Use correct default key file and normalize the usage

The provision command was defaulting to id_rsa.pub, however the deploy
command uses id_rsa_tripleo for initial setup.

When using the deploy command for provision as well, use the public
key, not the private id_rsa_tripleo.

This option was being processed in several different ways, this change
normalize it by creating a single function in the Command class, which
all inheriting methods will consume. Tests have been updated to
accomodate this change.

Related-Bug: #1863920
Change-Id: I221480f3cfc77545a8fcbef777829239c3bad0a0
Signed-off-by: Kevin Carter <kecarter@redhat.com>

tripleoclient/command.py

@@ -70,6 +70,29 @@ class Command(command.Command):
             key = '.ssh/tripleo-admin-rsa'
             return key, None
 
+    def get_key_pair(self, parsed_args):
+        """Autodetect or return a user defined key file.
+
+        :param parsed_args: An argparse object.
+        :type parsed_args: Object
+
+        :returns: String
+        """
+
+        if not parsed_args.overcloud_ssh_key:
+            key = utils.get_key(
+                stack=parsed_args.stack,
+                needs_pair=True
+            )
+            if not key:
+                raise oscexc.CommandError(
+                    'No key pair found, set the ssh key using'
+                    'the --overcloud-ssh-key switch.'
+                )
+            return key
+        else:
+            return parsed_args.overcloud_ssh_key
+
 
 class Lister(Command, command.Lister):
     pass

tripleoclient/tests/fakes.py

@@ -194,6 +194,10 @@ class FakePlaybookExecution(utils.TestCommand):
         )
         self.config_action.start()
         self.addCleanup(self.config_action.stop)
+        get_key = mock.patch('tripleoclient.utils.get_key')
+        get_key.start()
+        get_key.return_value = 'keyfile-path'
+        self.addCleanup(get_key.stop)
 
         if ansible_mock:
             get_stack = mock.patch('tripleoclient.utils.get_stack')

tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py

@@ -1580,6 +1580,9 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
         with open(ssh_key_path, 'w') as temp_file:
             temp_file.write('sekrit')
 
+        with open('{}.pub'.format(ssh_key_path), 'w') as f:
+            f.write('sekrit')
+
         arglist = [
             '--baremetal-deployment', bm_deploy_path,
             '--overcloud-ssh-key', ssh_key_path

tripleoclient/tests/v1/overcloud_ffwd_upgrade/test_overcloud_ffwd_upgrade.py

@@ -100,7 +100,7 @@ class TestFFWDUpgradePrepare(fakes.TestFFWDUpgradePrepare):
             mock_stack,
             parsed_args.overcloud_ssh_network,
             parsed_args.overcloud_ssh_user,
-            parsed_args.overcloud_ssh_key,
+            mock.ANY,
             parsed_args.overcloud_ssh_port_timeout
         )
         mock_overcloud_deploy.assert_called_once_with(parsed_args)

tripleoclient/tests/v1/overcloud_node/test_overcloud_node.py

@@ -1290,6 +1290,8 @@ class TestProvisionNode(fakes.TestOvercloudNode):
                     inp.flush()
                     keyf.write(b'I am a key')
                     keyf.flush()
+                    with open('{}.pub'.format(keyf.name), 'w') as f:
+                        f.write('I am a key')
 
                     argslist = ['--output', outp.name,
                                 '--overcloud-ssh-key', keyf.name,

tripleoclient/tests/v1/overcloud_upgrade/test_overcloud_upgrade.py

@@ -105,7 +105,7 @@ class TestOvercloudUpgradePrepare(fakes.TestOvercloudUpgradePrepare):
             mock_stack,
             parsed_args.overcloud_ssh_network,
             parsed_args.overcloud_ssh_user,
-            parsed_args.overcloud_ssh_key,
+            mock.ANY,
             parsed_args.overcloud_ssh_port_timeout
         )
 

tripleoclient/tests/v1/test_overcloud_admin.py

@@ -37,6 +37,6 @@ class TestAdminAuthorize(test_plugin.TestPluginV1):
             mock_stack,
             parsed_args.overcloud_ssh_network,
             parsed_args.overcloud_ssh_user,
-            parsed_args.overcloud_ssh_key,
+            mock.ANY,
             parsed_args.overcloud_ssh_port_timeout
         )

tripleoclient/tests/v1/test_plugin.py

@@ -13,6 +13,8 @@
 #   under the License.
 #
 
+import mock
+
 from tripleoclient.tests import base
 from tripleoclient.tests import fakes
 
@@ -35,3 +37,7 @@ class TestPluginV1(base.TestCommand):
             endpoint=fakes.AUTH_URL,
             token=fakes.AUTH_TOKEN,
         )
+        get_key = mock.patch('tripleoclient.utils.get_key')
+        get_key.start()
+        get_key.return_value = 'keyfile-path'
+        self.addCleanup(get_key.stop)

tripleoclient/utils.py

@@ -1404,7 +1404,7 @@ def get_config(clients, container):
     config_action.run(context=context)
 
 
-def get_key(stack):
+def get_key(stack, needs_pair=False):
     """Returns the private key from the local file system.
 
     Searches for and returns the stack private key. If the key is inaccessible
@@ -1414,18 +1414,26 @@ def get_key(stack):
     :params stack: name of the stack to use
     :type stack: String
 
+    :param needs_pair: Enable key pair search
+    :type needs_pair: Boolean
+
     :returns: String || None
     """
 
+    key_files = list()
     stack_dir = os.path.join(constants.DEFAULT_WORK_DIR, stack)
-    stack_key_file = os.path.join(stack_dir, 'ssh_private_key')
+    key_files.append(os.path.join(stack_dir, 'ssh_private_key'))
     user_dir = os.path.join(os.path.expanduser("~"), '.ssh')
-    user_key_file = os.path.join(user_dir, 'id_rsa_tripleo')
+    key_files.append(os.path.join(user_dir, 'id_rsa_tripleo'))
+    key_files.append(os.path.join(user_dir, 'id_rsa'))
     legacy_dir = os.path.join(constants.DEFAULT_WORK_DIR, '.ssh')
-    legacy_key_file = os.path.join(legacy_dir, 'tripleo-admin-rsa')
+    key_files.append(os.path.join(legacy_dir, 'tripleo-admin-rsa'))
-    for key_file in [stack_key_file, user_key_file, legacy_key_file]:
+    for key_file in key_files:
         try:
             if os.path.exists(key_file):
+                if needs_pair:
+                    if not os.path.exists('{}.pub'.format(key_file)):
+                        continue
                 with open(key_file):
                     return key_file
         except IOError:

tripleoclient/v1/overcloud_admin.py

@@ -12,7 +12,6 @@
 #   License for the specific language governing permissions and limitations
 #   under the License.
 #
-import os
 
 from oslo_config import cfg
 from oslo_log import log as logging
@@ -51,9 +50,9 @@ class Authorize(command.Command):
         )
         parser.add_argument(
             '--overcloud-ssh-key',
-            default=os.path.join(
+            default=None,
-                os.path.expanduser('~'), '.ssh', 'id_rsa'),
+            help=_('Key path for ssh access to overcloud nodes. When'
-            help=_('Key path for ssh access to overcloud nodes.')
+                   'undefined the key will be autodetected.')
         )
         parser.add_argument(
             '--overcloud-ssh-network',
@@ -83,6 +82,6 @@ class Authorize(command.Command):
             stack,
             parsed_args.overcloud_ssh_network,
             parsed_args.overcloud_ssh_user,
-            parsed_args.overcloud_ssh_key,
+            self.get_key_pair(parsed_args),
             parsed_args.overcloud_ssh_port_timeout
         )

tripleoclient/v1/overcloud_deploy.py

@@ -608,14 +608,17 @@ class DeployOvercloud(command.Command):
         with open(parsed_args.baremetal_deployment, 'r') as fp:
             roles = yaml.safe_load(fp)
 
-        with open(parsed_args.overcloud_ssh_key, 'rt') as fp:
+        key = self.get_key_pair(parsed_args)
+        with open('{}.pub'.format(key), 'rt') as fp:
             ssh_key = fp.read()
 
         parameter_defaults = baremetal.deploy_roles(
             self.app.client_manager,
             plan=parsed_args.stack,
-            roles=roles, ssh_keys=[ssh_key],
+            roles=roles,
-            ssh_user_name=parsed_args.overcloud_ssh_user)
+            ssh_keys=[ssh_key],
+            ssh_user_name=parsed_args.overcloud_ssh_user
+        )
 
         env_path, swift_path = self._write_user_environment(
             parameter_defaults,
@@ -674,9 +677,9 @@ class DeployOvercloud(command.Command):
         )
         parser.add_argument(
             '--overcloud-ssh-key',
-            default=os.path.join(
+            default=None,
-                os.path.expanduser('~'), '.ssh', 'id_rsa_tripleo'),
+            help=_('Key path for ssh access to overcloud nodes. When'
-            help=_('Key path for ssh access to overcloud nodes.')
+                   'undefined the key will be autodetected.')
         )
         parser.add_argument(
             '--overcloud-ssh-network',
@@ -956,7 +959,7 @@ class DeployOvercloud(command.Command):
                         stack,
                         parsed_args.overcloud_ssh_network,
                         parsed_args.overcloud_ssh_user,
-                        parsed_args.overcloud_ssh_key,
+                        self.get_key_pair(parsed_args),
                         parsed_args.overcloud_ssh_port_timeout
                     )
 

tripleoclient/v1/overcloud_ffwd_upgrade.py

@@ -95,7 +95,7 @@ class FFWDUpgradePrepare(DeployOvercloud):
             stack,
             parsed_args.overcloud_ssh_network,
             parsed_args.overcloud_ssh_user,
-            parsed_args.overcloud_ssh_key,
+            self.get_key_pair(parsed_args),
             parsed_args.overcloud_ssh_port_timeout
         )
 

tripleoclient/v1/overcloud_node.py

@@ -617,10 +617,10 @@ class ProvisionNode(command.Command):
                             help=_('User for SSH access to newly deployed '
                                    'nodes'))
         parser.add_argument('--overcloud-ssh-key',
-                            default=os.path.join(
+                            default=None,
-                                os.path.expanduser('~'), '.ssh', 'id_rsa.pub'),
+                            help=_('Key path for ssh access to'
-                            help=_('Public key path for SSH access to newly '
+                                   'overcloud nodes. When undefined the key'
-                                   'deployed nodes'))
+                                   'will be autodetected.'))
         return parser
 
     def take_action(self, parsed_args):
@@ -629,14 +629,17 @@ class ProvisionNode(command.Command):
         with open(parsed_args.input, 'r') as fp:
             roles = yaml.safe_load(fp)
 
-        with open(parsed_args.overcloud_ssh_key, 'rt') as fp:
+        key = self.get_key_pair(parsed_args)
+        with open('{}.pub'.format(key), 'rt') as fp:
             ssh_key = fp.read()
 
         output = baremetal.deploy_roles(
             self.app.client_manager,
             plan=parsed_args.stack,
-            roles=roles, ssh_keys=[ssh_key],
+            roles=roles,
-            ssh_user_name=parsed_args.overcloud_ssh_user)
+            ssh_keys=[ssh_key],
+            ssh_user_name=parsed_args.overcloud_ssh_user
+        )
 
         with open(parsed_args.output, 'w') as fp:
             yaml.safe_dump(output['environment'], fp, default_flow_style=False)

tripleoclient/v1/overcloud_upgrade.py

@@ -85,7 +85,7 @@ class UpgradePrepare(DeployOvercloud):
             stack,
             parsed_args.overcloud_ssh_network,
             parsed_args.overcloud_ssh_user,
-            parsed_args.overcloud_ssh_key,
+            self.get_key_pair(parsed_args),
             parsed_args.overcloud_ssh_port_timeout
         )