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.

Sign up for GitHub

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

Jump to bottom

(integ-tests): sdk and assertions can generate custom resource types with more than 60 characters causing validation errors #22055

Closed
dontirun opened this issue Sep 15, 2022 · 2 comments · Fixed by #22119
Closed

(integ-tests): sdk and assertions can generate custom resource types with more than 60 characters causing validation errors #22055

dontirun opened this issue Sep 15, 2022 · 2 comments · Fixed by #22119
Assignees
@kaizencc
Labels
@aws-cdk/assertions Related to the @aws-cdk/assertv2 package bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.

Comments

@dontirun
Copy link
Contributor

dontirun commented Sep 15, 2022 •
edited
Loading

Describe the bug

I can't use awsApiCall('Redshift', 'describeClusters', {...}) due to validation errors in CloudFormation.

Expected Behavior

Using assertions.awsApiCall generates a valid Resource Type that doesn't throw errors

Current Behavior

I received the following error when attempting to write integ tests using assertions.awsApiCall

Deployment failed: Error: Stack Deployments Failed: ValidationError: 1 validation error detected: Value '[AWS::IAM::Role, AWS::Lambda::Function, Custom::DeployAssert@SdkCallRedshiftdescribeClusterParameters]' at 'typeNameList' failed to satisfy constraint: Member must satisfy constraint: [Member must have length less than or equal to 204, Member must have length greater than or equal to 10, Member must satisfy regular expression pattern: [A-Za-z0-9]{2,64}::[A-Za-z0-9]{2,64}::[A-Za-z0-9]{2,64}(::MODULE){0,1}]

Custom::DeployAssert@SdkCallRedshiftdescribeClusterParameters did not match the expected regex pattern.

Edit:

Well it looks like this is the actual problem

from the docs

You can specify a custom resource type name up to a maximum length of 60 characters.

Custom::DeployAssert@SdkCallRedshiftdescribeClusterParameters. Just so happens to be 61 characters long ...

Reproduction Steps

Cut down integration test file

import * as ec2 from '@aws-cdk/aws-ec2';
import * as redshift from '@aws-cdk/aws-redshift';
import * as cdk from '@aws-cdk/core';
import * as integ from '@aws-cdk/integ-tests';

const app = new cdk.App();

const stack = new cdk.Stack(app, 'foo')
const vpc = new ec2.Vpc(stack, 'Vpc');
const cluster = new redshift.Cluster(stack, 'Cluster', {
  vpc: vpc,
  vpcSubnets: {
    subnetType: ec2.SubnetType.PUBLIC,
  },
  masterUser: {
    masterUsername: 'admin',
  },
});

const test = new integ.IntegTest(app, 'aws-cdk-redshift-reboot-test', {
  testCases: [stack],
  stackUpdateWorkflow: true,
  diffAssets: true,
});

test.assertions.awsApiCall('Redshift', 'describeClusters', {
  ClusterIdentifier: cluster.clusterName,
});

app.synth();

Possible Solution

Remove/replace the @ symbol or provide documentation on how to opt-in to allow that symbol

Additional Information/Context

I removed the @ symbol from the types defined here and rebuilt the integ-tests module.

When I did this, the problem went away

CDK CLI Version

2.41.0

Framework Version

No response

Node.js Version

v16.16.0

OS

OsX

Language

Typescript

Language Version

No response

Other information

No response
@dontirun dontirun added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 15, 2022
@github-actions github-actions bot added the @aws-cdk/assertions Related to the @aws-cdk/assertv2 package label Sep 15, 2022
@dontirun dontirun mentioned this issue Sep 15, 2022
Merged
4 tasks
@dontirun dontirun changed the title (integ-tests): unable to use sdk and assertions due to @ symbol (integ-tests): unable to use sdk and assertions due to custom resource name generating validation error Sep 16, 2022
@dontirun
Copy link
Contributor Author

Well it looks like this is the actual problem

from the docs

You can specify a custom resource type name up to a maximum length of 60 characters.

Custom::DeployAssert@SdkCallRedshiftdescribeClusterParameters. Just so happens to be 61 characters long ...

@dontirun dontirun changed the title (integ-tests): unable to use sdk and assertions due to custom resource name generating validation error (integ-tests): sdk and assertions can generate custom resource types with more than 60 characters causing validation errors Sep 16, 2022
This was referenced Sep 19, 2022
Merged
Merged
@mergify mergify bot closed this as completed in #22119 Sep 20, 2022
mergify bot pushed a commit that referenced this issue Sep 20, 2022
@dontirun
fix(integ-tests): AwsApiCall Custom Resource length could be greater …
35b2806 
…than 60 characters (#22119)

Limits the resource type name to 60 characters to account for CloudFormation limitations with Custom Resource Resource Types


Closes #22055


----

### All Submissions:

* [ ] 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

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] 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.

mergify bot pushed a commit that referenced this issue Oct 10, 2022
@dontirun
fix(core): Custom Resource type length validation (#22118)
c186e2d 
The resource type length validation did not include the `Custom::` prefix which CloudFormation counts towards the total character count

Related to #22055

----

### All Submissions:

* [ ] 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

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] 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*
mergify bot pushed a commit that referenced this issue Feb 17, 2023
@dontirun
feat(redshift): optionally reboot Clusters to apply parameter changes (…
f61d950 
…#22063)

Closes #22009

Currently waiting on #22055  and #22059 for the assertions in the integration test to successfully run

----

### 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
Assignees

@kaizencc kaizencc

Labels
@aws-cdk/assertions Related to the @aws-cdk/assertv2 package bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(integ-tests): AwsApiCall Custom Resource length could be greater than 60 characters dontirun/aws-cdk
2 participants
@dontirun @kaizencc