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.

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

Additional tag map isn't actually merged in with the tags #90

Closed
husain-aljamri opened this issue Jan 6, 2020 · 4 comments
Closed

Additional tag map isn't actually merged in with the tags #90

husain-aljamri opened this issue Jan 6, 2020 · 4 comments
Labels
invalid This doesn't seem right

Comments

@husain-aljamri
Copy link

I'v recently updated to TF 0.12 and I noticed that changes to the additional_tag_map variable does nothing. I've also noticed that the additional tags no longer exist in any of our resources. I'm not exactly sure, but seems as though the following line in main.tf is missing a reference to var.additional_tag_map.
The line is where the tags local is being set, and it is:

tags = merge(var.context.tags, local.generated_tags, var.tags)

I believe the correction should be:

tags = merge(var.context.tags, local.generated_tags, var.tags, var.additional_tag_map)

Making this change locally seems to have done the trick for me, but I might be missing something.

@osterman
Copy link
Member

Hrm... if that's the case, it seems to be a bug. Can you check the test/src folder to see if we check for this? If not, we should and we should update the module to merge the tags.

@osterman osterman added enhancement New feature or request help wanted Extra attention is needed labels Aug 10, 2020
@tibbing
Copy link
Contributor

tibbing commented Aug 13, 2020

👍 Doesn't seem that the tests cover this currently. The fix should probably use local.additional_tag_map so we also include any tags from the input context.

@osterman
Copy link
Member

It technically "works as designed", but I agree it's confusing. We can fix that.

See this for some context. #18

I am open to discussion on how to fix this. The original feature was added by @Jamie-BitFlight who can maybe add additional context. Now this is over 2 years ago, so my memory could be totally incorrect.

| additional_tag_map | {} | additional tags that get appended to each map in the list of maps, for the output tags_as_list_of_maps |

So it appears we added this for a narrow use-case specifically with Elastic Beanstalk. ElasticBeanstalk needs tags passed as a [ { }, ... { } ] list of maps. In that special situation we need additional tags passed for beanstalk, so I think we just add it to the tags as list of maps, not tags in general because we don't want them to pollute the normal tags, since we only wanted them on the beanstalk instance. e.g. the security group should not have propagate_at_launch = true.

@Nuru
Copy link
Contributor

Nuru commented Dec 22, 2020

Closing as "works as designed" and documented by #99

@Nuru Nuru closed this as completed Dec 22, 2020
@Nuru Nuru added invalid This doesn't seem right and removed enhancement New feature or request help wanted Extra attention is needed labels Aug 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

4 participants