Add "tenant", "labels_as_tags", and "descriptors" #132
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
osterman
previously approved these changes
Aug 17, 2021
|
/test all |
aknysh
reviewed
Aug 17, 2021
aknysh
reviewed
Aug 17, 2021
aknysh
reviewed
Aug 17, 2021
korenyoni
previously approved these changes
Aug 17, 2021
Nuru
commented
Aug 17, 2021
Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com> Co-authored-by: Yonatan Koren <me@yonatankoren.com>
|
/test all |
|
/test all |
|
/test all |
Nuru
removed
the
do not merge
aknysh
reviewed
Aug 20, 2021
aknysh
reviewed
Aug 20, 2021
aknysh
reviewed
Aug 20, 2021
aknysh
reviewed
Aug 20, 2021
aknysh
reviewed
Aug 20, 2021
aknysh
reviewed
Aug 20, 2021
aknysh
reviewed
Aug 20, 2021
aknysh
reviewed
Aug 20, 2021
main.tf
Outdated
| } | ||
|
|
||
| default_labels_as_tags = keys(local.tags_context) | ||
| # Unlike other inputs, the first setting of `labels_as_tags` cannot be later overridden | ||
| # We have to cover 2 cases. 1) context does not have a labels_as_tags key, 2) it is present and set to ["default"] |
There was a problem hiding this comment.
a little confused about "default" and "unset" in labels_as_tags
There was a problem hiding this comment.
Hopefully clearer now.
|
/test all |
aknysh
previously approved these changes
Aug 20, 2021
korenyoni
previously requested changes
Aug 20, 2021
README.yaml
Outdated
| becomes the new default for downstream modules. Also, collections are not overwritten, they are merged, | ||
| so once a tag is added, it will remain in the tag set and cannot be removed, although its value can | ||
| be overwritten. | ||
|
|
||
| Because the purpose of `labels_as_tags` is primarily to prevent tags from being generated | ||
| that would conflict with the AWS provider's `default_flags`, it is an exception to the |
There was a problem hiding this comment.
default_flags is a nonexistent AWS Provider argument — I'm sure you meant default_tags.
Suggested change
| that would conflict with the AWS provider's `default_flags`, it is an exception to the | |
| that would conflict with the AWS provider's `default_tags`, it is an exception to the |
Secondly, "conflict with" might suggest that the AWS Provider would somehow error out if the tags exist in two places. At the very least, it doesn't explain which of these two — the AWS Provider or null-label — takes precedent. Consider writing the following, which explains the situation a little more specifically:
Suggested change
| that would conflict with the AWS provider's `default_flags`, it is an exception to the | |
| that would override tags specified in the AWS provider's `default_tags` argument, it is an exception to the |
There was a problem hiding this comment.
Yes, the AWS provider errors out if the same tag is set in two places.
|
/test all |
osterman
approved these changes
Aug 20, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
what
idcomponent:tenantlabels_as_tagscontrols which labels are exported as tagsdescriptor_formatsgenerates new outputdescriptorsterraform-terraform-labelwhy
null-labeltenant,labels_as_tags,descriptor_formats, add additional clarification, stop promoting obsolete module