Skip to content
This repository has been archived by the owner on May 18, 2024. It is now read-only.

imdsv2 and tags - fixes #61

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

BenCodeOcean
Copy link
Contributor

Some fixes are suggested:

  1. Referring to the new imdsv2 feature, a few critical bugs require the fixes listed below.
  2. Reordering the tags handling to support any pattern of tags' keys and values.

Description of changes:

  1. IMDSV2 value is stored in the config file instead of a temporary environment variable.
  2. The config file rendering part moved before the initialize function calling to allow the function to use the IMDSV2 config parameter.
  3. Using eval instead of echo in the get_metadata() function.
  4. Separation between instance tags and ebs autoscale tags.
  5. Changing the way that the resource-id tag is added.
  6. Changing the --tag-specification tag to --tag-specifications.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@@ -32,19 +32,19 @@ function get_metadata() {
local key=$1
local metadata_ip='169.254.169.254'

if [ ! -z "$IMDSV2" ]; then
if [ ! -z "$(get_config_value .imdsv2)" ]; then
local token=$(curl -s -X PUT "http://$metadata_ip/latest/api/token" -H "X-aws-ec2-metadata-token-ttl-seconds: 60")
local token_wrapper='-H "X-aws-ec2-metadata-token: $token"'
Copy link

Choose a reason for hiding this comment

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

I'm testing this locally and it doesn't expand out properly when using single quotes.

> token_wrapper='-H "X-aws-ec2-metadata-token: $token"'
> echo $token_wrapper
-H "X-aws-ec2-metadata-token: $token"

Changing it to this works

> token_wrapper='-H "X-aws-ec2-metadata-token: '"$token"'"'
> echo $token_wrapper
-H "X-aws-ec2-metadata-token: <snip>"

The AMI I'm using is ami-0c6ee50d15e7364d4

This shows up in cloudwatch

image

Choose a reason for hiding this comment

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

Nevermind, I see that eval fixes it...

@nathanthorpe
Copy link

@wleepang would you be able to review this?

@microbioticajon
Copy link

Ah!!! I didnt see this PR till just now. I ran into the same issue of $token not expanding properly. My fix is slightly different: #62

@microbioticajon microbioticajon mentioned this pull request May 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants