From d17fc7d9be06a49004d935bd8ce3a4b1b9e51949 Mon Sep 17 00:00:00 2001 From: mariadb-AlanMologorsky Date: Fri, 4 Apr 2025 15:51:21 +0300 Subject: [PATCH] Review fixes. --- cmapi/cmapi_server/controllers/endpoints.py | 15 ++++++----- cmapi/cmapi_server/handlers/cej.py | 24 ++++++++++++++++- cmapi/cmapi_server/handlers/cluster.py | 2 +- cmapi/cmapi_server/helpers.py | 29 +++++++++++++++++---- cmapi/cmapi_server/node_manipulation.py | 2 +- cmapi/cmapi_server/test/test_txns.py | 2 +- 6 files changed, 59 insertions(+), 15 deletions(-) diff --git a/cmapi/cmapi_server/controllers/endpoints.py b/cmapi/cmapi_server/controllers/endpoints.py index 21c3416ee..870e07b35 100644 --- a/cmapi/cmapi_server/controllers/endpoints.py +++ b/cmapi/cmapi_server/controllers/endpoints.py @@ -351,13 +351,14 @@ class ConfigController: # the config file or apply the changes is_test = request_body.get('test', False) - mandatory = [request_revision, request_manager, request_timeout] + mandatory = (request_revision, request_manager, request_timeout) if None in mandatory: raise_422_error( module_logger, func_name, 'Mandatory attribute is missing.') request_mode = request_body.get('cluster_mode', None) - request_config = request_body.get('config', None) + xml_config = request_body.get('config', None) + sm_config = request_body.get('sm_config', None) mcs_config_filename = request_body.get( 'mcs_config_filename', DEFAULT_MCS_CONF_PATH ) @@ -366,9 +367,12 @@ class ConfigController: ) secrets = request_body.get('secrets', None) - if request_mode is None and request_config is None: + operation_params = (request_mode, xml_config, secrets) + # if no operation to apply, return 422 + if not any(operation_params): raise_422_error( - module_logger, func_name, 'Mandatory attribute is missing.' + module_logger, func_name, + 'Mandatory attribute is missing.' ) request_headers = cherrypy.request.headers @@ -394,11 +398,10 @@ class ConfigController: request_response = {'timestamp': str(datetime.now())} if secrets: + #TODO: validate incoming secrets? CEJPasswordHandler().save_secrets(secrets) node_config = NodeConfig() - xml_config = request_body.get('config', None) - sm_config = request_body.get('sm_config', None) if is_test: return request_response if request_mode is not None: diff --git a/cmapi/cmapi_server/handlers/cej.py b/cmapi/cmapi_server/handlers/cej.py index afa2248f6..9468314e3 100644 --- a/cmapi/cmapi_server/handlers/cej.py +++ b/cmapi/cmapi_server/handlers/cej.py @@ -77,6 +77,25 @@ class CEJPasswordHandler(): ) from None return secrets_json + @classmethod + def is_password_encrypted(cls, passwd: str) -> bool: + """Check if password is encrypted. + + :param passwd: CEJ password + :type passwd: str + :return: True if password is encrypted + :rtype: bool + """ + # minimal length of encrypted password + if len(passwd) < (AES_IV_HEX_SIZE*2): + return False + try: + # Try converting the string to an integer with base 16 + int(passwd, 16) + except ValueError: + return False + return True + @classmethod def decrypt_password( cls, enc_data: str, directory: str = MCS_DATA_PATH @@ -91,7 +110,10 @@ class CEJPasswordHandler(): :rtype: str """ secrets_file_path = os.path.join(directory, MCS_SECRETS_FILENAME) - if not cls.secretsfile_exists(directory=directory): + if ( + not cls.secretsfile_exists(directory=directory) or + not cls.is_password_encrypted(enc_data) + ): logging.warning('Unencrypted CrossEngineSupport password used.') return enc_data diff --git a/cmapi/cmapi_server/handlers/cluster.py b/cmapi/cmapi_server/handlers/cluster.py index 60ac56bb1..10a213fb7 100644 --- a/cmapi/cmapi_server/handlers/cluster.py +++ b/cmapi/cmapi_server/handlers/cluster.py @@ -47,7 +47,7 @@ def toggle_cluster_state( switch_node_maintenance(maintainance_flag) update_revision_and_manager() - broadcast_new_config(config) + broadcast_new_config(config, distribute_secrets=True) class ClusterHandler(): diff --git a/cmapi/cmapi_server/helpers.py b/cmapi/cmapi_server/helpers.py index 04845d8bb..53fb003e7 100644 --- a/cmapi/cmapi_server/helpers.py +++ b/cmapi/cmapi_server/helpers.py @@ -346,10 +346,10 @@ def broadcast_new_config( # TODO: do not restart cluster when put xml config only with # distribute secrets if not CEJPasswordHandler.secretsfile_exists(): - secrets_dict = CEJPasswordHandler.generate_secrets_data() - CEJPasswordHandler.save_secrets(secrets=secrets_dict) - secrets = CEJPasswordHandler.get_secrets_json() - body['secrets'] = secrets + logging.debug('No .secrets file found so not distrinuting it.') + else: + secrets = CEJPasswordHandler.get_secrets_json() + body['secrets'] = secrets # TODO: remove test mode here and replace it by mock in tests if test_mode: @@ -787,6 +787,7 @@ def get_cej_info(config_root): :return: cej_host, cej_port, cej_username, cej_password :rtype: tuple """ + #TODO: move this to cej.py? cej_node = config_root.find('./CrossEngineSupport') cej_host = cej_node.find('Host').text or '127.0.0.1' cej_port = cej_node.find('Port').text or '3306' @@ -802,8 +803,26 @@ def get_cej_info(config_root): 'Columnstore.xml has an empty CrossEngineSupport.Password tag' ) + if ( + not CEJPasswordHandler.secretsfile_exists() and + CEJPasswordHandler.is_password_encrypted(cej_password) + ): + logging.error( + 'CrossengineSupport password seems to be encrypted ' + 'but no .secrets file exist. May be it\'s eventually removed.' + ) + + if CEJPasswordHandler.secretsfile_exists() and cej_password: - cej_password = CEJPasswordHandler.decrypt_password(cej_password) + if CEJPasswordHandler.is_password_encrypted(cej_password): + cej_password = CEJPasswordHandler.decrypt_password(cej_password) + else: + logging.error( + 'CrossengineSupport password seems to be unencrypted but ' + '.secrets file exist. May be .secrets file generated by ' + 'mistake or password left encrypted after using cskeys ' + 'utility.' + ) return cej_host, cej_port, cej_username, cej_password diff --git a/cmapi/cmapi_server/node_manipulation.py b/cmapi/cmapi_server/node_manipulation.py index 76abdf8f5..7eee0ad18 100644 --- a/cmapi/cmapi_server/node_manipulation.py +++ b/cmapi/cmapi_server/node_manipulation.py @@ -486,7 +486,7 @@ def is_master(): "AND COMMAND LIKE 'Slave%';\"" ) - ret = subprocess.run(cmd, stdout=subprocess.PIPE, shell = True) + ret = subprocess.run(cmd, stdout=subprocess.PIPE, shell=True) if ret.returncode == 0: response = ret.stdout.decode("utf-8").strip() # Primary will have no slave_threads diff --git a/cmapi/cmapi_server/test/test_txns.py b/cmapi/cmapi_server/test/test_txns.py index 4bd03f3a9..f7bbf5ee6 100644 --- a/cmapi/cmapi_server/test/test_txns.py +++ b/cmapi/cmapi_server/test/test_txns.py @@ -131,7 +131,7 @@ class TestTransactions(unittest.TestCase): mcs_config_filename, cmapi_config_filename=cmapi_config_filename, test_mode=True, - nodes = result[2] + nodes=result[2] ) # not specifying nodes here to exercise the # nodes = None path