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

[ansible_runner] Add VaultCredential #19002

Merged

Conversation

NickLaMuro
Copy link
Member

@NickLaMuro NickLaMuro commented Jul 18, 2019

Adds Ansible::Runner::VaultCredential.

NOTE: The --vault-password-file is used instead of populating the env/passwords file here in case multiple credentials were used at the same time. This differs from how awx does things:

https://github.com/ansible/awx/blob/1242ee2b/awx/main/tasks.py#L1554

Where the env/passwords file (/usr/bin/expect style) is used, but there is also contextual awareness of all of the passwords being added in awx at the time of writing the file allowing this to work without adding conflicts. In the current case of MIQ, this is done for each credential type but overall scope of the play is missing, so we don't have the context available at when writing the files to ensure there aren't conflicts.

Steps for Testing/QA

Still working on wiring things up in MIQ to provide a way of testing this (this is much easier to test than with cloud credentials), so will update this section and most likely remove the [WIP] label once I do.

Using the following playbook repo:

https://github.com/NickLaMuro/ansible-tower-samples

  1. Enable EmbeddedAnsible
  2. In Automation -> Ansible -> Repositories, add the above repo's master branch
  3. In Automation -> Ansible -> Credentials, add a vault credential with a password of "vault"
  4. Create a catalog item running the hello_world_vault_encrypted.yml playbook
  5. Order the playbook an confirm you get the following output:
PLAY [Hello World Sample (Vault Encrypted)] ************************************
 
TASK [Gathering Facts] *********************************************************
 
ok: [localhost]
 
TASK [Hello Message] ***********************************************************
 
ok: [localhost] => {
    "msg": "Hello World! (NOTE: This message has been encrypted with ansible-vault)"
}
 
PLAY RECAP *********************************************************************
localhost                  : ok=2    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   

@miq-bot miq-bot added the wip label Jul 18, 2019
@NickLaMuro
Copy link
Member Author

@miq-bot add_label changelog/yes, core, embedded ansible, enhancement

cc @carbonin @Fryguy

@NickLaMuro
Copy link
Member Author

So just a quick update, needed a patch to get this to work:

diff --git a/app/models/manageiq/providers/embedded_ansible/automation_manager/configuration_script.rb b/app/models/manageiq/providers/embedded_ansible/automation_manager/configuration_script.rb
index be691cb..d3ef787 100644
--- a/app/models/manageiq/providers/embedded_ansible/automation_manager/configuration_script.rb
+++ b/app/models/manageiq/providers/embedded_ansible/automation_manager/configuration_script.rb
@@ -51,7 +51,7 @@ class ManageIQ::Providers::EmbeddedAnsible::AutomationManager::ConfigurationScri
   end

   def collect_credentials(options)
-    options.values_at(
+    options.symbolize_keys.values_at(
       :credential,
       :cloud_credential,
       :network_credential,

But using this playbook:

https://github.com/NickLaMuro/ansible-tower-samples/blob/9fb895c98159466d86f9fd98d24a29db3590e174/hello_world_vault_encrypted.yml#L1

Everything worked as expected. Will make a PR for the above patch tomorrow (err... later this morning...).

@carbonin
Copy link
Member

@NickLaMuro I'm not sure why you would need that patch ... That should be a HashWithIndifferentAccess based on https://github.com/ManageIQ/manageiq/blob/master/app/models/service_ansible_playbook.rb#L95

@NickLaMuro
Copy link
Member Author

Yup, @carbonin was right. After a out of band chat, we determined it was a combination of misleading debugging and incorrectly adding the vault credential (and didn't have a password).

Going to clean up the PR description, but I think this is mostly ready to be merged after.

@NickLaMuro
Copy link
Member Author

Actually, I am quickly going to change the code to use an environment variable instead (per carboni's suggestion). Back in a bit.

NOTE:  The `--vault-password-file` is used instead of populating the
`env/passwords` file here in case multiple credentials were used at the
same time.  This differs from how `awx` does things:

  https://github.com/ansible/awx/blob/1242ee2b/awx/main/tasks.py#L1554

Where as passwords expect file yaml file is used, but there is also
contextual awareness of all of the passwords being added at the time of
writing the file, where in the current case of MIQ, this is done for
each credential type, so we don't have the context available at when
writing the files.
@NickLaMuro NickLaMuro force-pushed the ansible_runner_vault_credential branch from 8de08a2 to 62dd90c Compare July 18, 2019 16:37
@NickLaMuro NickLaMuro changed the title [WIP][ansible_runner] Add VaultCredential [ansible_runner] Add VaultCredential Jul 18, 2019
@miq-bot miq-bot removed the wip label Jul 18, 2019
@miq-bot
Copy link
Member

miq-bot commented Jul 18, 2019

Checked commit NickLaMuro@62dd90c with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍪

@carbonin carbonin self-assigned this Jul 18, 2019
@carbonin carbonin merged commit 1a1b4a6 into ManageIQ:master Jul 18, 2019
@carbonin carbonin added this to the Sprint 116 Ending Jul 22, 2019 milestone Jul 18, 2019
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.

4 participants