fix(cloudformation-diff): cdk diff not picking up differences if old/…

…new value is in format n.n.n (#16050)

"cdk diff" in the current version doesn't pick up differences if the old/new value has a number-like format but actually isn't a number (e.g. 0.31.1)

Example: two version strings like "0.31.1-prod" and "0.31.2-prod" are both parsed into "0.31" (and hence incorrectly considered equal).

Closes #15935. 

----

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

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

@@ -141,11 +141,16 @@ export function unionOf(lv: string[] | Set<string>, rv: string[] | Set<string>):
  */
 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;
   }

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

@@ -581,3 +581,94 @@ test('when a property changes including equivalent DependsOn', () => {
   differences = diffTemplate(newTemplate, currentTemplate);
   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);
+  const currentTemplate = {
+    Resources: {
+      QueueResource: {
+        Type: 'AWS::SQS::Queue',
+      },
+      BucketResource: {
+        Type: 'AWS::S3::Bucket',
+        Properties: {
+          BucketName: bucketName,
+          Tags: oldTags,
+        },
+      },
+    },
+  };
+  const newTemplate = {
+    Resources: {
+      QueueResource: {
+        Type: 'AWS::SQS::Queue',
+      },
+      BucketResource: {
+        Type: 'AWS::S3::Bucket',
+        Properties: {
+          BucketName: bucketName,
+          Tags: newTags,
+        },
+      },
+    },
+  };
+
+  const differences = diffTemplate(currentTemplate, newTemplate);
+  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 },
+  });
+});
+
+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,
+        },
+      },
+    },
+  };
+
+  const differences = diffTemplate(currentTemplate, newTemplate);
+  expect(differences.differenceCount).toBe(0);
+  expect(differences.resources.differenceCount).toBe(0);
+  const difference = differences.resources.changes.BucketResource;
+  expect(difference).toBeUndefined();
+});