From: Sage Weil Date: Thu, 27 May 2021 23:25:06 +0000 (-0400) Subject: mgr/nfs: refactor 'nfs export update' and export validation X-Git-Tag: v17.1.0~1551^2~50 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=2b93356fac49a5bd50a9e4b97314db87ade51666;p=ceph.git mgr/nfs: refactor 'nfs export update' and export validation Move validation to the Export class, so it can check any object in place. Refactor update code to extract allowed changes checks from the old validation class. Signed-off-by: Sage Weil --- diff --git a/src/pybind/mgr/nfs/export.py b/src/pybind/mgr/nfs/export.py index 6a995653baf8..dd624b48314a 100644 --- a/src/pybind/mgr/nfs/export.py +++ b/src/pybind/mgr/nfs/export.py @@ -95,72 +95,6 @@ class NFSRados: return False -class ValidateExport: - @staticmethod - def pseudo_path(path): - if not isabs(path) or path == "/": - raise NFSInvalidOperation(f"pseudo path {path} is invalid. It should be an absolute " - "path and it cannot be just '/'.") - - @staticmethod - def squash(squash): - valid_squash_ls = ["root", "root_squash", "rootsquash", "rootid", "root_id_squash", - "rootidsquash", "all", "all_squash", "allsquash", "all_anomnymous", - "allanonymous", "no_root_squash", "none", "noidsquash"] - if squash not in valid_squash_ls: - raise NFSInvalidOperation(f"squash {squash} not in valid list {valid_squash_ls}") - - @staticmethod - def security_label(label): - if not isinstance(label, bool): - raise NFSInvalidOperation('Only boolean values allowed') - - @staticmethod - def protocols(proto): - for p in proto: - if p not in [3, 4]: - raise NFSInvalidOperation(f"Invalid protocol {p}") - if 3 in proto: - log.warning("NFS V3 is an old version, it might not work") - - @staticmethod - def transport(transport): - valid_transport = ["UDP", "TCP"] - for trans in transport: - if trans.upper() not in valid_transport: - raise NFSInvalidOperation(f'{trans} is not a valid transport protocol') - - @staticmethod - def access_type(access_type): - valid_ones = ['RW', 'RO'] - if access_type not in valid_ones: - raise NFSInvalidOperation(f'{access_type} is invalid, valid access type are' - f'{valid_ones}') - - @staticmethod - def fsal(mgr, old, new): - if old.name != new['name']: - raise NFSInvalidOperation('FSAL name change not allowed') - if old.user_id != new['user_id']: - raise NFSInvalidOperation('User ID modification is not allowed') - if new['sec_label_xattr']: - raise NFSInvalidOperation('Security label xattr cannot be changed') - if old.fs_name != new['fs_name']: - if not check_fs(mgr, new['fs_name']): - raise FSNotFound(new['fs_name']) - return 1 - - @staticmethod - def _client(client): - ValidateExport.access_type(client['access_type']) - ValidateExport.squash(client['squash']) - - @staticmethod - def clients(clients_ls): - for client in clients_ls: - ValidateExport._client(client) - - class ExportMgr: def __init__(self, mgr, namespace=None, export_ls=None): self.mgr = mgr @@ -270,7 +204,7 @@ class ExportMgr: GaneshaConfParser.write_block(export.to_export_block()), f'export-{export.export_id}', f'conf-nfs.{export.cluster_id}') - def format_path(self, path): + def format_path(self, path: str): if path: path = normpath(path.strip()) if path[:2] == "//": @@ -340,66 +274,24 @@ class ExportMgr: except Exception as e: return exception_handler(e, f'Failed to update export: {e}') + def import_export(self, export_config): + try: + if not export_config: + raise NFSInvalidOperation("Empty Config!!") + new_export = json.loads(export_config) + # check export type + return FSExport(self).import_export(new_export) + except NotImplementedError: + return 0, " Manual Restart of NFS PODS required for successful update of exports", "" + except Exception as e: + return exception_handler(e, f'Failed to import export: {e}') + class FSExport(ExportMgr): def __init__(self, export_mgr_obj): super().__init__(export_mgr_obj.mgr, export_mgr_obj.rados_namespace, export_mgr_obj._exports) - def _validate_export(self, new_export_dict): - if new_export_dict['cluster_id'] not in available_clusters(self.mgr): - raise ClusterNotFound() - - export = self._fetch_export(new_export_dict['cluster_id'], - new_export_dict['pseudo']) - out_msg = '' - if export: - # Check if export id matches - if export.export_id != new_export_dict['export_id']: - raise NFSInvalidOperation('Export ID changed, Cannot update export') - else: - # Fetch export based on export id object - export = self._fetch_export_obj(new_export_dict['export_id']) - if not export: - raise NFSObjectNotFound('Export does not exist') - else: - new_export_dict['pseudo'] = self.format_path(new_export_dict['pseudo']) - ValidateExport.pseudo_path(new_export_dict['pseudo']) - log.debug(f"Pseudo path has changed from {export.pseudo} to " - f"{new_export_dict['pseudo']}") - # Check if squash changed - if export.squash != new_export_dict['squash']: - if new_export_dict['squash']: - new_export_dict['squash'] = new_export_dict['squash'].lower() - ValidateExport.squash(new_export_dict['squash']) - log.debug(f"squash has changed from {export.squash} to {new_export_dict['squash']}") - # Security label check - if export.security_label != new_export_dict['security_label']: - ValidateExport.security_label(new_export_dict['security_label']) - # Protocol Checking - if export.protocols != new_export_dict['protocols']: - ValidateExport.protocols(new_export_dict['protocols']) - # Transport checking - if export.transports != new_export_dict['transports']: - ValidateExport.transport(new_export_dict['transports']) - # Path check - if export.path != new_export_dict['path']: - new_export_dict['path'] = self.format_path(new_export_dict['path']) - out_msg = 'update caps' - # Check Access Type - if export.access_type != new_export_dict['access_type']: - ValidateExport.access_type(new_export_dict['access_type']) - # Fsal block check - if export.fsal != new_export_dict['fsal']: - ret = ValidateExport.fsal(self.mgr, export.fsal, new_export_dict['fsal']) - if ret == 1 and not out_msg: - out_msg = 'update caps' - # Check client block - if export.clients != new_export_dict['clients']: - ValidateExport.clients(new_export_dict['clients']) - log.debug(f'Validation succeeded for Export {export.pseudo}') - return export, out_msg - def _update_user_id(self, path, access_type, fs_name, user_id): osd_cap = 'allow rw pool={} namespace={}, allow rw tag cephfs data={}'.format( self.rados_pool, self.rados_namespace, fs_name) @@ -437,7 +329,6 @@ class FSExport(ExportMgr): raise FSNotFound(fs_name) pseudo_path = self.format_path(pseudo_path) - ValidateExport.pseudo_path(pseudo_path) if cluster_id not in self.exports: self.exports[cluster_id] = [] @@ -459,11 +350,11 @@ class FSExport(ExportMgr): 'access_type': access_type, 'squash': squash, 'fsal': {"name": "CEPH", "user_id": user_id, - "fs_name": fs_name, "sec_label_xattr": ""}, + "fs_name": fs_name, "sec_label_xattr": "", + "cephx_key": key}, 'clients': clients } export = Export.from_dict(ex_id, ex_dict) - export.fsal.cephx_key = key self._save_export(export) result = { "bind": pseudo_path, @@ -476,12 +367,46 @@ class FSExport(ExportMgr): return 0, "", "Export already exists" def update_export(self, new_export): - old_export, update_user_caps = self._validate_export(new_export) - if update_user_caps: - self._update_user_id(new_export['path'], new_export['access_type'], - new_export['fsal']['fs_name'], new_export['fsal']['user_id']) + for k in ['cluster_id', 'path', 'pseudo']: + if k not in new_export: + raise NFSInvalidOperation(f'Export missing required field {k}') + if new_export['cluster_id'] not in available_clusters(self.mgr): + raise ClusterNotFound() + self.rados_namespace = new_export['cluster_id'] + + new_export['path'] = self.format_path(new_export['path']) + new_export['pseudo'] = self.format_path(new_export['pseudo']) + + old_export = self._fetch_export(new_export['cluster_id'], + new_export['pseudo']) + if old_export: + # Check if export id matches + if old_export.export_id != new_export['export_id']: + raise NFSInvalidOperation('Export ID changed, Cannot update export') + else: + # Fetch export based on export id object + old_export = self._fetch_export_obj(new_export['export_id']) + if not old_export: + raise NFSObjectNotFound('Export does not exist') + new_export = Export.from_dict(new_export['export_id'], new_export) + new_export.validate(self.mgr) + + if old_export.fsal.name != new_export.fsal.name: + raise NFSInvalidOperation('FSAL change not allowed') + if old_export.fsal.user_id != new_export.fsal.user_id: + raise NFSInvalidOperation('user_id change is not allowed') + if old_export.fsal.sec_label_xattr != new_export.fsal.sec_label_xattr: + raise NFSInvalidOperation('Security label xattr cannot be changed') + + if ( + old_export.fsal.fs_name != new_export.fsal.fs_name + or old_export.path != new_export.path + ): + self._update_user_id(new_export.path, new_export.access_type, + new_export.fsal.fs_name, new_export.fsal.user_id) new_export.fsal.cephx_key = old_export.fsal.cephx_key + self._update_export(new_export) export_ls = self.exports[self.rados_namespace] if old_export not in export_ls: diff --git a/src/pybind/mgr/nfs/export_utils.py b/src/pybind/mgr/nfs/export_utils.py index 593108c63713..cfbafa39d0bd 100644 --- a/src/pybind/mgr/nfs/export_utils.py +++ b/src/pybind/mgr/nfs/export_utils.py @@ -1,10 +1,14 @@ +from typing import cast, List, Dict, Any, Optional +from os.path import isabs + +from .exception import NFSInvalidOperation, FSNotFound +from .utils import check_fs + + class GaneshaConfParser: - def __init__(self, raw_config): + def __init__(self, raw_config: str): self.pos = 0 self.text = "" - self.clean_config(raw_config) - - def clean_config(self, raw_config): for line in raw_config.split("\n"): self.text += line if line.startswith("%"): @@ -152,24 +156,52 @@ class GaneshaConfParser: return conf_str -class CephFSFSal: - def __init__(self, name, user_id=None, fs_name=None, sec_label_xattr=None, - cephx_key=None): +class FSAL(object): + def __init__(self, name: str): self.name = name + + @classmethod + def from_dict(cls, fsal_dict: Dict[str, Any]) -> 'FSAL': + if fsal_dict.get('name') == 'CEPH': + return CephFSFSAL.from_dict(fsal_dict) + raise NFSInvalidOperation(f'Unknown FSAL {fsal_dict.get("name")}') + + @classmethod + def from_fsal_block(cls, fsal_block: Dict[str, Any]) -> 'FSAL': + if fsal_block.get('name') == 'CEPH': + return CephFSFSAL.from_fsal_block(fsal_block) + raise NFSInvalidOperation(f'Unknown FSAL {fsal_block.get("name")}') + + def to_fsal_block(self) -> Dict[str, Any]: + raise NotImplemented + + def to_dict(self) -> Dict[str, Any]: + raise NotImplemented + + +class CephFSFSAL(FSAL): + def __init__(self, + name: str, + user_id: Optional[str] = None, + fs_name: Optional[str] = None, + sec_label_xattr: Optional[str] = None, + cephx_key: Optional[str] = None): + super().__init__(name) + assert name == 'CEPH' self.fs_name = fs_name self.user_id = user_id self.sec_label_xattr = sec_label_xattr self.cephx_key = cephx_key @classmethod - def from_fsal_block(cls, fsal_block): + def from_fsal_block(cls, fsal_block: Dict[str, str]) -> 'CephFSFSAL': return cls(fsal_block['name'], - fsal_block.get('user_id', None), - fsal_block.get('filesystem', None), - fsal_block.get('sec_label_xattr', None), - fsal_block.get('secret_access_key', None)) + fsal_block.get('user_id'), + fsal_block.get('filesystem'), + fsal_block.get('sec_label_xattr'), + fsal_block.get('secret_access_key')) - def to_fsal_block(self): + def to_fsal_block(self) -> Dict[str, str]: result = { 'block_name': 'FSAL', 'name': self.name, @@ -185,29 +217,37 @@ class CephFSFSal: return result @classmethod - def from_dict(cls, fsal_dict): - return cls(fsal_dict['name'], fsal_dict['user_id'], - fsal_dict['fs_name'], fsal_dict['sec_label_xattr'], None) - - def to_dict(self): - return { - 'name': self.name, - 'user_id': self.user_id, - 'fs_name': self.fs_name, - 'sec_label_xattr': self.sec_label_xattr - } + def from_dict(cls, fsal_dict: Dict[str, str]) -> 'CephFSFSAL': + return cls(fsal_dict['name'], + fsal_dict.get('user_id'), + fsal_dict.get('fs_name'), + fsal_dict.get('sec_label_xattr'), + fsal_dict.get('cephx_key')) + + def to_dict(self) -> Dict[str, str]: + r = {'name': self.name} + if self.user_id: + r['user_id'] = self.user_id + if self.fs_name: + r['fs_name'] = self.fs_name + if self.sec_label_xattr: + r['sec_label_xattr'] = self.sec_label_xattr + return r class Client: - def __init__(self, addresses, access_type=None, squash=None): + def __init__(self, + addresses: List[str], + access_type: Optional[str] = None, + squash: Optional[str] = None): self.addresses = addresses self.access_type = access_type self.squash = squash @classmethod - def from_client_block(cls, client_block): - addresses = client_block['clients'] - if not isinstance(addresses, list): + def from_client_block(cls, client_block) -> 'Client': + addresses = client_block.get('clients', []) + if isinstance(addresses, str): addresses = [addresses] return cls(addresses, client_block.get('access_type', None), @@ -225,7 +265,7 @@ class Client: return result @classmethod - def from_dict(cls, client_dict): + def from_dict(cls, client_dict) -> 'Client': return cls(client_dict['addresses'], client_dict['access_type'], client_dict['squash']) @@ -238,8 +278,19 @@ class Client: class Export: - def __init__(self, export_id, path, cluster_id, pseudo, access_type, squash, security_label, - protocols, transports, fsal, clients=None): + def __init__( + self, + export_id: int, + path: str, + cluster_id: str, + pseudo: str, + access_type: str, + squash: str, + security_label: bool, + protocols: List[int], + transports: List[str], + fsal: FSAL, + clients=None): self.export_id = export_id self.path = path self.fsal = fsal @@ -255,8 +306,8 @@ class Export: @classmethod def from_export_block(cls, export_block, cluster_id): - fsal_block = [b for b in export_block['_blocks_'] - if b['block_name'] == "FSAL"] + fsal_blocks = [b for b in export_block['_blocks_'] + if b['block_name'] == "FSAL"] client_blocks = [b for b in export_block['_blocks_'] if b['block_name'] == "CLIENT"] @@ -278,7 +329,7 @@ class Export: export_block['security_label'], protocols, transports, - CephFSFSal.from_fsal_block(fsal_block[0]), + FSAL.from_fsal_block(fsal_blocks[0]), [Client.from_client_block(client) for client in client_blocks]) @@ -295,15 +346,18 @@ class Export: 'protocols': self.protocols, 'transports': self.transports, } - result['_blocks_'] = [self.fsal.to_fsal_block()] - result['_blocks_'].extend([client.to_client_block() - for client in self.clients]) + result['_blocks_'] = [ + self.fsal.to_fsal_block() + ] + [ + client.to_client_block() + for client in self.clients + ] return result @classmethod def from_dict(cls, export_id, ex_dict): return cls(export_id, - ex_dict['path'], + ex_dict.get('path', '/'), ex_dict['cluster_id'], ex_dict['pseudo'], ex_dict.get('access_type', 'R'), @@ -311,8 +365,8 @@ class Export: ex_dict.get('security_label', True), ex_dict.get('protocols', [4]), ex_dict.get('transports', ['TCP']), - CephFSFSal.from_dict(ex_dict['fsal']), - [Client.from_dict(client) for client in ex_dict['clients']]) + FSAL.from_dict(ex_dict.get('fsal', {})), + [Client.from_dict(client) for client in ex_dict.get('clients', [])]) def to_dict(self): return { @@ -328,3 +382,57 @@ class Export: 'fsal': self.fsal.to_dict(), 'clients': [client.to_dict() for client in self.clients] } + + @staticmethod + def validate_access_type(access_type): + valid_access_types = ['rw', 'ro', 'none'] + if access_type.lower() not in valid_access_types: + raise NFSInvalidOperation( + f'{access_type} is invalid, valid access type are' + f'{valid_access_types}' + ) + + @staticmethod + def validate_squash(squash): + valid_squash_ls = [ + "root", "root_squash", "rootsquash", "rootid", "root_id_squash", + "rootidsquash", "all", "all_squash", "allsquash", "all_anomnymous", + "allanonymous", "no_root_squash", "none", "noidsquash", + ] + if squash not in valid_squash_ls: + raise NFSInvalidOperation( + f"squash {squash} not in valid list {valid_squash_ls}" + ) + + def validate(self, mgr): + if not isabs(self.pseudo) or self.pseudo == "/": + raise NFSInvalidOperation( + f"pseudo path {self.pseudo} is invalid. It should be an absolute " + "path and it cannot be just '/'." + ) + + self.validate_squash(self.squash) + self.validate_access_type(self.access_type) + + if not isinstance(self.security_label, bool): + raise NFSInvalidOperation('security_label must be a boolean value') + + for p in self.protocols: + if p not in [3, 4]: + raise NFSInvalidOperation(f"Invalid protocol {p}") + + valid_transport = ["UDP", "TCP"] + for trans in self.transports: + if trans.upper() not in valid_transport: + raise NFSInvalidOperation(f'{trans} is not a valid transport protocol') + + for client in self.clients: + self.validate_squash(client.squash) + self.validate_access_type(client.access_type) + + if self.fsal.name == 'CEPH': + fs = cast(CephFSFSAL, self.fsal) + if not check_fs(mgr, fs.fs_name): + raise FSNotFound(fs.fs_name) + else: + raise NFSInvalidOperation('FSAL {self.fsal.name} not supported')