Skip to content

Commit

Permalink
feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
comcalvi committed Oct 1, 2024
1 parent eccec59 commit eb5263c
Show file tree
Hide file tree
Showing 7 changed files with 256 additions and 34 deletions.
30 changes: 25 additions & 5 deletions packages/aws-cdk-lib/cloudformation-include/lib/cfn-include.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
}

/**
Expand Down Expand Up @@ -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<string, string>;

constructor(scope: Construct, id: string, props: CfnIncludeProps) {
Expand All @@ -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)) {
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -315,38 +315,80 @@ 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 {
/** @default false */
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,
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
}
Original file line number Diff line number Diff line change
@@ -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
}
]
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
}
}
Loading

0 comments on commit eb5263c

Please sign in to comment.