feat: Add basic Yor support for default tags #3177
Conversation
…fig/simple tags env var
| } | ||
| } | ||
|
|
||
| sort.Strings(result) |
There was a problem hiding this comment.
This makes the output deterministic so we don't get random tests failing
|
|
||
| tags := make(map[string]string) | ||
|
|
||
| // external tags (e.g. yor) |
There was a problem hiding this comment.
is this the right precedence for yor tags? they will be "overwritten" by default tags and any explicitly defined tags?
There was a problem hiding this comment.
Yeah, I think this makes sense for the first customer use-case - we effectively only fall back to Yor tags if the code doesn't already specify them.
There was a problem hiding this comment.
but is that how terraform applies them? I would have guessed yor puts tags directly on resources so they would take precedence over default tags.
There was a problem hiding this comment.
Yeah I think that's fair. After testing yor locally, it seems like the precedence is actually: yor tags, resource tags, default tags. Since the yor "default" tags actually overwrite any resource tags I have.
There was a problem hiding this comment.
Yor overrides even regular tags - when it runs it rewrites the .tf files with the specified tag values. This is something we could do in future, but Yor gets a lot more complicated with conditionals etc., so for now we wanted to simply support taking values from the config to fall back to, regardless of conditionals etc.
But yes, it would be nice to fully support this in future, where we override defaults and tags with the Yor ones, and ideally only when conditionals match too. Yor is written in Go so we could probably just call it when a config is specified and run it "for real" once we have a Go runner.
There was a problem hiding this comment.
(sorry, wrote that before your comment appeared Alistair)
There was a problem hiding this comment.
Writing a PR to take Yor over the other tags now 👍
There was a problem hiding this comment.
I see your point -- if we're not actually supporting yor fully it seems safer to let other tags take precedence. I'm ok if you want to keep it as is, maybe just add the above as a comment explaining why they are the lowest precedence
| t.Setenv("YOR_SIMPLE_TAGS", `{"Team": "Engineering"}`) | ||
| GoldenFileCommandTest(t, testutil.CalcGoldenFileTestdataDirName(), []string{"breakdown", "--config-file", "./testdata/infracost-config-yor.yml", "--format", "json"}, &GoldenFileOptions{ | ||
| IsJSON: true, | ||
| RegexFilter: regexp.MustCompile(`"path": "[^"]*",`), |
There was a problem hiding this comment.
I'm curious why the
JSONInclude: regexp.MustCompile("^(defaultTags|tags|name)$"),
JSONExclude: regexp.MustCompile("^(costComponents|pastBreakdown)$"),
didn't work for you? It would think it made the golden files a lost less noisy?
There was a problem hiding this comment.
My main problem was misunderstanding how they worked tbh, I think they would work if I listed needed properties in the include - I had tried .* in include and path in exclude which failed to exclude path because an ancestor had already been included.
There was a problem hiding this comment.
I guess it's "Show me only keys that match JSONInclude anywhere they appear in the tree, unless they appear under a key that matches JSONExclude"
Adds basic Yor support to pull default tags values from a yaml config/
$YOR_SIMPLE_TAGSenv var.e.g. the following Yor config file will result in tags
AandBbeing applied to all resources which support tags.These are treated slightly differently from default tags in that they are still applied even if traditional default tags are not supported by the provider version/constraints.