Skip to content
This repository has been archived by the owner on Oct 30, 2018. It is now read-only.

Added the binary module for writing binary files to the filesystem. #142

Closed
wants to merge 2 commits into from

Conversation

deberon
Copy link

@deberon deberon commented Dec 10, 2014

The binary module accepts a base64 string and a destination parameter. With these it writes bytes out to the file system. While these bytes could technically be ASCII or Unicode (or other text-based encoding), the purpose of this module is to write out binary files to the file system. This allows a base64 string to be checked into version control or, as will be more likely, into group vars.

In my case the base64 string was included in the group vars in Ansible Tower and allowed me to write out separate binary files for separate groups. I submitted this request to the dev mailing list and did not hear any other way to accomplish what I needed. I figured I would submit a pull request and let somebody decide whether it was common enough of a task that a module should be included with Ansible.

@abadger
Copy link
Contributor

abadger commented Dec 10, 2014

tl;dr: This could be improved by making it more like the copy module (able to set owner and permissions for instance) but it's not clear if it needs to be its own module

Longer thoughts:

@bcoca and I discussed this a bit today. I think if we did take this it should be similar in parameters it handles to the copy module with the content parameter. But that also begs the question of whether there's some way to do this with the current copy module and the content= parameter. bcoca nad I explored using the b64decode jinja filter but there seem to be an unknown number of bugs standing in the way of that working whereas this just works.

import shutil
import tempfile
import base64
import hashlib
Copy link
Contributor

Choose a reason for hiding this comment

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

When possible, modules should be compatible with python2.4. hashlib is more recent than that. So using something like this is more appropriate:

try:
    from haslib import sha1
except ImportError:
    from sha import sha as sha1

Also note -- if we're just checksumming to confirm the file is the same we should use sha1. That's because sha1 is available when hashlib is not and md5 (the other hash in python-2.4's stdlib) is not available on systems running in FIPS-140-2 compliance mode.

@bcoca
Copy link
Member

bcoca commented Dec 10, 2014

actually, my thoughts is that this should be done in the template module and just have a filter internally.

any point at which ansible itself needs to handle the binary data it will be a problem.

@bcoca
Copy link
Member

bcoca commented Dec 10, 2014

why not dump the base64 decoded data at the master and then just use copy?

@deberon
Copy link
Author

deberon commented Dec 10, 2014

Would it make sense to have a "base64" or "base64_content" parameter for the copy module that explicitly accepted base64 encoded strings? Or should we try to shoe-horn it into the "content" parameter?

To me it makes sense to have this be a part of the copy module, since all the mechanisms are already in place for that to do the mode, owner, etc. But I'm just a new guy :)

@cchurch
Copy link
Contributor

cchurch commented Dec 11, 2014

@bcoca and @abadger: Earlier discussion from ansible-devel: https://groups.google.com/d/topic/ansible-devel/lR0k2S0GIQw/discussion

@abadger
Copy link
Contributor

abadger commented Dec 14, 2014

@danderson189 I think what @bcoca would really like to know is why you can't just put the binary content into a file and use ansible to copy the file to the remote system instead of storing a base64 version of the data inside of the playbook.

Regarding content and base64_content, I've been told that content is seen as a misfeature of the copy module and we would like to deprecate it if possible. template is what people should be using for putting content from a playbook into a file. As you found out with the filter (and I have similar experiences from other areas), templating with non-text has an unknown number of problems we'd have to overcome before it works, though. My gut feeling from dealing with python template modules in the past is that we'll have to play tricks internally to round trip bytes through jinja2 (setting its encoding to latin-1 or another encoding that includes all possible byte values). I think trying to implement that inside of template has a high probability of being a long-standing source of bugs (when someone touches the code they'll have to think of what happens when the code is dealing with textual-strings vs byte-strings. They won't be able to use things like string.split() or len(string) without accounting for both possibilities, etc) but I could be wrong and it really is just a matter of converting in a couple places and not caring anywhere else. The best way to tell is probably to come up with a modified version of template and see how many places have to be changed to handle binary data. if the changes are strewn throughout that code, that means that it will be easy to introduce bugs at a later date. If the changes are localized to only two or so areas then it's an indication that we either wrote stellar code to begin with (not possible ;-) or that it isn't so hard to deal with both bytes and text in the same module.

@deberon
Copy link
Author

deberon commented Dec 15, 2014

@abadger thanks for the very in-depth response. The base64 string is actually not a part of the playbook. Instead it is stored as a variable as part of the inventory in Tower. Each inventory has it's own base64 string that is used to write out environment specific corosync keys.

I'm a brand-new contributor so while I'm very happy being a part of this discussion, I'm not sure which suggestion I should follow. I'm currently solving a problem in my environment with a custom module, and I would like to instead use something that's a part of the official project as soon as possible.

@abadger
Copy link
Contributor

abadger commented Dec 16, 2014

@danderson189 Instead of storing the base64 string in a variable in tower and then using your binary module to write that out as a binary file can you upload the file itself to tower and then use a variable to tell ansible which file to copy to the remote machine for this host? I think that's the workaround that @bcoca is getting at.

As for ways forward for this module, I think we'd need to know if it's possible to make this be more similar to the template module as that seems to add something that we don't already have whereas writing a whole file as binary is something that should already be possible via copy.

If using a file in tower and the copy module doesn't work for your use case, it becomes more of a priority to make a decision about this module's fate whereas if a binary file and copy does work it might not be time-effective to rework this module to be more like template (at least, until someone else has a use case where copying a file doesn't work for them).

@deberon
Copy link
Author

deberon commented Dec 17, 2014

I personally like the idea of adding the functionality to an existing template since it will be able to leverage other properties of the module (mode, owner, etc.) In my use case, uploading the file to tower doesn't really work for me because Tower manages multiple inventories which use the same playbooks (dev, staging, prod, etc.) As I said earlier, I have a working solution with my custom module, so I'm in no rush. I would rather get things implemented the correct way. To me, modifying the "content" parameter of the copy module seems like the most fitting place to start.

@tiwoc
Copy link

tiwoc commented Feb 12, 2015

This is also an issue when using Ansible Vault to securely store secrets along with the playbook in git. If the secret is a binary file (e.g. a Java KeyStore), it cannot be stored in Vault without encoding it as text. A module which worked just as the copy module but allowed to feed it a variable containing base64 contents would solve the problem.

@sjagoe
Copy link

sjagoe commented Mar 31, 2015

I have run into an issue with deploying binary file content as well. My use case is deploying binary content fetched from etcd. I have not fully investigated the cause of failure with the copy module.

I tried the following scenario, which lands up mangling the file contents:

  • Use a lookup('etcd', 'key') | b64decode to fetch the value
  • Use copy: content="{{ now_binary_content }}" to deploy

I have worked around the issue by doing the following:

  • Use a lookup('etcd', 'key') to fetch the value (keeping the base64 encoding)
  • Use copy: content="{{ base64_content }}" as an interim deploy step
  • Use shell: base64 -d {{ base64_filename }} > {{ binary_filename }} to decode the binary file.

@neilk
Copy link

neilk commented Jun 10, 2015

Bumping this; at Sauce Labs we have the same problem. We need to store a binary file securely, so we base64 encoded it and used ansible-vault. But we couldn't decode it in a clean way on the other end. We used a temp file, but would prefer a more direct solution.

@gregdek
Copy link
Contributor

gregdek commented Jun 16, 2015

@bcoca @abadger Unclear on the status of this one: are you guys arguing that this module should be refactored, or that this module should be closed in favor of patches elsewhere? Is there a clear path forward for this PR?

@alex
Copy link

alex commented Jul 15, 2015

This would also be useful for a usecase I have. Is there a way I can install files/binary.py in my local ansible repo?

@deberon
Copy link
Author

deberon commented Jul 17, 2015

@alex you can always put this in your ansible library (defined in ansible.cfg) and use it that way. I didn't have any luck extending the copy module because of restrictions on the "content" parameter. So I'm back to square one with this module. If the maintainers still do not want to merge this in I will go ahead and withdraw this PR.

@deberon
Copy link
Author

deberon commented Jul 17, 2015

I just noticed a comment on my commit to update which hashing library I am using. I have made this change but I just need to test it. I will get it pushed ASAP.

@abadger
Copy link
Contributor

abadger commented Jul 28, 2015

Finally had an in-depth discussion about this today and decided that we won't be accepting this module into extras although there's nothing wrong with using it as a custom module (and if you want you could make a role out of it and upload it to galaxy.ansible.com so that others can use it easily as well). The reason is that we think that it duplicates the functionality already present by using the copy module and different filenames.

As an example, instead of storing the per-host binary content in an inventory variable, we'd store a filename prefix in inventory like this:

roan.lan per_host_prefix=roan

Then in our playbook we could reference the binary file like this:

- copy:
    src: "{{ per_host_prefix }}-source-file.bin"
    dest: "/path/on/destination"

Enhancements and variants on this idea could be achieved with a role instead of using the inventory variable directly and using a fact like ansible_hostname instead of explicitly specifying a per_host_prefix in inventory.

Sorry it took us so long to arrive at this decision; we had a lot of disparate ideas and viewpoints to work through both in terms of how to support binary files and also what role we wanted the extras repository to play for the project.

@abadger abadger closed this Jul 28, 2015
@alex
Copy link

alex commented Jul 29, 2015

@abadger FWIW, the reason I was interested in this is that copy, as far as I know, is not compatible with ansible-vault. I have some binary files that I want encrypted (JKS specifically :-().

@sjagoe
Copy link

sjagoe commented Jul 29, 2015

I had the same need as @alex. However, my workaround above (#142 (comment)) with base64 seems to be holding. I may test copy: content={{ binary }} again to remove the extra step.

@abadger
Copy link
Contributor

abadger commented Aug 20, 2015

Just got around to looking at this tab in my web browser... Might be a good idea to send your use-cases to the mailing list. Like I said, there were several different viewpoints and maybe one of the other developers will either know of a good way to do that or will change their mind because of it. Pasting the list information below:

Submission Received

Hi!

Thanks very much for your submission to Ansible. It sincerely means a lot to us.

We are interested in this idea and would like to see a wider discussion on it
on one of our lists. Reasons for this include:

  • At the moment, people think that this module would simply duplicate functionality available through using other modules and features of ansible. Bringing up your specific use cases may lead those people to find that the existing solutions are not usable in this case or it might lead to a more elegant way to do it in your playbook. Either way, it should make things better in the end.

Can you please post on ansible-development list so we can talk about this idea with the wider group?

Thank you once again for this and your interest in Ansible!

@tiwoc
Copy link

tiwoc commented Dec 5, 2016

Just a quick update: The copy module supports Vault-encrypted binary files since Ansible 2.1 (released on 2016-05-25). So for anyone who previously needed the base64-to-binary feature for writing out encrypted files, you can do so now in a much easier way, without even base64-encoding your file.

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

Successfully merging this pull request may close these issues.

None yet

9 participants