Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cloudformation): custom resource changes property casing (behind feature flag) #7034

Closed
9 changes: 8 additions & 1 deletion packages/@aws-cdk/aws-cloudformation/lib/custom-resource.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as lambda from '@aws-cdk/aws-lambda';
import * as sns from '@aws-cdk/aws-sns';
import { CfnResource, Construct, RemovalPolicy, Resource, Token } from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';
import { CfnCustomResource } from './cloudformation.generated';

/**
Expand Down Expand Up @@ -160,11 +161,17 @@ export class CustomResource extends Resource {

const type = renderResourceType(props.resourceType);
const providerConfig = props.provider.bind(this);

// change properties to upper case under a feature flag (since this is a breaking change).
const properties = this.node.tryGetContext(cxapi.DISABLE_CUSTOM_RESOURCE_UPPERCASE_PROPERTIES)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't use a feature flag for this because it can break "legacy" constructs used in "new" applications.

? (props.properties ?? {})
: uppercaseProperties(props.properties || {});

this.resource = new CfnResource(this, 'Default', {
type,
properties: {
ServiceToken: providerConfig.serviceToken,
...uppercaseProperties(props.properties || {})
...properties
}
});

Expand Down
82 changes: 81 additions & 1 deletion packages/@aws-cdk/aws-cloudformation/test/test.resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { expect, haveResource, ResourcePart } from '@aws-cdk/assert';
import * as lambda from '@aws-cdk/aws-lambda';
import * as sns from '@aws-cdk/aws-sns';
import * as cdk from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';
import { Test, testCase } from 'nodeunit';
import { CustomResource, CustomResourceProvider } from '../lib';

Expand Down Expand Up @@ -205,7 +206,65 @@ export = testCase({
// THEN
test.deepEqual(stack.resolve(res.resource.ref), { Ref: 'myResourceC6A188A9' });
test.done();
}
},

'@aws-cdk/core:disableCustomResourceUppercaseProperties': {

'disabled (default)': {

'Properties are uppercased'(test: Test) {
// GIVEN
const app = new cdk.App();

// WHEN
const stack = new cdk.Stack(app, 'Test');
new TestCustomResourceProperties(stack, 'Custom', {
properties: {
lowercase: 'value',
Uppercase: 'value'
}
});

// THEN
expect(stack).to(haveResource('AWS::CloudFormation::CustomResource', {
Lowercase: 'value',
Uppercase: 'value',
}, ResourcePart.Properties));

test.done();
}
},

'enabled': {

'Properties are not modified'(test: Test) {
// GIVEN
const app = new cdk.App({
context: {
[cxapi.DISABLE_CUSTOM_RESOURCE_UPPERCASE_PROPERTIES]: 'true'
}
});

// WHEN
const stack = new cdk.Stack(app, 'Test');
new TestCustomResourceProperties(stack, 'Custom', {
properties: {
lowercase: 'value',
Uppercase: 'value'
}
});

// THEN
expect(stack).to(haveResource('AWS::CloudFormation::CustomResource', {
lowercase: 'value',
Uppercase: 'value',
}, ResourcePart.Properties));

test.done();
},
}

},
});

class TestCustomResource extends cdk.Construct {
Expand All @@ -228,3 +287,24 @@ class TestCustomResource extends cdk.Construct {
});
}
}

class TestCustomResourceProperties extends cdk.Construct {
public readonly resource: CustomResource;

constructor(scope: cdk.Construct, id: string, properties: { [key: string]: any }) {
super(scope, id);

const singletonLambda = new lambda.SingletonFunction(this, 'Lambda', {
uuid: 'TestCustomResourceProvider',
code: new lambda.InlineCode('def hello(): pass'),
runtime: lambda.Runtime.PYTHON_2_7,
handler: 'index.hello',
timeout: cdk.Duration.minutes(5),
});

this.resource = new CustomResource(this, 'Resource', {
...properties,
provider: CustomResourceProvider.fromLambda(singletonLambda),
});
}
}
6 changes: 6 additions & 0 deletions packages/@aws-cdk/cx-api/lib/features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ export const ENABLE_DIFF_NO_FAIL_CONTEXT = 'aws-cdk:enableDiffNoFail';
/** @deprecated use `ENABLE_DIFF_NO_FAIL_CONTEXT` */
export const ENABLE_DIFF_NO_FAIL = ENABLE_DIFF_NO_FAIL_CONTEXT;

/**
* If this is set, properties of a custom resource will not be uppercased.
*/
export const DISABLE_CUSTOM_RESOURCE_UPPERCASE_PROPERTIES = '@aws-cdk/aws-cloudformation:disableCustomResourceUppercaseProperties';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a _CONTEXT suffix please


/**
* This map includes context keys and values for feature flags that enable
* capabilities "from the future", which we could not introduce as the default
Expand All @@ -45,4 +50,5 @@ export const ENABLE_DIFF_NO_FAIL = ENABLE_DIFF_NO_FAIL_CONTEXT;
export const FUTURE_FLAGS = {
[ENABLE_STACK_NAME_DUPLICATES_CONTEXT]: 'true',
[ENABLE_DIFF_NO_FAIL_CONTEXT]: 'true',
[DISABLE_CUSTOM_RESOURCE_UPPERCASE_PROPERTIES]: 'true',
};