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

bug: handle lowercase name tag when explicitly defining Name tag #802

Closed
wants to merge 1 commit into from

Conversation

sgtoj
Copy link
Sponsor Contributor

@sgtoj sgtoj commented Aug 7, 2023

what

  • gracefully handle lowercase name tag when explicitly defining Name tag
    • setting name to null means the provider ignore the key-value pair and therefore creates no conflicts with Name

why

  • aws provider (least with latest provider) does not allow both name and Name keys to exists for tags variable

references

  • n/a

@sgtoj sgtoj requested review from a team as code owners August 7, 2023 12:35
Copy link

@bateller bateller left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

Copy link
Sponsor Contributor

@Nuru Nuru left a comment

Choose a reason for hiding this comment

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

@sgtoj Can you please explain this use case? This solution only handles the name tag, but you would have the same problem if you included other tags where the keys were distinct under case-sensitive matches but identical under case-insensitive matches. So unless we have a comprehensive solution, I'm inclined to leave it to users to clean up their inputs.

@sgtoj
Copy link
Sponsor Contributor Author

sgtoj commented Aug 9, 2023

So unless we have a comprehensive solution, I'm inclined to leave it to users to clean up their inputs.

Yes. Whenever label_key_case is set to lower in the global stack file, then both name and Name will exists in the situations this issue resolves.

I also realize sometimes Name is intentional even if all other tags are normalized to lowercase because the AWS web console will use the value of Name (must be upper case) as the name of resource if it exists.

The solution addresses Name/name becuase that's the only tag explicitly being defined and does so without using processing through cloudposse/label/null module (so it can be normalized per configuration).

@Nuru Nuru added the wontfix This will not be worked on label Aug 9, 2023
@Nuru
Copy link
Sponsor Contributor

Nuru commented Aug 9, 2023

I am closing this as "wontfix" because it necessarily adds warts to the code without solving the underlying problem. @sgtoj Feel free to open an issue on null-label about filtering out conflicting tags.

@Nuru Nuru closed this Aug 9, 2023
@sgtoj
Copy link
Sponsor Contributor Author

sgtoj commented Aug 9, 2023

@Nuru - I dont think this solves the problem outlined in the OP that exists now this repo's current state. The 3 components cannot be deployed as is if label_key_case is set to lower. At the very least, the tags should be normalized in via null-label instead of explicitly setting Name.

Opening in issue on the null-label repo is a good idea nevertheless, but for different reasons.

@sgtoj
Copy link
Sponsor Contributor Author

sgtoj commented Aug 9, 2023

@Nuru - I have no issue submitting an updated PR that normalizes the tags as I suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants