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

Error: Resource's path part only allow [a-zA-Z0-9:._-], an optional trailing '+'(module name) #27083

Closed
fd008 opened this issue Sep 10, 2023 · 6 comments · Fixed by #27619
Closed
Labels
bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2 package/tools Related to AWS CDK Tools or CLI

Comments

@fd008
Copy link

fd008 commented Sep 10, 2023

Describe the bug

CDK v2 api gateway won't allow "$" sign in the url, but it "$" is allows using AWS CLI and Console

Error: Resource's path part only allow [a-zA-Z0-9:._-], an optional trailing '+'
and curly braces at the beginning and the end: $test
at validateResourcePathPart

Expected Behavior

No error when using $ in the api path just like AWS CLI or Console

Current Behavior

Error: Resource's path part only allow [a-zA-Z0-9:._-], an optional trailing '+'
and curly braces at the beginning and the end: $process-message
at validateResourcePathPart

Reproduction Steps

const restApi = new RestApi(this, 'restApi', {
deploy: true,
})

restApi.root?.addResource("$test")

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.95.1 (build ae455d8)

Framework Version

No response

Node.js Version

v18.16.1

OS

Ubuntu 22.04.2 LTS

Language

Typescript

Language Version

5.2.2

Other information

No response

@fd008 fd008 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 10, 2023
@github-actions github-actions bot added the package/tools Related to AWS CDK Tools or CLI label Sep 10, 2023
@indrora
Copy link
Contributor

indrora commented Sep 11, 2023

If I understand this right, this is a CloudFormation limitation, not a CDK limitation at its heart.

The CLI and Console do not interact with CloudFormation, thus are not subject to its limitations.

@indrora indrora added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Sep 11, 2023
@peterwoodworth
Copy link
Contributor

@indrora are there any docs from CloudFormation stating this limitation? Have we tried working around this with escape hatches to confirm?

@peterwoodworth peterwoodworth removed the needs-triage This issue or PR still needs to be triaged. label Sep 11, 2023
@indrora
Copy link
Contributor

indrora commented Sep 11, 2023

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Sep 11, 2023
@peterwoodworth
Copy link
Contributor

peterwoodworth commented Sep 11, 2023

I see, the issue is that we are taking whatever is passed into addResource() and are directly adding it to the logical ID.

There isn't any issue with setting the pathPart to be accurate with an escape hatch:

    const restApi = new apigw.RestApi(this, 'restApi', {
      deploy: true,
    });
      
    const apiResource = restApi.root?.addResource("testwithoutdollar");
    (apiResource.node.defaultChild as apigw.CfnResource).addPropertyOverride('PathPart', '$testwithoutdollar');
    restApi.root.addMethod('GET');

Deploying this results in the following:
Screenshot 2023-09-11 at 1 07 54 PM

I think it should work fine if we change the code found at validateResourcePathPart. We've had to make this more lenient in the past #22531

function validateResourcePathPart(part: string) {

Edit: Actually, on a further look, special characters like : or $ get sanitized out of the synthesized logical IDs anyway. Scratch anything related to that

@peterwoodworth peterwoodworth added p2 effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md labels Sep 11, 2023
@lawofcycles
Copy link
Contributor

I may be able to implement this fix.

@mergify mergify bot closed this as completed in #27619 Oct 26, 2023
mergify bot pushed a commit that referenced this issue Oct 26, 2023
… resources beginning with dollar sign (#27619)

This PR adjusts the validation rules for path parts in resources created using the APIGateway library to allow the use of dollar mark in pathParts, as requested in #27083. This enables the creation of resources with paths such as $test(from the reproduction steps in the related issue).

An existing resource unit test ('url for a resource') has been updated to ensure that a dollar mark in a pathPart does not throw an error and properly reflects in the output of RestApi.urlForPath. The integration test integ.restapi has also been updated, wherein the appliances resource now has a path of $appliances:all instead of appliances:all.

Closes #27083.

#### All Submissions:
- [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)
#### Adding new Unconventional Dependencies:
- [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)
#### New Features
- [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
  - [x] Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

mrgrain pushed a commit that referenced this issue Nov 1, 2023
… resources beginning with dollar sign (#27619)

This PR adjusts the validation rules for path parts in resources created using the APIGateway library to allow the use of dollar mark in pathParts, as requested in #27083. This enables the creation of resources with paths such as $test(from the reproduction steps in the related issue).

An existing resource unit test ('url for a resource') has been updated to ensure that a dollar mark in a pathPart does not throw an error and properly reflects in the output of RestApi.urlForPath. The integration test integ.restapi has also been updated, wherein the appliances resource now has a path of $appliances:all instead of appliances:all.

Closes #27083.

#### All Submissions:
- [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)
#### Adding new Unconventional Dependencies:
- [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)
#### New Features
- [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
  - [x] Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
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. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2 package/tools Related to AWS CDK Tools or CLI
Projects
None yet
4 participants