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

update offhours tag restrictions #8808

Merged

Conversation

connorSanderson
Copy link
Contributor

Update escape characters to allow the hyphen character to be escaped for resources which do not allow it as a tag value. For example, ecs services' tags.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 8, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@kapilt
Copy link
Collaborator

kapilt commented Aug 14, 2023

if this is ecs specific it feels like should handle it specific to that type, rather than force the default for resources where it is compatible. although its not clear if we do that with rds and others, tbd.

@kentnsw
Copy link
Collaborator

kentnsw commented Sep 8, 2023

@connorSanderson @kapilt

Just want to add my understanding here. The "escape" feature in c7n is "decode" only, no "encode". It is to "decode" those escaped characters in tag value back to normal, eg, from u28 to (.

If the ( is supported by the tag value, user does not have to escape it, eg, in EC2 tag value, we can use normal string like off=(M-F,19);on=(M-F,7). But when comes to RDS, user has to "encode" ( to u28 to adapt to the restriction.

So IMHO, this PR looks good.

@kapilt
Copy link
Collaborator

kapilt commented Sep 17, 2023

thanks @kentnsw for the explanation

Copy link
Collaborator

@kapilt kapilt left a comment

Choose a reason for hiding this comment

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

lgtm

@kapilt kapilt merged commit 81f7577 into cloud-custodian:main Sep 17, 2023
21 checks passed
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.

None yet

3 participants