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.

Sign up for GitHub

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

Jump to bottom

ec2_metadata_facts: Add support to query instance tags in metadata #1186

Merged
merged 9 commits into from
Oct 27, 2022
Merged

ec2_metadata_facts: Add support to query instance tags in metadata #1186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b331533
Add support to query instance tags in metadata
mandar242 Oct 20, 2022
ca5d6c1
Sanity fix
mandar242 Oct 20, 2022
bae65ad
minor fix
mandar242 Oct 20, 2022
d875b4e
Return empty list if instance_metadata_tags disabled
mandar242 Oct 24, 2022
cef2142
Added changelogs fragment
mandar242 Oct 24, 2022
c9dc206
Add required min botocore version to integration tests
mandar242 Oct 24, 2022
7944783
Add required min botocore version to integration tests
mandar242 Oct 24, 2022
783b934
Add required min botocore version to integration tests
mandar242 Oct 25, 2022
9545aae
Add version_added
mandar242 Oct 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/1186-ec2_metadata_facts-query-instance-metadata-tags.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
minor_changes:
- ec2_metadata_facts - added support to query instance tags in metadata (https://github.com/ansible-collections/amazon.aws/pull/1186).
25 changes: 24 additions & 1 deletion plugins/modules/ec2_metadata_facts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,14 @@
description: The purchasing option of the instance.
type: str
sample: "on-demand"
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.
Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

type: list
elements: str
sample: ["tagKey1", "tag_key2"]
version_added: 5.1.0
ansible_ec2_instance_type:
description: The type of the instance.
type: str
Expand Down Expand Up @@ -445,14 +453,25 @@
class Ec2Metadata(object):
ec2_metadata_token_uri = 'http://169.254.169.254/latest/api/token'
ec2_metadata_uri = 'http://169.254.169.254/latest/meta-data/'
ec2_metadata_instance_tags_uri = 'http://169.254.169.254/latest/meta-data/tags/instance'
ec2_sshdata_uri = 'http://169.254.169.254/latest/meta-data/public-keys/0/openssh-key'
ec2_userdata_uri = 'http://169.254.169.254/latest/user-data/'
ec2_dynamicdata_uri = 'http://169.254.169.254/latest/dynamic/'

def __init__(self, module, ec2_metadata_token_uri=None, ec2_metadata_uri=None, ec2_sshdata_uri=None, ec2_userdata_uri=None, ec2_dynamicdata_uri=None):
def __init__(
self,
module,
ec2_metadata_token_uri=None,
ec2_metadata_uri=None,
ec2_metadata_instance_tags_uri=None,
ec2_sshdata_uri=None,
ec2_userdata_uri=None,
ec2_dynamicdata_uri=None,
):
self.module = module
self.uri_token = ec2_metadata_token_uri or self.ec2_metadata_token_uri
self.uri_meta = ec2_metadata_uri or self.ec2_metadata_uri
self.uri_instance_tags = ec2_metadata_instance_tags_uri or self.ec2_metadata_instance_tags_uri
self.uri_user = ec2_userdata_uri or self.ec2_userdata_uri
self.uri_ssh = ec2_sshdata_uri or self.ec2_sshdata_uri
self.uri_dynamic = ec2_dynamicdata_uri or self.ec2_dynamicdata_uri
Expand Down Expand Up @@ -576,6 +595,10 @@ def run(self):
data.update(dyndata)
data = self.fix_invalid_varnames(data)

instance_tags_keys = self._fetch(self.uri_instance_tags)
Copy link
Member

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?

Copy link
Member

@goneri goneri Oct 24, 2022 •
edited
Loading

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)

Copy link
Member

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",

Copy link
Contributor Author

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?

Copy link
Contributor Author

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

Copy link
Member

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.

Copy link
Contributor Author

@mandar242 mandar242 Oct 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Fixed @gravesm

Copy link
Member

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.

instance_tags_keys = instance_tags_keys.split('\n') if instance_tags_keys != "None" else []
data[self._prefix % 'instance_tags_keys'] = instance_tags_keys

# Maintain old key for backwards compatibility
if 'ansible_ec2_instance_identity_document_region' in data:
data['ansible_ec2_placement_region'] = data['ansible_ec2_instance_identity_document_region']
Expand Down
8 changes: 6 additions & 2 deletions tests/integration/targets/ec2_metadata_facts/meta/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
dependencies:
- setup_ec2_facts
- setup_sshkey
- setup_ec2_facts
- setup_sshkey
#required for run_instances with MetadataOptions.InstanceMetadataTags
- role: setup_botocore_pip
vars:
botocore_version: '1.23.30'
13 changes: 12 additions & 1 deletion tests/integration/targets/ec2_metadata_facts/playbooks/setup.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@
- include_role:
name: '../setup_ec2_facts'

- include_role:
name: '../setup_botocore_pip'
vars:
botocore_version: '1.23.30'

- set_fact:
availability_zone: '{{ ec2_availability_zone_names[0] }}'

Expand Down Expand Up @@ -115,9 +120,15 @@
network:
assign_public_ip: true
delete_on_termination: true
metadata_options:
Copy link
Member

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()).

instance_metadata_tags: enabled
tags:
snake_case_key: a_snake_case_value
camelCaseKey: aCamelCaseValue
wait: True

register: ec2_instance
vars:
ansible_python_interpreter: "{{ botocore_virtualenv_interpreter }}"

- set_fact:
ec2_instance_id: "{{ ec2_instance.instances[0].instance_id }}"
Expand Down
2 changes: 2 additions & 0 deletions tests/integration/targets/ec2_metadata_facts/playbooks/test_metadata.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,5 @@
- ansible_ec2_placement_availability_zone == availability_zone
- ansible_ec2_security_groups == "{{ resource_prefix }}-sg"
- ansible_ec2_user_data == "None"
- ansible_ec2_instance_tags_keys is defined
- ansible_ec2_instance_tags_keys | length == 3