Add a content parameter to aws_s3 put operation

[onitake]

SUMMARY

This is a port of the original pull request at ansible/ansible#67091 to collections. There might still be compatibility issues due to renamed packages.

Add support for uploading S3 objects from in-memory content in addition to on-disk files.

Fixes ansible/ansible#67086

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

aws_s3

ADDITIONAL INFORMATION

Example usage:

- name: PUT operation from a rendered template
  aws_s3:
    bucket: mybucket
    object: /object.yaml
    content: "{{ lookup('template', 'templates/object.yaml.j2') }}"
    mode: put

[onitake]

Comment from original PR:

Hmm... Looks like strings aren't autocast into byte strings.
If possible, I'd like to keep the possibility to pass a binary data into the content parameter. Is this possible at all? Should I change the content parameter type to string?

[onitake]

Another comment:

So, I added a unit test for the content parameter and it looks like the feature's working as expected. Shippable had a timeout in a completely unrelated test, perhaps a retry is in order.

I changed the content parameter type to string, as I don't see a way to pass byte strings into Ansible modules. If there's a way to do this, please tell me. The string is converted to UTF-8 before sending it to S3.

What the unit tests don't cover yet is a multi-part put. This is probably crucial, as the the new calculate_etag_content utility function will not have full test coverage otherwise. I'll try to address this.

[tremble]

Thanks for migrating this over. I've not tested this locally, but some some minor comments

[onitake]

I also noticed that there a ton of % formats in the module.
Since Python 2 is now EOL, wouldn't it also be good to change them into f-strings or at least str.format()?

[onitake]

I addressed your comments and also fixed a bug with etag calculation that popped up thanks to the idempotency test.

My other two comments regarding dest checking and refactoring % into format() have nothing to do with this PR, so please disregard them.

[tremble]

Short verison on % : Yes we probably should (although I don't always remember)

The only trouble with testing that dest exists is that you need to check that the parent folder exists since you may be creating the target. (but it would be worth tweaking that to type=path too)

[tremble]

@jillr What are we supposed to be using for 'version_added' at the minute?

[jillr]

@tremble the collection version the patch will land in, using semver. We should be targeting version_added: 1.1.0 right now.

[jillr]

Regarding python2, Ansible will support 2.7 as long as we have supported platforms that ship with 2.7 (like RHEL). So it would be good to update these but it's not urgent.

[jillr]

@onitake following up on your question:

If possible, I'd like to keep the possibility to pass a binary data into the content parameter. Is this possible at all? Should I change the content parameter type to string?

Unfortunately there's not any good ways to take multiple types of content in the same parameter. The best way to handle this would be to have a content parameter that is for str data, and another option such as base64_content for bytes data.

[abadger]

And the base64_content parameter would take a base64 encoded byte string (not a raw byte string). There's no way in ansible to pass a raw byte string through all the layers.

[onitake]

Thanks for the suggestions, I will add such a parameter.

[onitake]

I implemented the mentioned content_base64 parameter, but now we we have a bit of asymmetry:
There is no getbin mode to obtain binary object contents without writing to disk.
I think this is an acceptable tradeoff, but if you want it added as well, I'd rather submit a separate PR for that feature.

[ansibullbot]

cc @lwade @s-hertel @wimnat
click here for bot help

[tremble]

Many thanks for your efforts on this. I'm sorry it's taken so long.

Given how long you've been waiting on us for this I've added a changelog fragment and tweaked the 'added_in' value.

[onitake]

Thanks for taking time to review this & merging it!