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

AWS CDK CLI: China partition not taken into account when validating notification Sns Topic #29719

Open
LennartC opened this issue Apr 4, 2024 · 2 comments
Labels
bug This issue is a bug. cli Issues related to the CDK CLI effort/medium Medium work item – several days of effort p2 package/tools Related to AWS CDK Tools or CLI

Comments

@LennartC
Copy link

LennartC commented Apr 4, 2024

Describe the bug

The cdk deploy commands allows you to specify SNS topics for CloudFormation notifications with the --notification-arns option.

However, when deploying in a China region, with a valid SNS Topic in China, the command will error out with:

Notification arn arn:aws-cn:sns:cn-northwest-1:012345678901:topic is not a valid arn for an SNS topic

Expected Behavior

The CLI should allow the China partition (aws-cn) when validating the SNS topic.

Current Behavior

The CLI exits.

Reproduction Steps

cdk deploy --notification-arns arn:aws-cn:sns:cn-northwest-1:012345678901:my-topic

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.135.0

Framework Version

No response

Node.js Version

v18.18.2

OS

Ubuntu

Language

TypeScript

Language Version

No response

Other information

No response

@LennartC LennartC added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 4, 2024
@github-actions github-actions bot added the package/tools Related to AWS CDK Tools or CLI label Apr 4, 2024
@nmussy
Copy link
Contributor

nmussy commented Apr 4, 2024

The validation regex is less than ideal, we should probably switch to using Arn.split instead:

export function validateSnsTopicArn(arn: string): boolean {
return /^arn:aws:sns:[a-z0-9\-]+:[0-9]+:[a-z0-9\-\_]+$/i.test(arn);
}

@tim-finnigan tim-finnigan self-assigned this Apr 4, 2024
@tim-finnigan tim-finnigan added investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed needs-triage This issue or PR still needs to be triaged. labels Apr 4, 2024
@tim-finnigan
Copy link

Thanks for reporting this issue and creating the PR. I agree with the comment above that using split() is probably the better approach to account for partitions in ARNs.

@tim-finnigan tim-finnigan added p2 effort/medium Medium work item – several days of effort and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Apr 4, 2024
@tim-finnigan tim-finnigan removed their assignment Apr 4, 2024
@pahud pahud added the cli Issues related to the CDK CLI label May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. cli Issues related to the CDK CLI effort/medium Medium work item – several days of effort p2 package/tools Related to AWS CDK Tools or CLI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants