fix(appsync): sanitized datasource name isn't exported

[ramblingenzyme]

Closes #23743
When providing a name with unsupported characters to an AppSync datasource, it is sanitized internally. #22378 introduced a bug where the sanitized name was no longer exposed on DataSource class instances.

---

All Submissions:

• Have you followed the guidelines in our Contributing guide?

Adding new Construct Runtime Dependencies:

• This PR adds new construct runtime dependencies following the process described here

New Features

• Have you added the new feature to an integration test?
  • 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

The above code generates the shown diff. This is due to #23743 where the name is sanitised, but not when setting this.name.

[gitpod-io]

[aws-cdk-automation]

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

[MrArnoldPalmer]

hey @ramblingenzyme good catch on this. Are you able to add a unit test here to assert that the sanitized name is correctly returned on this.name? An integ test isn't necessary.

[ramblingenzyme]

@MrArnoldPalmer Happy to try!

[ramblingenzyme]

Now that I'm looking at this, this fix is only going to apply to stuff without tokens, i.e. if a token returns a string with a - or there just is one in the static part of the string.

I'm a bit unsure about how AppSync/CF deals with this, and if that situation will just fail to deploy.

✅ Updated pull request passes all PRLinter validations. Dissmissing previous PRLinter review.

[ramblingenzyme]

@MrArnoldPalmer I've added a test case for what's definitely fixed by this PR and 2 which aren't covered. I'm unsure if these are possible to be resolved, but are definitely beyond me at this point.

If these are solvable issues, I'm happy to open issues that get linked in the test case comment similar to https://github.com/aws/aws-cdk/blob/74318c7d22bfc00de9e005f68a0a6aaa58c7db39/packages/%40aws-cdk/pipelines/test/docker-credentials.test.ts

[MrArnoldPalmer]

So I'm not sure the exact case of having a string that is part token then concatenated with - unless the user is interpolating that themselves, in which case the name will not be sanitized and the deploy will fail with an error saying the name is invalid right? Basically I don't think we need to cover this case. Sanitizing is a way to ensure that names originating in the CDK app somewhere wont cause a failure, but if it originates elsewhere there isn't much we can do about it until resolution time. We could make the sanitization itself lazy but I'm not sure how to assure the token is resolved before the sanitization function occurs.

Basically I think we can remove the two failing test cases and merge the fix as is to solve the existing issue.

Also can you add a couple of sentences to the PR message for the changelog?

[ramblingenzyme]

Just wasn't sure if that was a normal thing to do, since one of the test cases added in #22378 also has a template string with an embedded token. Removed those test cases and updating the PR message now.

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 91f4e72
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).