Rename aws_s3 to s3_object (and deprecate bucket creation/deleting) #869
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tremble
added
the
do_not_backport
Docs Build 📝Thank you for contribution!✨ This PR has been merged and your docs changes will be incorporated when they are next published. |
|
I totally agree, |
ansibullbot
added
community_review
feature
This comment was marked as outdated.
This comment was marked as outdated.
tremble
force-pushed
the
rename/s3_object
branch
3 times, most recently
from
2568a8c
to
f8ad593
Compare
This comment was marked as resolved.
This comment was marked as resolved.
tremble
force-pushed
the
rename/s3_object
branch
2 times, most recently
from
4667670
to
2b4fa8c
Compare
…ule and its action plugin...
jatorcasso
pushed a commit
to jatorcasso/amazon.aws
that referenced
this pull request
Jun 27, 2022
…nsible-collections#869) Rename aws_s3 to s3_object (and deprecate bucket creation/deleting) SUMMARY The aws_s3 module (as it's known today) is primarily for managing objects within S3. While it provides minimal support for creating S3 buckets, the feature set is very limited. Support for the advanced bucket management features is provided via the s3_bucket modules (such as managing encryption settings). Because the name aws_s3 often puts the module at the top of the list of modules, well away from the s3_bucket module, it can be difficult for folks to discover the s3_bucket module leading them to assume that we simply have no support for the more complex s3_bucket management features. As such, I suggest renaming the module to s3_object to make the intended scope more obvious and to improve the discoverability of s3_bucket. At this time I do not recommend setting a deprecation date for the alias, the cost of an alias is minimal and we've had a lot of churn recently. Additionally, deprecates the duplicated (but very limited) bucket creation/deletion functionality of aws_s3/s3_object ISSUE TYPE Feature Pull Request COMPONENT NAME aws_s3 (s3_object) ADDITIONAL INFORMATION See for example ansible-collections#866 where there was an attempt to create duplicate functionality. Reviewed-by: Alina Buzachis <None> Reviewed-by: Mark Chappell <None> Reviewed-by: Jill R <None>
1 task
1 task
1 task
1 task
abikouo
pushed a commit
to abikouo/amazon.aws
that referenced
this pull request
Sep 18, 2023
… integration tests (ansible-collections#870) Add support for tagging certificates. Fix deprecated tasks in aws_acm integration tests SUMMARY This PR adds support for configuring arbitrary tags when importing a certificate using the aws_acm module. Previously, it was only possible to set the 'Name' tag. Additionally, this PR fixes issues with the aws_acm integration tests. The integration tests were using deprecated tasks or attributes, such as openssl_certificate. ISSUE TYPE Bugfix Pull Request COMPONENT NAME aws_acm ADDITIONAL INFORMATION Changes to the aws_acm.py module: Add new tags and purge_tags attributes. The certificate_arn attribute is now allowed when state='present'. A playbook should be allowed to modify an existing certificate entry by providing the ARN. For example, a play may want to add, modify, remove tags on an existing certificate. The aws_acm module returns the updated tags. See example below. Refactor aws_acm.py to improve code reuse and make it possible to set arbitrary tags. This should also help to 1) improve readability. 2) prepare for ansible-collections#869 which I am planning to work on next. Backwards-compatibility is retained, even though it might make sense to normalize some of the attributes. Example return value: "certificate": { "arn": "arn:aws:acm:us-west-1:account:certificate/f85abf9d-4bda-4dcc-98c3-770664a68243", "domain_name": "acm1.949058644.ansible.com", "tags": { "Application": "search", "Environment": "development", "Name": "ansible-test-78006277-398b5796f999_949058644_1" } } Integration tests: The openssl_certificate task is deprecated. Migrate to x509_certificate. The signature_algorithms attribute is no longer supported by the new x509_certificate task. Using selfsigned_digest instead. The integration tests for the aws_acm module pass locally. I see ansible/ansible#67788 has been closed, but tests/integration/targets/aws_acm/aliases still has unstable. I am not sure what to do about it. I was able to run the tests in my local workspace after making the above changes. Reviewed-by: Markus Bergholz <git@osuv.de> Reviewed-by: Sebastien Rosset <None> Reviewed-by: Mark Woolley <mw@marknet15.com> Reviewed-by: Alina Buzachis <None>
abikouo
pushed a commit
to abikouo/amazon.aws
that referenced
this pull request
Sep 18, 2023
… integration tests (ansible-collections#870) Add support for tagging certificates. Fix deprecated tasks in aws_acm integration tests SUMMARY This PR adds support for configuring arbitrary tags when importing a certificate using the aws_acm module. Previously, it was only possible to set the 'Name' tag. Additionally, this PR fixes issues with the aws_acm integration tests. The integration tests were using deprecated tasks or attributes, such as openssl_certificate. ISSUE TYPE Bugfix Pull Request COMPONENT NAME aws_acm ADDITIONAL INFORMATION Changes to the aws_acm.py module: Add new tags and purge_tags attributes. The certificate_arn attribute is now allowed when state='present'. A playbook should be allowed to modify an existing certificate entry by providing the ARN. For example, a play may want to add, modify, remove tags on an existing certificate. The aws_acm module returns the updated tags. See example below. Refactor aws_acm.py to improve code reuse and make it possible to set arbitrary tags. This should also help to 1) improve readability. 2) prepare for ansible-collections#869 which I am planning to work on next. Backwards-compatibility is retained, even though it might make sense to normalize some of the attributes. Example return value: "certificate": { "arn": "arn:aws:acm:us-west-1:account:certificate/f85abf9d-4bda-4dcc-98c3-770664a68243", "domain_name": "acm1.949058644.ansible.com", "tags": { "Application": "search", "Environment": "development", "Name": "ansible-test-78006277-398b5796f999_949058644_1" } } Integration tests: The openssl_certificate task is deprecated. Migrate to x509_certificate. The signature_algorithms attribute is no longer supported by the new x509_certificate task. Using selfsigned_digest instead. The integration tests for the aws_acm module pass locally. I see ansible/ansible#67788 has been closed, but tests/integration/targets/aws_acm/aliases still has unstable. I am not sure what to do about it. I was able to run the tests in my local workspace after making the above changes. Reviewed-by: Markus Bergholz <git@osuv.de> Reviewed-by: Sebastien Rosset <None> Reviewed-by: Mark Woolley <mw@marknet15.com> Reviewed-by: Alina Buzachis <None>
abikouo
pushed a commit
to abikouo/amazon.aws
that referenced
this pull request
Oct 24, 2023
… integration tests (ansible-collections#870) Add support for tagging certificates. Fix deprecated tasks in aws_acm integration tests SUMMARY This PR adds support for configuring arbitrary tags when importing a certificate using the aws_acm module. Previously, it was only possible to set the 'Name' tag. Additionally, this PR fixes issues with the aws_acm integration tests. The integration tests were using deprecated tasks or attributes, such as openssl_certificate. ISSUE TYPE Bugfix Pull Request COMPONENT NAME aws_acm ADDITIONAL INFORMATION Changes to the aws_acm.py module: Add new tags and purge_tags attributes. The certificate_arn attribute is now allowed when state='present'. A playbook should be allowed to modify an existing certificate entry by providing the ARN. For example, a play may want to add, modify, remove tags on an existing certificate. The aws_acm module returns the updated tags. See example below. Refactor aws_acm.py to improve code reuse and make it possible to set arbitrary tags. This should also help to 1) improve readability. 2) prepare for ansible-collections#869 which I am planning to work on next. Backwards-compatibility is retained, even though it might make sense to normalize some of the attributes. Example return value: "certificate": { "arn": "arn:aws:acm:us-west-1:account:certificate/f85abf9d-4bda-4dcc-98c3-770664a68243", "domain_name": "acm1.949058644.ansible.com", "tags": { "Application": "search", "Environment": "development", "Name": "ansible-test-78006277-398b5796f999_949058644_1" } } Integration tests: The openssl_certificate task is deprecated. Migrate to x509_certificate. The signature_algorithms attribute is no longer supported by the new x509_certificate task. Using selfsigned_digest instead. The integration tests for the aws_acm module pass locally. I see ansible/ansible#67788 has been closed, but tests/integration/targets/aws_acm/aliases still has unstable. I am not sure what to do about it. I was able to run the tests in my local workspace after making the above changes. Reviewed-by: Markus Bergholz <git@osuv.de> Reviewed-by: Sebastien Rosset <None> Reviewed-by: Mark Woolley <mw@marknet15.com> Reviewed-by: Alina Buzachis <None>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
The
aws_s3module (as it's known today) is primarily for managing objects within S3. While it provides minimal support for creating S3 buckets, the feature set is very limited. Support for the advanced bucket management features is provided via thes3_bucketmodules (such as managing encryption settings).Because the name
aws_s3often puts the module at the top of the list of modules, well away from thes3_bucketmodule, it can be difficult for folks to discover thes3_bucketmodule leading them to assume that we simply have no support for the more complexs3_bucketmanagement features.As such, I suggest renaming the module to
s3_objectto make the intended scope more obvious and to improve the discoverability ofs3_bucket. At this time I do not recommend setting a deprecation date for the alias, the cost of an alias is minimal and we've had a lot of churn recently.Additionally, deprecates the duplicated (but very limited) bucket creation/deletion functionality of
aws_s3/s3_objectISSUE TYPE
COMPONENT NAME
(s3_object)
ADDITIONAL INFORMATION
See for example #866 where there was an attempt to create duplicate functionality.