Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ec2 provider: Add support for specifying ssh keypair #2390

Merged
merged 1 commit into from
Oct 23, 2019
Merged

ec2 provider: Add support for specifying ssh keypair #2390

merged 1 commit into from
Oct 23, 2019

Conversation

jmpsf
Copy link
Contributor

@jmpsf jmpsf commented Oct 22, 2019

Right now each time a molecule job is run with the ec2 provider, a new key ssh key is created into AWS with the name molecule_key.

This means that if several CI jobs are run at the same time, they will have a racing condition, and one of them is going to fail because it can't connect to the created instance.

So in this PR, I add the opportunity to use a specific ssh private key, instead of generating one each job.

As there can be multiple platforms I've decided to set up the configuration in the molecule.yml under the driver section.

driver:
  name: ec2
  keypair_path: /path/to/ssh/private/key

Changes are backward compatible and shouldn't affect current users.

PR Type

  • Feature Pull Request

Signed-off-by: Josetxu <jmp@icij.org>
Copy link
Contributor

@decentral1se decentral1se left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, I just have been reminded that this driver has no schema so we cannot valiate the keys 🤕 I don't understand where the task that generates the SSH key is for this driver (for example, here is the Hetzner one:

- name: Create SSH key
user:
name: "{{ lookup('env', 'USER') }}"
generate_ssh_key: true
ssh_key_file: "{{ ssh_path }}"
force: true
register: generated_ssh_key
- name: Register the SSH key name
set_fact:
ssh_key_name: "molecule-generated-{{ 12345 | random | to_uuid }}"
- name: Register SSH key for test instance(s)
hcloud_ssh_key:
name: "{{ ssh_key_name }}"
public_key: "{{ generated_ssh_key.ssh_public_key }}"
state: present
)? A race condition shouldn't be happening because the task should just find an SSH key in place already and use it?

@jmpsf
Copy link
Contributor Author

jmpsf commented Oct 23, 2019

Hi @decentral1se, the problem is that each job uses a different ssh key path, so they are not aware of the existence of previous keys. With my PR I implement the functionality of a global ssh testing key.

This is how I see the race condition.

- name: Test for presence of local keypair
stat:
path: "{{ keypair_path }}"
register: keypair_local

Tests if a keypair exist locally, beware that keypair_path is set at

keypair_path: "{{ lookup('env', 'MOLECULE_EPHEMERAL_DIRECTORY') }}/ssh_key"

So each role will have a different keypair_path.

Then if the ssh key doesn't exist in the keypair_path it deletes the AWS keypair called molecule_key

- name: Delete remote keypair
ec2_key:
name: "{{ keypair_name }}"
state: absent
when: not keypair_local.stat.exists

Then asks AWS to generate a keypair

- name: Create keypair
ec2_key:
name: "{{ keypair_name }}"
register: keypair

And persists it into the keypair_path file

- name: Persist the keypair
copy:
dest: "{{ keypair_path }}"
content: "{{ keypair.key.private_key }}"
mode: 0600
when: keypair.changed

So Imagine the first role is being tested, the ssh key is generated by AWS and is now about to launch the instance.

That's when the second role starts to be tested. as it's keypair_path is different from role 1, there is no ssh_key present there, so it deletes the AWS molecule_key ssh keypair and generates a new one also called molecule_key.

Now the role 1 launches the instance with the AWS molecule_key ssh keypair.

- name: Create molecule instance(s)
ec2:
key_name: "{{ keypair_name }}"

But as it's the ssh keypair of the role 2, once it tries to prepare or converge we get a Permission Denied error.

Cheers

Copy link
Contributor

@decentral1se decentral1se left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid detective work @jmpsf!

LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants