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

implement sops decrypt --extract for lookup plugin #200

Merged
merged 11 commits into from
Aug 24, 2024

Conversation

niuleh
Copy link
Contributor

@niuleh niuleh commented Aug 20, 2024

Motivation

We have large encrypted yaml files and would like to lookup only specific keys.

Changes description

Modified the Sops.decrypt function to accept an "extract" option.

Copy link

github-actions bot commented Aug 20, 2024

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and the docs are now incorporated into main:
https://ansible-collections.github.io/community.sops/branch/main

Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! Can you please add a changelog fragment? Thanks.

Also please remove unrelated changes (like line 153 in the lookup and line 94 in the module utils). Thanks.

plugins/lookup/sops.py Outdated Show resolved Hide resolved
plugins/lookup/sops.py Show resolved Hide resolved
plugins/module_utils/sops.py Outdated Show resolved Hide resolved
plugins/module_utils/sops.py Outdated Show resolved Hide resolved
niuleh and others added 6 commits August 20, 2024 20:51
Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Felix Fontein <felix@fontein.de>
* Adds rough explanation of extract syntax.
* Adds extract test case.
@niuleh
Copy link
Contributor Author

niuleh commented Aug 20, 2024

Thanks for your contribution! Can you please add a changelog fragment? Thanks.

Also please remove unrelated changes (like line 153 in the lookup and line 94 in the module utils). Thanks.

@felixfontein Thanks for your review! Not sure if I've adequately addressed the changelog fragment. Also cluess about how to address the failing CI checks, is that about pep8 and pylint linters not liking my code?

changelogs/changelog.yaml Outdated Show resolved Hide resolved
plugins/module_utils/sops.py Outdated Show resolved Hide resolved
plugins/lookup/sops.py Outdated Show resolved Hide resolved
nh_fedora and others added 2 commits August 21, 2024 07:58
description:
- Tell SOPS to extract a specific key from a JSON or YAML file.
- Expects the same 'querystring' syntax as SOPS' C(--encrypt) option,
for example V('["somekey"][0]').
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you sure the single quotes are needed? I'm pretty sure they are not, and in fact will result in an error. In the integration tests you don't have single quotes there either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Quotes are needed, without them sops throws an error the likes of parsing --extract path: strconv.Atoi: parsing "hello": invalid syntax.
What I've learnt is that the entire querystring needs to be quoted, either with single or double quotes. Inside the querystring the other kind of quotes must be used.

What do you think about going with another syntax, I don't like the sops syntax either. A more pythonic syntax like dict.key[0] ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's because if you enter a command in a shell, the shell will also handle quotation marks. But this is not for using SOPS in a shell, but for using Ansible, which calls SOPS without going through a shell.

For example this should work:

    - name: Test extract
      set_fact:
        simple_yaml_with_extract: "{{ lookup('community.sops.sops', 'extract.yaml', extract=extract) }}"
      vars:
        extract: >-
          ['foo']

What do you think about going with another syntax, I don't like the sops syntax either. A more pythonic syntax like dict.key[0] ?

If you want to implement an expression parser / converter, you could do that, but that sounds like quite a lot of work for little gain.

You can also simply extract a specific value from the lookup's return value without having to use the extract option in the first place:

- set_fact:
    value: '{{ (lookup("community.sops.sops", "/path/to/file.yml") | from_yaml).somekey[0] }}'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now I get it, I've updated the description once more to account for this.

Regarding the syntax, imho its more readable than the ansible native | from_yaml version, even if having to escape quotes.

nh_fedora and others added 2 commits August 23, 2024 13:04
* adds link to PR in changelog fragment
* Improve readability of tests

Co-authored-by: Felix Fontein <felix@fontein.de>
@felixfontein felixfontein merged commit f40b008 into ansible-collections:main Aug 24, 2024
65 checks passed
@felixfontein
Copy link
Collaborator

@niuleh thanks a lot for your contribution!

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

Successfully merging this pull request may close these issues.

2 participants