From a38707b178973a01356037aa8f29f61593e056e2 Mon Sep 17 00:00:00 2001 From: mazyu36 Date: Tue, 7 May 2024 03:24:49 +0900 Subject: [PATCH 1/4] docs(ses): fix typos (#30068) Fix typos in VdmAttributes docs. * `Deliverablity ` -> `Deliverability` ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/aws-cdk-lib/aws-ses/lib/vdm-attributes.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/aws-cdk-lib/aws-ses/lib/vdm-attributes.ts b/packages/aws-cdk-lib/aws-ses/lib/vdm-attributes.ts index 0e72ed669e0e5..891f606cb97a6 100644 --- a/packages/aws-cdk-lib/aws-ses/lib/vdm-attributes.ts +++ b/packages/aws-cdk-lib/aws-ses/lib/vdm-attributes.ts @@ -3,11 +3,11 @@ import { CfnVdmAttributes } from './ses.generated'; import { IResource, Resource } from '../../core'; /** - * Virtual Deliverablity Manager (VDM) attributes + * Virtual Deliverability Manager (VDM) attributes */ export interface IVdmAttributes extends IResource { /** - * The name of the resource behind the Virtual Deliverablity Manager attributes. + * The name of the resource behind the Virtual Deliverability Manager attributes. * * @attribute */ @@ -15,7 +15,7 @@ export interface IVdmAttributes extends IResource { } /** - * Properties for the Virtual Deliverablity Manager (VDM) attributes + * Properties for the Virtual Deliverability Manager (VDM) attributes */ export interface VdmAttributesProps { /** @@ -34,11 +34,11 @@ export interface VdmAttributesProps { } /** - * Virtual Deliverablity Manager (VDM) attributes + * Virtual Deliverability Manager (VDM) attributes */ export class VdmAttributes extends Resource implements IVdmAttributes { /** - * Use an existing Virtual Deliverablity Manager attributes resource + * Use an existing Virtual Deliverability Manager attributes resource */ public static fromVdmAttributesName(scope: Construct, id: string, vdmAttributesName: string): IVdmAttributes { class Import extends Resource implements IVdmAttributes { @@ -50,7 +50,7 @@ export class VdmAttributes extends Resource implements IVdmAttributes { public readonly vdmAttributesName: string; /** - * Resource ID for the Virtual Deliverablity Manager attributes + * Resource ID for the Virtual Deliverability Manager attributes * * @attribute */ From 50331a19cfbe30e3d46f8eed15d74d5975fb1527 Mon Sep 17 00:00:00 2001 From: LaurenceWarne <17688577+LaurenceWarne@users.noreply.github.com> Date: Mon, 6 May 2024 19:53:24 +0100 Subject: [PATCH 2/4] feat(rds): implement setting parameter group name (#29965) ### Reason for this change I'd like to be able to set RDS parameter group names using CDK. ### Description of changes I've added a new field `name` to `ParameterGroupProps` used to populate `dbClusterParameterGroupName` and `dbParameterGroupName` where appropriate - AFAICS this couldn't be done at this level previously? ### Description of how you validated changes I've altered a couple of unit tests and an integration test. However I'm unable to run the integration test as the postgres version in the existing snapshot is deprecated (I believe this needs to be deployed before the new snapshot?) which blocks me from deploying. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- ...-rds-instance-with-rds-parameter-group.assets.json | 4 ++-- ...ds-instance-with-rds-parameter-group.template.json | 5 +++-- .../manifest.json | 2 +- .../tree.json | 5 +++-- .../test/integ.instance-with-parameter-group.ts | 5 +++-- packages/aws-cdk-lib/aws-rds/README.md | 1 + packages/aws-cdk-lib/aws-rds/lib/parameter-group.ts | 11 +++++++++++ .../aws-cdk-lib/aws-rds/test/parameter-group.test.ts | 4 ++++ 8 files changed, 28 insertions(+), 9 deletions(-) diff --git a/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.assets.json b/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.assets.json index cf9ecfa597261..b2db8913fb2a3 100644 --- a/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.assets.json +++ b/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.assets.json @@ -1,7 +1,7 @@ { "version": "36.0.0", "files": { - "27d09999310ee1193509c78be177319a1946d9d28632d2362b5f6168fd2078f1": { + "239d9f5d8cff7634197e9978eaf8133a6904b71cd0f07fbb239f839c7ce32886": { "source": { "path": "aws-cdk-rds-instance-with-rds-parameter-group.template.json", "packaging": "file" @@ -9,7 +9,7 @@ "destinations": { "current_account-current_region": { "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}", - "objectKey": "27d09999310ee1193509c78be177319a1946d9d28632d2362b5f6168fd2078f1.json", + "objectKey": "239d9f5d8cff7634197e9978eaf8133a6904b71cd0f07fbb239f839c7ce32886.json", "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}" } } diff --git a/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 b/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 index 08798fdefb756..4fb3b3c5d1aa0 100644 --- a/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 +++ b/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": [ "", diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-rds/test/integ.instance-with-parameter-group.js.snapshot/manifest.json b/packages/@aws-cdk-testing/framework-integ/test/aws-rds/test/integ.instance-with-parameter-group.js.snapshot/manifest.json index bbaf8c75314a2..5ab6b54ad58d9 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-rds/test/integ.instance-with-parameter-group.js.snapshot/manifest.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-rds/test/integ.instance-with-parameter-group.js.snapshot/manifest.json @@ -18,7 +18,7 @@ "validateOnSynth": false, "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}", "cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}", - "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/27d09999310ee1193509c78be177319a1946d9d28632d2362b5f6168fd2078f1.json", + "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/239d9f5d8cff7634197e9978eaf8133a6904b71cd0f07fbb239f839c7ce32886.json", "requiresBootstrapStackVersion": 6, "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version", "additionalDependencies": [ diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-rds/test/integ.instance-with-parameter-group.js.snapshot/tree.json b/packages/@aws-cdk-testing/framework-integ/test/aws-rds/test/integ.instance-with-parameter-group.js.snapshot/tree.json index e2c109b5469fc..9ab09e1f659fd 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-rds/test/integ.instance-with-parameter-group.js.snapshot/tree.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-rds/test/integ.instance-with-parameter-group.js.snapshot/tree.json @@ -661,8 +661,9 @@ "attributes": { "aws:cdk:cloudformation:type": "AWS::RDS::DBParameterGroup", "aws:cdk:cloudformation:props": { + "dbParameterGroupName": "name", "description": "desc", - "family": "postgres15", + "family": "postgres16", "parameters": {} } }, @@ -834,7 +835,7 @@ }, "enableIamDatabaseAuthentication": true, "engine": "postgres", - "engineVersion": "15.2", + "engineVersion": "16.2", "masterUsername": { "Fn::Join": [ "", diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-rds/test/integ.instance-with-parameter-group.ts b/packages/@aws-cdk-testing/framework-integ/test/aws-rds/test/integ.instance-with-parameter-group.ts index 141fefb491cc1..83a9caefd940b 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-rds/test/integ.instance-with-parameter-group.ts +++ b/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, diff --git a/packages/aws-cdk-lib/aws-rds/README.md b/packages/aws-cdk-lib/aws-rds/README.md index f44921d277e47..6fad15b4734b5 100644 --- a/packages/aws-cdk-lib/aws-rds/README.md +++ b/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', }, diff --git a/packages/aws-cdk-lib/aws-rds/lib/parameter-group.ts b/packages/aws-cdk-lib/aws-rds/lib/parameter-group.ts index c529bc859d401..ef1f43aac56b1 100644 --- a/packages/aws-cdk-lib/aws-rds/lib/parameter-group.ts +++ b/packages/aws-cdk-lib/aws-rds/lib/parameter-group.ts @@ -70,6 +70,13 @@ export interface ParameterGroupProps { */ readonly engine: IEngine; + /** + * The name of this parameter group. + * + * @default - CloudFormation-generated name + */ + readonly name?: string; + /** * Description for this parameter group * @@ -126,6 +133,7 @@ export class ParameterGroup extends Resource implements IParameterGroup { private readonly family: string; private readonly removalPolicy?: RemovalPolicy; private readonly description?: string; + private readonly name?: string; private clusterCfnGroup?: CfnDBClusterParameterGroup; private instanceCfnGroup?: CfnDBParameterGroup; @@ -139,6 +147,7 @@ export class ParameterGroup extends Resource implements IParameterGroup { } this.family = family; this.description = props.description; + this.name = props.name; this.parameters = props.parameters ?? {}; this.removalPolicy = props.removalPolicy; } @@ -149,6 +158,7 @@ export class ParameterGroup extends Resource implements IParameterGroup { this.clusterCfnGroup = new CfnDBClusterParameterGroup(this, id, { description: this.description || `Cluster parameter group for ${this.family}`, family: this.family, + dbClusterParameterGroupName: this.name, parameters: Lazy.any({ produce: () => this.parameters }), }); } @@ -166,6 +176,7 @@ export class ParameterGroup extends Resource implements IParameterGroup { this.instanceCfnGroup = new CfnDBParameterGroup(this, id, { description: this.description || `Parameter group for ${this.family}`, family: this.family, + dbParameterGroupName: this.name, parameters: Lazy.any({ produce: () => this.parameters }), }); } diff --git a/packages/aws-cdk-lib/aws-rds/test/parameter-group.test.ts b/packages/aws-cdk-lib/aws-rds/test/parameter-group.test.ts index cb8ee7adefc4c..a47d76953bd11 100644 --- a/packages/aws-cdk-lib/aws-rds/test/parameter-group.test.ts +++ b/packages/aws-cdk-lib/aws-rds/test/parameter-group.test.ts @@ -29,6 +29,7 @@ describe('parameter group', () => { const parameterGroup = new ParameterGroup(stack, 'Params', { engine: DatabaseClusterEngine.AURORA, description: 'desc', + name: 'name', parameters: { key: 'value', }, @@ -37,6 +38,7 @@ describe('parameter group', () => { // THEN Template.fromStack(stack).hasResourceProperties('AWS::RDS::DBParameterGroup', { + DBParameterGroupName: 'name', Description: 'desc', Family: 'aurora5.6', Parameters: { @@ -53,6 +55,7 @@ describe('parameter group', () => { const parameterGroup = new ParameterGroup(stack, 'Params', { engine: DatabaseClusterEngine.AURORA, description: 'desc', + name: 'name', parameters: { key: 'value', }, @@ -61,6 +64,7 @@ describe('parameter group', () => { // THEN Template.fromStack(stack).hasResourceProperties('AWS::RDS::DBClusterParameterGroup', { + DBClusterParameterGroupName: 'name', Description: 'desc', Family: 'aurora5.6', Parameters: { From 2c53cf959d4de8a2d7cadfb24985087df1199305 Mon Sep 17 00:00:00 2001 From: Joshua Weber <57131123+daschaa@users.noreply.github.com> Date: Mon, 6 May 2024 21:22:51 +0200 Subject: [PATCH 3/4] chore(lambda): hide warning if skipPermissions is set (#30060) ### Issue #29887 Closes #29887 ### Reason for this change If an user imports a lambda and wants to add permissions a warning is show. This warning should be skippable with the skipPermissions flag. ### Description of how you validated changes Unit tests for checking if the warning is shown/not shown depending on the value of `skipPermissions` are added. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../aws-lambda/lib/function-base.ts | 4 +- .../aws-lambda/test/function.test.ts | 51 +++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/aws-lambda/lib/function-base.ts b/packages/aws-cdk-lib/aws-lambda/lib/function-base.ts index 9e6055eb86b4b..22d20a8202fbf 100644 --- a/packages/aws-cdk-lib/aws-lambda/lib/function-base.ts +++ b/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; } diff --git a/packages/aws-cdk-lib/aws-lambda/test/function.test.ts b/packages/aws-cdk-lib/aws-lambda/test/function.test.ts index f4c5382707641..0b1142baf17c5 100644 --- a/packages/aws-cdk-lib/aws-lambda/test/function.test.ts +++ b/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); From e4b1f43c48c98e38e65eb18b5fedf1a653c5bdef Mon Sep 17 00:00:00 2001 From: Jakob Berg Date: Tue, 7 May 2024 10:25:26 -0400 Subject: [PATCH 4/4] changes --- .../cloudformation-diff/lib/diff-template.ts | 137 +++++++++++------- .../cloudformation-diff/lib/diff/index.ts | 5 +- .../cloudformation-diff/lib/diff/types.ts | 1 + 3 files changed, 87 insertions(+), 56 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts index 118df03c6ebc5..22257cb41338c 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts +++ b/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,24 +29,14 @@ 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); @@ -78,7 +44,7 @@ export function diffTemplate( 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 } = {}; 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 ?? []) { diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff/index.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff/index.ts index 4ab45f7f09fb3..a362d7e2183c2 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff/index.ts +++ b/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, diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts index 4b65ef484e919..c058aac55bb43 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts @@ -661,6 +661,7 @@ export class ResourceDifference implements IDifference { 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; }