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

Add sops vars plugin #59641

Closed
wants to merge 6 commits into from
Closed

Conversation

endorama
Copy link
Contributor

@endorama endorama commented Jul 26, 2019

SUMMARY

Add a new vars plugin for using sops to loading sops-encrypted vars files.

Fixes #36982

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

plugin

ADDITIONAL INFORMATION

This mimic the behaviour of host_group_vars, loading all variables from files with specified file extensions (default are [".sops.yml", ".sops.yaml", ".sops.json"]).

This plugin require the sops command to be available in the path.

Superseed #38843: #38843 PR has a Python implementation (more a reimplementation in Python, as currently sops is a golang program). This one instead relies on the presence of the sops executable delegating the hard work to it, being only a wrapper to integrate the command execution in the Ansible context.

Related to #59639

@ansibot ansibot added affects_2.9 This issue/PR affects Ansible v2.9 core_review In order to be merged, this PR must follow the core review workflow. feature This issue/PR relates to a feature request. has_issue needs_triage Needs a first human triage before being processed. new_plugin This PR includes a new plugin. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Jul 26, 2019
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Jul 26, 2019
@samdoran samdoran removed the needs_triage Needs a first human triage before being processed. label Jul 30, 2019
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Aug 7, 2019
@endorama endorama mentioned this pull request Aug 8, 2019
@cameronelliott
Copy link

cameronelliott commented Sep 28, 2019

@sivel @jwitko @endorama @samdoran
Actually, I commented on the wrong issue, but they both seem super useful.

The one PR I have used a little bit is this one, which provides sops encrypted var support.
But, I suspect as I learn more Ansible, I would probably want to use the lookup feature also.
Anyway, I just want to offer my small vote for the usefulness of these two PRs!
Sorry if I am like a bull in a china shop, I am newer to Ansible.

vars:
#36982
#59641

lookup:
#47770
#59639

Copy link
Contributor

@mhumeSF mhumeSF left a comment

This is awesome! Facing same issue here with the decrypt_with_sops Popen function needing the encoding attribute.

lib/ansible/plugins/vars/sops_vars.py Show resolved Hide resolved
Copy link
Contributor

@mhumeSF mhumeSF left a comment

👍

lib/ansible/plugins/vars/sops_vars.py Outdated Show resolved Hide resolved
lib/ansible/plugins/vars/sops_vars.py Outdated Show resolved Hide resolved
lib/ansible/plugins/vars/sops_vars.py Outdated Show resolved Hide resolved
lib/ansible/plugins/vars/sops_vars.py Show resolved Hide resolved
@jamescarr
Copy link

jamescarr commented Jan 8, 2020

Any update on this? Looks really useful.

Co-Authored-By: eenblam <eenblam@users.noreply.github.com>
@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jan 23, 2020
Co-Authored-By: Felix Fontein <felix@fontein.de>
@endorama
Copy link
Contributor Author

endorama commented Jan 23, 2020

Everyone, sorry for the long delay! I completely missed the notification about the requested changes! :(

Very sorry for this. I'll keep an eye on this to have everything ready for merge as soon as possible!

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Feb 1, 2020
@hd-deman
Copy link

hd-deman commented Feb 19, 2020

How to deal with include_vars module? https://docs.ansible.com/ansible/latest/modules/include_vars_module.html

@felixfontein
Copy link
Contributor

felixfontein commented Feb 19, 2020

@hd-deman parts of the functionality can be done with the sops lookup plugin from PR #59639 combined with set_facts.

@felixfontein
Copy link
Contributor

felixfontein commented Feb 23, 2020

(Another option would be to add an action plugin include_sops_vars.)

@mattclay mattclay added the ci_verified Changes made in this PR are causing tests to fail. label Feb 24, 2020
@hd-deman
Copy link

hd-deman commented Feb 25, 2020

include_sops_vars option looks much better, because set_facts has lower priority than
include_vars.


https://docs.ansible.com/ansible/latest/user_guide/playbooks_variables.html
> Here is the order of precedence from least to greatest (the last listed variables winning prioritization):
> 
> command line values (eg “-u user”)
> role defaults [1]
> inventory file or script group vars [2]
> inventory group_vars/all [3]
> playbook group_vars/all [3]
> inventory group_vars/* [3]
> playbook group_vars/* [3]
> inventory file or script host vars [2]
> inventory host_vars/* [3]
> playbook host_vars/* [3]
> host facts / cached set_facts [4]
> play vars
> play vars_prompt
> play vars_files
> role vars (defined in role/vars/main.yml)
> block vars (only for tasks in block)
> task vars (only for the task)
> include_vars
> set_facts / registered vars
> role (and include_role) params
> include params
> extra vars (always win precedence)

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Mar 4, 2020
pm98zz-c added a commit to codeenigma/ce-provision that referenced this pull request Mar 17, 2020
@abeluck
Copy link

abeluck commented Apr 16, 2020

I tested this plugin, replacing the found_files section with the code from @mikhail-nikitin and it works well.. except:

The plugin is called for every task. This causes a play to run incredibly slowly. (this has nothing to do with @mikhail-nikitin 's proposal)

We use sops with PGP encrypted secrets. Each call to decrypt_with_sops takes several seconds as it needs to make a round trip to a PGP smart card. This causes every task to take at least 3-4 seconds.

Looking at the code, it only caches the paths of the file. This seems like a strange thing to cache considering it takes several orders of magnitude more time to perform the decryption step than it does to look for files on disk.

Adding a cache for the contents itself fixes the issue and makes plays run at normal speed again.

I added:

# up around the FOUND = {} line
DECRYPTED = {}

...
# down in VarsModule
...
                    for found in found_files:
                        if cache and found in DECRYPTED:
                            file_content = DECRYPTED[found]
                        else:
                            file_content = decrypt_with_sops(found)
                            DECRYPTED[found] = file_content
                        new_data = loader.load(file_content)
                        if new_data:  # ignore empty files
                            data = combine_vars(data, new_data)
...

@danihodovic
Copy link
Contributor

danihodovic commented May 19, 2020

@endorama has this PR been abandoned? 😿

@sivel
Copy link
Member

sivel commented May 19, 2020

We are no longer accepting new plugins in this repository. This PR would need to be resubmitted to https://github.com/ansible-collections/community.general for potential consideration.

@felixfontein
Copy link
Contributor

felixfontein commented May 19, 2020

If someone wants to move this PR (and the other sops PR) to community.general, I'd be happy to help reviewing and eventually merging.

@mhumeSF
Copy link
Contributor

mhumeSF commented May 20, 2020

@felixfontein I opened up ansible-collections/community.general#376. The lookup plugin made since, though will vars plugins go into this repo? Seems sops would be the first.

@felixfontein
Copy link
Contributor

felixfontein commented May 20, 2020

Both plugins should go into that repo. Let's get the lookup plugin merged first, since it already has tests.

@cognifloyd
Copy link
Contributor

cognifloyd commented May 20, 2020

Instead of dumping them in the community.general collection, make a new sops collection. That way, the sops plugins can be very clearly versioned and released without regard to everything else in that community collection.

@hho
Copy link

hho commented Jun 10, 2020

That would've been a great idea, especially because the dependency on a SOPS version, but the lookup plugin has now already been merged to community.general 😟

@felixfontein
Copy link
Contributor

felixfontein commented Jun 10, 2020

community.general has not yet been released, so it can easily be removed again. The main question is who will create and maintain a sops specific collection.

@jimi-c
Copy link
Member

jimi-c commented Jun 10, 2020

Based on the conversation above, it appears this should be closed here now?

@endorama
Copy link
Contributor Author

endorama commented Jun 12, 2020

Hello, I'm sorry I completely missed the notifications on this one 😞

@danihodovic I'm still keeping an eye at it, with delays

@felixfontein may I propose myself as a maintainer? I already wrote the code and I keep using it at my company.

I agree that a separate collection would probably help in agility and maintenance.

How would you prefer I move this forward? Shall this collection be under ansible-collections GitHub org? I can start from creating it in my account and then transferring if that's the place it should be.

@gundalow
Copy link
Contributor

gundalow commented Jun 12, 2020

Hi
I help look after the Community Collections at Ansible.
It's great to see the discussion here and see people working together.

  1. As @felixfontein mentioned above community.general hasn't been released yet, so we still have time to move plugins between different collections

  2. I'm OK with there being a new dedicated community.sops collection under https://github.com/ansible-collections. Examples of similar collections hosted here would be community.grafana and community.mongodb

  3. If we have a few people that would be interested in being maintainers of this new collection, you can request a new repo setup via Request a new repo for a Collection.

  4. Once the repo has been setup the SOPS files and PRs can be moved from here (ansible/ansible) and community.general into the new repo. We can put in basic CI (ansible-test sanity) to start with, then extend to include integration tests against SOPS over time. For example community.grafana test against multiple Grafana versions https://github.com/ansible-collections/community.grafana/blob/12c83a278ff0e752dd5266766d1925d2e01bdddd/.github/workflows/ansible-test.yml#L56

@felixfontein
Copy link
Contributor

felixfontein commented Jun 13, 2020

@felixfontein may I propose myself as a maintainer? I already wrote the code and I keep using it at my company.

For the plugin you already are a maintainer, since you are listed as the author. (The bot also looks as the author field if it contains GitHub usernames, it will treat these users automatically as maintainers if they haven't been explicitly put on the ignore list.)

Once the ansible-community/community.sops repo exists, I will move the plugin over. If you prefer another location (in theory you could put it whereever you want, though you'd have to use another namespace then) I can handle the removal part from community.general.

Also note that we should have a first proper release (0.2.0) of community.general next week (a "couple of days" after ansible-base 2.10 beta is released). While before 1.0.0 removing stuff is still ok, it's nicer if we manage to do the move before the 0.2.0 release so the plugin isn't leaving traces in the changelog.

@felixfontein
Copy link
Contributor

felixfontein commented Jun 15, 2020

@endorama if you don't want the sops lookup to stay in community.general for some time, you should really apply for the community.sops repo now.

@endorama
Copy link
Contributor Author

endorama commented Jun 16, 2020

@felixfontein done.

@felixfontein
Copy link
Contributor

felixfontein commented Jun 16, 2020

The lookup plugin has been moved to the new collection (ansible-collections/community.sops#1, ansible-collections/community.general#518), and I've closed the PR for the vars plugin in community.general (ansible-collections/community.general#376).

@bcoca
Copy link
Member

bcoca commented Jun 17, 2020

closing as per above

@bcoca bcoca closed this Jun 17, 2020
@ansible ansible locked and limited conversation to collaborators Jul 15, 2020
@samdoran
Copy link
Contributor

samdoran commented Jul 22, 2020

This PR moved to ansible-collections/community.sops#6

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.9 This issue/PR affects Ansible v2.9 ci_verified Changes made in this PR are causing tests to fail. feature This issue/PR relates to a feature request. has_issue needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. new_plugin This PR includes a new plugin. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integration with Mozilla SOPS for encrypted vars