Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add docker_container_copy_into module #545

Merged
merged 27 commits into from
Jan 9, 2023

Conversation

felixfontein
Copy link
Collaborator

SUMMARY

Create a module to copy files into a container.

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

docker_container_copy_into

@github-actions
Copy link

github-actions bot commented Jan 2, 2023

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and the docs are now incorporated into main:
https://ansible-collections.github.io/community.docker/branch/main

@felixfontein felixfontein changed the title [WIP] Add docker_container_copy_into module Add docker_container_copy_into module Jan 4, 2023
@felixfontein felixfontein marked this pull request as ready for review January 4, 2023 22:26
Copy link
Contributor

@briantist briantist left a comment

Choose a reason for hiding this comment

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

I have two suggestions (that could be done in other PRs):

A content option (a la copy module) for writing small-ish content directly without having an actual source file.

A base64_content (name could be changed) that does similar to content but you supply it base64 pre-encoded content, which is decoded on the target.

plugins/module_utils/copy.py Outdated Show resolved Hide resolved
plugins/modules/docker_container_copy_into.py Outdated Show resolved Hide resolved
@felixfontein felixfontein added the docker-plain plain Docker (no swarm, no compose, no stack) label Jan 6, 2023
@felixfontein
Copy link
Collaborator Author

felixfontein commented Jan 6, 2023

Things left to implement:

  • diff mode
  • force=true (disable idempotency checks)
  • content and content_base64, or content and content_is_b64 options (the latter is used by docker_config and docker_secret)

@felixfontein felixfontein marked this pull request as draft January 6, 2023 14:09
@felixfontein felixfontein marked this pull request as ready for review January 8, 2023 20:52
@felixfontein
Copy link
Collaborator Author

Ok, I think this is ready for review again :)

Copy link
Contributor

@briantist briantist left a comment

Choose a reason for hiding this comment

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

None of my inline questions/comments are blocking, this is great stuff!

plugins/module_utils/copy.py Outdated Show resolved Hide resolved
plugins/modules/docker_container_copy_into.py Outdated Show resolved Hide resolved
content_is_b64=dict(type='bool', default=False),

# Undocumented parameters for use by the action plugin
_max_file_size_for_diff=dict(type='int'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be exposed as a documented/configurable option for the module, which defaults to the Ansible configuration value if not otherwise set? Or do you think it's better not to let this be set independently?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, I think for now I'd like to keep it this way. It can still be renamed / documented at any point.

@felixfontein felixfontein merged commit e198e4a into ansible-collections:main Jan 9, 2023
@felixfontein felixfontein deleted the copy branch January 9, 2023 10:52
@felixfontein
Copy link
Collaborator Author

@briantist thanks a lot for reviewing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docker-plain plain Docker (no swarm, no compose, no stack)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants