Review fixes.

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:

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
 

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():

cmapi/cmapi_server/helpers.py

@@ -346,8 +346,8 @@ 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)
+            logging.debug('No .secrets file found so not distrinuting it.')
+        else:
             secrets = CEJPasswordHandler.get_secrets_json()
             body['secrets'] = secrets
 
@@ -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:
+        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
 

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

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