Merge branch 'main' into bergjaak/29731

packages/@aws-cdk-testing/framework-integ/test/aws-rds/test/integ.instance-with-parameter-group.js.snapshot/aws-cdk-rds-instance-with-rds-parameter-group.template.json

@@ -394,8 +394,9 @@
   "ParameterGroup5E32DECB": {
    "Type": "AWS::RDS::DBParameterGroup",
    "Properties": {
+    "DBParameterGroupName": "name",
     "Description": "desc",
-    "Family": "postgres15",
+    "Family": "postgres16",
     "Parameters": {}
    },
    "UpdateReplacePolicy": "Delete",
@@ -481,7 +482,7 @@
     },
     "EnableIAMDatabaseAuthentication": true,
     "Engine": "postgres",
-    "EngineVersion": "15.2",
+    "EngineVersion": "16.2",
     "MasterUserPassword": {
      "Fn::Join": [
       "",

packages/@aws-cdk-testing/framework-integ/test/aws-rds/test/integ.instance-with-parameter-group.ts

@@ -11,13 +11,14 @@ const stack = new cdk.Stack(app, 'aws-cdk-rds-instance-with-rds-parameter-group'
 const vpc = new ec2.Vpc(stack, 'VPC', { maxAzs: 2, restrictDefaultSecurityGroup: false });
 
 const parameterGroup = new rds.ParameterGroup(stack, 'ParameterGroup', {
-  engine: rds.DatabaseInstanceEngine.postgres({ version: rds.PostgresEngineVersion.VER_15_2 }),
+  engine: rds.DatabaseInstanceEngine.postgres({ version: rds.PostgresEngineVersion.VER_16_2 }),
   description: 'desc',
   removalPolicy: cdk.RemovalPolicy.DESTROY,
+  name: 'name',
 });
 
 new rds.DatabaseInstance(stack, 'Instance', {
-  engine: rds.DatabaseInstanceEngine.postgres({ version: rds.PostgresEngineVersion.VER_15_2 }),
+  engine: rds.DatabaseInstanceEngine.postgres({ version: rds.PostgresEngineVersion.VER_16_2 }),
   vpc,
   multiAz: false,
   publiclyAccessible: true,

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

@@ -9,30 +9,6 @@ export * from './diff/types';
 
 export type DescribeChangeSetOutput = DescribeChangeSet;
 
-type DiffHandler = (diff: types.ITemplateDiff, oldValue: any, newValue: any) => void;
-type HandlerRegistry = { [section: string]: DiffHandler };
-
-const DIFF_HANDLERS: HandlerRegistry = {
-  AWSTemplateFormatVersion: (diff, oldValue, newValue) =>
-    diff.awsTemplateFormatVersion = impl.diffAttribute(oldValue, newValue),
-  Description: (diff, oldValue, newValue) =>
-    diff.description = impl.diffAttribute(oldValue, newValue),
-  Metadata: (diff, oldValue, newValue) =>
-    diff.metadata = new types.DifferenceCollection(diffKeyedEntities(oldValue, newValue, impl.diffMetadata)),
-  Parameters: (diff, oldValue, newValue) =>
-    diff.parameters = new types.DifferenceCollection(diffKeyedEntities(oldValue, newValue, impl.diffParameter)),
-  Mappings: (diff, oldValue, newValue) =>
-    diff.mappings = new types.DifferenceCollection(diffKeyedEntities(oldValue, newValue, impl.diffMapping)),
-  Conditions: (diff, oldValue, newValue) =>
-    diff.conditions = new types.DifferenceCollection(diffKeyedEntities(oldValue, newValue, impl.diffCondition)),
-  Transform: (diff, oldValue, newValue) =>
-    diff.transform = impl.diffAttribute(oldValue, newValue),
-  Resources: (diff, oldValue, newValue) =>
-    diff.resources = new types.DifferenceCollection(diffKeyedEntities(oldValue, newValue, impl.diffResource)),
-  Outputs: (diff, oldValue, newValue) =>
-    diff.outputs = new types.DifferenceCollection(diffKeyedEntities(oldValue, newValue, impl.diffOutput)),
-};
-
 /**
  * Compare two CloudFormation templates and return semantic differences between them.
  *
@@ -53,32 +29,22 @@ export function fullDiff(
 
   normalize(currentTemplate);
   normalize(newTemplate);
-  const theDiff = diffTemplate(currentTemplate, newTemplate);
-  if (changeSet) {
-    filterFalsePositives(theDiff, changeSet);
-    addImportInformation(theDiff, changeSet);
-  } else if (isImport) {
-    makeAllResourceChangesImports(theDiff);
-  }
 
-  return theDiff;
-}
-
-export function diffTemplate(
-  currentTemplate: { [key: string]: any },
-  newTemplate: { [key: string]: any },
-): types.TemplateDiff {
+  // The ChangeSet can include changes that aren't included in the template diff. For example, if the identifier of a resource is an ssm parameter,
+  // and the value of the ssm parameter changes -- then the diff between the old and new template may be the same while the changeSet will resolve
+  // the ssm parameter and thus cause a resource replacement. Issue: https://github.com/aws/aws-cdk/issues/29731
+  const replacementsFromChangeSet = findResourceReplacements(changeSet);
 
   // Base diff
-  const theDiff = calculateTemplateDiff(currentTemplate, newTemplate);
+  const theDiff = calculateTemplateDiff(currentTemplate, newTemplate, replacementsFromChangeSet);
 
   // We're going to modify this in-place
   const newTemplateCopy = deepCopy(newTemplate);
 
   let didPropagateReferenceChanges;
   let diffWithReplacements;
   do {
-    diffWithReplacements = calculateTemplateDiff(currentTemplate, newTemplateCopy);
+    diffWithReplacements = calculateTemplateDiff(currentTemplate, newTemplateCopy, replacementsFromChangeSet);
 
     // Propagate replacements for replaced resources
     didPropagateReferenceChanges = false;
@@ -104,6 +70,13 @@ export function diffTemplate(
       }
     });
 
+  if (changeSet) {
+    addImportInformation(theDiff, changeSet);
+    filterFalsePositives(theDiff, replacementsFromChangeSet);
+  } else if (isImport) {
+    makeAllResourceChangesImports(theDiff);
+  }
+
   return theDiff;
 }
 
@@ -123,33 +96,84 @@ function propagatePropertyReplacement(source: types.ResourceDifference, dest: ty
   }
 }
 
-function calculateTemplateDiff(currentTemplate: { [key: string]: any }, newTemplate: { [key: string]: any }): types.TemplateDiff {
+function calculateTemplateDiff(
+  currentTemplate: { [key: string]: any },
+  newTemplate: { [key: string]: any },
+  replacementsFromChangeSet?: types.ResourceReplacements,
+): types.TemplateDiff {
   const differences: types.ITemplateDiff = {};
   const unknown: { [key: string]: types.Difference<any> } = {};
   for (const key of unionOf(Object.keys(currentTemplate), Object.keys(newTemplate)).sort()) {
     const oldValue = currentTemplate[key];
     const newValue = newTemplate[key];
-    if (deepEqual(oldValue, newValue)) {
+    if (key != 'Resources' && deepEqual(oldValue, newValue)) {
+      // If the key is Resources, then we may have changes that come from the ChangeSet despite new and old templates being deepEqual.
+      // So include Resources in the diff always. (For example, this can happen if the user has ssm parameters in their template.)
       continue;
     }
-    const handler: DiffHandler = DIFF_HANDLERS[key]
-                  || ((_diff, oldV, newV) => unknown[key] = impl.diffUnknown(oldV, newV));
-    handler(differences, oldValue, newValue);
+
+    switch (key) {
+      case 'AWSTemplateFormatVersion':
+        differences.awsTemplateFormatVersion = impl.diffAttribute(oldValue, newValue);
+        break;
+      case 'Description':
+        differences.description = impl.diffAttribute(oldValue, newValue);
+        break;
+      case 'Transform':
+        differences.transform = impl.diffAttribute(oldValue, newValue);
+        break;
+      case 'Metadata':
+        differences.metadata = new types.DifferenceCollection(diffKeyedEntities(oldValue, newValue, impl.diffMetadata));
+        break;
+      case 'Parameters':
+        differences.parameters = new types.DifferenceCollection(diffKeyedEntities(oldValue, newValue, impl.diffParameter));
+        break;
+      case 'Mappings':
+        differences.mappings = new types.DifferenceCollection(diffKeyedEntities(oldValue, newValue, impl.diffMapping));
+        break;
+      case 'Conditions':
+        differences.conditions = new types.DifferenceCollection(diffKeyedEntities(oldValue, newValue, impl.diffCondition));
+        break;
+      case 'Resources':
+        for (const logicalId of Object.keys(newValue || {})) {
+          if (logicalId in (replacementsFromChangeSet || {})) {
+            for (const [propertyName, changeSetReplacement] of Object.entries(replacementsFromChangeSet![logicalId].propertiesReplaced)) {
+              if (deepEqual(newValue[logicalId].Properties[propertyName], oldValue[logicalId].Properties[propertyName])) {
+                newValue[logicalId].Properties[propertyName].ReplacementFromChangeSet = changeSetReplacement;
+              }
+            }
+          }
+        };
+
+        const resourcesDiff = diffKeyedEntities(oldValue, newValue, impl.diffResource);
+
+        for (const logicalId of unionOf(Object.keys(oldValue || {}), Object.keys(newValue || {}))) {
+          if (logicalId in (replacementsFromChangeSet || {})) {
+            for (const propertyName of Object.keys(replacementsFromChangeSet![logicalId].propertiesReplaced)) {
+              delete newValue[logicalId].Properties[propertyName].ReplacementFromChangeSet;
+            }
+          }
+        };
+
+        differences.resources = new types.DifferenceCollection(resourcesDiff);
+        break;
+      case 'Outputs':
+        differences.outputs = new types.DifferenceCollection(diffKeyedEntities(oldValue, newValue, impl.diffOutput));
+        break;
+      default:
+        unknown[key] = impl.diffUnknown(oldValue, newValue);
+        break;
+    }
   }
+
   if (Object.keys(unknown).length > 0) {
+    // There can potentially be multiple unknown keys, so do this outside of the switch statement.
     differences.unknown = new types.DifferenceCollection(unknown);
   }
 
   return new types.TemplateDiff(differences);
 }
 
-/**
- * Compare two CloudFormation resources and return semantic differences between them
- */
-export function diffResource(oldValue: types.Resource, newValue: types.Resource): types.ResourceDifference {
-  return impl.diffResource(oldValue, newValue);
-}
-
 /**
  * Replace all references to the given logicalID on the given template, in-place
  *
@@ -229,8 +253,7 @@ function makeAllResourceChangesImports(diff: types.TemplateDiff) {
   });
 }
 
-function filterFalsePositives(diff: types.TemplateDiff, changeSet: DescribeChangeSetOutput) {
-  const replacements = findResourceReplacements(changeSet);
+function filterFalsePositives(diff: types.TemplateDiff, replacements: types.ResourceReplacements) {
   diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => {
     if (change.resourceType.includes('AWS::Serverless')) {
       // CFN applies the SAM transform before creating the changeset, so the changeset contains no information about SAM resources
@@ -281,8 +304,12 @@ function findResourceImports(changeSet: DescribeChangeSetOutput): string[] {
   return importedResourceLogicalIds;
 }
 
-function findResourceReplacements(changeSet: DescribeChangeSetOutput): types.ResourceReplacements {
+function findResourceReplacements(changeSet: DescribeChangeSetOutput | undefined): types.ResourceReplacements {
   const replacements: types.ResourceReplacements = {};
+  if (changeSet === undefined) {
+    return replacements;
+  }
+
   for (const resourceChange of changeSet.Changes ?? []) {
     const propertiesReplaced: { [propName: string]: types.ChangeSetReplacement } = {};
     for (const propertyChange of resourceChange.ResourceChange?.Details ?? []) {

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

@@ -26,7 +26,10 @@ export function diffParameter(oldValue: types.Parameter, newValue: types.Paramet
   return new types.ParameterDifference(oldValue, newValue);
 }
 
-export function diffResource(oldValue?: types.Resource, newValue?: types.Resource): types.ResourceDifference {
+export function diffResource(
+  oldValue?: types.Resource,
+  newValue?: types.Resource,
+): types.ResourceDifference {
   const resourceType = {
     oldType: oldValue && oldValue.Type,
     newType: newValue && newValue.Type,

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

@@ -661,6 +661,7 @@ export class ResourceDifference implements IDifference<Resource> {
     if (this.isImport) {
       return ResourceImpact.WILL_IMPORT;
     }
+
     // Check the Type first
     if (this.resourceTypes.oldType !== this.resourceTypes.newType) {
       if (this.resourceTypes.oldType === undefined) { return ResourceImpact.WILL_CREATE; }

packages/aws-cdk-lib/aws-lambda/lib/function-base.ts

@@ -344,7 +344,9 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC
    */
   public addPermission(id: string, permission: Permission) {
     if (!this.canCreatePermissions) {
-      Annotations.of(this).addWarningV2('UnclearLambdaEnvironment', `addPermission() has no effect on a Lambda Function with region=${this.env.region}, account=${this.env.account}, in a Stack with region=${Stack.of(this).region}, account=${Stack.of(this).account}. Suppress this warning if this is is intentional, or pass sameEnvironment=true to fromFunctionAttributes() if you would like to add the permissions.`);
+      if (!this._skipPermissions) {
+        Annotations.of(this).addWarningV2('UnclearLambdaEnvironment', `addPermission() has no effect on a Lambda Function with region=${this.env.region}, account=${this.env.account}, in a Stack with region=${Stack.of(this).region}, account=${Stack.of(this).account}. Suppress this warning if this is is intentional, or pass sameEnvironment=true to fromFunctionAttributes() if you would like to add the permissions.`);
+      }
       return;
     }
 

packages/aws-cdk-lib/aws-lambda/test/function.test.ts

@@ -7,6 +7,7 @@ import { ProfilingGroup } from '../../aws-codeguruprofiler';
 import * as ec2 from '../../aws-ec2';
 import * as efs from '../../aws-efs';
 import * as iam from '../../aws-iam';
+import { AccountPrincipal } from '../../aws-iam';
 import * as kms from '../../aws-kms';
 import * as logs from '../../aws-logs';
 import * as s3 from '../../aws-s3';
@@ -15,6 +16,7 @@ import * as sns from '../../aws-sns';
 import * as sqs from '../../aws-sqs';
 import * as cdk from '../../core';
 import { Aspects, Lazy, Size } from '../../core';
+import { getWarnings } from '../../core/test/util';
 import * as cxapi from '../../cx-api';
 import * as lambda from '../lib';
 import { AdotLambdaLayerJavaSdkVersion } from '../lib/adot-layers';
@@ -223,6 +225,55 @@ describe('function', () => {
       fn.addPermission('S4', { principal: new iam.OrganizationPrincipal('my:org') });
     });
 
+    test('does not show warning if skipPermissions is set', () => {
+      const app = new cdk.App();
+      const stack = new cdk.Stack(app);
+      const imported = lambda.Function.fromFunctionAttributes(stack, 'Imported', {
+        functionArn: 'arn:aws:lambda:us-west-2:123456789012:function:my-function',
+        skipPermissions: true,
+      });
+      imported.addPermission('Permission', {
+        action: 'lambda:InvokeFunction',
+        principal: new AccountPrincipal('123456789010'),
+      });
+
+      expect(getWarnings(app.synth()).length).toBe(0);
+    });
+
+    test('shows warning if skipPermissions is not set', () => {
+      const app = new cdk.App();
+      const stack = new cdk.Stack(app);
+      const imported = lambda.Function.fromFunctionAttributes(stack, 'Imported', {
+        functionArn: 'arn:aws:lambda:us-west-2:123456789012:function:my-function',
+      });
+      imported.addPermission('Permission', {
+        action: 'lambda:InvokeFunction',
+        principal: new AccountPrincipal('123456789010'),
+      });
+
+      expect(getWarnings(app.synth())).toEqual([
+        {
+          message: {
+            'Fn::Join': [
+              '',
+              [
+                'addPermission() has no effect on a Lambda Function with region=us-west-2, account=123456789012, in a Stack with region=',
+                {
+                  Ref: 'AWS::Region',
+                },
+                ', account=',
+                {
+                  Ref: 'AWS::AccountId',
+                },
+                '. Suppress this warning if this is is intentional, or pass sameEnvironment=true to fromFunctionAttributes() if you would like to add the permissions. [ack: UnclearLambdaEnvironment]',
+              ],
+            ],
+          },
+          path: '/Default/Imported',
+        },
+      ]);
+    });
+
     test('applies source account/ARN conditions if the principal has conditions', () => {
       const stack = new cdk.Stack();
       const fn = newTestLambda(stack);

packages/aws-cdk-lib/aws-rds/README.md

@@ -1078,6 +1078,7 @@ const parameterGroup = new rds.ParameterGroup(this, 'ParameterGroup', {
   engine: rds.DatabaseInstanceEngine.sqlServerEe({
     version: rds.SqlServerEngineVersion.VER_11,
   }),
+  name: 'my-parameter-group',
   parameters: {
     locks: '100',
   },