diff mbox series

[18/22] Acceptance Tests: set up existing ssh keys by default

Message ID 20210203172357.1422425-19-crosa@redhat.com
State New
Headers show
Series Acceptance Test: introduce base class for Linux based tests | expand

Commit Message

Cleber Rosa Feb. 3, 2021, 5:23 p.m. UTC
It's questionable wether it's necessary to create one brand new pair
for each test.  It's not questionable that it takes less time and
resources to just use the keys available at "tests/keys" that exist
for that exact reason.

If a location for the public key is not given explicitly, the
LinuxTest will now set up the existing pair of keys as the default.
This removes the need for a lot of boiler plate code.

To avoid the ssh client from erroring on permission issues, a
directory with restricive permissions is created for the private key.
This should still be a lot cheaper than creating a new key.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/avocado_qemu/__init__.py | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Marc-André Lureau Feb. 11, 2021, 10:15 a.m. UTC | #1
Hi

On Wed, Feb 3, 2021 at 10:07 PM Cleber Rosa <crosa@redhat.com> wrote:
>
> It's questionable wether it's necessary to create one brand new pair

whether

> for each test.  It's not questionable that it takes less time and
> resources to just use the keys available at "tests/keys" that exist
> for that exact reason.
>
> If a location for the public key is not given explicitly, the
> LinuxTest will now set up the existing pair of keys as the default.
> This removes the need for a lot of boiler plate code.

boilerplate

>
> To avoid the ssh client from erroring on permission issues, a
> directory with restricive permissions is created for the private key.

restrictive

> This should still be a lot cheaper than creating a new key.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  tests/acceptance/avocado_qemu/__init__.py | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index 8156224625..5f4dd6b9ec 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -11,6 +11,7 @@
>  import logging
>  import os
>  import re
> +import shutil
>  import sys
>  import uuid
>  import tempfile
> @@ -301,8 +302,21 @@ class LinuxTest(Test, LinuxSSHMixIn):
>          self.vm.add_args('-smp', '2')
>          self.vm.add_args('-m', '1024')
>          self.set_up_boot()
> +        if ssh_pubkey is None:
> +            ssh_pubkey, self.ssh_key = self.set_up_existing_ssh_keys()
>          self.set_up_cloudinit(ssh_pubkey)
>
> +    def set_up_existing_ssh_keys(self):
> +        ssh_public_key = os.path.join(SOURCE_DIR, 'tests', 'keys', 'id_rsa.pub')
> +        source_private_key = os.path.join(SOURCE_DIR, 'tests', 'keys', 'id_rsa')
> +        ssh_dir = os.path.join(self.workdir, '.ssh')
> +        os.mkdir(ssh_dir, mode=0o700)
> +        ssh_private_key = os.path.join(ssh_dir,
> +                                       os.path.basename(source_private_key))
> +        shutil.copyfile(source_private_key, ssh_private_key)
> +        os.chmod(ssh_private_key, 0o600)
> +        return (ssh_public_key, ssh_private_key)
> +
>      def download_boot(self):
>          self.log.debug('Looking for and selecting a qemu-img binary to be '
>                         'used to create the bootable snapshot image')
> --
> 2.25.4
>
>
Willian Rampazzo Feb. 15, 2021, 7:25 p.m. UTC | #2
On Wed, Feb 3, 2021 at 2:25 PM Cleber Rosa <crosa@redhat.com> wrote:
>
> It's questionable wether it's necessary to create one brand new pair
> for each test.  It's not questionable that it takes less time and
> resources to just use the keys available at "tests/keys" that exist
> for that exact reason.
>
> If a location for the public key is not given explicitly, the
> LinuxTest will now set up the existing pair of keys as the default.
> This removes the need for a lot of boiler plate code.
>
> To avoid the ssh client from erroring on permission issues, a
> directory with restricive permissions is created for the private key.
> This should still be a lot cheaper than creating a new key.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/avocado_qemu/__init__.py | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>
Cleber Rosa Feb. 16, 2021, 3:28 a.m. UTC | #3
On Thu, Feb 11, 2021 at 02:15:32PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Feb 3, 2021 at 10:07 PM Cleber Rosa <crosa@redhat.com> wrote:
> >
> > It's questionable wether it's necessary to create one brand new pair
> 
> whether
>

Yep, thanks!

> > for each test.  It's not questionable that it takes less time and
> > resources to just use the keys available at "tests/keys" that exist
> > for that exact reason.
> >
> > If a location for the public key is not given explicitly, the
> > LinuxTest will now set up the existing pair of keys as the default.
> > This removes the need for a lot of boiler plate code.
> 
> boilerplate
>

You're right again.

> >
> > To avoid the ssh client from erroring on permission issues, a
> > directory with restricive permissions is created for the private key.
> 
> restrictive
>

OK.



> > This should still be a lot cheaper than creating a new key.
> >
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> 
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>

Thanks for the review,
- Cleber.
diff mbox series

Patch

diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index 8156224625..5f4dd6b9ec 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -11,6 +11,7 @@ 
 import logging
 import os
 import re
+import shutil
 import sys
 import uuid
 import tempfile
@@ -301,8 +302,21 @@  class LinuxTest(Test, LinuxSSHMixIn):
         self.vm.add_args('-smp', '2')
         self.vm.add_args('-m', '1024')
         self.set_up_boot()
+        if ssh_pubkey is None:
+            ssh_pubkey, self.ssh_key = self.set_up_existing_ssh_keys()
         self.set_up_cloudinit(ssh_pubkey)
 
+    def set_up_existing_ssh_keys(self):
+        ssh_public_key = os.path.join(SOURCE_DIR, 'tests', 'keys', 'id_rsa.pub')
+        source_private_key = os.path.join(SOURCE_DIR, 'tests', 'keys', 'id_rsa')
+        ssh_dir = os.path.join(self.workdir, '.ssh')
+        os.mkdir(ssh_dir, mode=0o700)
+        ssh_private_key = os.path.join(ssh_dir,
+                                       os.path.basename(source_private_key))
+        shutil.copyfile(source_private_key, ssh_private_key)
+        os.chmod(ssh_private_key, 0o600)
+        return (ssh_public_key, ssh_private_key)
+
     def download_boot(self):
         self.log.debug('Looking for and selecting a qemu-img binary to be '
                        'used to create the bootable snapshot image')