-
Notifications
You must be signed in to change notification settings - Fork 522
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
fix: propagate default provider tags to volume tags #3070
Conversation
@@ -49,6 +49,14 @@ func ParseTags(defaultTags *map[string]string, r *schema.ResourceData) *map[stri | |||
keysAndValues = append(keysAndValues, k, v) | |||
} | |||
|
|||
if r.Type == "aws_instance" { | |||
for k, v := range *defaultTags { |
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.
Are we sure this works correctly? I didn't think terraform applied default tags to volume tags like this.
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.
yup after some testing with AWS it doesn't seem like AWS propagates default tags to volume_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.
No sorry, default tags are actually propagated to the volume_tags but only in v5.39.0
and above hashicorp/terraform-provider-aws#19890. We'll have to write some logic to detect the provider version and propagate accordingly.
Fixes issue where provider tags were not being used as defaults in volume tags. We now first add these defaults with the `volume_tags` prefix if fetching tags for an applicable resource (`aws_instance`).
@tim775 can you have another look at this - see the updated PR desc for more info. Note: the team decided the propagation of default tags to volume_tags should happen by default unless a user has explicitly specified a version that is below |
Adds a line to clarify the `volume_tags` functionality introduced in infracost/infracost#3070
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 makes me wonder if the tag propogation should be handled in the hcl parsing or in the infracost json like we're doing now. I had been thinking it should probably be done in hcl, but maybe not if that would mean we have to add all the resource and provider specific rules in there.
@tim775 yeah I'm thinking it would be a lot easier/cleaner if we do it in HCL, but for now it's probably just a bit too much. |
Adds a line to clarify the `volume_tags` functionality introduced in infracost/infracost#3070
Fixes issue where provider tags were not being used as defaults in volume tags. Provider defaults propagating to
volume_tags
was introduced in versionv5.39.0
of the aws provider hashicorp/terraform-provider-aws#19890.To accomodate this functionality we add any of the default tags with the
volume_tags
prefix if the current resource supportsvolume_tags
(currently justaws_instance
). This is only done if the version constraint specified at the root levelterraform
require_providers
block is compatible with the5.39.0
version. e.g.>= 5.0
. Versions which specify a maximum version, e.g.~> 3.0
below5.39
will not have the default tags passed to volume tags. If no version constraint is found/can be parsed we add default tags to volume_tags by default. This is to ensure we have no false positive tagging policy failures.