Add a feature to lookup a file, and get it. #10917
Conversation
ansibullbot
added
lookup
This comment was marked as outdated.
This comment was marked as outdated.
ansibullbot
added
ci_verified
ansibullbot
removed
ci_verified
felixfontein
added
the
check-before-release
There was a problem hiding this comment.
Hi @rdemongeot # #
Thank you for the contribution! Please add a changelog fragment.
I left a couple of comments.
| content: "{{ lookup('community.general.bitwarden', 'VPN Config', attachment='vpn-server.key') | first }}" | ||
| dest: /etc/vpn/server.key | ||
| mode: '0600' |
There was a problem hiding this comment.
Not necessarily a problem, but I can't help remembering that the lookup plugins run on the controller, whilst the copy task (unless otherwise set) will run on the target(s). This feature has potential to make a dent on the network bandwidth, depending on the file size and number of targets. It would be nice to have at least a warning in the documentation, for new and possibly unwise users.
That begin said, when slurping a file, the result comes encoded as base64 - I wonder why that is so and whether it would make sense to make that in this case as well - take this with a large grain of salt, I have no hard evidence to support any claim.
There was a problem hiding this comment.
A comment was putted just now.
Don't sure that store file from bitwarden is the better idea; but works for now, with couple of stored files.
For network traffic, from Ansible computer to target, any copy will generate network traffic. In this case we'll add from bitwarden/vaultwarden to ansible computer.
Can you verify if i'm correct or not? Regards, |
| @@ -0,0 +1,2 @@ | |||
| minor_changes: | |||
| - bitwarden - add availbility to get Attachment file from bitwarden (https://github.com/ansible-collections/community.general/pull/10917). | |||
There was a problem hiding this comment.
| - bitwarden - add availbility to get Attachment file from bitwarden (https://github.com/ansible-collections/community.general/pull/10917). | |
| - bitwarden lookup plugin - add availbility to get attachment file from bitwarden (https://github.com/ansible-collections/community.general/pull/10917). |
| @@ -0,0 +1,2 @@ | |||
| minor_changes: | |||
| - bitwarden lookup plugin - add availbility to get attachment file from bitwarden (https://github.com/ansible-collections/community.general/pull/10917). | |||
There was a problem hiding this comment.
Do you mean "ability" or "availability" (an "a" is missing for that)? I think the former makes more sense:
| - bitwarden lookup plugin - add availbility to get attachment file from bitwarden (https://github.com/ansible-collections/community.general/pull/10917). | |
| - bitwarden lookup plugin - add ability to get attachment file from bitwarden (https://github.com/ansible-collections/community.general/pull/10917). |
There was a problem hiding this comment.
It is a bit odd as "availability", indeed.
| attachment: | ||
| description: | ||
| - Name of the attachment to download from the item. | ||
| - When set, the plugin will download the attachment content in raw format. |
There was a problem hiding this comment.
What does "raw format" mean? Do you mean "without further processing"? What if the file is binary data (Ansible doesn't like to handle binary data, one usually encodes it as Base64 because of that)?
| - name: "Save attachment to file" | ||
| ansible.builtin.copy: | ||
| content: "{{ lookup('community.general.bitwarden', 'VPN Config', attachment='vpn-server.key') | first }}" |
There was a problem hiding this comment.
While this isn't the first time | first was used in the examples: what's the goal of using | first here? Does this always return a list of more than one elemnts? If yes, | first is fine. If not, | first is problematic because lookup() itself is problematic - if the lookup plugin returns a one-element list, it automatically takes the single element and returns that instead of the list. (And I think you get None or "" if the list is empty - forgot which one.) Usually it's better to use query(), since that will always return the list, even if it has zero or one elements.
|
|
||
| try: | ||
| params = ['get', 'attachment', attachment_name, '--itemid', item_id, '--raw'] | ||
| out, err = self._run(params) |
There was a problem hiding this comment.
out and err are unicode strings; the output of the CLI is parsed as UTF-8. Is this what is meant by "raw format"?
(Can Bitwarden actually store binary files, or must all files be UTF-8 encoded text?)
5 participants
SUMMARY
Need to store files into BitWarden / ValtWarden; and get them from Ansible.