Allow to tell sops lookup to return '' for non-existant files

[felixfontein]

Motivation

This makes implementing things a lot easier. Instead of first having to do a stat to see if it really exists, one can simply specify empty_on_not_exist=true to the lookup and obtain an empty string instead of an error.

I'm using this in https://github.com/felixfontein/ansible-acme/ (I'm right now working on adding sops-encrypted keys support).

[codecov]

Codecov Report

Merging #33 (c6b396f) into main (810b4c1) will increase coverage by 0.15%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #33      +/-   ##
==========================================
+ Coverage   86.29%   86.45%   +0.15%     
==========================================
  Files           6        6              
  Lines         343      347       +4     
  Branches       61       62       +1     
==========================================
+ Hits          296      300       +4     
  Misses         32       32              
  Partials       15       15              

Impacted Files	Coverage Δ
plugins/lookup/sops.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 810b4c1...c6b396f. Read the comment docs.

[felixfontein]

FYI, this is a full example using the new option for the sops lookup and sops_encrypted, together with the new openssl_privatekey_pipe module from community.crypto: https://github.com/felixfontein/ansible-acme/blob/40ed248c7d3b18e604bf6f35aa9d9bdf56377b82/roles/acme_certificate/tasks/main.yml#L71-L101

[endorama]

👍 I did something very similar for our internal use in my company, so I'm definitely going to keep an eye on felixfontein/ansible-acme

[endorama]

I was wondering if it make sense to update the behaviour instead of adding a new one. I don't think it would break compatibility as due to how it is implemented right now (you must perform a stat before, and with that safe guard you would not run the plugin at all).

What do you think?

[felixfontein]

Hmm, I'm generally no fan of eating up errors when not explicitly being asked so. For calling roles/playbooks, it would be surprising (IMO) if the community.sops.sops lookup returns '' for non-existant files, while the ansible.builtin.file lookup errors out.

Just image if you're writing a private key to a location on the remote server (by combining copy module with the sops lookup), and because you made a typo in the filename you don't get the key written there but an empty file, and you only notice that when some service doesn't come back up after a restart.

[felixfontein]

@endorama what do you think of releasing 0.2.0 soon? I'd really like to be able to use this new option without having to install from git (or even from this branch) :)

[felixfontein]

@endorama can you be chance look at this PR again? (Maybe even at the others?) It would be really awesome to have this in a release, it's a lot easier to use than installing collections from a git repo branch (which doesn't work for Ansible 2.9).

[endorama]

@felixfontein I'm sorry for the long delay, but these notifications were buried under others so I missed them.

Hmm, I'm generally no fan of eating up errors when not explicitly being asked so. For calling roles/playbooks, it would be surprising (IMO) if the community.sops.sops lookup returns '' for non-existant files, while the ansible.builtin.file lookup errors out.

I pretty much agree with this.

From the liked usage in felixfontein/ansible-acme this change would allow using | default(omit, true) pattern, which is not now possible due to the triggered error, is this correct?

I'm specifically referring to these lines: https://github.com/felixfontein/ansible-acme/blob/40ed248c7d3b18e604bf6f35aa9d9bdf56377b82/roles/acme_certificate/tasks/main.yml#L76-L80

This looks a clean and useful, so I'm definitely ok with the change. I also agree is better to have it under a specific option to explicit the behaviour.

[felixfontein]

From the liked usage in felixfontein/ansible-acme this change would allow using | default(omit, true) pattern, which is not now possible due to the triggered error, is this correct?

Exactly. Otherwise you first would have to use a set_fact task with register and ignore_errors: true, and then in a next task check whether set_fact failed or not, and depending on that use the new fact or omit. Which is not really fun :)

[felixfontein]

@endorama thanks for reviewing this one as well!

[felixfontein]

@endorama do you mind if I release a 0.2.0 version of the collection? (Or do you want to do that yourself?)