Fix: bootstrap: add informative logging for generating new ssh keypairs

crmsh/bootstrap.py

@@ -948,7 +948,9 @@ def _init_ssh_for_secondary_user_on_remote_nodes(
         if not sh.SSHShell(cluster_shell.local_shell, user).can_run_as(node, user):
             for key in local_keys:
                 authorized_key_manager.add(node, user, key)
-            remote_keys = key_file_manager.ensure_key_pair_exists_for_user(node, user)
+            is_generated, remote_keys = key_file_manager.ensure_key_pair_exists_for_user(node, user)
+            if is_generated:
+                logger.info("A new ssh keypair is generated for user %s@%s.", user, node)
             for key in remote_keys:
                 authorized_key_manager.add(None, user, key)
 
@@ -1037,8 +1039,10 @@ def configure_ssh_key(user):
     shell = sh.LocalShell()
     key_file_manager = ssh_key.KeyFileManager(sh.ClusterShellAdaptorForLocalShell(shell))
     authorized_key_manager = ssh_key.AuthorizedKeyManager(sh.SSHShell(shell, None))
-    key = key_file_manager.ensure_key_pair_exists_for_user(None, user)[0]
-    authorized_key_manager.add(None, user, key)
+    is_generated, keys = key_file_manager.ensure_key_pair_exists_for_user(None, user)
+    if is_generated:
+        logger.info("A new ssh keypair is generated for user %s.", user)
+    authorized_key_manager.add(None, user, keys[0])
 
 
 def generate_ssh_key_pair_on_remote(
@@ -1809,9 +1813,11 @@ def join_ssh_with_ssh_agent(
 def swap_public_ssh_key_for_secondary_user(shell: sh.ClusterShell, host: str, user: str):
     key_file_manager = ssh_key.KeyFileManager(shell)
     local_key = ssh_key.KeyFile(key_file_manager.list_public_key_for_user(None, user)[0])
-    remote_key = key_file_manager.ensure_key_pair_exists_for_user(host, user)[0]
+    is_generated, remote_keys = key_file_manager.ensure_key_pair_exists_for_user(host, user)
+    if is_generated:
+        logger.info("A new ssh keypair is generated for user %s@%s.", user, host)
     authorized_key_manager = ssh_key.AuthorizedKeyManager(shell)
-    authorized_key_manager.add(None, user, remote_key)
+    authorized_key_manager.add(None, user, remote_keys[0])
     authorized_key_manager.add(host, user, local_key)
 
 
@@ -2092,7 +2098,7 @@ def swap_key_for_hacluster(other_node_list):
     key_file_manager = ssh_key.KeyFileManager(shell)
     authorized_key_manager = ssh_key.AuthorizedKeyManager(shell)
     keys: typing.List[ssh_key.Key] = [
-        key_file_manager.ensure_key_pair_exists_for_user(node, 'hacluster')[0]
+        key_file_manager.ensure_key_pair_exists_for_user(node, 'hacluster')[1][0]
         for node in other_node_list
     ]
     keys.append(ssh_key.KeyFile(key_file_manager.list_public_key_for_user(None, 'hacluster')[0]))

crmsh/ssh_key.py

@@ -217,9 +217,21 @@ def load_public_keys_for_user(self, host: typing.Optional[str], user: str) -> ty
             raise sh.CommandFailure(cmd, host, user, sh.Utils.decode_str(result.stderr).strip())
         return [InMemoryPublicKey(line) for line in sh.Utils.decode_str(result.stdout).splitlines()]
 
-    def ensure_key_pair_exists_for_user(self, host: typing.Optional[str], user: str) -> typing.List[InMemoryPublicKey]:
+    def ensure_key_pair_exists_for_user(
+            self,
+            host: typing.Optional[str],
+            user: str,
+    ) -> typing.Tuple[bool, typing.List[InMemoryPublicKey]]:
+        """Ensure at least one keypair exists for the specified user. If it does not exist, generate a new one.
+
+        Return (is_generated, list_of_public_keys):
+
+        * is_generated: whether a new keypair is generated
+        * list_of_public_keys: all public keys of known types, including the newly generated one
+        """
         script = '''if [ ! \\( {condition} \\) ]; then
     ssh-keygen -t rsa -f ~/.ssh/id_rsa -q -C "Cluster internal on $(hostname)" -N '' <> /dev/null
+    echo 'GENERATED=1'
 fi
 for file in ~/.ssh/id_{{{pattern}}}; do
     if [ -f "$file" ]; then
@@ -243,4 +255,11 @@ def ensure_key_pair_exists_for_user(self, host: typing.Optional[str], user: str)
             print(script)
             print(result.stdout)
             raise sh.CommandFailure(f'Script({script[:16]}...) failed. rc = {result.returncode}', host, user, sh.Utils.decode_str(result.stderr).strip())
-        return [InMemoryPublicKey(line) for line in sh.Utils.decode_str(result.stdout).splitlines()]
+        generated = False
+        keys = list()
+        for line in sh.Utils.decode_str(result.stdout).splitlines():
+            if line == 'GENERATED=1':
+                generated = True
+            else:
+                keys.append(InMemoryPublicKey(line))
+        return generated, keys

test/unittests/test_bootstrap.py

@@ -540,7 +540,7 @@ def _test_configure_ssh_key(self, mock_change_shell, mock_key_files, mock_detect
     @mock.patch('crmsh.ssh_key.KeyFileManager.ensure_key_pair_exists_for_user')
     def test_configure_ssh_key(self, mock_ensure_key_pair, mock_add):
         public_key = crmsh.ssh_key.InMemoryPublicKey('foo')
-        mock_ensure_key_pair.return_value = [public_key]
+        mock_ensure_key_pair.return_value = (True, [public_key])
         bootstrap.configure_ssh_key('alice')
         mock_ensure_key_pair.assert_called_once_with(None, 'alice')
         mock_add.assert_called_once_with(None, 'alice', public_key)
@@ -613,8 +613,10 @@ def test_join_ssh(
     @mock.patch('crmsh.ssh_key.KeyFile.public_key')
     @mock.patch('crmsh.ssh_key.KeyFileManager.ensure_key_pair_exists_for_user')
     @mock.patch('crmsh.ssh_key.KeyFileManager.list_public_key_for_user')
+    @mock.patch('logging.Logger.info')
     def test_swap_public_ssh_key_for_secondary_user(
             self,
+            mock_log_info,
             mock_list_public_key_for_user,
             mock_ensure_key_pair_exists_for_user,
             mock_public_key,
@@ -626,10 +628,10 @@ def test_swap_public_ssh_key_for_secondary_user(
             user_of_host=mock.Mock(crmsh.user_of_host.UserOfHost),
         )
         mock_list_public_key_for_user.return_value = ['~/.ssh/id_rsa', '~/.ssh/id_ed25519']
-        mock_ensure_key_pair_exists_for_user.return_value = [
+        mock_ensure_key_pair_exists_for_user.return_value = (True, [
             crmsh.ssh_key.InMemoryPublicKey('foo'),
             crmsh.ssh_key.InMemoryPublicKey('bar'),
-        ]
+        ])
         mock_public_key.return_value = 'public_key'
         crmsh.bootstrap.swap_public_ssh_key_for_secondary_user(mock_shell, 'node1', 'alice')
         mock_list_public_key_for_user.assert_called_once_with(None, 'alice')
@@ -638,6 +640,7 @@ def test_swap_public_ssh_key_for_secondary_user(
             mock.call(None, 'alice', crmsh.ssh_key.InMemoryPublicKey('foo')),
             mock.call('node1', 'alice', crmsh.ssh_key.KeyFile('~/.ssh/id_rsa')),
         ])
+        mock_log_info.assert_called_with("A new ssh keypair is generated for user %s@%s.", 'alice', 'node1')
 
     @mock.patch('crmsh.bootstrap.change_user_shell')
     @mock.patch('crmsh.sh.LocalShell.get_stdout_or_raise_error')