Skip to content

Commit

Permalink
fix(core): Custom Resource type length validation (#22118)
Browse files Browse the repository at this point in the history
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*
  • Loading branch information
dontirun authored Oct 10, 2022
1 parent 064a1a6 commit c186e2d
Show file tree
Hide file tree
Showing 9 changed files with 91 additions and 32 deletions.
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"version":"20.0.0"}
{"version":"21.0.0"}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "20.0.0",
"version": "21.0.0",
"files": {
"94be4e33263da312b2e3d038a855c4b46686a35cc3188e9736010642b8a9078a": {
"source": {
Expand All @@ -14,15 +14,15 @@
}
}
},
"4872bc3dc3f60af09e34f4244eab889b8fc0afc8372641cc559e0fcb55a986c0": {
"b724f17976eebd5e5ec6f838bd9cc66577d187ef286025c01d5d6c9ca099a368": {
"source": {
"path": "custom-resource-test.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "4872bc3dc3f60af09e34f4244eab889b8fc0afc8372641cc559e0fcb55a986c0.json",
"objectKey": "b724f17976eebd5e5ec6f838bd9cc66577d187ef286025c01d5d6c9ca099a368.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,20 @@
},
"UpdateReplacePolicy": "Delete",
"DeletionPolicy": "Delete"
},
"MyLengthyTypeResource": {
"Type": "Custom::Given_Resource_Type_Is_Exactly_Sixty_Characters_Long",
"Properties": {
"ServiceToken": {
"Fn::GetAtt": [
"CustomReflectCustomResourceProviderHandler2E189D0B",
"Arn"
]
},
"physicalResourceId": "MyPhysicalLengthyType"
},
"UpdateReplacePolicy": "Delete",
"DeletionPolicy": "Delete"
}
},
"Outputs": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "20.0.0",
"version": "21.0.0",
"testCases": {
"integ.core-custom-resources": {
"stacks": [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "20.0.0",
"version": "21.0.0",
"artifacts": {
"Tree": {
"type": "cdk:tree",
Expand All @@ -23,7 +23,7 @@
"validateOnSynth": false,
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
"cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/4872bc3dc3f60af09e34f4244eab889b8fc0afc8372641cc559e0fcb55a986c0.json",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/b724f17976eebd5e5ec6f838bd9cc66577d187ef286025c01d5d6c9ca099a368.json",
"requiresBootstrapStackVersion": 6,
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
"additionalDependencies": [
Expand Down Expand Up @@ -57,6 +57,12 @@
"data": "MyResource"
}
],
"/custom-resource-test/MyLengthyTypeResource/Default": [
{
"type": "aws:cdk:logicalId",
"data": "MyLengthyTypeResource"
}
],
"/custom-resource-test/Ref": [
{
"type": "aws:cdk:logicalId",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"path": "Tree",
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.85"
"version": "10.1.108"
}
},
"custom-resource-test": {
Expand All @@ -24,30 +24,30 @@
"id": "Staging",
"path": "custom-resource-test/Custom::ReflectCustomResourceProvider/Staging",
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.85"
"fqn": "@aws-cdk/core.AssetStaging",
"version": "0.0.0"
}
},
"Role": {
"id": "Role",
"path": "custom-resource-test/Custom::ReflectCustomResourceProvider/Role",
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.85"
"fqn": "@aws-cdk/core.CfnResource",
"version": "0.0.0"
}
},
"Handler": {
"id": "Handler",
"path": "custom-resource-test/Custom::ReflectCustomResourceProvider/Handler",
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.85"
"fqn": "@aws-cdk/core.CfnResource",
"version": "0.0.0"
}
}
},
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.85"
"fqn": "@aws-cdk/core.CustomResourceProvider",
"version": "0.0.0"
}
},
"MyResource": {
Expand All @@ -58,50 +58,68 @@
"id": "Default",
"path": "custom-resource-test/MyResource/Default",
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.85"
"fqn": "@aws-cdk/core.CfnResource",
"version": "0.0.0"
}
}
},
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.85"
"fqn": "@aws-cdk/core.CustomResource",
"version": "0.0.0"
}
},
"MyLengthyTypeResource": {
"id": "MyLengthyTypeResource",
"path": "custom-resource-test/MyLengthyTypeResource",
"children": {
"Default": {
"id": "Default",
"path": "custom-resource-test/MyLengthyTypeResource/Default",
"constructInfo": {
"fqn": "@aws-cdk/core.CfnResource",
"version": "0.0.0"
}
}
},
"constructInfo": {
"fqn": "@aws-cdk/core.CustomResource",
"version": "0.0.0"
}
},
"Ref": {
"id": "Ref",
"path": "custom-resource-test/Ref",
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.85"
"fqn": "@aws-cdk/core.CfnOutput",
"version": "0.0.0"
}
},
"GetAtt.Attribute1": {
"id": "GetAtt.Attribute1",
"path": "custom-resource-test/GetAtt.Attribute1",
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.85"
"fqn": "@aws-cdk/core.CfnOutput",
"version": "0.0.0"
}
},
"GetAtt.Attribute2": {
"id": "GetAtt.Attribute2",
"path": "custom-resource-test/GetAtt.Attribute2",
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.85"
"fqn": "@aws-cdk/core.CfnOutput",
"version": "0.0.0"
}
}
},
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.85"
"fqn": "@aws-cdk/core.Stack",
"version": "0.0.0"
}
}
},
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.85"
"fqn": "@aws-cdk/core.App",
"version": "0.0.0"
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class TestStack extends Stack {
super(scope, id);

const resourceType = 'Custom::Reflect';
const lengthyResourceType = 'Custom::Given_Resource_Type_Is_Exactly_Sixty_Characters_Long';

const serviceToken = CustomResourceProvider.getOrCreate(this, resourceType, {
codeDirectory: `${__dirname}/core-custom-resource-provider-fixture`,
Expand All @@ -35,6 +36,14 @@ class TestStack extends Stack {
},
});

new CustomResource(this, 'MyLengthyTypeResource', {
resourceType: lengthyResourceType,
serviceToken,
properties: {
physicalResourceId: 'MyPhysicalLengthyType',
},
});

new CfnOutput(this, 'Ref', { value: cr.ref });
new CfnOutput(this, 'GetAtt.Attribute1', { value: Token.asString(cr.getAtt('Attribute1')) });
new CfnOutput(this, 'GetAtt.Attribute2', { value: Token.asString(cr.getAtt('Attribute2')) });
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/core/lib/custom-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,11 +203,11 @@ function renderResourceType(resourceType?: string) {
throw new Error(`Custom resource type must begin with "Custom::" (${resourceType})`);
}

const typeName = resourceType.slice(resourceType.indexOf('::') + 2);
if (typeName.length > 60) {
if (resourceType.length > 60) {
throw new Error(`Custom resource type length > 60 (${resourceType})`);
}

const typeName = resourceType.slice(resourceType.indexOf('::') + 2);
if (!/^[a-z0-9_@-]+$/i.test(typeName)) {
throw new Error(`Custom resource type name can only include alphanumeric characters and _@- (${typeName})`);
}
Expand Down
12 changes: 12 additions & 0 deletions packages/@aws-cdk/core/test/custom-resource.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,18 @@ describe('custom resource', () => {
})).toThrow(/Custom resource type must begin with "Custom::"/);
});

test('Custom resource type length must be less than 60 characters', () => {
// GIVEN
const stack = new Stack();

// WHEN
expect(() => new CustomResource(stack, 'MyCustomResource', {
resourceType: 'Custom::Adding_An_Additional_Fifty_Three_Characters_For_Error',
serviceToken: 'FooBar',
// THEN
})).toThrow(/Custom resource type length > 60/);
});

test('properties can be pascal-cased', () => {
// GIVEN
const stack = new Stack();
Expand Down

0 comments on commit c186e2d

Please sign in to comment.