-
Notifications
You must be signed in to change notification settings - Fork 398
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
ssm_parameter: add support for tags (#1573) #1575
ssm_parameter: add support for tags (#1573) #1575
Conversation
Docs Build 📝Thank you for contribution!✨ This PR has been merged and your docs changes will be incorporated when they are next published. |
Hi @alinabuzachis, I think I've addressed all the comments so far. Is there anything else I can do to help increase the chances of this PR getting merged? |
cc @116davinder @jillr @markuman @mikedlr @nathanwebsterdotme @ozbillwang @s-hertel @tremble |
LGTM |
@@ -208,12 +236,37 @@ | |||
description: Parameter version number | |||
example: 3 | |||
returned: success | |||
tags: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@markuman What if we only return tags as a dictionary rather than a list of dicts and remove tags_dict from the response?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@markuman @alinabuzachis To provide a little context, tags_dict was copied over from plugins/modules/secretsmanager_secret.py. I'd be happy to remove the code if you'd like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
secretsmanager_secret has some compatibility code in there because it originally returned the "boto3 style" (list of dicts) format, rather than the normal simple dictionary.
We often just convert the resource objects the APIs return from CamelCase to snake_case, and add that as part of what the module returns. When AWS suddenly adds support for Tags to a resource and changes what the API returns, we start returning the list-of-dict style tags, and need to go through a deprecation cycle before we can return the simple dict.
Since returning tags is new to this module, we can skip the "list-of-dict" and just return the "dict" format as "tags".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(There are also some cases where reviews simply missed that the module originally returned the boto3 style tags)
- We're trying to tidy this up :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks for this, and I'm sorry we missed responding in time to get this into 5.2.0. For the sake of getting this merged I'll update the version information and drop the boto3-style tags.
plugins/modules/ssm_parameter.py
Outdated
- amazon.aws.tags | ||
|
||
notes: | ||
- Support for I(tags) and I(purge_tags) was added in release 5.2.0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Support for I(tags) and I(purge_tags) was added in release 5.2.0. | |
- Support for I(tags) and I(purge_tags) was added in release 5.3.0. |
@@ -208,12 +236,37 @@ | |||
description: Parameter version number | |||
example: 3 | |||
returned: success | |||
tags: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
secretsmanager_secret has some compatibility code in there because it originally returned the "boto3 style" (list of dicts) format, rather than the normal simple dictionary.
We often just convert the resource objects the APIs return from CamelCase to snake_case, and add that as part of what the module returns. When AWS suddenly adds support for Tags to a resource and changes what the API returns, we start returning the list-of-dict style tags, and need to go through a deprecation cycle before we can return the simple dict.
Since returning tags is new to this module, we can skip the "list-of-dict" and just return the "dict" format as "tags".
23e57e4
to
7642cee
Compare
Backport to stable-5: 💚 backport PR created✅ Backport PR branch: Backported as #1691 🤖 @patchback |
ssm_parameter: add support for tags (#1573) SUMMARY Adding support for tags following community guidelines and other practices from other modules. secretsmanager_secret was used along with helper functions from ec2 code. Addresses open issue for feature request #1573. ISSUE TYPE Feature Pull Request COMPONENT NAME ssm_parameter ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis <None> Reviewed-by: Michael Haskell (mikehas) <None> Reviewed-by: Dennis Qian <None> Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Mark Chappell <None> (cherry picked from commit 7a2b5d9)
@mikehas thanks for taking the time to submit this PR, I'm sorry it got stuck for a while. |
[PR #1575/7a2b5d98 backport][stable-5] ssm_parameter: add support for tags (#1573) This is a backport of PR #1575 as merged into main (7a2b5d9). SUMMARY Adding support for tags following community guidelines and other practices from other modules. secretsmanager_secret was used along with helper functions from ec2 code. Addresses open issue for feature request #1573. ISSUE TYPE Feature Pull Request COMPONENT NAME ssm_parameter ADDITIONAL INFORMATION Reviewed-by: Mark Chappell <None>
… is compressed (ansible-collections#1575) ec2_metadata_facts - Handle decompression when EC2 instance user-data is compressed SUMMARY Handle decompression when user-data is compressed. The fetch_url method from ansible.module_utils.urls does not decompress the user-data because the header is missing (The API does not set 'Content-Encoding' = 'gzip'). ISSUE TYPE Bugfix Pull Request COMPONENT NAME ec2_metadata_facts ADDITIONAL INFORMATION Reviewed-by: Mike Graves <[email protected]> Reviewed-by: Mark Chappell Reviewed-by: Alina Buzachis
SUMMARY
Adding support for tags following community guidelines and other practices from other modules. secretsmanager_secret was used along with helper functions from ec2 code. Addresses open issue for feature request #1573.
ISSUE TYPE
COMPONENT NAME
ssm_parameter
ADDITIONAL INFORMATION