diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff/util.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff/util.ts index 159cd9a0ed717..c8fcbf694c17e 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff/util.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff/util.ts @@ -4,6 +4,10 @@ * object prototype into account for the purpose of the comparison, only the values of * properties reported by +Object.keys+. * + * If both operands can be parsed to equivalent numbers, will return true. + * This makes diff consistent with CloudFormation, where a numeric 10 and a literal "10" + * are considered equivalent. + * * @param lvalue the left operand of the equality comparison. * @param rvalue the right operand of the equality comparison. * @@ -11,6 +15,9 @@ */ export function deepEqual(lvalue: any, rvalue: any): boolean { if (lvalue === rvalue) { return true; } + // allows a numeric 10 and a literal "10" to be equivalent; + // this is consistent with CloudFormation. + if (parseFloat(lvalue) === parseFloat(rvalue)) { return true; } if (typeof lvalue !== typeof rvalue) { return false; } if (Array.isArray(lvalue) !== Array.isArray(rvalue)) { return false; } if (Array.isArray(lvalue) /* && Array.isArray(rvalue) */) { diff --git a/packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts b/packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts index d772fa9e1f21d..e351162863cb3 100644 --- a/packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts +++ b/packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts @@ -321,3 +321,56 @@ test('resource replacement is tracked through references', () => { // THEN expect(differences.resources.differenceCount).toBe(3); }); + +test('adding and removing quotes from a numeric property causes no changes', () => { + const currentTemplate = { + Resources: { + Bucket: { + Type: 'AWS::S3::Bucket', + Properties: { + CorsConfiguration: { + CorsRules: [ + { + AllowedMethods: [ + 'GET', + ], + AllowedOrigins: [ + '*', + ], + MaxAge: 10, + }, + ], + }, + }, + }, + }, + }; + + const newTemplate = { + Resources: { + Bucket: { + Type: 'AWS::S3::Bucket', + Properties: { + CorsConfiguration: { + CorsRules: [ + { + AllowedMethods: [ + 'GET', + ], + AllowedOrigins: [ + '*', + ], + MaxAge: '10', + }, + ], + }, + }, + }, + }, + }; + let differences = diffTemplate(currentTemplate, newTemplate); + expect(differences.resources.differenceCount).toBe(0); + + differences = diffTemplate(newTemplate, currentTemplate); + expect(differences.resources.differenceCount).toBe(0); +}); diff --git a/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts index 113e58773a242..0a2a636a546c3 100644 --- a/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts @@ -112,6 +112,14 @@ describe('CDK Include', () => { includeTestTemplate(stack, 'fn-sub-${}-only.json'); }).toThrow(/Element referenced in Fn::Sub expression with logical ID: '' was not found in the template/); }); + + test('throws an error when a template supplies an invalid string to a number parameter', () => { + includeTestTemplate(stack, 'alphabetical-string-passed-to-number.json'); + + expect(() => { + SynthUtils.synthesize(stack); + }).toThrow(/"abc" should be a number/); + }); }); function includeTestTemplate(scope: core.Construct, testTemplate: string): inc.CfnInclude { diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/alphabetical-string-passed-to-number.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/alphabetical-string-passed-to-number.json new file mode 100644 index 0000000000000..c97850b3ba0e7 --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/alphabetical-string-passed-to-number.json @@ -0,0 +1,18 @@ +{ + "Resources": { + "Bucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "CorsConfiguration": { + "CorsRules": [ + { + "AllowedMethods": ["GET"], + "AllowedOrigins": ["*"], + "MaxAge": "abc" + } + ] + } + } + } + } +} diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/parsing-as-numbers.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/parsing-as-numbers.json new file mode 100644 index 0000000000000..a623ff02c6217 --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/parsing-as-numbers.json @@ -0,0 +1,18 @@ +{ + "Resources": { + "Bucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "CorsConfiguration": { + "CorsRules": [ + { + "AllowedMethods": ["GET"], + "AllowedOrigins": ["*"], + "MaxAge": "10" + } + ] + } + } + } + } +} diff --git a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts index 369e1c9ef1b66..6b7d3835c9287 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -98,6 +98,14 @@ describe('CDK Include', () => { ); }); + test('correctly parse strings as integers as needed', () => { + includeTestTemplate(stack, 'parsing-as-numbers.json'); + + expect(stack).toMatchTemplate( + loadTestFileToJsObject('parsing-as-numbers.json'), + ); + }); + xtest('correctly changes the logical IDs, including references, if imported with preserveLogicalIds=false', () => { const cfnTemplate = includeTestTemplate(stack, 'bucket-with-encryption-key.json', { preserveLogicalIds: false, diff --git a/packages/@aws-cdk/core/lib/cfn-parse.ts b/packages/@aws-cdk/core/lib/cfn-parse.ts index 13662e4633244..bab5b50d8b49d 100644 --- a/packages/@aws-cdk/core/lib/cfn-parse.ts +++ b/packages/@aws-cdk/core/lib/cfn-parse.ts @@ -51,6 +51,8 @@ export class FromCloudFormation { return value; } + // won't always return a string; if the input can't be resolved to a string, + // the input will be returned. public static getString(value: any): string { // if the string is a deploy-time value, serialize it to a Token if (isResolvableObject(value)) { @@ -62,13 +64,24 @@ export class FromCloudFormation { return value; } + // won't always return a number; if the input can't be parsed to a number, + // the input will be returned. public static getNumber(value: any): number { // if the string is a deploy-time value, serialize it to a Token if (isResolvableObject(value)) { return Token.asNumber(value); } - // in all other cases, just return the input, + // return a number, if the input can be parsed as one + let parsedValue; + if (typeof value === 'string') { + parsedValue = parseFloat(value); + if (!isNaN(parsedValue)) { + return parsedValue; + } + } + + // otherwise return the input, // and let a validator handle it if it's not a number return value; }