Skip to content

Commit

Permalink
fix(cfn-diff): correctly handle version strings like '0.0.0' (#13022)
Browse files Browse the repository at this point in the history
Turns out, our cloudformation-diff interprets all version strings like '0.2.3' as just 0,
which means diff misses any changes to those kind of resources.

Make sure that returning the number 0 only happens for strings that are actually equal to '0'.

Fixes #13016

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
skinny85 authored Feb 13, 2021
1 parent 09ba573 commit 34a921b
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 1 deletion.
20 changes: 19 additions & 1 deletion packages/@aws-cdk/cloudformation-diff/lib/diff/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ export function deepEqual(lvalue: any, rvalue: any): boolean {
}
// allows a numeric 10 and a literal "10" to be equivalent;
// this is consistent with CloudFormation.
if (((typeof lvalue === 'string') || (typeof rvalue === 'string')) && (parseFloat(lvalue) === parseFloat(rvalue))) {
if ((typeof lvalue === 'string' || typeof rvalue === 'string') &&
safeParseFloat(lvalue) === safeParseFloat(rvalue)) {
return true;
}
if (typeof lvalue !== typeof rvalue) { return false; }
Expand Down Expand Up @@ -132,3 +133,20 @@ export function unionOf(lv: string[] | Set<string>, rv: string[] | Set<string>):
}
return new Array(...result);
}

/**
* A parseFloat implementation that does the right thing for
* strings like '0.0.0'
* (for which JavaScript's parseFloat() returns 0).
*/
function safeParseFloat(str: string): number {
const ret = parseFloat(str);
if (ret === 0) {
// if the str is exactly '0', that's OK;
// but parseFloat() also returns 0 for things like '0.0';
// in this case, return NaN, so we'll fall back to string comparison
return str === '0' ? ret : NaN;
} else {
return ret;
}
}
28 changes: 28 additions & 0 deletions packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,34 @@ test('adding and removing quotes from a numeric property causes no changes', ()
expect(differences.resources.differenceCount).toBe(0);
});

test('versions are correctly detected as not numbers', () => {
const currentTemplate = {
Resources: {
ImageBuilderComponent: {
Type: 'AWS::ImageBuilder::Component',
Properties: {
Platform: 'Linux',
Version: '0.0.1',
},
},
},
};
const newTemplate = {
Resources: {
ImageBuilderComponent: {
Type: 'AWS::ImageBuilder::Component',
Properties: {
Platform: 'Linux',
Version: '0.0.2',
},
},
},
};

const differences = diffTemplate(currentTemplate, newTemplate);
expect(differences.resources.differenceCount).toBe(1);
});

test('single element arrays are equivalent to the single element in DependsOn expressions', () => {
// GIVEN
const currentTemplate = {
Expand Down

0 comments on commit 34a921b

Please sign in to comment.