fix(cfn-diff): correctly handle Date strings in diff (#16591)

Turns out, `parseFloat()` in JavaScript is even crazier than we thought,
and returns nonsense like `2021` for a string containing a Date like `'2021-10-25'`.
For that reason, add an explicit check that the string parsed looks like a number before calling `parseFloat()`.

Fixes #16444

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Author: skinny85

packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts

@@ -99,13 +99,17 @@ function calculateTemplateDiff(currentTemplate: { [key: string]: any }, newTempl
   for (const key of unionOf(Object.keys(currentTemplate), Object.keys(newTemplate)).sort()) {
     const oldValue = currentTemplate[key];
     const newValue = newTemplate[key];
-    if (deepEqual(oldValue, newValue)) { continue; }
+    if (deepEqual(oldValue, newValue)) {
+      continue;
+    }
     const handler: DiffHandler = DIFF_HANDLERS[key]
                   || ((_diff, oldV, newV) => unknown[key] = impl.diffUnknown(oldV, newV));
     handler(differences, oldValue, newValue);
 
   }
-  if (Object.keys(unknown).length > 0) { differences.unknown = new types.DifferenceCollection(unknown); }
+  if (Object.keys(unknown).length > 0) {
+    differences.unknown = new types.DifferenceCollection(unknown);
+  }
 
   return new types.TemplateDiff(differences);
 }

packages/@aws-cdk/cloudformation-diff/lib/diff/util.ts

@@ -138,20 +138,10 @@ export function unionOf(lv: string[] | Set<string>, rv: string[] | Set<string>):
  * A parseFloat implementation that does the right thing for
  * strings like '0.0.0'
  * (for which JavaScript's parseFloat() returns 0).
+ * We return NaN for all of these strings that do not represent numbers,
+ * and so comparing them fails,
+ * and doesn't short-circuit the diff logic.
  */
 function safeParseFloat(str: string): number {
-  const ret = parseFloat(str);
-  const nonNumericRegex = /\d*\.\d+\./;
-  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 if (nonNumericRegex.test(str)) {
-    // if the str contains non-numeric characters,
-    // return NaN, so we'll fall back to string comparison
-    return NaN;
-  } else {
-    return ret;
-  }
+  return Number(str);
 }

packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts

@@ -582,84 +582,67 @@ test('when a property changes including equivalent DependsOn', () => {
   expect(differences.resources.differenceCount).toBe(1);
 });
 
-test('when a property with a number-like format changes', () => {
-  const bucketName = 'ShineyBucketName';
-  const tagChanges = {
-    '0.31.1-prod': '0.31.2-prod',
-    '8.0.5.5.4-identifier': '8.0.5.5.5-identifier',
-    '1.1.1.1': '1.1.2.2',
-    '1.2.3': '1.2.4',
-    '2.2.2.2': '2.2.3.2',
-    '3.3.3.3': '3.4.3.3',
-  };
-  const oldTags = Object.keys(tagChanges);
-  const newTags = Object.values(tagChanges);
+test.each([
+  ['0.31.1-prod', '0.31.2-prod'],
+  ['8.0.5.5.4-identifier', '8.0.5.5.5-identifier'],
+  ['1.1.1.1', '1.1.1.2'],
+  ['1.2.3', '1.2.4'],
+  ['2.2.2.2', '2.2.3.2'],
+  ['3.3.3.3', '3.4.3.3'],
+  ['2021-10-23T06:07:08.000Z', '2021-10-23T09:10:11.123Z'],
+])("reports a change when a string property with a number-like format changes from '%s' to '%s'", (oldValue, newValue) => {
+  // GIVEN
   const currentTemplate = {
     Resources: {
-      QueueResource: {
-        Type: 'AWS::SQS::Queue',
-      },
       BucketResource: {
         Type: 'AWS::S3::Bucket',
         Properties: {
-          BucketName: bucketName,
-          Tags: oldTags,
+          Tags: [oldValue],
         },
       },
     },
   };
   const newTemplate = {
     Resources: {
-      QueueResource: {
-        Type: 'AWS::SQS::Queue',
-      },
       BucketResource: {
         Type: 'AWS::S3::Bucket',
         Properties: {
-          BucketName: bucketName,
-          Tags: newTags,
+          Tags: [newValue],
         },
       },
     },
   };
-
+  // WHEN
   const differences = diffTemplate(currentTemplate, newTemplate);
+
+  // THEN
   expect(differences.differenceCount).toBe(1);
   expect(differences.resources.differenceCount).toBe(1);
   const difference = differences.resources.changes.BucketResource;
   expect(difference).not.toBeUndefined();
   expect(difference?.oldResourceType).toEqual('AWS::S3::Bucket');
   expect(difference?.propertyUpdates).toEqual({
-    Tags: { oldValue: oldTags, newValue: newTags, changeImpact: ResourceImpact.WILL_UPDATE, isDifferent: true },
+    Tags: { oldValue: [oldValue], newValue: [newValue], changeImpact: ResourceImpact.WILL_UPDATE, isDifferent: true },
   });
 });
 
 test('when a property with a number-like format doesn\'t change', () => {
-  const bucketName = 'ShineyBucketName';
   const tags = ['0.31.1-prod', '8.0.5.5.4-identifier', '1.1.1.1', '1.2.3'];
   const currentTemplate = {
     Resources: {
-      QueueResource: {
-        Type: 'AWS::SQS::Queue',
-      },
       BucketResource: {
         Type: 'AWS::S3::Bucket',
         Properties: {
-          BucketName: bucketName,
           Tags: tags,
         },
       },
     },
   };
   const newTemplate = {
     Resources: {
-      QueueResource: {
-        Type: 'AWS::SQS::Queue',
-      },
       BucketResource: {
         Type: 'AWS::S3::Bucket',
         Properties: {
-          BucketName: bucketName,
           Tags: tags,
         },
       },
@@ -671,4 +654,4 @@ test('when a property with a number-like format doesn\'t change', () => {
   expect(differences.resources.differenceCount).toBe(0);
   const difference = differences.resources.changes.BucketResource;
   expect(difference).toBeUndefined();
-});
+});