-
Notifications
You must be signed in to change notification settings - Fork 348
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
ec2_metadata_facts: Add support to query instance tags in metadata #1186
ec2_metadata_facts: Add support to query instance tags in metadata #1186
Conversation
Docs Build 📝Thank you for contribution!✨ This PR has been merged and your docs changes will be incorporated when they are next published. |
This comment was marked as outdated.
This comment was marked as outdated.
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.
This also needs a changelog fragment.
@@ -576,6 +594,10 @@ def run(self): | |||
data.update(dyndata) | |||
data = self.fix_invalid_varnames(data) | |||
|
|||
instance_tags_keys = self._fetch(self.uri_instance_tags) | |||
instance_tags_keys = instance_tags_keys.split('\n') if '\n' in instance_tags_keys else [instance_tags_keys] |
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.
Unless I'm missing something here you can just do:
instance_tag_keys = instance_tags_keys.split("\n")
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.
oh right, fixed.
@@ -576,6 +594,10 @@ def run(self): | |||
data.update(dyndata) | |||
data = self.fix_invalid_varnames(data) | |||
|
|||
instance_tags_keys = self._fetch(self.uri_instance_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.
My understanding is that tags are not enabled on instance metadata by default. Isn't this going to fail in that case?
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.
This is what I get if tag is disabled (the default):
"ansible_ec2_instance_tags_keys": [
"None"
],
Actually, we just need this to update the cache, the key(s) will be automatically included in the final result:
self.fetch(self.uri_instance_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.
For the record, this is what I get with an instance and 2 tags:
"ansible_ec2_tags_instance_Name": "test-goneri-tag",
"ansible_ec2_tags_instance_foo": "var",
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.
@gravesm , it doesn't fail, it returns str
"None" but I wasn't sure how to handle it
in case if the instance metadata tags are enabled, we are returning a list
of tag keys like "ansible_ec2_instance_tags_keys": [ "Name", "snake_case_key" ]
, so I decided to return None
in a list like "ansible_ec2_instance_tags_keys": [ "None" ]
, to maintain the output data type.
should we want to return ansible_ec2_instance_tags_keys: "None"
instead?
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.
For the record, this is what I get with an instance and 2 tags:
"ansible_ec2_tags_instance_Name": "test-goneri-tag",
"ansible_ec2_tags_instance_foo": "var",
@goneri , these keys were being returned before the code change in this PR, but there was no single output param to access all the tag keys earlier
with the code change in this PR, we return new output param ansible_ec2_instance_tags_keys
which is a list of str having tag keys
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.
should we want to return ansible_ec2_instance_tags_keys: "None" instead?
I think we should return ansible_ec2_instance_tags_keys: []
, which is what the documentation for the return value suggests it will do already.
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.
Fixed @gravesm
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.
Thank you @mandar242 for the clarification, I was far away.
@@ -115,8 +115,11 @@ | |||
network: | |||
assign_public_ip: true | |||
delete_on_termination: true | |||
metadata_options: |
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.
The instance type is wrong since you need Nitro support (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance-types.html#ec2-nitro-instances). I personally don't care if there is functional test or not since the code follow the same logic than the rest of the facts (assuming here you just use self.fetch()
).
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
recheck |
ansible_ec2_instance_tags_keys: | ||
description: | ||
- The list of tags keys of the instance. | ||
- Returns empty list if access to tags (InstanceMetadataTags) in instance metadata is not enabled. |
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.
Please add version_added: 5.1.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.
Added.
recheck |
…ansible-collections#1186) Tagging - remove default empty dict where purge_tags default is False Depends-On: ansible-collections#844 SUMMARY Deprecate purge_tags=False Remove default of empty dict for tags ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/modules/aws_kms.py plugins/modules/cloudfront_distribution.py plugins/modules/ec2_vpc_vpn.py plugins/modules/rds_param_group.py ADDITIONAL INFORMATION Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Alina Buzachis <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@374bcfd
…nsible-collections#1186) ec2_metadata_facts: Add support to query instance tags in metadata SUMMARY Fixes ansible-collections#684 Added support to be able to query instance tags using ec2_metadata_facts. This PR adds a field in returned ansible_facts named ansible_ec2_instance_tags_keys. Sample "ansible_ec2_instance_tags_keys": [ "Name", "snake_case_key" ], ISSUE TYPE Feature Pull Request COMPONENT NAME ec2_metadata_facts ADDITIONAL INFORMATION Support to enable instance metadata tags already exists in amazon.aws.ec2_instance Reviewed-by: Mike Graves <[email protected]> Reviewed-by: Mandar Kulkarni <[email protected]> Reviewed-by: Gonéri Le Bouder <[email protected]> Reviewed-by: Alina Buzachis <None>
…nsible-collections#1186) ec2_metadata_facts: Add support to query instance tags in metadata SUMMARY Fixes ansible-collections#684 Added support to be able to query instance tags using ec2_metadata_facts. This PR adds a field in returned ansible_facts named ansible_ec2_instance_tags_keys. Sample "ansible_ec2_instance_tags_keys": [ "Name", "snake_case_key" ], ISSUE TYPE Feature Pull Request COMPONENT NAME ec2_metadata_facts ADDITIONAL INFORMATION Support to enable instance metadata tags already exists in amazon.aws.ec2_instance Reviewed-by: Mike Graves <[email protected]> Reviewed-by: Mandar Kulkarni <[email protected]> Reviewed-by: Gonéri Le Bouder <[email protected]> Reviewed-by: Alina Buzachis <None>
…nstance tags in metadata (#1186) (#1488) [manual backport stable-5] ec2_metadata_facts: Add support to query instance tags in metadata (#1186) ec2_metadata_facts: Add support to query instance tags in metadata SUMMARY Fixes #684 Added support to be able to query instance tags using ec2_metadata_facts. This PR adds a field in returned ansible_facts named ansible_ec2_instance_tags_keys. Sample "ansible_ec2_instance_tags_keys": [ "Name", "snake_case_key" ], ISSUE TYPE Feature Pull Request COMPONENT NAME ec2_metadata_facts ADDITIONAL INFORMATION Support to enable instance metadata tags already exists in amazon.aws.ec2_instance Reviewed-by: Mike Graves [email protected] Reviewed-by: Mandar Kulkarni [email protected] Reviewed-by: Gonéri Le Bouder [email protected] Reviewed-by: Alina Buzachis SUMMARY ISSUE TYPE Bugfix Pull Request Docs Pull Request Feature Pull Request New Module Pull Request COMPONENT NAME ADDITIONAL INFORMATION Reviewed-by: Mark Chappell
…ansible-collections#1186) Tagging - remove default empty dict where purge_tags default is False Depends-On: ansible-collections#844 SUMMARY Deprecate purge_tags=False Remove default of empty dict for tags ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/modules/aws_kms.py plugins/modules/cloudfront_distribution.py plugins/modules/ec2_vpc_vpn.py plugins/modules/rds_param_group.py ADDITIONAL INFORMATION Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Alina Buzachis <None>
…ansible-collections#1186) Tagging - remove default empty dict where purge_tags default is False Depends-On: ansible-collections#844 SUMMARY Deprecate purge_tags=False Remove default of empty dict for tags ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/modules/aws_kms.py plugins/modules/cloudfront_distribution.py plugins/modules/ec2_vpc_vpn.py plugins/modules/rds_param_group.py ADDITIONAL INFORMATION Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Alina Buzachis <None>
…ansible-collections#1186) Tagging - remove default empty dict where purge_tags default is False Depends-On: ansible-collections#844 SUMMARY Deprecate purge_tags=False Remove default of empty dict for tags ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/modules/aws_kms.py plugins/modules/cloudfront_distribution.py plugins/modules/ec2_vpc_vpn.py plugins/modules/rds_param_group.py ADDITIONAL INFORMATION Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Alina Buzachis <None>
…ansible-collections#1186) Tagging - remove default empty dict where purge_tags default is False Depends-On: ansible-collections#844 SUMMARY Deprecate purge_tags=False Remove default of empty dict for tags ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/modules/aws_kms.py plugins/modules/cloudfront_distribution.py plugins/modules/ec2_vpc_vpn.py plugins/modules/rds_param_group.py ADDITIONAL INFORMATION Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Alina Buzachis <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@374bcfd
…ansible-collections#1186) Tagging - remove default empty dict where purge_tags default is False Depends-On: ansible-collections#844 SUMMARY Deprecate purge_tags=False Remove default of empty dict for tags ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/modules/aws_kms.py plugins/modules/cloudfront_distribution.py plugins/modules/ec2_vpc_vpn.py plugins/modules/rds_param_group.py ADDITIONAL INFORMATION Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Alina Buzachis <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@374bcfd
…ansible-collections#1186) Tagging - remove default empty dict where purge_tags default is False Depends-On: ansible-collections#844 SUMMARY Deprecate purge_tags=False Remove default of empty dict for tags ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/modules/aws_kms.py plugins/modules/cloudfront_distribution.py plugins/modules/ec2_vpc_vpn.py plugins/modules/rds_param_group.py ADDITIONAL INFORMATION Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Alina Buzachis <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@374bcfd
…ansible-collections#1186) Tagging - remove default empty dict where purge_tags default is False Depends-On: ansible-collections#844 SUMMARY Deprecate purge_tags=False Remove default of empty dict for tags ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/modules/aws_kms.py plugins/modules/cloudfront_distribution.py plugins/modules/ec2_vpc_vpn.py plugins/modules/rds_param_group.py ADDITIONAL INFORMATION Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Alina Buzachis <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@374bcfd
SUMMARY
Fixes #684
Added support to be able to query instance tags using ec2_metadata_facts.
This PR adds a field in returned
ansible_facts
named ansible_ec2_instance_tags_keys.Sample
ISSUE TYPE
COMPONENT NAME
ec2_metadata_facts
ADDITIONAL INFORMATION
Support to enable instance metadata tags already exists in amazon.aws.ec2_instance