From c186e2ddcffb25647b971ba1b90faa850552d219 Mon Sep 17 00:00:00 2001 From: Arun Donti Date: Mon, 10 Oct 2022 08:38:46 -0400 Subject: [PATCH] fix(core): Custom Resource type length validation (#22118) The resource type length validation did not include the `Custom::` prefix which CloudFormation counts towards the total character count Related to https://github.com/aws/aws-cdk/issues/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* --- .../cdk.out | 2 +- .../custom-resource-test.assets.json | 6 +- .../custom-resource-test.template.json | 14 ++++ .../integ.json | 2 +- .../manifest.json | 10 ++- .../tree.json | 64 ++++++++++++------- .../test/integ.core-custom-resources.ts | 9 +++ packages/@aws-cdk/core/lib/custom-resource.ts | 4 +- .../core/test/custom-resource.test.ts | 12 ++++ 9 files changed, 91 insertions(+), 32 deletions(-) diff --git a/packages/@aws-cdk/aws-cloudformation/test/core-custom-resources.integ.snapshot/cdk.out b/packages/@aws-cdk/aws-cloudformation/test/core-custom-resources.integ.snapshot/cdk.out index 588d7b269d34f..8ecc185e9dbee 100644 --- a/packages/@aws-cdk/aws-cloudformation/test/core-custom-resources.integ.snapshot/cdk.out +++ b/packages/@aws-cdk/aws-cloudformation/test/core-custom-resources.integ.snapshot/cdk.out @@ -1 +1 @@ -{"version":"20.0.0"} \ No newline at end of file +{"version":"21.0.0"} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-cloudformation/test/core-custom-resources.integ.snapshot/custom-resource-test.assets.json b/packages/@aws-cdk/aws-cloudformation/test/core-custom-resources.integ.snapshot/custom-resource-test.assets.json index a3b81b93daa59..6a7f6efef5cd8 100644 --- a/packages/@aws-cdk/aws-cloudformation/test/core-custom-resources.integ.snapshot/custom-resource-test.assets.json +++ b/packages/@aws-cdk/aws-cloudformation/test/core-custom-resources.integ.snapshot/custom-resource-test.assets.json @@ -1,5 +1,5 @@ { - "version": "20.0.0", + "version": "21.0.0", "files": { "94be4e33263da312b2e3d038a855c4b46686a35cc3188e9736010642b8a9078a": { "source": { @@ -14,7 +14,7 @@ } } }, - "4872bc3dc3f60af09e34f4244eab889b8fc0afc8372641cc559e0fcb55a986c0": { + "b724f17976eebd5e5ec6f838bd9cc66577d187ef286025c01d5d6c9ca099a368": { "source": { "path": "custom-resource-test.template.json", "packaging": "file" @@ -22,7 +22,7 @@ "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}" } } diff --git a/packages/@aws-cdk/aws-cloudformation/test/core-custom-resources.integ.snapshot/custom-resource-test.template.json b/packages/@aws-cdk/aws-cloudformation/test/core-custom-resources.integ.snapshot/custom-resource-test.template.json index 193f5b13c7f6b..4e49ec531a691 100644 --- a/packages/@aws-cdk/aws-cloudformation/test/core-custom-resources.integ.snapshot/custom-resource-test.template.json +++ b/packages/@aws-cdk/aws-cloudformation/test/core-custom-resources.integ.snapshot/custom-resource-test.template.json @@ -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": { diff --git a/packages/@aws-cdk/aws-cloudformation/test/core-custom-resources.integ.snapshot/integ.json b/packages/@aws-cdk/aws-cloudformation/test/core-custom-resources.integ.snapshot/integ.json index 7d0373db56b0d..e994528491139 100644 --- a/packages/@aws-cdk/aws-cloudformation/test/core-custom-resources.integ.snapshot/integ.json +++ b/packages/@aws-cdk/aws-cloudformation/test/core-custom-resources.integ.snapshot/integ.json @@ -1,5 +1,5 @@ { - "version": "20.0.0", + "version": "21.0.0", "testCases": { "integ.core-custom-resources": { "stacks": [ diff --git a/packages/@aws-cdk/aws-cloudformation/test/core-custom-resources.integ.snapshot/manifest.json b/packages/@aws-cdk/aws-cloudformation/test/core-custom-resources.integ.snapshot/manifest.json index 9963d16bfd115..f558211a3def3 100644 --- a/packages/@aws-cdk/aws-cloudformation/test/core-custom-resources.integ.snapshot/manifest.json +++ b/packages/@aws-cdk/aws-cloudformation/test/core-custom-resources.integ.snapshot/manifest.json @@ -1,5 +1,5 @@ { - "version": "20.0.0", + "version": "21.0.0", "artifacts": { "Tree": { "type": "cdk:tree", @@ -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": [ @@ -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", diff --git a/packages/@aws-cdk/aws-cloudformation/test/core-custom-resources.integ.snapshot/tree.json b/packages/@aws-cdk/aws-cloudformation/test/core-custom-resources.integ.snapshot/tree.json index 66d17aa2a53ac..cd2972d13a0a2 100644 --- a/packages/@aws-cdk/aws-cloudformation/test/core-custom-resources.integ.snapshot/tree.json +++ b/packages/@aws-cdk/aws-cloudformation/test/core-custom-resources.integ.snapshot/tree.json @@ -9,7 +9,7 @@ "path": "Tree", "constructInfo": { "fqn": "constructs.Construct", - "version": "10.1.85" + "version": "10.1.108" } }, "custom-resource-test": { @@ -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": { @@ -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" } } } \ No newline at end of file diff --git a/packages/@aws-cdk/aws-cloudformation/test/integ.core-custom-resources.ts b/packages/@aws-cdk/aws-cloudformation/test/integ.core-custom-resources.ts index f98c131af7218..cdd25e7d4c179 100644 --- a/packages/@aws-cdk/aws-cloudformation/test/integ.core-custom-resources.ts +++ b/packages/@aws-cdk/aws-cloudformation/test/integ.core-custom-resources.ts @@ -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`, @@ -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')) }); diff --git a/packages/@aws-cdk/core/lib/custom-resource.ts b/packages/@aws-cdk/core/lib/custom-resource.ts index 22f07453a1843..5e77f7f77e220 100644 --- a/packages/@aws-cdk/core/lib/custom-resource.ts +++ b/packages/@aws-cdk/core/lib/custom-resource.ts @@ -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})`); } diff --git a/packages/@aws-cdk/core/test/custom-resource.test.ts b/packages/@aws-cdk/core/test/custom-resource.test.ts index 7e721235adef9..79d513657309a 100644 --- a/packages/@aws-cdk/core/test/custom-resource.test.ts +++ b/packages/@aws-cdk/core/test/custom-resource.test.ts @@ -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();