From 5a9d5c9a1901c6939a461bed07dfb0b23ec3c41a Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Fri, 20 Sep 2024 10:06:48 -0700 Subject: [PATCH 01/16] reprod --- packages/@aws-cdk/cx-api/FEATURE_FLAGS.md | 22 ++++++++- .../test/invalid-templates.test.ts | 6 +++ .../tags-with-invalid-intrinsics.json | 0 .../update-policy-with-intrinsics.json | 47 +++++++++++++++++++ .../test/valid-templates.test.ts | 10 ++-- 5 files changed, 80 insertions(+), 5 deletions(-) rename packages/aws-cdk-lib/cloudformation-include/test/test-templates/{ => invalid}/tags-with-invalid-intrinsics.json (100%) create mode 100644 packages/aws-cdk-lib/cloudformation-include/test/test-templates/update-policy-with-intrinsics.json diff --git a/packages/@aws-cdk/cx-api/FEATURE_FLAGS.md b/packages/@aws-cdk/cx-api/FEATURE_FLAGS.md index 634630f6e9b41..beadd60aa4ed2 100644 --- a/packages/@aws-cdk/cx-api/FEATURE_FLAGS.md +++ b/packages/@aws-cdk/cx-api/FEATURE_FLAGS.md @@ -73,6 +73,7 @@ Flags come in three types: | [@aws-cdk/custom-resources:logApiResponseDataPropertyTrueDefault](#aws-cdkcustom-resourceslogapiresponsedatapropertytruedefault) | When enabled, the custom resource used for `AwsCustomResource` will configure the `logApiResponseData` property as true by default | 2.145.0 | (fix) | | [@aws-cdk/aws-s3:keepNotificationInImportedBucket](#aws-cdkaws-s3keepnotificationinimportedbucket) | When enabled, Adding notifications to a bucket in the current stack will not remove notification from imported stack. | 2.155.0 | (fix) | | [@aws-cdk/aws-stepfunctions-tasks:useNewS3UriParametersForBedrockInvokeModelTask](#aws-cdkaws-stepfunctions-tasksusenews3uriparametersforbedrockinvokemodeltask) | When enabled, use new props for S3 URI field in task definition of state machine for bedrock invoke model. | 2.156.0 | (fix) | +| [@aws-cdk/aws-ecs:reduceEc2FargateCloudWatchPermissions](#aws-cdkaws-ecsreduceec2fargatecloudwatchpermissions) | When enabled, we will only grant the necessary permissions when users specify cloudwatch log group through logConfiguration | 2.159.0 | (fix) | @@ -134,7 +135,8 @@ The following json shows the current recommended set of flags, as `cdk init` wou "@aws-cdk/aws-ec2:ebsDefaultGp3Volume": true, "@aws-cdk/aws-ecs:removeDefaultDeploymentAlarm": true, "@aws-cdk/custom-resources:logApiResponseDataPropertyTrueDefault": false, - "@aws-cdk/aws-s3:keepNotificationInImportedBucket": false + "@aws-cdk/aws-s3:keepNotificationInImportedBucket": false, + "@aws-cdk/aws-ecs:reduceEc2FargateCloudWatchPermissions": true } } ``` @@ -1378,4 +1380,22 @@ When this feature flag is enabled, specify newly introduced props 's3InputUri' a **Compatibility with old behavior:** Disable the feature flag to use input and output path fields for s3 URI +### @aws-cdk/aws-ecs:reduceEc2FargateCloudWatchPermissions + +*When enabled, we will only grant the necessary permissions when users specify cloudwatch log group through logConfiguration* (fix) + +Currently, we automatically add a number of cloudwatch permissions to the task role when no cloudwatch log group is +specified as logConfiguration and it will grant 'Resources': ['*'] to the task role. + +When this feature flag is enabled, we will only grant the necessary permissions when users specify cloudwatch log group. + + +| Since | Default | Recommended | +| ----- | ----- | ----- | +| (not in v1) | | | +| 2.159.0 | `false` | `true` | + +**Compatibility with old behavior:** Disable the feature flag to continue grant permissions to log group when no log group is specified + + diff --git a/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts b/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts index 984a394adaa05..b4e35bca11725 100644 --- a/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts +++ b/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts @@ -245,6 +245,12 @@ describe('CDK Include', () => { }, ); }); + + test('throws an exception if Tags contains invalid intrinsics', () => { + expect(() => { + includeTestTemplate(stack, 'tags-with-invalid-intrinsics.json'); + }).toThrow(/expression does not exist in the template/); + }); }); interface IncludeTestTemplateProps { diff --git a/packages/aws-cdk-lib/cloudformation-include/test/test-templates/tags-with-invalid-intrinsics.json b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/tags-with-invalid-intrinsics.json similarity index 100% rename from packages/aws-cdk-lib/cloudformation-include/test/test-templates/tags-with-invalid-intrinsics.json rename to packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/tags-with-invalid-intrinsics.json diff --git a/packages/aws-cdk-lib/cloudformation-include/test/test-templates/update-policy-with-intrinsics.json b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/update-policy-with-intrinsics.json new file mode 100644 index 0000000000000..607443aa6c129 --- /dev/null +++ b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/update-policy-with-intrinsics.json @@ -0,0 +1,47 @@ +{ + "Conditions": { + "AutoReplaceHosts": { + "Fn::Equals": [ + 2, + 2 + ] + }, + "SetMinInstancesInServiceToZero": { + "Fn::Equals": [ + 2, + 2 + ] + } + }, + "Resources": { + "ASG": { + "Type": "AWS::AutoScaling::AutoScalingGroup", + "Properties": { + "MaxSize": 10, + "MinSize": 1 + }, + "UpdatePolicy": { + "AutoScalingRollingUpdate": { + "Fn::If": [ + "AutoReplaceHosts", + { + "MinInstancesInService": { + "Fn::If": [ + "SetMinInstancesInServiceToZero", + 0, + 1 + ] + }, + "MaxBatchSize": 2, + "PauseTime": "PT5M", + "WaitOnResourceSignals": "true" + }, + { + "Ref": "AWS::NoValue" + } + ] + } + } + } + } +} \ No newline at end of file diff --git a/packages/aws-cdk-lib/cloudformation-include/test/valid-templates.test.ts b/packages/aws-cdk-lib/cloudformation-include/test/valid-templates.test.ts index 8876e5c3fe50a..e7c38c36652fa 100644 --- a/packages/aws-cdk-lib/cloudformation-include/test/valid-templates.test.ts +++ b/packages/aws-cdk-lib/cloudformation-include/test/valid-templates.test.ts @@ -1130,10 +1130,12 @@ describe('CDK Include', () => { ); }); - test('throws an exception if Tags contains invalid intrinsics', () => { - expect(() => { - includeTestTemplate(stack, 'tags-with-invalid-intrinsics.json'); - }).toThrow(/expression does not exist in the template/); + test('Intrinsics can be used in update policy attribute', () => { + includeTestTemplate(stack, 'update-policy-with-intrinsics.json'); + + Template.fromStack(stack).templateMatches( + loadTestFileToJsObject('update-policy-with-intrinsics.json'), + ); }); }); From 38e8963734c7a030fc28747d0861972eb561e287 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Thu, 26 Sep 2024 10:06:57 -0700 Subject: [PATCH 02/16] tests --- .../test/invalid-templates.test.ts | 48 ++++++++++ .../intrinsics-create-policy-autoscaling.json | 23 +++++ .../intrinsics-create-policy-complex.json | 42 +++++++++ ...rinsics-create-policy-resource-signal.json | 24 +++++ ...e-policy-autoscaling-replacing-update.json | 22 +++++ ...ate-policy-autoscaling-rolling-update.json | 37 ++++++++ ...e-policy-autoscaling-scheduled-action.json | 24 +++++ ...olicy-code-deploy-lambda-alias-update.json | 34 +++++++ .../intrinsics-update-policy-complex.json | 33 +++++++ .../intrinsics-create-policy-autoscaling.json | 28 ++++++ ...rinsics-create-policy-resource-signal.json | 36 ++++++++ .../invalid/intrinsics-create-policy.json | 42 +++++++++ ...e-policy-autoscaling-replacing-update.json | 32 +++++++ ...ate-policy-autoscaling-rolling-update.json | 38 ++++++++ ...e-policy-autoscaling-scheduled-action.json | 32 +++++++ ...olicy-code-deploy-lambda-alias-update.json | 39 ++++++++ .../invalid/intrinsics-update-policy.json | 40 ++++++++ .../resource-attribute-update-policy.json | 61 ------------- .../test/valid-templates.test.ts | 91 +++++++++++++++---- .../core/lib/helpers-internal/cfn-parse.ts | 19 ++++ 20 files changed, 668 insertions(+), 77 deletions(-) create mode 100644 packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-create-policy-autoscaling.json create mode 100644 packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-create-policy-complex.json create mode 100644 packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-create-policy-resource-signal.json create mode 100644 packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-update-policy-autoscaling-replacing-update.json create mode 100644 packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-update-policy-autoscaling-rolling-update.json create mode 100644 packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-update-policy-autoscaling-scheduled-action.json create mode 100644 packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-update-policy-code-deploy-lambda-alias-update.json create mode 100644 packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-update-policy-complex.json create mode 100644 packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-create-policy-autoscaling.json create mode 100644 packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-create-policy-resource-signal.json create mode 100644 packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-create-policy.json create mode 100644 packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy-autoscaling-replacing-update.json create mode 100644 packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy-autoscaling-rolling-update.json create mode 100644 packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy-autoscaling-scheduled-action.json create mode 100644 packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy-code-deploy-lambda-alias-update.json create mode 100644 packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy.json delete mode 100644 packages/aws-cdk-lib/cloudformation-include/test/test-templates/resource-attribute-update-policy.json diff --git a/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts b/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts index b4e35bca11725..383d17cb09ac5 100644 --- a/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts +++ b/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts @@ -251,6 +251,54 @@ describe('CDK Include', () => { includeTestTemplate(stack, 'tags-with-invalid-intrinsics.json'); }).toThrow(/expression does not exist in the template/); }); + + test('non-leaf Intrinsics cannot be used in the top-level creation policy', () => { + expect(() => { + includeTestTemplate(stack, 'intrinsics-create-policy.json'); + }).toThrow(/intrinsics cannot be used in update, deletion, or update-replace policies./); + }); + + test('Intrinsics cannot be used in the autoscaling creation policy', () => { + expect(() => { + includeTestTemplate(stack, 'intrinsics-create-policy-autoscaling.json'); + }).toThrow(/intrinsics cannot be used in update, deletion, or update-replace policies./); + }); + + test('Intrinsics cannot be used in the create policy resource signal', () => { + expect(() => { + includeTestTemplate(stack, 'intrinsics-create-policy-resource-signal.json'); + }).toThrow(/intrinsics cannot be used in update, deletion, or update-replace policies./); + }); + + test('Intrinsics cannot be used in the top-level update policy', () => { + expect(() => { + includeTestTemplate(stack, 'intrinsics-update-policy.json'); + }).toThrow(/intrinsics cannot be used in update, deletion, or update-replace policies./); + }); + + test('Intrinsics cannot be used in the auto scaling rolling update update policy', () => { + expect(() => { + includeTestTemplate(stack, 'intrinsics-update-policy-autoscaling-rolling-update.json'); + }).toThrow(/intrinsics cannot be used in update, deletion, or update-replace policies./); + }); + + test('Intrinsics cannot be used in the auto scaling replacing update update policy', () => { + expect(() => { + includeTestTemplate(stack, 'intrinsics-update-policy-autoscaling-replacing-update.json'); + }).toThrow(/intrinsics cannot be used in update, deletion, or update-replace policies./); + }); + + test('Intrinsics cannot be used in the auto scaling scheduled action update policy', () => { + expect(() => { + includeTestTemplate(stack, 'intrinsics-update-policy-autoscaling-scheduled-action.json'); + }).toThrow(/intrinsics cannot be used in update, deletion, or update-replace policies./); + }); + + test('Intrinsics cannot be used in the code deploy lambda alias update policy', () => { + expect(() => { + includeTestTemplate(stack, 'intrinsics-update-policy-code-deploy-lambda-alias-update.json'); + }).toThrow(/intrinsics cannot be used in update, deletion, or update-replace policies./); + }); }); interface IncludeTestTemplateProps { diff --git a/packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-create-policy-autoscaling.json b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-create-policy-autoscaling.json new file mode 100644 index 0000000000000..2730a163d5770 --- /dev/null +++ b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-create-policy-autoscaling.json @@ -0,0 +1,23 @@ +{ + "Parameters": { + "MinSuccessfulInstancesPercent": { + "Type": "Number" + } + }, + "Resources": { + "AutoScalingCreationPolicyIntrinsic": { + "Type": "AWS::AutoScaling::AutoScalingGroup", + "Properties": { + "MinSize": "1", + "MaxSize": "5" + }, + "CreationPolicy": { + "AutoScalingCreationPolicy": { + "MinSuccessfulInstancesPercent": { + "Ref": "MinSuccessfulInstancesPercent" + } + } + } + } + } +} diff --git a/packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-create-policy-complex.json b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-create-policy-complex.json new file mode 100644 index 0000000000000..82f46093f68d4 --- /dev/null +++ b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-create-policy-complex.json @@ -0,0 +1,42 @@ +{ + "Parameters": { + "CountParameter": { + "Type": "Number", + "Default": 3 + } + }, + "Conditions": { + "SomeCondition": { + "Fn::Equals": [ + 2, + 2 + ] + } + }, + "Resources": { + "ASG": { + "Type": "AWS::AutoScaling::AutoScalingGroup", + "Properties": { + "MinSize": "1", + "MaxSize": "5" + }, + "CreationPolicy": { + "AutoScalingCreationPolicy": { + "MinSuccessfulInstancesPercent": 50 + }, + "ResourceSignal": { + "Count": { + "Fn::If": [ + "SomeCondition", + { + "Ref": "CountParameter" + }, + 4 + ] + }, + "Timeout":"PT5H4M3S" + } + } + } + } +} diff --git a/packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-create-policy-resource-signal.json b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-create-policy-resource-signal.json new file mode 100644 index 0000000000000..40919f1e39b5b --- /dev/null +++ b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-create-policy-resource-signal.json @@ -0,0 +1,24 @@ +{ + "Parameters": { + "CountParameter": { + "Type": "Number", + "Default": 3 + } + }, + "Resources": { + "ResourceSignalIntrinsic": { + "Type": "AWS::AutoScaling::AutoScalingGroup", + "Properties": { + "MinSize": "1", + "MaxSize": "5" + }, + "CreationPolicy": { + "ResourceSignal": { + "Count": { + "Ref": "CountParameter" + } + } + } + } + } +} \ No newline at end of file diff --git a/packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-update-policy-autoscaling-replacing-update.json b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-update-policy-autoscaling-replacing-update.json new file mode 100644 index 0000000000000..dd80cf6146a6c --- /dev/null +++ b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-update-policy-autoscaling-replacing-update.json @@ -0,0 +1,22 @@ +{ + "Parameters": { + "WillReplace": { + "Type": "Boolean", + "Default": false + } + }, + "Resources": { + "ASG": { + "Type": "AWS::AutoScaling::AutoScalingGroup", + "Properties": { + "MinSize": "1", + "MaxSize": "10" + }, + "UpdatePolicy": { + "AutoScalingReplacingUpdate": { + "WillReplace" : { "Ref": "WillReplace" } + } + } + } + } +} \ No newline at end of file diff --git a/packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-update-policy-autoscaling-rolling-update.json b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-update-policy-autoscaling-rolling-update.json new file mode 100644 index 0000000000000..bd55715595887 --- /dev/null +++ b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-update-policy-autoscaling-rolling-update.json @@ -0,0 +1,37 @@ +{ + "Parameters": { + "MinInstances": { + "Type": "Number", + "Default": 1 + }, + "MaxBatchSize": { + "Type": "Number", + "Default": 1 + }, + "PauseTime": { + "Type": "String", + "Default": "PT5M" + }, + "WaitOnResourceSignals": { + "Type": "Boolean", + "Default": true + } + }, + "Resources": { + "ASG": { + "Type": "AWS::AutoScaling::AutoScalingGroup", + "Properties": { + "MinSize": "1", + "MaxSize": "10" + }, + "UpdatePolicy": { + "AutoScalingRollingUpdate": { + "MinInstancesInService": { "Ref": "MinInstances" }, + "MaxBatchSize": { "Ref": "MaxBatchSize" }, + "PauseTime": { "Ref": "PauseTime" }, + "WaitOnResourceSignals": { "Ref": "WaitOnResourceSignals" } + } + } + } + } +} \ No newline at end of file diff --git a/packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-update-policy-autoscaling-scheduled-action.json b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-update-policy-autoscaling-scheduled-action.json new file mode 100644 index 0000000000000..27daa8a4f8972 --- /dev/null +++ b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-update-policy-autoscaling-scheduled-action.json @@ -0,0 +1,24 @@ +{ + "Parameters": { + "IgnoreUnmodifiedGroupSizeProperties": { + "Type": "Boolean", + "Default": false + } + }, + "Resources": { + "ASG": { + "Type": "AWS::AutoScaling::AutoScalingGroup", + "Properties": { + "MinSize": "1", + "MaxSize": "10" + }, + "UpdatePolicy": { + "AutoScalingScheduledAction": { + "IgnoreUnmodifiedGroupSizeProperties": { + "Ref": "IgnoreUnmodifiedGroupSizeProperties" + } + } + } + } + } +} \ No newline at end of file diff --git a/packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-update-policy-code-deploy-lambda-alias-update.json b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-update-policy-code-deploy-lambda-alias-update.json new file mode 100644 index 0000000000000..382b04a767b89 --- /dev/null +++ b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-update-policy-code-deploy-lambda-alias-update.json @@ -0,0 +1,34 @@ +{ + "Parameters": { + "ApplicationName": { + "Type": "String" + }, + "DeploymentGroupName": { + "Type": "String" + }, + "BeforeAllowTrafficHook": { + "Type": "String" + }, + "AfterAllowTrafficHook": { + "Type": "String" + } + }, + "Resources": { + "Alias": { + "Type": "AWS::Lambda::Alias", + "Properties": { + "FunctionName": "SomeLambda", + "FunctionVersion": "SomeVersion", + "Name": "MyAlias" + }, + "UpdatePolicy": { + "CodeDeployLambdaAliasUpdate": { + "ApplicationName": { "Ref": "ApplicationName" }, + "DeploymentGroupName": { "Ref": "DeploymentGroupName" }, + "BeforeAllowTrafficHook": { "Ref": "BeforeAllowTrafficHook" }, + "AfterAllowTrafficHook": { "Ref": "AfterAllowTrafficHook" } + } + } + } + } +} \ No newline at end of file diff --git a/packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-update-policy-complex.json b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-update-policy-complex.json new file mode 100644 index 0000000000000..6b0cdb351f00b --- /dev/null +++ b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/intrinsics-update-policy-complex.json @@ -0,0 +1,33 @@ +{ + "Conditions": { + "SomeCondition": { + "Fn::Equals": [ + 2, + 2 + ] + } + }, + "Resources": { + "ASG": { + "Type": "AWS::AutoScaling::AutoScalingGroup", + "Properties": { + "MinSize": "1", + "MaxSize": "10" + }, + "UpdatePolicy": { + "AutoScalingRollingUpdate": { + "MinInstancesInService": { + "Fn::If": [ + "SomeCondition", + 1, + 2 + ] + }, + "MaxBatchSize": 2, + "PauseTime": "PT5M", + "WaitOnResourceSignals": true + } + } + } + } +} \ No newline at end of file diff --git a/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-create-policy-autoscaling.json b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-create-policy-autoscaling.json new file mode 100644 index 0000000000000..9d6cfaddf2df2 --- /dev/null +++ b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-create-policy-autoscaling.json @@ -0,0 +1,28 @@ +{ + "Conditions": { + "SomeCondition": { + "Fn::Equals": [ + 2, + 2 + ] + } + }, + "Resources": { + "AutoScalingCreationPolicyIntrinsic": { + "Type": "AWS::AutoScaling::AutoScalingGroup", + "Properties": { + "MinSize": 1, + "MaxSize": 5 + }, + "CreationPolicy": { + "AutoScalingCreationPolicy": { + "Fn::If": [ + "SomeCondition", + { "MinSuccessfulInstancesPercent": 50 }, + { "MinSuccessfulInstancesPercent": 25 } + ] + } + } + } + } +} diff --git a/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-create-policy-resource-signal.json b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-create-policy-resource-signal.json new file mode 100644 index 0000000000000..5fc823d4731d5 --- /dev/null +++ b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-create-policy-resource-signal.json @@ -0,0 +1,36 @@ +{ + "Parameters": { + "CountParameter": { + "Type": "Number", + "Default": 3 + } + }, + "Conditions": { + "UseCountParameter": { + "Fn::Equals": [ + 2, + 2 + ] + } + }, + "Resources": { + "ResourceSignalIntrinsic": { + "Type": "AWS::AutoScaling::AutoScalingGroup", + "Properties": { + "MinSize": 1, + "MaxSize": 5 + }, + "CreationPolicy": { + "ResourceSignal": { + "Fn::If": [ + "UseCountParameter", + { + "Count": { "Ref": "CountParameter" } + }, + 5 + ] + } + } + } + } +} diff --git a/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-create-policy.json b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-create-policy.json new file mode 100644 index 0000000000000..2afe984191fcc --- /dev/null +++ b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-create-policy.json @@ -0,0 +1,42 @@ +{ + "Parameters": { + "CountParameter": { + "Type": "Number", + "Default": 3 + } + }, + "Conditions": { + "SomeCondition": { + "Fn::Equals": [ + 2, + 2 + ] + } + }, + "Resources": { + "CreationPolicyIntrinsic": { + "Type": "AWS::AutoScaling::AutoScalingGroup", + "Properties": { + "MinSize": 1, + "MaxSize": 5 + }, + "CreationPolicy": { + "Fn::If": [ + "SomeCondition", + { + "AutoScalingCreationPolicy": { + "MinSuccessfulInstancesPercent": 50 + }, + "ResourceSignal": { + "Count": 5, + "Timeout": "PT5H4M3S" + } + }, + { + "Ref": "AWS::NoValue" + } + ] + } + } + } +} diff --git a/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy-autoscaling-replacing-update.json b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy-autoscaling-replacing-update.json new file mode 100644 index 0000000000000..e302385e89139 --- /dev/null +++ b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy-autoscaling-replacing-update.json @@ -0,0 +1,32 @@ +{ + "Conditions": { + "SomeCondition": { + "Fn::Equals": [ + 2, + 2 + ] + } + }, + "Resources": { + "ASG": { + "Type": "AWS::AutoScaling::AutoScalingGroup", + "Properties": { + "MaxSize": 10, + "MinSize": 1 + }, + "UpdatePolicy": { + "AutoScalingReplacingUpdate": { + "Fn::If": [ + "SomeCondition", + { + "WillReplace" : true + }, + { + "WillReplace" : false + } + ] + } + } + } + } +} \ No newline at end of file diff --git a/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy-autoscaling-rolling-update.json b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy-autoscaling-rolling-update.json new file mode 100644 index 0000000000000..8d793770dda2a --- /dev/null +++ b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy-autoscaling-rolling-update.json @@ -0,0 +1,38 @@ +{ + "Conditions": { + "SomeCondition": { + "Fn::Equals": [ + 2, + 2 + ] + } + }, + "Resources": { + "ASG": { + "Type": "AWS::AutoScaling::AutoScalingGroup", + "Properties": { + "MaxSize": 10, + "MinSize": 1 + }, + "UpdatePolicy": { + "AutoScalingRollingUpdate": { + "Fn::If": [ + "SomeCondition", + { + "MinInstancesInService": 1, + "MaxBatchSize": 2, + "PauseTime": "PT5M", + "WaitOnResourceSignals": "true" + }, + { + "MinInstancesInService": 1, + "MaxBatchSize": 2, + "PauseTime": "PT5M", + "WaitOnResourceSignals": "true" + } + ] + } + } + } + } +} \ No newline at end of file diff --git a/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy-autoscaling-scheduled-action.json b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy-autoscaling-scheduled-action.json new file mode 100644 index 0000000000000..2a6141f18fffb --- /dev/null +++ b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy-autoscaling-scheduled-action.json @@ -0,0 +1,32 @@ +{ + "Conditions": { + "SomeCondition": { + "Fn::Equals": [ + 2, + 2 + ] + } + }, + "Resources": { + "ASG": { + "Type": "AWS::AutoScaling::AutoScalingGroup", + "Properties": { + "MaxSize": 10, + "MinSize": 1 + }, + "UpdatePolicy": { + "AutoScalingScheduledAction": { + "Fn::If": [ + "SomeCondition", + { + "IgnoreUnmodifiedGroupSizeProperties" : true + }, + { + "IgnoreUnmodifiedGroupSizeProperties" : false + } + ] + } + } + } + } +} \ No newline at end of file diff --git a/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy-code-deploy-lambda-alias-update.json b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy-code-deploy-lambda-alias-update.json new file mode 100644 index 0000000000000..92911296e5764 --- /dev/null +++ b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy-code-deploy-lambda-alias-update.json @@ -0,0 +1,39 @@ +{ + "Conditions": { + "SomeCondition": { + "Fn::Equals": [ + 2, + 2 + ] + } + }, + "Resources": { + "Alias": { + "Type": "AWS::Lambda::Alias", + "Properties": { + "FunctionName": "SomeLambda", + "FunctionVersion": "SomeVersion", + "Name": "MyAlias" + }, + "UpdatePolicy": { + "CodeDeployLambdaAliasUpdate": { + "Fn::If": [ + "SomeCondition", + { + "ApplicationName": "SomeApp", + "DeploymentGroupName": "SomeDeploymentGroup", + "BeforeAllowTrafficHook": "SomeHook", + "AfterAllowTrafficHook": "SomeOtherHook" + }, + { + "ApplicationName": "SomeOtherApp", + "DeploymentGroupName": "SomeOtherDeploymentGroup", + "BeforeAllowTrafficHook": "SomeOtherHook", + "AfterAllowTrafficHook": "SomeOtherOtherHook" + } + ] + } + } + } + } +} \ No newline at end of file diff --git a/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy.json b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy.json new file mode 100644 index 0000000000000..fe284e4e05828 --- /dev/null +++ b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy.json @@ -0,0 +1,40 @@ +{ + "Conditions": { + "SomeCondition": { + "Fn::Equals": [ + 2, + 2 + ] + } + }, + "Resources": { + "ASG": { + "Type": "AWS::AutoScaling::AutoScalingGroup", + "Properties": { + "MaxSize": 10, + "MinSize": 1 + }, + "UpdatePolicy": { + "Fn::If": [ + "SomeCondition", + { + "AutoScalingRollingUpdate": { + "MinInstancesInService": 1, + "MaxBatchSize": 2, + "PauseTime": "PT5M", + "WaitOnResourceSignals": "true" + } + }, + { + "AutoScalingRollingUpdate": { + "MinInstancesInService": 3, + "MaxBatchSize": 4, + "PauseTime": "PT5M", + "WaitOnResourceSignals": "false" + } + } + ] + } + } + } +} \ No newline at end of file diff --git a/packages/aws-cdk-lib/cloudformation-include/test/test-templates/resource-attribute-update-policy.json b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/resource-attribute-update-policy.json deleted file mode 100644 index e1440a46193be..0000000000000 --- a/packages/aws-cdk-lib/cloudformation-include/test/test-templates/resource-attribute-update-policy.json +++ /dev/null @@ -1,61 +0,0 @@ -{ - "Parameters": { - "WaitOnResourceSignals": { - "Type": "String", - "Default": "true" - } - }, - "Resources": { - "CodeDeployApp": { - "Type": "AWS::CodeDeploy::Application" - }, - "CodeDeployDg": { - "Type": "AWS::CodeDeploy::DeploymentGroup", - "Properties": { - "ApplicationName": { "Ref": "CodeDeployApp" }, - "ServiceRoleArn": "my-role-arn" - } - }, - "Bucket": { - "Type": "AWS::S3::Bucket", - "UpdatePolicy": { - "AutoScalingReplacingUpdate": { - "WillReplace": false - }, - "AutoScalingRollingUpdate": { - "MaxBatchSize" : 1, - "MinInstancesInService" : 2, - "MinSuccessfulInstancesPercent" : 3, - "PauseTime" : "PT4M3S", - "SuspendProcesses" : [ - "Launch", - "Terminate", - "HealthCheck", - "ReplaceUnhealthy", - "AZRebalance", - "AlarmNotification", - "ScheduledActions", - "AddToLoadBalancer" - ], - "WaitOnResourceSignals" : { - "Fn::Equals": [ - "true", - { "Ref": "WaitOnResourceSignals" } - ] - } - }, - "AutoScalingScheduledAction": { - "IgnoreUnmodifiedGroupSizeProperties": true - }, - "CodeDeployLambdaAliasUpdate" : { - "AfterAllowTrafficHook" : "Lambda1", - "ApplicationName" : { "Ref": "CodeDeployApp" }, - "BeforeAllowTrafficHook" : "Lambda2", - "DeploymentGroupName" : { "Ref": "CodeDeployDg" } - }, - "EnableVersionUpgrade": true, - "UseOnlineResharding": false - } - } - } -} diff --git a/packages/aws-cdk-lib/cloudformation-include/test/valid-templates.test.ts b/packages/aws-cdk-lib/cloudformation-include/test/valid-templates.test.ts index e7c38c36652fa..2f7b132f4aaf8 100644 --- a/packages/aws-cdk-lib/cloudformation-include/test/valid-templates.test.ts +++ b/packages/aws-cdk-lib/cloudformation-include/test/valid-templates.test.ts @@ -611,24 +611,91 @@ describe('CDK Include', () => { }); }); - test('correctly handles the CreationPolicy resource attribute', () => { - const cfnTemplate = includeTestTemplate(stack, 'resource-attribute-creation-policy.json'); - const cfnBucket = cfnTemplate.getResource('Bucket'); + test('Intrinsics can be used in the leaf nodes of autoscaling creation policy', () => { + const cfnTemplate = includeTestTemplate(stack, 'intrinsics-create-policy-autoscaling.json'); + const cfnBucket = cfnTemplate.getResource('AutoScalingCreationPolicyIntrinsic'); expect(cfnBucket.cfnOptions.creationPolicy).toBeDefined(); Template.fromStack(stack).templateMatches( - loadTestFileToJsObject('resource-attribute-creation-policy.json'), + loadTestFileToJsObject('intrinsics-create-policy-autoscaling.json'), ); }); - test('correctly handles the UpdatePolicy resource attribute', () => { - const cfnTemplate = includeTestTemplate(stack, 'resource-attribute-update-policy.json'); - const cfnBucket = cfnTemplate.getResource('Bucket'); + test('Nested intrinsics can be used in the leaf nodes of autoscaling creation policy', () => { + const cfnTemplate = includeTestTemplate(stack, 'intrinsics-create-policy-complex.json'); + const cfnBucket = cfnTemplate.getResource('ASG'); + + expect(cfnBucket.cfnOptions.creationPolicy).toBeDefined(); + + Template.fromStack(stack).templateMatches( + loadTestFileToJsObject('intrinsics-create-policy-complex.json'), + ); + }); + + test('intrinsics can be used in the leaf nodes of autoscaling resource signal creation policy', () => { + const cfnTemplate = includeTestTemplate(stack, 'intrinsics-create-policy-resource-signal.json'); + const cfnBucket = cfnTemplate.getResource('ResourceSignalIntrinsic'); + + expect(cfnBucket.cfnOptions.creationPolicy).toBeDefined(); + + Template.fromStack(stack).templateMatches( + loadTestFileToJsObject('intrinsics-create-policy-resource-signal.json'), + ); + }); + + test('Intrinsics can be used in the leaf nodes of autoscaling rolling update policy', () => { + const cfnTemplate = includeTestTemplate(stack, 'intrinsics-update-policy-autoscaling-rolling-update.json'); + const cfnBucket = cfnTemplate.getResource('ASG'); + + expect(cfnBucket.cfnOptions.updatePolicy).toBeDefined(); + + Template.fromStack(stack).templateMatches( + loadTestFileToJsObject('intrinsics-update-policy-autoscaling-rolling-update.json'), + ); + }); + + test('Intrinsics can be used in the leaf nodes of autoscaling replacing update policy', () => { + const cfnTemplate = includeTestTemplate(stack, 'intrinsics-update-policy-autoscaling-replacing-update.json'); + const cfnBucket = cfnTemplate.getResource('ASG'); + + expect(cfnBucket.cfnOptions.updatePolicy).toBeDefined(); + + Template.fromStack(stack).templateMatches( + loadTestFileToJsObject('intrinsics-update-policy-autoscaling-replacing-update.json'), + ); + }); + + test('Intrinsics can be used in the leaf nodes of autoscaling scheduled-action update policy', () => { + const cfnTemplate = includeTestTemplate(stack, 'intrinsics-update-policy-autoscaling-scheduled-action.json'); + const cfnBucket = cfnTemplate.getResource('ASG'); expect(cfnBucket.cfnOptions.updatePolicy).toBeDefined(); + + Template.fromStack(stack).templateMatches( + loadTestFileToJsObject('intrinsics-update-policy-autoscaling-scheduled-action.json'), + ); + }); + + test('Intrinsics can be used in the leaf nodes of code deploy lambda alias update policy', () => { + const cfnTemplate = includeTestTemplate(stack, 'intrinsics-update-policy-code-deploy-lambda-alias-update.json'); + const cfnBucket = cfnTemplate.getResource('Alias'); + + expect(cfnBucket.cfnOptions.updatePolicy).toBeDefined(); + Template.fromStack(stack).templateMatches( - loadTestFileToJsObject('resource-attribute-update-policy.json'), + loadTestFileToJsObject('intrinsics-update-policy-code-deploy-lambda-alias-update.json'), + ); + }); + + test('Nested Intrinsics can be used in the leaf nodes of autoscaling rolling update policy', () => { + const cfnTemplate = includeTestTemplate(stack, 'intrinsics-update-policy-complex.json'); + const cfnBucket = cfnTemplate.getResource('ASG'); + + expect(cfnBucket.cfnOptions.updatePolicy).toBeDefined(); + + Template.fromStack(stack).templateMatches( + loadTestFileToJsObject('intrinsics-update-policy-complex.json'), ); }); @@ -1129,14 +1196,6 @@ describe('CDK Include', () => { loadTestFileToJsObject('tags-with-intrinsics.json'), ); }); - - test('Intrinsics can be used in update policy attribute', () => { - includeTestTemplate(stack, 'update-policy-with-intrinsics.json'); - - Template.fromStack(stack).templateMatches( - loadTestFileToJsObject('update-policy-with-intrinsics.json'), - ); - }); }); interface IncludeTestTemplateProps { diff --git a/packages/aws-cdk-lib/core/lib/helpers-internal/cfn-parse.ts b/packages/aws-cdk-lib/core/lib/helpers-internal/cfn-parse.ts index fbbb26c0c566e..2949b780c902e 100644 --- a/packages/aws-cdk-lib/core/lib/helpers-internal/cfn-parse.ts +++ b/packages/aws-cdk-lib/core/lib/helpers-internal/cfn-parse.ts @@ -383,6 +383,8 @@ export class CfnParser { private parseCreationPolicy(policy: any): CfnCreationPolicy | undefined { if (typeof policy !== 'object') { return undefined; } + this.throwIfIsIntrinsic(policy); + const self = this; // change simple JS values to their CDK equivalents policy = this.parseValue(policy); @@ -393,6 +395,7 @@ export class CfnParser { }); function parseAutoScalingCreationPolicy(p: any): CfnResourceAutoScalingCreationPolicy | undefined { + self.throwIfIsIntrinsic(p); if (typeof p !== 'object') { return undefined; } return undefinedIfAllValuesAreEmpty({ @@ -402,6 +405,7 @@ export class CfnParser { function parseResourceSignal(p: any): CfnResourceSignal | undefined { if (typeof p !== 'object') { return undefined; } + self.throwIfIsIntrinsic(p); return undefinedIfAllValuesAreEmpty({ count: FromCloudFormation.getNumber(p.Count).value, @@ -412,6 +416,8 @@ export class CfnParser { private parseUpdatePolicy(policy: any): CfnUpdatePolicy | undefined { if (typeof policy !== 'object') { return undefined; } + this.throwIfIsIntrinsic(policy); + const self = this; // change simple JS values to their CDK equivalents policy = this.parseValue(policy); @@ -427,6 +433,7 @@ export class CfnParser { function parseAutoScalingReplacingUpdate(p: any): CfnAutoScalingReplacingUpdate | undefined { if (typeof p !== 'object') { return undefined; } + self.throwIfIsIntrinsic(p); return undefinedIfAllValuesAreEmpty({ willReplace: p.WillReplace, @@ -435,6 +442,7 @@ export class CfnParser { function parseAutoScalingRollingUpdate(p: any): CfnAutoScalingRollingUpdate | undefined { if (typeof p !== 'object') { return undefined; } + self.throwIfIsIntrinsic(p); return undefinedIfAllValuesAreEmpty({ maxBatchSize: FromCloudFormation.getNumber(p.MaxBatchSize).value, @@ -447,6 +455,7 @@ export class CfnParser { } function parseCodeDeployLambdaAliasUpdate(p: any): CfnCodeDeployLambdaAliasUpdate | undefined { + self.throwIfIsIntrinsic(p); if (typeof p !== 'object') { return undefined; } return { @@ -458,6 +467,7 @@ export class CfnParser { } function parseAutoScalingScheduledAction(p: any): CfnAutoScalingScheduledAction | undefined { + self.throwIfIsIntrinsic(p); if (typeof p !== 'object') { return undefined; } return undefinedIfAllValuesAreEmpty({ @@ -674,6 +684,15 @@ export class CfnParser { } } + private throwIfIsIntrinsic(object: object): void { + // Top-level parsing functions check before we call `parseValue`, which requires + // calling `looksLikeCfnIntrinsic`. Helper parsing functions check after we call + // `parseValue`, which requires calling `isResolvableObject`. + if (isResolvableObject(object ?? {}) || this.looksLikeCfnIntrinsic(object ?? {})) { + throw new Error('intrinsics cannot be used in update, deletion, or update-replace policies.'); + } + } + private looksLikeCfnIntrinsic(object: object): string | undefined { const objectKeys = Object.keys(object); // a CFN intrinsic is always an object with a single key From 0f27437c88d2f4043bc56db0d7b9b581f2d78c65 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Thu, 26 Sep 2024 13:05:27 -0700 Subject: [PATCH 03/16] it doesn't throw here, which is fine --- .../test/auto-scaling-group.test.ts | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/aws-autoscaling/test/auto-scaling-group.test.ts b/packages/aws-cdk-lib/aws-autoscaling/test/auto-scaling-group.test.ts index 1ccbdd5f423ba..65a8fce324433 100644 --- a/packages/aws-cdk-lib/aws-autoscaling/test/auto-scaling-group.test.ts +++ b/packages/aws-cdk-lib/aws-autoscaling/test/auto-scaling-group.test.ts @@ -8,7 +8,7 @@ import * as iam from '../../aws-iam'; import * as sns from '../../aws-sns'; import * as ssm from '../../aws-ssm'; import * as cdk from '../../core'; -import { AUTOSCALING_GENERATE_LAUNCH_TEMPLATE } from '../../cx-api'; +import { AUTOSCALING_GENERATE_LAUNCH_TEMPLATE, REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS } from '../../cx-api'; import * as autoscaling from '../lib'; import { OnDemandAllocationStrategy, SpotAllocationStrategy } from '../lib'; @@ -2481,6 +2481,38 @@ describe('auto scaling group', () => { }).toThrow("Setting \'keyPair\' must not be set when \'launchTemplate\' or \'mixedInstancesPolicy\' is set"); }); + test('complex intrinsics are forbidden in update policies when the FF is enabled', () => { + // GIVEN + const stack = new cdk.Stack(); + stack.node.setContext(REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); + const vpc = mockVpc(stack); + + const condition = new cdk.CfnCondition(stack, 'SomeCondition', { + expression: cdk.Fn.conditionNot(cdk.Fn.conditionEquals(stack.region, cdk.Aws.REGION)), + }); + + // WHEN + new autoscaling.AutoScalingGroup(stack, 'mip-asg', { + vpc, + machineImage: new ec2.AmazonLinuxImage(), + instanceType: ec2.InstanceType.of(ec2.InstanceClass.M4, ec2.InstanceSize.MICRO), + updatePolicy: { + _renderUpdatePolicy(): cdk.CfnUpdatePolicy { + return { + autoScalingReplacingUpdate: cdk.Fn.conditionIf(condition.logicalId, + { willReplace: true }, + { willReplace: false }, + ) as any, + }; + }, + }, + }); + + // THEN + expect(() => { + Template.fromStack(stack); + }).toThrow('bad'); + }); }); function mockVpc(stack: cdk.Stack) { From d04f76560385e6555161b3c94ccd11f5bba6c1fa Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Thu, 26 Sep 2024 13:05:51 -0700 Subject: [PATCH 04/16] remove test --- .../test/auto-scaling-group.test.ts | 34 +------------------ 1 file changed, 1 insertion(+), 33 deletions(-) diff --git a/packages/aws-cdk-lib/aws-autoscaling/test/auto-scaling-group.test.ts b/packages/aws-cdk-lib/aws-autoscaling/test/auto-scaling-group.test.ts index 65a8fce324433..1ccbdd5f423ba 100644 --- a/packages/aws-cdk-lib/aws-autoscaling/test/auto-scaling-group.test.ts +++ b/packages/aws-cdk-lib/aws-autoscaling/test/auto-scaling-group.test.ts @@ -8,7 +8,7 @@ import * as iam from '../../aws-iam'; import * as sns from '../../aws-sns'; import * as ssm from '../../aws-ssm'; import * as cdk from '../../core'; -import { AUTOSCALING_GENERATE_LAUNCH_TEMPLATE, REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS } from '../../cx-api'; +import { AUTOSCALING_GENERATE_LAUNCH_TEMPLATE } from '../../cx-api'; import * as autoscaling from '../lib'; import { OnDemandAllocationStrategy, SpotAllocationStrategy } from '../lib'; @@ -2481,38 +2481,6 @@ describe('auto scaling group', () => { }).toThrow("Setting \'keyPair\' must not be set when \'launchTemplate\' or \'mixedInstancesPolicy\' is set"); }); - test('complex intrinsics are forbidden in update policies when the FF is enabled', () => { - // GIVEN - const stack = new cdk.Stack(); - stack.node.setContext(REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); - const vpc = mockVpc(stack); - - const condition = new cdk.CfnCondition(stack, 'SomeCondition', { - expression: cdk.Fn.conditionNot(cdk.Fn.conditionEquals(stack.region, cdk.Aws.REGION)), - }); - - // WHEN - new autoscaling.AutoScalingGroup(stack, 'mip-asg', { - vpc, - machineImage: new ec2.AmazonLinuxImage(), - instanceType: ec2.InstanceType.of(ec2.InstanceClass.M4, ec2.InstanceSize.MICRO), - updatePolicy: { - _renderUpdatePolicy(): cdk.CfnUpdatePolicy { - return { - autoScalingReplacingUpdate: cdk.Fn.conditionIf(condition.logicalId, - { willReplace: true }, - { willReplace: false }, - ) as any, - }; - }, - }, - }); - - // THEN - expect(() => { - Template.fromStack(stack); - }).toThrow('bad'); - }); }); function mockVpc(stack: cdk.Stack) { From f6d023d15f38fa5b4c6491ee4a14a3ad8f2b352b Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Thu, 26 Sep 2024 13:23:53 -0700 Subject: [PATCH 05/16] flags --- .../test/invalid-templates.test.ts | 15 +++++++++++++++ .../core/lib/helpers-internal/cfn-parse.ts | 14 ++++++++++++-- packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md | 16 ++++++++++++++++ packages/aws-cdk-lib/cx-api/lib/features.ts | 14 ++++++++++++++ 4 files changed, 57 insertions(+), 2 deletions(-) diff --git a/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts b/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts index 383d17cb09ac5..5dec3bfe524bb 100644 --- a/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts +++ b/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts @@ -253,52 +253,67 @@ describe('CDK Include', () => { }); test('non-leaf Intrinsics cannot be used in the top-level creation policy', () => { + stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); expect(() => { includeTestTemplate(stack, 'intrinsics-create-policy.json'); }).toThrow(/intrinsics cannot be used in update, deletion, or update-replace policies./); }); test('Intrinsics cannot be used in the autoscaling creation policy', () => { + stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); expect(() => { includeTestTemplate(stack, 'intrinsics-create-policy-autoscaling.json'); }).toThrow(/intrinsics cannot be used in update, deletion, or update-replace policies./); }); test('Intrinsics cannot be used in the create policy resource signal', () => { + stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); expect(() => { includeTestTemplate(stack, 'intrinsics-create-policy-resource-signal.json'); }).toThrow(/intrinsics cannot be used in update, deletion, or update-replace policies./); }); test('Intrinsics cannot be used in the top-level update policy', () => { + stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); expect(() => { includeTestTemplate(stack, 'intrinsics-update-policy.json'); }).toThrow(/intrinsics cannot be used in update, deletion, or update-replace policies./); }); test('Intrinsics cannot be used in the auto scaling rolling update update policy', () => { + stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); expect(() => { includeTestTemplate(stack, 'intrinsics-update-policy-autoscaling-rolling-update.json'); }).toThrow(/intrinsics cannot be used in update, deletion, or update-replace policies./); }); test('Intrinsics cannot be used in the auto scaling replacing update update policy', () => { + stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); expect(() => { includeTestTemplate(stack, 'intrinsics-update-policy-autoscaling-replacing-update.json'); }).toThrow(/intrinsics cannot be used in update, deletion, or update-replace policies./); }); test('Intrinsics cannot be used in the auto scaling scheduled action update policy', () => { + stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); expect(() => { includeTestTemplate(stack, 'intrinsics-update-policy-autoscaling-scheduled-action.json'); }).toThrow(/intrinsics cannot be used in update, deletion, or update-replace policies./); }); test('Intrinsics cannot be used in the code deploy lambda alias update policy', () => { + stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); expect(() => { includeTestTemplate(stack, 'intrinsics-update-policy-code-deploy-lambda-alias-update.json'); }).toThrow(/intrinsics cannot be used in update, deletion, or update-replace policies./); }); + + test('FF toggles error checking', () => { + stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, false); + expect(() => { + includeTestTemplate(stack, 'intrinsics-update-policy-code-deploy-lambda-alias-update.json'); + }).not.toThrow(); + }); }); interface IncludeTestTemplateProps { diff --git a/packages/aws-cdk-lib/core/lib/helpers-internal/cfn-parse.ts b/packages/aws-cdk-lib/core/lib/helpers-internal/cfn-parse.ts index 2949b780c902e..dd01b44dac21b 100644 --- a/packages/aws-cdk-lib/core/lib/helpers-internal/cfn-parse.ts +++ b/packages/aws-cdk-lib/core/lib/helpers-internal/cfn-parse.ts @@ -1,3 +1,4 @@ +import { CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS } from '../../../cx-api'; import { CfnCondition } from '../cfn-condition'; import { CfnElement } from '../cfn-element'; import { Fn } from '../cfn-fn'; @@ -9,10 +10,12 @@ import { CfnCreationPolicy, CfnDeletionPolicy, CfnResourceAutoScalingCreationPolicy, CfnResourceSignal, CfnUpdatePolicy, } from '../cfn-resource-policy'; import { CfnTag } from '../cfn-tag'; +import { FeatureFlags } from '../feature-flags'; import { Lazy } from '../lazy'; import { CfnReference, ReferenceRendering } from '../private/cfn-reference'; import { IResolvable } from '../resolvable'; import { Validator } from '../runtime'; +import { Stack } from '../stack'; import { isResolvableObject, Token } from '../token'; import { undefinedIfAllValuesAreEmpty } from '../util'; @@ -343,6 +346,7 @@ export interface ParseCfnOptions { */ export class CfnParser { private readonly options: ParseCfnOptions; + private stack?: Stack; constructor(options: ParseCfnOptions) { this.options = options; @@ -350,6 +354,7 @@ export class CfnParser { public handleAttributes(resource: CfnResource, resourceAttributes: any, logicalId: string): void { const cfnOptions = resource.cfnOptions; + this.stack = Stack.of(resource); cfnOptions.creationPolicy = this.parseCreationPolicy(resourceAttributes.CreationPolicy); cfnOptions.updatePolicy = this.parseUpdatePolicy(resourceAttributes.UpdatePolicy); @@ -688,8 +693,13 @@ export class CfnParser { // Top-level parsing functions check before we call `parseValue`, which requires // calling `looksLikeCfnIntrinsic`. Helper parsing functions check after we call // `parseValue`, which requires calling `isResolvableObject`. - if (isResolvableObject(object ?? {}) || this.looksLikeCfnIntrinsic(object ?? {})) { - throw new Error('intrinsics cannot be used in update, deletion, or update-replace policies.'); + if (!this.stack) { + throw new Error('cannot call this method before handleAttributes!'); + } + if (FeatureFlags.of(this.stack).isEnabled(CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS)) { + if (isResolvableObject(object ?? {}) || this.looksLikeCfnIntrinsic(object ?? {})) { + throw new Error('intrinsics cannot be used in update, deletion, or update-replace policies.'); + } } } diff --git a/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md b/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md index 5c88640f1a502..6cb4f498f1db3 100644 --- a/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md +++ b/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md @@ -1415,5 +1415,21 @@ When this feature flag is enabled, if both initOptions.timeout and resourceSigna | (not in v1) | | | | 2.160.0 | `false` | `true` | +### @aws-cdk/core:cfnIncludeRejectComplexResourceUpdateCreatePolicyIntrinsics + +*When enabled, CFN templates added with `cfn-include` will error if the template contains Resource Update or Create policies with CFN Intrinsics that include non-primitive values.* (fix) + +Without enabling this feature flag, `cfn-include` will silently drop resource update or create policies that contain CFN Intrinsics if they include non-primitive values. + +Enabling this feature flag will make `cfn-include` reject these templates. + + +| Since | Default | Recommended | +| ----- | ----- | ----- | +| (not in v1) | | | +| V2NEXT | `false` | `true` | + +**Compatibility with old behavior:** Disable the feature flag to silently drop these policies instead of throwing errors. + diff --git a/packages/aws-cdk-lib/cx-api/lib/features.ts b/packages/aws-cdk-lib/cx-api/lib/features.ts index 2eed0dcfd6c6d..4906ae238c54b 100644 --- a/packages/aws-cdk-lib/cx-api/lib/features.ts +++ b/packages/aws-cdk-lib/cx-api/lib/features.ts @@ -109,6 +109,7 @@ export const S3_KEEP_NOTIFICATION_IN_IMPORTED_BUCKET = '@aws-cdk/aws-s3:keepNoti export const USE_NEW_S3URI_PARAMETERS_FOR_BEDROCK_INVOKE_MODEL_TASK = '@aws-cdk/aws-stepfunctions-tasks:useNewS3UriParametersForBedrockInvokeModelTask'; export const REDUCE_EC2_FARGATE_CLOUDWATCH_PERMISSIONS = '@aws-cdk/aws-ecs:reduceEc2FargateCloudWatchPermissions'; export const EC2_SUM_TIMEOUT_ENABLED = '@aws-cdk/aws-ec2:ec2SumTImeoutEnabled'; +export const CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS = '@aws-cdk/core:cfnIncludeRejectComplexResourceUpdateCreatePolicyIntrinsics'; export const FLAGS: Record = { ////////////////////////////////////////////////////////////////////// @@ -1157,6 +1158,19 @@ export const FLAGS: Record = { recommendedValue: true, introducedIn: { v2: '2.160.0' }, }, + + ////////////////////////////////////////////////////////////////////// + [CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS]: { + type: FlagType.BugFix, + summary: 'When enabled, CFN templates added with `cfn-include` will error if the template contains Resource Update or Create policies with CFN Intrinsics that include non-primitive values.', + detailsMd: ` + Without enabling this feature flag, \`cfn-include\` will silently drop resource update or create policies that contain CFN Intrinsics if they include non-primitive values. + + Enabling this feature flag will make \`cfn-include\` throw on these templates. + `, + recommendedValue: true, + introducedIn: { v2: '2.160.0' }, + }, }; const CURRENT_MV = 'v2'; From 6ce9b2961fe84ca5eea4cfbac03786336661c694 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Thu, 26 Sep 2024 13:57:49 -0700 Subject: [PATCH 06/16] tests --- .../integ.intrinsic-update-create-policy.ts | 21 ++++++++++ .../invalid/intrinsics-create-policy.json | 42 +++++++++++++++++++ .../invalid/intrinsics-update-policy.json | 40 ++++++++++++++++++ 3 files changed, 103 insertions(+) create mode 100644 packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/integ.intrinsic-update-create-policy.ts create mode 100644 packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/test-templates/invalid/intrinsics-create-policy.json create mode 100644 packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy.json diff --git a/packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/integ.intrinsic-update-create-policy.ts b/packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/integ.intrinsic-update-create-policy.ts new file mode 100644 index 0000000000000..8577e618f1887 --- /dev/null +++ b/packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/integ.intrinsic-update-create-policy.ts @@ -0,0 +1,21 @@ +import * as core from 'aws-cdk-lib'; +import * as inc from 'aws-cdk-lib/cloudformation-include'; +import * as integ from '@aws-cdk/integ-tests-alpha'; +import { CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS } from 'aws-cdk-lib/cx-api'; + +const app = new core.App(); + +const stack = new core.Stack(app, 'Stack'); +stack.node.setContext(CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, false); + +// invalid without the FF +new inc.CfnInclude(stack, 'Stack', { + templateFile: 'test-templates/invalid/intrinsics-update-policy.json', +}); +new inc.CfnInclude(stack, 'Stack', { + templateFile: 'test-templates/invalid/intrinsics-create-policy.json', +}); + +new integ.IntegTest(app, 'IntrinsicsUpdatePolicy', { + testCases: [stack], +}); diff --git a/packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/test-templates/invalid/intrinsics-create-policy.json b/packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/test-templates/invalid/intrinsics-create-policy.json new file mode 100644 index 0000000000000..2afe984191fcc --- /dev/null +++ b/packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/test-templates/invalid/intrinsics-create-policy.json @@ -0,0 +1,42 @@ +{ + "Parameters": { + "CountParameter": { + "Type": "Number", + "Default": 3 + } + }, + "Conditions": { + "SomeCondition": { + "Fn::Equals": [ + 2, + 2 + ] + } + }, + "Resources": { + "CreationPolicyIntrinsic": { + "Type": "AWS::AutoScaling::AutoScalingGroup", + "Properties": { + "MinSize": 1, + "MaxSize": 5 + }, + "CreationPolicy": { + "Fn::If": [ + "SomeCondition", + { + "AutoScalingCreationPolicy": { + "MinSuccessfulInstancesPercent": 50 + }, + "ResourceSignal": { + "Count": 5, + "Timeout": "PT5H4M3S" + } + }, + { + "Ref": "AWS::NoValue" + } + ] + } + } + } +} diff --git a/packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy.json b/packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy.json new file mode 100644 index 0000000000000..fe284e4e05828 --- /dev/null +++ b/packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy.json @@ -0,0 +1,40 @@ +{ + "Conditions": { + "SomeCondition": { + "Fn::Equals": [ + 2, + 2 + ] + } + }, + "Resources": { + "ASG": { + "Type": "AWS::AutoScaling::AutoScalingGroup", + "Properties": { + "MaxSize": 10, + "MinSize": 1 + }, + "UpdatePolicy": { + "Fn::If": [ + "SomeCondition", + { + "AutoScalingRollingUpdate": { + "MinInstancesInService": 1, + "MaxBatchSize": 2, + "PauseTime": "PT5M", + "WaitOnResourceSignals": "true" + } + }, + { + "AutoScalingRollingUpdate": { + "MinInstancesInService": 3, + "MaxBatchSize": 4, + "PauseTime": "PT5M", + "WaitOnResourceSignals": "false" + } + } + ] + } + } + } +} \ No newline at end of file From 3201024ef737c47258d07f2f18541d3bf5457b10 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Fri, 27 Sep 2024 09:01:56 -0700 Subject: [PATCH 07/16] yesyes --- packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md b/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md index 6cb4f498f1db3..3d1be600cacac 100644 --- a/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md +++ b/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md @@ -75,6 +75,7 @@ Flags come in three types: | [@aws-cdk/aws-stepfunctions-tasks:useNewS3UriParametersForBedrockInvokeModelTask](#aws-cdkaws-stepfunctions-tasksusenews3uriparametersforbedrockinvokemodeltask) | When enabled, use new props for S3 URI field in task definition of state machine for bedrock invoke model. | 2.156.0 | (fix) | | [@aws-cdk/aws-ecs:reduceEc2FargateCloudWatchPermissions](#aws-cdkaws-ecsreduceec2fargatecloudwatchpermissions) | When enabled, we will only grant the necessary permissions when users specify cloudwatch log group through logConfiguration | 2.159.0 | (fix) | | [@aws-cdk/aws-ec2:ec2SumTImeoutEnabled](#aws-cdkaws-ec2ec2sumtimeoutenabled) | When enabled, initOptions.timeout and resourceSignalTimeout values will be summed together. | 2.160.0 | (fix) | +| [@aws-cdk/core:cfnIncludeRejectComplexResourceUpdateCreatePolicyIntrinsics](#aws-cdkcorecfnincluderejectcomplexresourceupdatecreatepolicyintrinsics) | When enabled, CFN templates added with `cfn-include` will error if the template contains Resource Update or Create policies with CFN Intrinsics that include non-primitive values. | 2.160.0 | (fix) | @@ -138,7 +139,8 @@ The following json shows the current recommended set of flags, as `cdk init` wou "@aws-cdk/custom-resources:logApiResponseDataPropertyTrueDefault": false, "@aws-cdk/aws-s3:keepNotificationInImportedBucket": false, "@aws-cdk/aws-ecs:reduceEc2FargateCloudWatchPermissions": true, - "@aws-cdk/aws-ec2:ec2SumTImeoutEnabled": true + "@aws-cdk/aws-ec2:ec2SumTImeoutEnabled": true, + "@aws-cdk/core:cfnIncludeRejectComplexResourceUpdateCreatePolicyIntrinsics": true } } ``` @@ -1415,6 +1417,7 @@ When this feature flag is enabled, if both initOptions.timeout and resourceSigna | (not in v1) | | | | 2.160.0 | `false` | `true` | + ### @aws-cdk/core:cfnIncludeRejectComplexResourceUpdateCreatePolicyIntrinsics *When enabled, CFN templates added with `cfn-include` will error if the template contains Resource Update or Create policies with CFN Intrinsics that include non-primitive values.* (fix) @@ -1431,5 +1434,4 @@ Enabling this feature flag will make `cfn-include` reject these templates. **Compatibility with old behavior:** Disable the feature flag to silently drop these policies instead of throwing errors. - From 36f3c0985dcd964c65e3cc166126927401778ad7 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Mon, 30 Sep 2024 09:29:07 -0700 Subject: [PATCH 08/16] integ --- ...date-create-policy-unhydrated-resources.ts | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/integ.intrinsic-update-create-policy-unhydrated-resources.ts diff --git a/packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/integ.intrinsic-update-create-policy-unhydrated-resources.ts b/packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/integ.intrinsic-update-create-policy-unhydrated-resources.ts new file mode 100644 index 0000000000000..6c0b4240fa34a --- /dev/null +++ b/packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/integ.intrinsic-update-create-policy-unhydrated-resources.ts @@ -0,0 +1,22 @@ +import * as core from 'aws-cdk-lib'; +import * as inc from 'aws-cdk-lib/cloudformation-include'; +import * as integ from '@aws-cdk/integ-tests-alpha'; +import { CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS } from 'aws-cdk-lib/cx-api'; + +const app = new core.App(); + +const stack = new core.Stack(app, 'Stack'); +stack.node.setContext(CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); + +// invalid without the FF +new inc.CfnInclude(stack, 'Stack', { + templateFile: 'test-templates/invalid/intrinsics-update-policy.json', +}); +new inc.CfnInclude(stack, 'Stack', { + templateFile: 'test-templates/invalid/intrinsics-create-policy.json', +}); + +new integ.IntegTest(app, 'IntrinsicsUpdatePolicy', { + testCases: [stack], +}); + From fb97f185270b0e9e429dba214a8cfea9d54821c8 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Mon, 30 Sep 2024 13:39:39 -0700 Subject: [PATCH 09/16] removing integ tests --- ...date-create-policy-unhydrated-resources.ts | 22 ---------- .../integ.intrinsic-update-create-policy.ts | 21 ---------- .../invalid/intrinsics-create-policy.json | 42 ------------------- .../invalid/intrinsics-update-policy.json | 40 ------------------ .../cloudformation-include/lib/cfn-include.ts | 13 ++++++ .../test/invalid-templates.test.ts | 38 +++++++++++++---- .../core/lib/helpers-internal/cfn-parse.ts | 39 ++++++++++------- packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md | 5 +-- 8 files changed, 70 insertions(+), 150 deletions(-) delete mode 100644 packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/integ.intrinsic-update-create-policy-unhydrated-resources.ts delete mode 100644 packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/integ.intrinsic-update-create-policy.ts delete mode 100644 packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/test-templates/invalid/intrinsics-create-policy.json delete mode 100644 packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy.json diff --git a/packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/integ.intrinsic-update-create-policy-unhydrated-resources.ts b/packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/integ.intrinsic-update-create-policy-unhydrated-resources.ts deleted file mode 100644 index 6c0b4240fa34a..0000000000000 --- a/packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/integ.intrinsic-update-create-policy-unhydrated-resources.ts +++ /dev/null @@ -1,22 +0,0 @@ -import * as core from 'aws-cdk-lib'; -import * as inc from 'aws-cdk-lib/cloudformation-include'; -import * as integ from '@aws-cdk/integ-tests-alpha'; -import { CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS } from 'aws-cdk-lib/cx-api'; - -const app = new core.App(); - -const stack = new core.Stack(app, 'Stack'); -stack.node.setContext(CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); - -// invalid without the FF -new inc.CfnInclude(stack, 'Stack', { - templateFile: 'test-templates/invalid/intrinsics-update-policy.json', -}); -new inc.CfnInclude(stack, 'Stack', { - templateFile: 'test-templates/invalid/intrinsics-create-policy.json', -}); - -new integ.IntegTest(app, 'IntrinsicsUpdatePolicy', { - testCases: [stack], -}); - diff --git a/packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/integ.intrinsic-update-create-policy.ts b/packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/integ.intrinsic-update-create-policy.ts deleted file mode 100644 index 8577e618f1887..0000000000000 --- a/packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/integ.intrinsic-update-create-policy.ts +++ /dev/null @@ -1,21 +0,0 @@ -import * as core from 'aws-cdk-lib'; -import * as inc from 'aws-cdk-lib/cloudformation-include'; -import * as integ from '@aws-cdk/integ-tests-alpha'; -import { CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS } from 'aws-cdk-lib/cx-api'; - -const app = new core.App(); - -const stack = new core.Stack(app, 'Stack'); -stack.node.setContext(CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, false); - -// invalid without the FF -new inc.CfnInclude(stack, 'Stack', { - templateFile: 'test-templates/invalid/intrinsics-update-policy.json', -}); -new inc.CfnInclude(stack, 'Stack', { - templateFile: 'test-templates/invalid/intrinsics-create-policy.json', -}); - -new integ.IntegTest(app, 'IntrinsicsUpdatePolicy', { - testCases: [stack], -}); diff --git a/packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/test-templates/invalid/intrinsics-create-policy.json b/packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/test-templates/invalid/intrinsics-create-policy.json deleted file mode 100644 index 2afe984191fcc..0000000000000 --- a/packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/test-templates/invalid/intrinsics-create-policy.json +++ /dev/null @@ -1,42 +0,0 @@ -{ - "Parameters": { - "CountParameter": { - "Type": "Number", - "Default": 3 - } - }, - "Conditions": { - "SomeCondition": { - "Fn::Equals": [ - 2, - 2 - ] - } - }, - "Resources": { - "CreationPolicyIntrinsic": { - "Type": "AWS::AutoScaling::AutoScalingGroup", - "Properties": { - "MinSize": 1, - "MaxSize": 5 - }, - "CreationPolicy": { - "Fn::If": [ - "SomeCondition", - { - "AutoScalingCreationPolicy": { - "MinSuccessfulInstancesPercent": 50 - }, - "ResourceSignal": { - "Count": 5, - "Timeout": "PT5H4M3S" - } - }, - { - "Ref": "AWS::NoValue" - } - ] - } - } - } -} diff --git a/packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy.json b/packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy.json deleted file mode 100644 index fe284e4e05828..0000000000000 --- a/packages/@aws-cdk-testing/framework-integ/test/cloudformation-include/test/test-templates/invalid/intrinsics-update-policy.json +++ /dev/null @@ -1,40 +0,0 @@ -{ - "Conditions": { - "SomeCondition": { - "Fn::Equals": [ - 2, - 2 - ] - } - }, - "Resources": { - "ASG": { - "Type": "AWS::AutoScaling::AutoScalingGroup", - "Properties": { - "MaxSize": 10, - "MinSize": 1 - }, - "UpdatePolicy": { - "Fn::If": [ - "SomeCondition", - { - "AutoScalingRollingUpdate": { - "MinInstancesInService": 1, - "MaxBatchSize": 2, - "PauseTime": "PT5M", - "WaitOnResourceSignals": "true" - } - }, - { - "AutoScalingRollingUpdate": { - "MinInstancesInService": 3, - "MaxBatchSize": 4, - "PauseTime": "PT5M", - "WaitOnResourceSignals": "false" - } - } - ] - } - } - } -} \ No newline at end of file diff --git a/packages/aws-cdk-lib/cloudformation-include/lib/cfn-include.ts b/packages/aws-cdk-lib/cloudformation-include/lib/cfn-include.ts index 9a1018e79e026..2bb2d17427c17 100644 --- a/packages/aws-cdk-lib/cloudformation-include/lib/cfn-include.ts +++ b/packages/aws-cdk-lib/cloudformation-include/lib/cfn-include.ts @@ -66,6 +66,15 @@ export interface CfnIncludeProps { * @default - will throw an error on detecting any cyclical references */ readonly allowCyclicalReferences?: boolean; + + /** + * Specifies a list of LogicalIDs for resources that will be included in the CDK Stack, + * but which will not be accessible through `getResource` or other accessors. + * + * This allows you to use CFN templates that rely on Intrinsic placement that `cfn-include` + * will not parse, such as non-primitive values in resource update policies. + */ + readonly unhydratedResources?: string[]; } /** @@ -109,6 +118,7 @@ export class CfnInclude extends core.CfnElement { private readonly template: any; private readonly preserveLogicalIds: boolean; private readonly allowCyclicalReferences: boolean; + private readonly unhydratedResources?: string[]; private logicalIdToPlaceholderMap: Map; constructor(scope: Construct, id: string, props: CfnIncludeProps) { @@ -125,6 +135,8 @@ export class CfnInclude extends core.CfnElement { this.preserveLogicalIds = props.preserveLogicalIds ?? true; + this.unhydratedResources = props.unhydratedResources; + // check if all user specified parameter values exist in the template for (const logicalId of Object.keys(this.parametersToReplace)) { if (!(logicalId in (this.template.Parameters || {}))) { @@ -659,6 +671,7 @@ export class CfnInclude extends core.CfnElement { const cfnParser = new cfn_parse.CfnParser({ finder, parameters: this.parametersToReplace, + unhydratedResources: this.unhydratedResources, }); const resourceAttributes: any = this.template.Resources[logicalId]; diff --git a/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts b/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts index 5dec3bfe524bb..76dae8778df19 100644 --- a/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts +++ b/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts @@ -256,56 +256,56 @@ describe('CDK Include', () => { stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); expect(() => { includeTestTemplate(stack, 'intrinsics-create-policy.json'); - }).toThrow(/intrinsics cannot be used in update, deletion, or update-replace policies./); + }).toThrow(/Resource 'CreationPolicyIntrinsic' uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify 'CreationPolicyIntrinsic' in the 'unhydratedResources' prop to include this resource./); }); test('Intrinsics cannot be used in the autoscaling creation policy', () => { stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); expect(() => { includeTestTemplate(stack, 'intrinsics-create-policy-autoscaling.json'); - }).toThrow(/intrinsics cannot be used in update, deletion, or update-replace policies./); + }).toThrow(/Resource 'AutoScalingCreationPolicyIntrinsic' uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify 'AutoScalingCreationPolicyIntrinsic' in the 'unhydratedResources' prop to include this resource./); }); test('Intrinsics cannot be used in the create policy resource signal', () => { stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); expect(() => { includeTestTemplate(stack, 'intrinsics-create-policy-resource-signal.json'); - }).toThrow(/intrinsics cannot be used in update, deletion, or update-replace policies./); + }).toThrow(/Resource 'ResourceSignalIntrinsic' uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify 'ResourceSignalIntrinsic' in the 'unhydratedResources' prop to include this resource./); }); test('Intrinsics cannot be used in the top-level update policy', () => { stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); expect(() => { includeTestTemplate(stack, 'intrinsics-update-policy.json'); - }).toThrow(/intrinsics cannot be used in update, deletion, or update-replace policies./); + }).toThrow(/Resource 'ASG' uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify 'ASG' in the 'unhydratedResources' prop to include this resource./); }); test('Intrinsics cannot be used in the auto scaling rolling update update policy', () => { stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); expect(() => { includeTestTemplate(stack, 'intrinsics-update-policy-autoscaling-rolling-update.json'); - }).toThrow(/intrinsics cannot be used in update, deletion, or update-replace policies./); + }).toThrow(/Resource 'ASG' uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify 'ASG' in the 'unhydratedResources' prop to include this resource./); }); test('Intrinsics cannot be used in the auto scaling replacing update update policy', () => { stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); expect(() => { includeTestTemplate(stack, 'intrinsics-update-policy-autoscaling-replacing-update.json'); - }).toThrow(/intrinsics cannot be used in update, deletion, or update-replace policies./); + }).toThrow(/Resource 'ASG' uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify 'ASG' in the 'unhydratedResources' prop to include this resource./); }); test('Intrinsics cannot be used in the auto scaling scheduled action update policy', () => { stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); expect(() => { includeTestTemplate(stack, 'intrinsics-update-policy-autoscaling-scheduled-action.json'); - }).toThrow(/intrinsics cannot be used in update, deletion, or update-replace policies./); + }).toThrow(/Resource 'ASG' uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify 'ASG' in the 'unhydratedResources' prop to include this resource./); }); test('Intrinsics cannot be used in the code deploy lambda alias update policy', () => { stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); expect(() => { includeTestTemplate(stack, 'intrinsics-update-policy-code-deploy-lambda-alias-update.json'); - }).toThrow(/intrinsics cannot be used in update, deletion, or update-replace policies./); + }).toThrow(/Resource 'Alias' uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify 'Alias' in the 'unhydratedResources' prop to include this resource./); }); test('FF toggles error checking', () => { @@ -314,17 +314,39 @@ describe('CDK Include', () => { includeTestTemplate(stack, 'intrinsics-update-policy-code-deploy-lambda-alias-update.json'); }).not.toThrow(); }); + + test('FF disabled with unhydratedResources does not throw', () => { + stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, false); + expect(() => { + includeTestTemplate(stack, 'intrinsics-update-policy-code-deploy-lambda-alias-update.json', { + unhydratedResources: ['Alias'], + }); + }).not.toThrow(); + }); + + test('unhydrated resources appear in the template', () => { + stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); + includeTestTemplate(stack, 'intrinsics-update-policy-code-deploy-lambda-alias-update.json', { + unhydratedResources: ['Alias'], + }); + + expect(Template.fromStack(stack).hasResource('AWS::Lambda::Alias', {})); + }); }); interface IncludeTestTemplateProps { /** @default false */ readonly allowCyclicalReferences?: boolean; + + /** @default none */ + readonly unhydratedResources?: string[]; } function includeTestTemplate(scope: constructs.Construct, testTemplate: string, props: IncludeTestTemplateProps = {}): inc.CfnInclude { return new inc.CfnInclude(scope, 'MyScope', { templateFile: _testTemplateFilePath(testTemplate), allowCyclicalReferences: props.allowCyclicalReferences, + unhydratedResources: props.unhydratedResources, }); } diff --git a/packages/aws-cdk-lib/core/lib/helpers-internal/cfn-parse.ts b/packages/aws-cdk-lib/core/lib/helpers-internal/cfn-parse.ts index dd01b44dac21b..686b3dbf7d910 100644 --- a/packages/aws-cdk-lib/core/lib/helpers-internal/cfn-parse.ts +++ b/packages/aws-cdk-lib/core/lib/helpers-internal/cfn-parse.ts @@ -314,6 +314,11 @@ export enum CfnParsingContext { * The options for `FromCloudFormation.parseValue`. */ export interface ParseCfnOptions { + /** + * resources to not parse some data for + */ + unhydratedResources?: string[]; + /** * The finder interface used to resolve references in the template. */ @@ -346,18 +351,24 @@ export interface ParseCfnOptions { */ export class CfnParser { private readonly options: ParseCfnOptions; + private readonly unhydratedResources: string[]; private stack?: Stack; constructor(options: ParseCfnOptions) { this.options = options; + this.unhydratedResources = options.unhydratedResources ?? []; } public handleAttributes(resource: CfnResource, resourceAttributes: any, logicalId: string): void { const cfnOptions = resource.cfnOptions; this.stack = Stack.of(resource); - cfnOptions.creationPolicy = this.parseCreationPolicy(resourceAttributes.CreationPolicy); - cfnOptions.updatePolicy = this.parseUpdatePolicy(resourceAttributes.UpdatePolicy); + cfnOptions.creationPolicy = this.unhydratedResources.includes(logicalId) + ? resourceAttributes.CreationPolicy + : this.parseCreationPolicy(resourceAttributes.CreationPolicy, logicalId); + cfnOptions.updatePolicy = this.unhydratedResources.includes(logicalId) + ? resourceAttributes.UpdatePolicy + : this.parseUpdatePolicy(resourceAttributes.UpdatePolicy, logicalId); cfnOptions.deletionPolicy = this.parseDeletionPolicy(resourceAttributes.DeletionPolicy); cfnOptions.updateReplacePolicy = this.parseDeletionPolicy(resourceAttributes.UpdateReplacePolicy); cfnOptions.version = this.parseValue(resourceAttributes.Version); @@ -386,9 +397,9 @@ export class CfnParser { } } - private parseCreationPolicy(policy: any): CfnCreationPolicy | undefined { + private parseCreationPolicy(policy: any, logicalId: string): CfnCreationPolicy | undefined { if (typeof policy !== 'object') { return undefined; } - this.throwIfIsIntrinsic(policy); + this.throwIfIsIntrinsic(policy, logicalId); const self = this; // change simple JS values to their CDK equivalents @@ -400,7 +411,7 @@ export class CfnParser { }); function parseAutoScalingCreationPolicy(p: any): CfnResourceAutoScalingCreationPolicy | undefined { - self.throwIfIsIntrinsic(p); + self.throwIfIsIntrinsic(p, logicalId); if (typeof p !== 'object') { return undefined; } return undefinedIfAllValuesAreEmpty({ @@ -410,7 +421,7 @@ export class CfnParser { function parseResourceSignal(p: any): CfnResourceSignal | undefined { if (typeof p !== 'object') { return undefined; } - self.throwIfIsIntrinsic(p); + self.throwIfIsIntrinsic(p, logicalId); return undefinedIfAllValuesAreEmpty({ count: FromCloudFormation.getNumber(p.Count).value, @@ -419,9 +430,9 @@ export class CfnParser { } } - private parseUpdatePolicy(policy: any): CfnUpdatePolicy | undefined { + private parseUpdatePolicy(policy: any, logicalId: string): CfnUpdatePolicy | undefined { if (typeof policy !== 'object') { return undefined; } - this.throwIfIsIntrinsic(policy); + this.throwIfIsIntrinsic(policy, logicalId); const self = this; // change simple JS values to their CDK equivalents @@ -438,7 +449,7 @@ export class CfnParser { function parseAutoScalingReplacingUpdate(p: any): CfnAutoScalingReplacingUpdate | undefined { if (typeof p !== 'object') { return undefined; } - self.throwIfIsIntrinsic(p); + self.throwIfIsIntrinsic(p, logicalId); return undefinedIfAllValuesAreEmpty({ willReplace: p.WillReplace, @@ -447,7 +458,7 @@ export class CfnParser { function parseAutoScalingRollingUpdate(p: any): CfnAutoScalingRollingUpdate | undefined { if (typeof p !== 'object') { return undefined; } - self.throwIfIsIntrinsic(p); + self.throwIfIsIntrinsic(p, logicalId); return undefinedIfAllValuesAreEmpty({ maxBatchSize: FromCloudFormation.getNumber(p.MaxBatchSize).value, @@ -460,7 +471,7 @@ export class CfnParser { } function parseCodeDeployLambdaAliasUpdate(p: any): CfnCodeDeployLambdaAliasUpdate | undefined { - self.throwIfIsIntrinsic(p); + self.throwIfIsIntrinsic(p, logicalId); if (typeof p !== 'object') { return undefined; } return { @@ -472,7 +483,7 @@ export class CfnParser { } function parseAutoScalingScheduledAction(p: any): CfnAutoScalingScheduledAction | undefined { - self.throwIfIsIntrinsic(p); + self.throwIfIsIntrinsic(p, logicalId); if (typeof p !== 'object') { return undefined; } return undefinedIfAllValuesAreEmpty({ @@ -689,7 +700,7 @@ export class CfnParser { } } - private throwIfIsIntrinsic(object: object): void { + private throwIfIsIntrinsic(object: object, logicalId: string): void { // Top-level parsing functions check before we call `parseValue`, which requires // calling `looksLikeCfnIntrinsic`. Helper parsing functions check after we call // `parseValue`, which requires calling `isResolvableObject`. @@ -698,7 +709,7 @@ export class CfnParser { } if (FeatureFlags.of(this.stack).isEnabled(CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS)) { if (isResolvableObject(object ?? {}) || this.looksLikeCfnIntrinsic(object ?? {})) { - throw new Error('intrinsics cannot be used in update, deletion, or update-replace policies.'); + throw new Error(`Resource '${logicalId}' uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify '${logicalId}' in the 'unhydratedResources' prop to include this resource.`); } } } diff --git a/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md b/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md index 3d1be600cacac..aa7be14e31ad9 100644 --- a/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md +++ b/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md @@ -1424,14 +1424,13 @@ When this feature flag is enabled, if both initOptions.timeout and resourceSigna Without enabling this feature flag, `cfn-include` will silently drop resource update or create policies that contain CFN Intrinsics if they include non-primitive values. -Enabling this feature flag will make `cfn-include` reject these templates. +Enabling this feature flag will make `cfn-include` throw on these templates. | Since | Default | Recommended | | ----- | ----- | ----- | | (not in v1) | | | -| V2NEXT | `false` | `true` | +| 2.160.0 | `false` | `true` | -**Compatibility with old behavior:** Disable the feature flag to silently drop these policies instead of throwing errors. From bad6498c40330d44944cfcb68b5a16d0990f9d9a Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Mon, 30 Sep 2024 14:51:17 -0700 Subject: [PATCH 10/16] docstring --- .../aws-cdk-lib/cloudformation-include/lib/cfn-include.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/aws-cdk-lib/cloudformation-include/lib/cfn-include.ts b/packages/aws-cdk-lib/cloudformation-include/lib/cfn-include.ts index 2bb2d17427c17..15a5ad9c4a22d 100644 --- a/packages/aws-cdk-lib/cloudformation-include/lib/cfn-include.ts +++ b/packages/aws-cdk-lib/cloudformation-include/lib/cfn-include.ts @@ -69,10 +69,9 @@ export interface CfnIncludeProps { /** * Specifies a list of LogicalIDs for resources that will be included in the CDK Stack, - * but which will not be accessible through `getResource` or other accessors. - * - * This allows you to use CFN templates that rely on Intrinsic placement that `cfn-include` - * will not parse, such as non-primitive values in resource update policies. + * but will not be parsed and converted to CDK types. This allows you to use CFN templates + * that rely on Intrinsic placement that `cfn-include` + * would otherwise reject, such as non-primitive values in resource update policies. */ readonly unhydratedResources?: string[]; } From c8655b8fa816478f15bb928bf7fdfd9a59184ab6 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Mon, 30 Sep 2024 14:57:53 -0700 Subject: [PATCH 11/16] conflcits ; --- packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md b/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md index 1996338af8c3d..a512fbd6b4cb9 100644 --- a/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md +++ b/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md @@ -75,12 +75,9 @@ Flags come in three types: | [@aws-cdk/aws-stepfunctions-tasks:useNewS3UriParametersForBedrockInvokeModelTask](#aws-cdkaws-stepfunctions-tasksusenews3uriparametersforbedrockinvokemodeltask) | When enabled, use new props for S3 URI field in task definition of state machine for bedrock invoke model. | 2.156.0 | (fix) | | [@aws-cdk/aws-ecs:reduceEc2FargateCloudWatchPermissions](#aws-cdkaws-ecsreduceec2fargatecloudwatchpermissions) | When enabled, we will only grant the necessary permissions when users specify cloudwatch log group through logConfiguration | 2.159.0 | (fix) | | [@aws-cdk/aws-ec2:ec2SumTImeoutEnabled](#aws-cdkaws-ec2ec2sumtimeoutenabled) | When enabled, initOptions.timeout and resourceSignalTimeout values will be summed together. | 2.160.0 | (fix) | -<<<<<<< HEAD -| [@aws-cdk/core:cfnIncludeRejectComplexResourceUpdateCreatePolicyIntrinsics](#aws-cdkcorecfnincluderejectcomplexresourceupdatecreatepolicyintrinsics) | When enabled, CFN templates added with `cfn-include` will error if the template contains Resource Update or Create policies with CFN Intrinsics that include non-primitive values. | 2.160.0 | (fix) | -======= | [@aws-cdk/aws-appsync:appSyncGraphQLAPIScopeLambdaPermission](#aws-cdkaws-appsyncappsyncgraphqlapiscopelambdapermission) | When enabled, a Lambda authorizer Permission created when using GraphqlApi will be properly scoped with a SourceArn. | V2NEXT | (fix) | | [@aws-cdk/aws-rds:setCorrectValueForDatabaseInstanceReadReplicaInstanceResourceId](#aws-cdkaws-rdssetcorrectvaluefordatabaseinstancereadreplicainstanceresourceid) | When enabled, the value of property `instanceResourceId` in construct `DatabaseInstanceReadReplica` will be set to the correct value which is `DbiResourceId` instead of currently `DbInstanceArn` | V2NEXT | (fix) | ->>>>>>> b0e4a544aecce86e8b41e7cd148a139c2e34bfbd +| [@aws-cdk/core:cfnIncludeRejectComplexResourceUpdateCreatePolicyIntrinsics](#aws-cdkcorecfnincluderejectcomplexresourceupdatecreatepolicyintrinsics) | When enabled, CFN templates added with `cfn-include` will error if the template contains Resource Update or Create policies with CFN Intrinsics that include non-primitive values. | 2.160.0 | (fix) | From eccec599d70aed758d866d5472568c73d86528ae Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Mon, 30 Sep 2024 14:58:58 -0700 Subject: [PATCH 12/16] conflcits --- packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md b/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md index a512fbd6b4cb9..04b533b0862b6 100644 --- a/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md +++ b/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md @@ -142,12 +142,9 @@ The following json shows the current recommended set of flags, as `cdk init` wou "@aws-cdk/aws-s3:keepNotificationInImportedBucket": false, "@aws-cdk/aws-ecs:reduceEc2FargateCloudWatchPermissions": true, "@aws-cdk/aws-ec2:ec2SumTImeoutEnabled": true, -<<<<<<< HEAD + "@aws-cdk/aws-appsync:appSyncGraphQLAPIScopeLambdaPermission": true, + "@aws-cdk/aws-rds:setCorrectValueForDatabaseInstanceReadReplicaInstanceResourceId": true, "@aws-cdk/core:cfnIncludeRejectComplexResourceUpdateCreatePolicyIntrinsics": true -======= - "@aws-cdk/aws-appsync:appSyncGraphQLAPIScopeLambdaPermission": true - "@aws-cdk/aws-rds:setCorrectValueForDatabaseInstanceReadReplicaInstanceResourceId": true ->>>>>>> b0e4a544aecce86e8b41e7cd148a139c2e34bfbd } } ``` From eb5263c1e46a392a098d0767003c2c74db321996 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Tue, 1 Oct 2024 14:09:55 -0700 Subject: [PATCH 13/16] feedback --- .../cloudformation-include/lib/cfn-include.ts | 30 ++++++-- .../test/invalid-templates.test.ts | 72 +++++++++++++++---- .../test/nested-stacks.test.ts | 71 ++++++++++++++++++ .../invalid/resource-all-attributes.json | 27 +++++++ .../nested/child-dehydrated.json | 32 +++++++++ .../nested/parent-dehydrated.json | 41 +++++++++++ .../core/lib/helpers-internal/cfn-parse.ts | 17 +---- 7 files changed, 256 insertions(+), 34 deletions(-) create mode 100644 packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/resource-all-attributes.json create mode 100644 packages/aws-cdk-lib/cloudformation-include/test/test-templates/nested/child-dehydrated.json create mode 100644 packages/aws-cdk-lib/cloudformation-include/test/test-templates/nested/parent-dehydrated.json diff --git a/packages/aws-cdk-lib/cloudformation-include/lib/cfn-include.ts b/packages/aws-cdk-lib/cloudformation-include/lib/cfn-include.ts index 15a5ad9c4a22d..edd6d6b88d495 100644 --- a/packages/aws-cdk-lib/cloudformation-include/lib/cfn-include.ts +++ b/packages/aws-cdk-lib/cloudformation-include/lib/cfn-include.ts @@ -72,8 +72,10 @@ export interface CfnIncludeProps { * but will not be parsed and converted to CDK types. This allows you to use CFN templates * that rely on Intrinsic placement that `cfn-include` * would otherwise reject, such as non-primitive values in resource update policies. + * + * @default - All resources are hydrated */ - readonly unhydratedResources?: string[]; + readonly dehydratedResources?: string[]; } /** @@ -117,7 +119,7 @@ export class CfnInclude extends core.CfnElement { private readonly template: any; private readonly preserveLogicalIds: boolean; private readonly allowCyclicalReferences: boolean; - private readonly unhydratedResources?: string[]; + private readonly dehydratedResources: string[]; private logicalIdToPlaceholderMap: Map; constructor(scope: Construct, id: string, props: CfnIncludeProps) { @@ -134,7 +136,7 @@ export class CfnInclude extends core.CfnElement { this.preserveLogicalIds = props.preserveLogicalIds ?? true; - this.unhydratedResources = props.unhydratedResources; + this.dehydratedResources = props.dehydratedResources ?? []; // check if all user specified parameter values exist in the template for (const logicalId of Object.keys(this.parametersToReplace)) { @@ -670,13 +672,31 @@ export class CfnInclude extends core.CfnElement { const cfnParser = new cfn_parse.CfnParser({ finder, parameters: this.parametersToReplace, - unhydratedResources: this.unhydratedResources, }); const resourceAttributes: any = this.template.Resources[logicalId]; let l1Instance: core.CfnResource; - if (this.nestedStacksToInclude[logicalId]) { + if (this.nestedStacksToInclude[logicalId] && this.dehydratedResources.includes(logicalId)) { + throw new Error(`nested stack '${logicalId}' was marked as dehydrated - nested stacks cannot be dehydrated`); + } else if (this.nestedStacksToInclude[logicalId]) { l1Instance = this.createNestedStack(logicalId, cfnParser); + } else if (this.dehydratedResources.includes(logicalId)) { + + l1Instance = new core.CfnResource(this, logicalId, { + type: resourceAttributes.Type, + properties: resourceAttributes.Properties, + }); + const cfnOptions = l1Instance.cfnOptions; + cfnOptions.creationPolicy = resourceAttributes.CreationPolicy; + cfnOptions.updatePolicy = resourceAttributes.UpdatePolicy; + cfnOptions.deletionPolicy = resourceAttributes.DeletionPolicy; + cfnOptions.updateReplacePolicy = resourceAttributes.UpdateReplacePolicy; + cfnOptions.version = resourceAttributes.Version; + cfnOptions.description = resourceAttributes.Description; + cfnOptions.metadata = resourceAttributes.Metadata; + this.resources[logicalId] = l1Instance; + + return l1Instance; } else { const l1ClassFqn = cfn_type_to_l1_mapping.lookup(resourceAttributes.Type); // The AWS::CloudFormation::CustomResource type corresponds to the CfnCustomResource class. diff --git a/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts b/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts index 76dae8778df19..0a256eb885b2a 100644 --- a/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts +++ b/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts @@ -256,56 +256,56 @@ describe('CDK Include', () => { stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); expect(() => { includeTestTemplate(stack, 'intrinsics-create-policy.json'); - }).toThrow(/Resource 'CreationPolicyIntrinsic' uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify 'CreationPolicyIntrinsic' in the 'unhydratedResources' prop to include this resource./); + }).toThrow(/Cannot convert resource 'CreationPolicyIntrinsic' to CDK objects: it uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify 'CreationPolicyIntrinsic' in the 'dehydratedResources' prop to skip parsing this resource, while still including it in the output./); }); test('Intrinsics cannot be used in the autoscaling creation policy', () => { stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); expect(() => { includeTestTemplate(stack, 'intrinsics-create-policy-autoscaling.json'); - }).toThrow(/Resource 'AutoScalingCreationPolicyIntrinsic' uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify 'AutoScalingCreationPolicyIntrinsic' in the 'unhydratedResources' prop to include this resource./); + }).toThrow(/Cannot convert resource 'AutoScalingCreationPolicyIntrinsic' to CDK objects: it uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify 'AutoScalingCreationPolicyIntrinsic' in the 'dehydratedResources' prop to skip parsing this resource, while still including it in the output./); }); test('Intrinsics cannot be used in the create policy resource signal', () => { stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); expect(() => { includeTestTemplate(stack, 'intrinsics-create-policy-resource-signal.json'); - }).toThrow(/Resource 'ResourceSignalIntrinsic' uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify 'ResourceSignalIntrinsic' in the 'unhydratedResources' prop to include this resource./); + }).toThrow(/Cannot convert resource 'ResourceSignalIntrinsic' to CDK objects: it uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify 'ResourceSignalIntrinsic' in the 'dehydratedResources' prop to skip parsing this resource, while still including it in the output./); }); test('Intrinsics cannot be used in the top-level update policy', () => { stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); expect(() => { includeTestTemplate(stack, 'intrinsics-update-policy.json'); - }).toThrow(/Resource 'ASG' uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify 'ASG' in the 'unhydratedResources' prop to include this resource./); + }).toThrow(/Cannot convert resource 'ASG' to CDK objects: it uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify 'ASG' in the 'dehydratedResources' prop to skip parsing this resource, while still including it in the output./); }); test('Intrinsics cannot be used in the auto scaling rolling update update policy', () => { stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); expect(() => { includeTestTemplate(stack, 'intrinsics-update-policy-autoscaling-rolling-update.json'); - }).toThrow(/Resource 'ASG' uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify 'ASG' in the 'unhydratedResources' prop to include this resource./); + }).toThrow(/Cannot convert resource 'ASG' to CDK objects: it uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify 'ASG' in the 'dehydratedResources' prop to skip parsing this resource, while still including it in the output./); }); test('Intrinsics cannot be used in the auto scaling replacing update update policy', () => { stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); expect(() => { includeTestTemplate(stack, 'intrinsics-update-policy-autoscaling-replacing-update.json'); - }).toThrow(/Resource 'ASG' uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify 'ASG' in the 'unhydratedResources' prop to include this resource./); + }).toThrow(/Cannot convert resource 'ASG' to CDK objects: it uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify 'ASG' in the 'dehydratedResources' prop to skip parsing this resource, while still including it in the output./); }); test('Intrinsics cannot be used in the auto scaling scheduled action update policy', () => { stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); expect(() => { includeTestTemplate(stack, 'intrinsics-update-policy-autoscaling-scheduled-action.json'); - }).toThrow(/Resource 'ASG' uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify 'ASG' in the 'unhydratedResources' prop to include this resource./); + }).toThrow(/Cannot convert resource 'ASG' to CDK objects: it uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify 'ASG' in the 'dehydratedResources' prop to skip parsing this resource, while still including it in the output./); }); test('Intrinsics cannot be used in the code deploy lambda alias update policy', () => { stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); expect(() => { includeTestTemplate(stack, 'intrinsics-update-policy-code-deploy-lambda-alias-update.json'); - }).toThrow(/Resource 'Alias' uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify 'Alias' in the 'unhydratedResources' prop to include this resource./); + }).toThrow(/Cannot convert resource 'Alias' to CDK objects: it uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify 'Alias' in the 'dehydratedResources' prop to skip parsing this resource, while still including it in the output./); }); test('FF toggles error checking', () => { @@ -315,23 +315,65 @@ describe('CDK Include', () => { }).not.toThrow(); }); - test('FF disabled with unhydratedResources does not throw', () => { + test('FF disabled with dehydratedResources does not throw', () => { stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, false); expect(() => { includeTestTemplate(stack, 'intrinsics-update-policy-code-deploy-lambda-alias-update.json', { - unhydratedResources: ['Alias'], + dehydratedResources: ['Alias'], }); }).not.toThrow(); }); - test('unhydrated resources appear in the template', () => { + test('dehydrated resources retain attributes with complex Intrinsics', () => { stack.node.setContext(cxapi.CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS, true); includeTestTemplate(stack, 'intrinsics-update-policy-code-deploy-lambda-alias-update.json', { - unhydratedResources: ['Alias'], + dehydratedResources: ['Alias'], }); - expect(Template.fromStack(stack).hasResource('AWS::Lambda::Alias', {})); + expect(Template.fromStack(stack).hasResource('AWS::Lambda::Alias', { + UpdatePolicy: { + CodeDeployLambdaAliasUpdate: { + 'Fn::If': [ + 'SomeCondition', + { + AfterAllowTrafficHook: 'SomeOtherHook', + ApplicationName: 'SomeApp', + BeforeAllowTrafficHook: 'SomeHook', + DeploymentGroupName: 'SomeDeploymentGroup', + }, + { + AfterAllowTrafficHook: 'SomeOtherOtherHook', + ApplicationName: 'SomeOtherApp', + BeforeAllowTrafficHook: 'SomeOtherHook', + DeploymentGroupName: 'SomeOtherDeploymentGroup', + + }, + ], + }, + }, + })); + }); + + test('dehydrated resources retain all attributes', () => { + includeTestTemplate(stack, 'resource-all-attributes.json', { + dehydratedResources: ['Foo'], + }); + + expect(Template.fromStack(stack).hasResource('AWS::Foo::Bar', { + Properties: { Blinky: 'Pinky' }, + Type: 'AWS::Foo::Bar', + CreationPolicy: { Inky: 'Clyde' }, + DeletionPolicy: { DeletionPolicyKey: 'DeletionPolicyValue' }, + Metadata: { SomeKey: 'SomeValue' }, + Version: '1.2.3.4.5.6', + UpdateReplacePolicy: { Oh: 'No' }, + Description: 'This resource does not match the spec, but it does have every possible attribute', + UpdatePolicy: { + Foo: 'Bar', + }, + })); }); + }); interface IncludeTestTemplateProps { @@ -339,14 +381,14 @@ interface IncludeTestTemplateProps { readonly allowCyclicalReferences?: boolean; /** @default none */ - readonly unhydratedResources?: string[]; + readonly dehydratedResources?: string[]; } function includeTestTemplate(scope: constructs.Construct, testTemplate: string, props: IncludeTestTemplateProps = {}): inc.CfnInclude { return new inc.CfnInclude(scope, 'MyScope', { templateFile: _testTemplateFilePath(testTemplate), allowCyclicalReferences: props.allowCyclicalReferences, - unhydratedResources: props.unhydratedResources, + dehydratedResources: props.dehydratedResources, }); } diff --git a/packages/aws-cdk-lib/cloudformation-include/test/nested-stacks.test.ts b/packages/aws-cdk-lib/cloudformation-include/test/nested-stacks.test.ts index 6d43433c3b74b..06fb19716d3cf 100644 --- a/packages/aws-cdk-lib/cloudformation-include/test/nested-stacks.test.ts +++ b/packages/aws-cdk-lib/cloudformation-include/test/nested-stacks.test.ts @@ -743,6 +743,77 @@ describe('CDK Include for nested stacks', () => { }); }); }); + + describe('dehydrated resources', () => { + let parentStack: core.Stack; + let childStack: core.Stack; + + beforeEach(() => { + parentStack = new core.Stack(); + }); + + test('dehydrated resources are included in child templates, even if they are otherwise invalid', () => { + const parentTemplate = new inc.CfnInclude(parentStack, 'ParentStack', { + templateFile: testTemplateFilePath('parent-dehydrated.json'), + dehydratedResources: ['ASG'], + loadNestedStacks: { + 'ChildStack': { + templateFile: testTemplateFilePath('child-dehydrated.json'), + dehydratedResources: ['ChildASG'], + }, + }, + }); + childStack = parentTemplate.getNestedStack('ChildStack').stack; + + Template.fromStack(childStack).templateMatches({ + "Conditions": { + "SomeCondition": { + "Fn::Equals": [ + 2, + 2, + ], + }, + }, + "Resources": { + "ChildStackChildASGF815DFE9": { + "Type": "AWS::AutoScaling::AutoScalingGroup", + "Properties": { + "MaxSize": 10, + "MinSize": 1, + }, + "UpdatePolicy": { + "AutoScalingScheduledAction": { + "Fn::If": [ + "SomeCondition", + { + "IgnoreUnmodifiedGroupSizeProperties": true, + }, + { + "IgnoreUnmodifiedGroupSizeProperties": false, + }, + ], + }, + }, + }, + }, + }); + }); + + test('throws if a nested stack is marked dehydrated', () => { + expect(() => { + new inc.CfnInclude(parentStack, 'ParentStack', { + templateFile: testTemplateFilePath('parent-dehydrated.json'), + dehydratedResources: ['ChildStack'], + loadNestedStacks: { + 'ChildStack': { + templateFile: testTemplateFilePath('child-dehydrated.json'), + dehydratedResources: ['ChildASG'], + }, + }, + }); + }).toThrow(/nested stack 'ChildStack' was marked as dehydrated - nested stacks cannot be dehydrated/); + }); + }); }); function loadTestFileToJsObject(testTemplate: string): any { diff --git a/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/resource-all-attributes.json b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/resource-all-attributes.json new file mode 100644 index 0000000000000..03316390e4e3b --- /dev/null +++ b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/resource-all-attributes.json @@ -0,0 +1,27 @@ +{ + "Resources": { + "Foo": { + "Type": "AWS::Foo::Bar", + "Properties": { + "Blinky": "Pinky" + }, + "CreationPolicy": { + "Inky": "Clyde" + }, + "UpdatePolicy": { + "Foo": "Bar" + }, + "DeletionPolicy": { + "DeletionPolicyKey": "DeletionPolicyValue" + }, + "UpdateReplacePolicy": { + "Oh": "No" + }, + "Version": "1.2.3.4.5.6" , + "Description": "This resource does not match the spec, but it does have every possible attribute", + "Metadata": { + "SomeKey": "SomeValue" + } + } + } +} \ No newline at end of file diff --git a/packages/aws-cdk-lib/cloudformation-include/test/test-templates/nested/child-dehydrated.json b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/nested/child-dehydrated.json new file mode 100644 index 0000000000000..b390fdc70d22b --- /dev/null +++ b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/nested/child-dehydrated.json @@ -0,0 +1,32 @@ +{ + "Conditions": { + "SomeCondition": { + "Fn::Equals": [ + 2, + 2 + ] + } + }, + "Resources": { + "ChildASG": { + "Type": "AWS::AutoScaling::AutoScalingGroup", + "Properties": { + "MaxSize": 10, + "MinSize": 1 + }, + "UpdatePolicy": { + "AutoScalingScheduledAction": { + "Fn::If": [ + "SomeCondition", + { + "IgnoreUnmodifiedGroupSizeProperties" : true + }, + { + "IgnoreUnmodifiedGroupSizeProperties" : false + } + ] + } + } + } + } +} \ No newline at end of file diff --git a/packages/aws-cdk-lib/cloudformation-include/test/test-templates/nested/parent-dehydrated.json b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/nested/parent-dehydrated.json new file mode 100644 index 0000000000000..ee0b92688962a --- /dev/null +++ b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/nested/parent-dehydrated.json @@ -0,0 +1,41 @@ +{ + "Conditions": { + "SomeCondition": { + "Fn::Equals": [ + 2, + 2 + ] + } + }, + "Resources": { + "ASG": { + "Type": "AWS::AutoScaling::AutoScalingGroup", + "Properties": { + "MaxSize": 10, + "MinSize": 1 + }, + "UpdatePolicy": { + "AutoScalingScheduledAction": { + "Fn::If": [ + "SomeCondition", + { + "IgnoreUnmodifiedGroupSizeProperties" : true + }, + { + "IgnoreUnmodifiedGroupSizeProperties" : false + } + ] + } + } + }, + "ChildStack": { + "Type": "AWS::CloudFormation::Stack", + "Properties": { + "TemplateURL": "https://cfn-templates-set.s3.amazonaws.com/child-dehydrated-stack.json", + "Parameters": { + "SomeParam": "SomeValue" + } + } + } + } +} diff --git a/packages/aws-cdk-lib/core/lib/helpers-internal/cfn-parse.ts b/packages/aws-cdk-lib/core/lib/helpers-internal/cfn-parse.ts index 686b3dbf7d910..839684b2e791f 100644 --- a/packages/aws-cdk-lib/core/lib/helpers-internal/cfn-parse.ts +++ b/packages/aws-cdk-lib/core/lib/helpers-internal/cfn-parse.ts @@ -314,11 +314,6 @@ export enum CfnParsingContext { * The options for `FromCloudFormation.parseValue`. */ export interface ParseCfnOptions { - /** - * resources to not parse some data for - */ - unhydratedResources?: string[]; - /** * The finder interface used to resolve references in the template. */ @@ -351,24 +346,18 @@ export interface ParseCfnOptions { */ export class CfnParser { private readonly options: ParseCfnOptions; - private readonly unhydratedResources: string[]; private stack?: Stack; constructor(options: ParseCfnOptions) { this.options = options; - this.unhydratedResources = options.unhydratedResources ?? []; } public handleAttributes(resource: CfnResource, resourceAttributes: any, logicalId: string): void { const cfnOptions = resource.cfnOptions; this.stack = Stack.of(resource); - cfnOptions.creationPolicy = this.unhydratedResources.includes(logicalId) - ? resourceAttributes.CreationPolicy - : this.parseCreationPolicy(resourceAttributes.CreationPolicy, logicalId); - cfnOptions.updatePolicy = this.unhydratedResources.includes(logicalId) - ? resourceAttributes.UpdatePolicy - : this.parseUpdatePolicy(resourceAttributes.UpdatePolicy, logicalId); + cfnOptions.creationPolicy = this.parseCreationPolicy(resourceAttributes.CreationPolicy, logicalId); + cfnOptions.updatePolicy = this.parseUpdatePolicy(resourceAttributes.UpdatePolicy, logicalId); cfnOptions.deletionPolicy = this.parseDeletionPolicy(resourceAttributes.DeletionPolicy); cfnOptions.updateReplacePolicy = this.parseDeletionPolicy(resourceAttributes.UpdateReplacePolicy); cfnOptions.version = this.parseValue(resourceAttributes.Version); @@ -709,7 +698,7 @@ export class CfnParser { } if (FeatureFlags.of(this.stack).isEnabled(CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS)) { if (isResolvableObject(object ?? {}) || this.looksLikeCfnIntrinsic(object ?? {})) { - throw new Error(`Resource '${logicalId}' uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify '${logicalId}' in the 'unhydratedResources' prop to include this resource.`); + throw new Error(`Cannot convert resource '${logicalId}' to CDK objects: it uses an intrinsic in a resource update or deletion policy to represent a non-primitive value. Specify '${logicalId}' in the 'dehydratedResources' prop to skip parsing this resource, while still including it in the output.`); } } } From e020e6af68496bbb6dd70c870a41bc7bea9f659c Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Thu, 3 Oct 2024 09:15:11 -0700 Subject: [PATCH 14/16] flag version' g --- packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md b/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md index 04b533b0862b6..c2813160d9f03 100644 --- a/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md +++ b/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md @@ -77,7 +77,7 @@ Flags come in three types: | [@aws-cdk/aws-ec2:ec2SumTImeoutEnabled](#aws-cdkaws-ec2ec2sumtimeoutenabled) | When enabled, initOptions.timeout and resourceSignalTimeout values will be summed together. | 2.160.0 | (fix) | | [@aws-cdk/aws-appsync:appSyncGraphQLAPIScopeLambdaPermission](#aws-cdkaws-appsyncappsyncgraphqlapiscopelambdapermission) | When enabled, a Lambda authorizer Permission created when using GraphqlApi will be properly scoped with a SourceArn. | V2NEXT | (fix) | | [@aws-cdk/aws-rds:setCorrectValueForDatabaseInstanceReadReplicaInstanceResourceId](#aws-cdkaws-rdssetcorrectvaluefordatabaseinstancereadreplicainstanceresourceid) | When enabled, the value of property `instanceResourceId` in construct `DatabaseInstanceReadReplica` will be set to the correct value which is `DbiResourceId` instead of currently `DbInstanceArn` | V2NEXT | (fix) | -| [@aws-cdk/core:cfnIncludeRejectComplexResourceUpdateCreatePolicyIntrinsics](#aws-cdkcorecfnincluderejectcomplexresourceupdatecreatepolicyintrinsics) | When enabled, CFN templates added with `cfn-include` will error if the template contains Resource Update or Create policies with CFN Intrinsics that include non-primitive values. | 2.160.0 | (fix) | +| [@aws-cdk/core:cfnIncludeRejectComplexResourceUpdateCreatePolicyIntrinsics](#aws-cdkcorecfnincluderejectcomplexresourceupdatecreatepolicyintrinsics) | When enabled, CFN templates added with `cfn-include` will error if the template contains Resource Update or Create policies with CFN Intrinsics that include non-primitive values. | V2NEXT | (fix) | @@ -1471,4 +1471,5 @@ Enabling this feature flag will make `cfn-include` throw on these templates, unl | (not in v1) | | | | V2NEXT | `false` | `true` | + From 54f3b69e53688fb40323e953cc1f19c02d627449 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Thu, 3 Oct 2024 09:20:03 -0700 Subject: [PATCH 15/16] newline --- .../cloudformation-include/test/invalid-templates.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts b/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts index 0a256eb885b2a..3f3bad52156c2 100644 --- a/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts +++ b/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts @@ -373,7 +373,6 @@ describe('CDK Include', () => { }, })); }); - }); interface IncludeTestTemplateProps { From f5657018560404da824e31d1fe89ec58a1763576 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Thu, 3 Oct 2024 11:07:34 -0700 Subject: [PATCH 16/16] tests --- .../cloudformation-include/lib/cfn-include.ts | 6 +++ .../test/invalid-templates.test.ts | 52 ++++++++++++++++++ .../intrinsics-tags-resource-validation.json | 54 +++++++++++++++++++ 3 files changed, 112 insertions(+) create mode 100644 packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-tags-resource-validation.json diff --git a/packages/aws-cdk-lib/cloudformation-include/lib/cfn-include.ts b/packages/aws-cdk-lib/cloudformation-include/lib/cfn-include.ts index edd6d6b88d495..a2849e5c794a6 100644 --- a/packages/aws-cdk-lib/cloudformation-include/lib/cfn-include.ts +++ b/packages/aws-cdk-lib/cloudformation-include/lib/cfn-include.ts @@ -138,6 +138,12 @@ export class CfnInclude extends core.CfnElement { this.dehydratedResources = props.dehydratedResources ?? []; + for (const logicalId of this.dehydratedResources) { + if (!Object.keys(this.template.Resources).includes(logicalId)) { + throw new Error(`Logical ID '${logicalId}' was specified in 'dehydratedResources', but does not belong to a resource in the template.`); + } + } + // check if all user specified parameter values exist in the template for (const logicalId of Object.keys(this.parametersToReplace)) { if (!(logicalId in (this.template.Parameters || {}))) { diff --git a/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts b/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts index 3f3bad52156c2..3802c00477d37 100644 --- a/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts +++ b/packages/aws-cdk-lib/cloudformation-include/test/invalid-templates.test.ts @@ -373,6 +373,58 @@ describe('CDK Include', () => { }, })); }); + + test('synth-time validation does not run on dehydrated resources', () => { + // synth-time validation fails if resource is hydrated + expect(() => { + includeTestTemplate(stack, 'intrinsics-tags-resource-validation.json'); + Template.fromStack(stack); + }).toThrow(`Resolution error: Supplied properties not correct for \"CfnLoadBalancerProps\" + tags: element 1: {} should have a 'key' and a 'value' property.`); + + app = new core.App({ context: { [cxapi.NEW_STYLE_STACK_SYNTHESIS_CONTEXT]: false } }); + stack = new core.Stack(app); + + // synth-time validation not run if resource is dehydrated + includeTestTemplate(stack, 'intrinsics-tags-resource-validation.json', { + dehydratedResources: ['MyLoadBalancer'], + }); + + expect(Template.fromStack(stack).hasResource('AWS::ElasticLoadBalancingV2::LoadBalancer', { + Properties: { + Tags: [ + { + Key: 'Name', + Value: 'MyLoadBalancer', + }, + { + data: [ + 'IsExtraTag', + { + Key: 'Name2', + Value: 'MyLoadBalancer2', + }, + { + data: 'AWS::NoValue', + type: 'Ref', + isCfnFunction: true, + }, + ], + type: 'Fn::If', + isCfnFunction: true, + }, + ], + }, + })); + }); + + test('throws on dehydrated resources not present in the template', () => { + expect(() => { + includeTestTemplate(stack, 'intrinsics-tags-resource-validation.json', { + dehydratedResources: ['ResourceNotExistingHere'], + }); + }).toThrow(/Logical ID 'ResourceNotExistingHere' was specified in 'dehydratedResources', but does not belong to a resource in the template./); + }); }); interface IncludeTestTemplateProps { diff --git a/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-tags-resource-validation.json b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-tags-resource-validation.json new file mode 100644 index 0000000000000..b162016bb2577 --- /dev/null +++ b/packages/aws-cdk-lib/cloudformation-include/test/test-templates/invalid/intrinsics-tags-resource-validation.json @@ -0,0 +1,54 @@ +{ + "AWSTemplateFormatVersion": "2010-09-09", + "Parameters": { + "IsExtraTag": { + "Type": "String", + "AllowedValues": [ + "true", + "false" + ], + "Default": "false" + } + }, + "Conditions": { + "AddExtraTag": { + "Fn::Equals": [ + { + "data": "IsExtraTag", + "type": "Ref", + "isCfnFunction": true + }, + "true" + ] + } + }, + "Resources": { + "MyLoadBalancer": { + "Type": "AWS::ElasticLoadBalancingV2::LoadBalancer", + "Properties": { + "Tags": [ + { + "Key": "Name", + "Value": "MyLoadBalancer" + }, + { + "data": [ + "IsExtraTag", + { + "Key": "Name2", + "Value": "MyLoadBalancer2" + }, + { + "data": "AWS::NoValue", + "type": "Ref", + "isCfnFunction": true + } + ], + "type": "Fn::If", + "isCfnFunction": true + } + ] + } + } + } +} \ No newline at end of file