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

fix: code generating enum values containing !, =, ~ are stripped #1738

Merged
merged 3 commits into from
Dec 9, 2023

Conversation

vinayak-kukreja
Copy link
Contributor

This has been fixed in json2jsii. Allowlisting characters !, =, ~.

Fixes #229

Signed-off-by: Vinayak Kukreja <vinakuk@amazon.com>
Signed-off-by: Vinayak Kukreja <vinakuk@amazon.com>
Comment on lines +54 to +58
enum:
- '!='
- =
- =~
- '!~'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are now working. Unfortunately the diff is huge so it does not show up in the web ui. But, you can take this locally and verify it.

This is what it looks now,

/**
 * Match operation available with AlertManager >= v0.22.0 and takes precedence over Regex (deprecated) if non-empty.
 *
 * @schema AlertmanagerConfigSpecInhibitRulesSourceMatchMatchType
 */
export enum AlertmanagerConfigSpecInhibitRulesSourceMatchMatchType {
  /** != */
  VALUE_NOT_EQUALS_TO = \\"!=\\",
  /** = */
  VALUE_ = \\"=\\",
  /** =~ */
  VALUE_EQUAL_TILDE = \\"=~\\",
  /** !~ */
  VALUE_NEGATION_TILDE = \\"!~\\",
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

VALUE_ = \\"=\\",

Fixing this in Json2Jsii now

Choose a reason for hiding this comment

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

What is the purpose of this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So this is a Custom Resource Definition that is failing in the issue mentioned. We can add these to the tests and they create a snapshot that we can verify if the generated code is as expected.

This CRD was failing code generation due to symbols that were not handled correctly by Json2Jsii. So I fixed that first and then allowlisted these in cdk8s.

Since = was missed, the code generation just gave us VALUE_ which is incorrect. So I will update the json2jsii version with the fix you just approved and that should fix this. Until then we cannot merge this.

Choose a reason for hiding this comment

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

lgtm, thanks for explaining. Added a do-not-merge tag, feel free to remove and merge once json2jsii version is updated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great, thank you.

After the update it is,

/**
 * Match operation available with AlertManager >= v0.22.0 and takes precedence over Regex (deprecated) if non-empty.
 *
 * @schema AlertmanagerConfigSpecInhibitRulesSourceMatchMatchType
 */
export enum AlertmanagerConfigSpecInhibitRulesSourceMatchMatchType {
  /** != */
  VALUE_NOT_EQUALS_TO = \\"!=\\",
  /** = */
  VALUE_EQUALS = \\"=\\",
  /** =~ */
  VALUE_EQUAL_TILDE = \\"=~\\",
  /** !~ */
  VALUE_NEGATION_TILDE = \\"!~\\",
}

github-merge-queue bot pushed a commit to cdklabs/json2jsii that referenced this pull request Dec 9, 2023
Fix for: cdk8s-team/cdk8s-cli#1738

Signed-off-by: Vinayak Kukreja <vinakuk@amazon.com>
@vinayak-kukreja vinayak-kukreja added the backport-to-1.x Backport a PR to the 1.x branch label Dec 9, 2023
@paulhcsun paulhcsun added the pr/do-not-merge Do not auto merge the PR label Dec 9, 2023
Signed-off-by: Vinayak Kukreja <vinakuk@amazon.com>
@paulhcsun paulhcsun removed the pr/do-not-merge Do not auto merge the PR label Dec 9, 2023
@mergify mergify bot merged commit 53815be into 2.x Dec 9, 2023
12 checks passed
@mergify mergify bot deleted the vkukreja/fix branch December 9, 2023 00:40
cdk8s-automation pushed a commit that referenced this pull request Dec 9, 2023
This has been fixed in json2jsii. Allowlisting characters `!, =, ~`.

Fixes #229

(cherry picked from commit 53815be)
Signed-off-by: Vinayak Kukreja <78971045+vinayak-kukreja@users.noreply.github.com>
@cdk8s-automation
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
1.x

Questions ?

Please refer to the Backport tool documentation

mergify bot pushed a commit that referenced this pull request Dec 9, 2023
…) (#1739)

# Backport

This will backport the following commits from `2.x` to `1.x`:
 - [fix: code generating enum values containing !, =, ~ are stripped (#1738)](#1738)



### Questions ?
Please refer to the [Backport tool documentation](https://github.com/sqren/backport)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-1.x Backport a PR to the 1.x branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

non alpha-numeric enum values cannot be imported
3 participants