#141 creates /root/.ssh/id_rsa.pub if does not exist
Merged 6 years ago by astepano. Opened 6 years ago by bgoncalv.
bgoncalv/standard-test-roles rhts-public-key  into  master

@@ -179,6 +179,11 @@ 

    args:

      creates: /root/.ssh/id_rsa

  

+ - name: Create root SSH public key

+   shell: ssh-keygen -y -f /root/.ssh/id_rsa > /root/.ssh/id_rsa.pub

+   args:

+     creates: /root/.ssh/id_rsa.pub

+ 

  - name: Procure root public key

    shell: cat /root/.ssh/id_rsa.pub

    register: my_pub_key_output

I propose to define a clear and obvious place for generating SSH keys that can be used by different roles.
This code should create necessary keys for test_environment and test_runner, etc.
You propose to create SSH keys in rhts role.
My point is: this ssh keys could be used by different roles.
Imagine a tests.yml that uses two different roles.
Does this imply that each role creates a keypair for self?
Can we put this code to "str-common" role?
There is other way to create a SSH keypiars at: https://pagure.io/standard-test-roles/blob/master/f/inventory/standard-inventory-qcow2#_18 (but this is crated by preparation scripts).

We could probably create keypair in str-common, but rhts role does not use str-common.

My fix was just to solve an specific problem where the server has private key, but does not have public key. Missing public key caused rhts role to fail. I did not see the same issue when running beakerlib or basic role.

I see your point. Okay. Lets do this step together with first command. I see this two commands as one logical step. Private and public keys are bound together. And task name has "key pair".

- name: Create root SSH key pair
  shell: |
      ssh-keygen -q -t rsa -N '' -f /root/.ssh/id_rsa
      ssh-keygen -y -f /root/.ssh/id_rsa > /root/.ssh/id_rsa.pub
  args:
    creates: /root/.ssh/id_rsa

I see your point. Okay. Lets do this step together with first command. I see this two commands as one logical step. Private and public keys are bound together. And task name has "key pair".
- name: Create root SSH key pair
shell: |
ssh-keygen -q -t rsa -N '' -f /root/.ssh/id_rsa
ssh-keygen -y -f /root/.ssh/id_rsa > /root/.ssh/id_rsa.pub
args:
creates: /root/.ssh/id_rsa

The problem is this does not work.
This shell command will only execute if /root/.ssh/id_rsa does not exist. The issue my patch fixes is when /root/.ssh/id_rsa exists, but /root/.ssh/id_rsa.pub does not. The new task creates the public key using the private one executing ssh-keygen -y -f /root/.ssh/id_rsa > /root/.ssh/id_rsa.pub if /root/.ssh/id_rsa.pub does not exist.

@bgoncalv OK if you see this tasks as two independent, and something is creating /root/.ssh/id_rsa without /root/.ssh/id_rsa.pub ok!

Please change job names, and I am fine with merge it. Thank you!

- name: Create root SSH key pair

to

- name: Create root SSH private key

and

- name:  Create root SSH public key if it does not exist

to:

- name: Create root SSH public key

Actually the task names seem appropriate for me.

task: Create root SSH key pair the shell command will create private and public keys if server does not have private key.

task: Create root SSH public key if it does not exist will create a public key from the private key in case server only has private key and not a public key.
The shell command here will only be executed if script from Create root SSH key pair did not run as server already has private key, but for some strange reason it does not have a public key.

Why do specify a condition in task name?

We have:

- name: Create root SSH key pair

This task has condition

We do not write task name as: Create root SSH key pair if it is not exist.

Many Ansible tasks have different conditions. Check how many times we specify when.

Let's keep clean code.

Ansible will mark step as skipped. This will explain that key is present.

rebased onto c015781

6 years ago

Commit 6ae32fe fixes this pull-request

Pull-Request has been merged by astepano

6 years ago

Pull-Request has been merged by astepano

6 years ago