TerryHowe / ansible-modules-hashivault

Ansible module for Hashicorp Vault.
https://pypi.python.org/pypi/ansible-modules-hashivault
MIT License
454 stars 158 forks source link

hashivault_read_to_file does a base64 decode. #219

Closed jamielennox closed 4 years ago

jamielennox commented 4 years ago

In the middle of hashivault_read_to_file it calls b64decode.

https://github.com/TerryHowe/ansible-modules-hashivault/blob/259ae6bcac83759442ea59d407841df3a2aa7ab4/ansible/plugins/action/hashivault_read_to_file.py#L74

That's an interesting assumption that probably makes sense in one of the secret engines, however it massively messes up trying to fetch things like a CA certficate from a PKI engine.

What's this assumption, can we have a flag that says don't do this?

jamielennox commented 4 years ago

Note if i just remove the base64.b64decode i get

TypeError: a bytes-like object is required, not 'AnsibleUnsafeText'

I have to replace it with a:

local_tmp.write(content.encode('utf-8'))

and i get the actual secret/content written to file.

jamielennox commented 4 years ago

Ok - i see now it's a partner to the hashivault_write_to_file which base64 encodes on upload. IMO it's useful on it's own as i'm going to do this a lot, but ok.

TerryHowe commented 4 years ago

Dude! Good to hear from you, looks like you moved.

So, you are looking for a no encode/decode upload? I've been feeling for a while those to_file/from_file modules should be deprecated and parameters added to the read/write modules to handle file I/O.

jamielennox commented 4 years ago

Hey! I did catch the name and was going to say hi, just wasn't sure an issue report was the best place. Yep, finally got back to doing some development :)

So I worked around it, and it's maybe the easiest way to just let people do that themselves.

Workaround is either to use a lookup or just multiple steps:

- name: Fetch CA certificates
  become: yes
  become_user: root
  copy:
    dest: "{{ item.dest | mandatory }}"
    content: "{{ lookup('hashivault', item.secret, 'certificate') }}"
    owner: "{{ item.owner | default('root') }}"
    group: "{{ item.group | default('root') }}"
    mode: "{{ item.mode | default('0644') }}"
  with_items: "{{ vault_certificates_cas }}"

Mainly the thing that caught me was that the module was called read_to_file, which is exactly what i was trying to do, and then ended up with garbage in the file.

So I mean i think you could improve the module, just add a base64: true default to read and write that can be flagged off, but yea given the amount of ways you might want to use that file maybe just deprecation is easier.

jamielennox commented 4 years ago

Going to close this as having thought about it i think the lookup plugin is really the right way to do this.

I'd agree that deprecating the to_file,from_file makes sense given you can get more control over that behaviour with standard ansible.