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

Allow the characters _:./+- to be used in tag keys during deploy #1727

Closed
wants to merge 1 commit into from

Conversation

RobReus
Copy link

@RobReus RobReus commented Jan 15, 2020

Why is this change necessary?
Using the aws cloudformation deploy --tags command, you are allowed to use these characters in the tag keys. Documentation also lists these characters as supported and allowed. With sam deploy --tags, it did not work properly due to a regex limiting supported characters to A-Za-z0-9.

How does it address the issue?
By adding a regex specifically for tag keys. By using this new regex, sam deploy --tags 'owner:name=son of anton' no longer converts the tag key to name instead of owner:name.
Before:

{"name": "son of anton"}

After:

{"owner:name": "son of anton"}

What side effects does this change have?
None. More characters allowed in keys and fully backwards compatible.

Checklist:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@sriram-mv sriram-mv self-requested a review January 15, 2020 18:14
Copy link
Contributor

@sriram-mv sriram-mv left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! This looks good 👍

@vladb-metapack
Copy link

Hi,

I was trying to tag my entire stack with some tags which include colons in them
and using sam deploy to deploy them (sam version 0.40 and awscli version 1.16.230)
and had the same issue as this. Everything before the : including the : itself was removed in the tags.

I have upgraded the aws versions and downgraded sam to 0.15 which enabled me to use this, but alas for safety reasons we went ahead with just using aws cloudformation deploy.

@RobReus
Copy link
Author

RobReus commented Jan 16, 2020

Hi,

I was trying to tag my entire stack with some tags which include colons in them
and using sam deploy to deploy them (sam version 0.40 and awscli version 1.16.230)
and had the same issue as this. Everything before the : including the : itself was removed in the tags.

I have upgraded the aws versions and downgraded sam to 0.15 which enabled me to use this, but alas for safety reasons we went ahead with just using aws cloudformation deploy.

This was the exact reason for me to create this PR. As per AWS best practices, we too use colons to separate between parts of the tag keys. I got tired of having to use sam package and then aws cloudformation deploy.

@sriram-mv I just realized I did not make the regex a bit nicer. I left some other allowed characters out (like @) and did not add the constraints of the key value (must start and end with A-Za-z0-9). Would you want me to fix this before merging this PR?

@sriram-mv
Copy link
Contributor

@RobReus Yes please, I think its a good idea to add some more constraints on this.

@sriram-mv
Copy link
Contributor

Closing because I was not able to push to the fork.

#1798 is the new PR.

@sriram-mv sriram-mv closed this Feb 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants