Skip to content

Commit f1fd56a

Browse files
chore: remove sdkv2 from cloudformation-diff (#29730)
### Issue # (if applicable) Closes #<issue number here>. ### Reason for this change ### Description of changes ### Description of how you validated changes ### Checklist - [ ] 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*
1 parent bf2cf51 commit f1fd56a

File tree

7 files changed

+464
-40
lines changed

7 files changed

+464
-40
lines changed

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
// The SDK is only used to reference `DescribeChangeSetOutput`, so the SDK is added as a devDependency.
22
// The SDK should not make network calls here
3-
import type { CloudFormation } from 'aws-sdk';
3+
import type { DescribeChangeSetOutput as DescribeChangeSet } from '@aws-sdk/client-cloudformation';
44
import * as impl from './diff';
55
import * as types from './diff/types';
66
import { deepEqual, diffKeyedEntities, unionOf } from './diff/util';
77

88
export * from './diff/types';
99

10+
export type DescribeChangeSetOutput = DescribeChangeSet;
11+
1012
type DiffHandler = (diff: types.ITemplateDiff, oldValue: any, newValue: any) => void;
1113
type HandlerRegistry = { [section: string]: DiffHandler };
1214

@@ -45,7 +47,7 @@ const DIFF_HANDLERS: HandlerRegistry = {
4547
export function fullDiff(
4648
currentTemplate: { [key: string]: any },
4749
newTemplate: { [key: string]: any },
48-
changeSet?: CloudFormation.DescribeChangeSetOutput,
50+
changeSet?: DescribeChangeSetOutput,
4951
isImport?: boolean,
5052
): types.TemplateDiff {
5153

@@ -212,7 +214,7 @@ function deepCopy(x: any): any {
212214
return x;
213215
}
214216

215-
function addImportInformation(diff: types.TemplateDiff, changeSet: CloudFormation.DescribeChangeSetOutput) {
217+
function addImportInformation(diff: types.TemplateDiff, changeSet: DescribeChangeSetOutput) {
216218
const imports = findResourceImports(changeSet);
217219
diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => {
218220
if (imports.includes(logicalId)) {
@@ -227,7 +229,7 @@ function makeAllResourceChangesImports(diff: types.TemplateDiff) {
227229
});
228230
}
229231

230-
function filterFalsePositives(diff: types.TemplateDiff, changeSet: CloudFormation.DescribeChangeSetOutput) {
232+
function filterFalsePositives(diff: types.TemplateDiff, changeSet: DescribeChangeSetOutput) {
231233
const replacements = findResourceReplacements(changeSet);
232234
diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => {
233235
if (change.resourceType.includes('AWS::Serverless')) {
@@ -268,7 +270,7 @@ function filterFalsePositives(diff: types.TemplateDiff, changeSet: CloudFormatio
268270
});
269271
}
270272

271-
function findResourceImports(changeSet: CloudFormation.DescribeChangeSetOutput): string[] {
273+
function findResourceImports(changeSet: DescribeChangeSetOutput): string[] {
272274
const importedResourceLogicalIds = [];
273275
for (const resourceChange of changeSet.Changes ?? []) {
274276
if (resourceChange.ResourceChange?.Action === 'Import') {
@@ -279,7 +281,7 @@ function findResourceImports(changeSet: CloudFormation.DescribeChangeSetOutput):
279281
return importedResourceLogicalIds;
280282
}
281283

282-
function findResourceReplacements(changeSet: CloudFormation.DescribeChangeSetOutput): types.ResourceReplacements {
284+
function findResourceReplacements(changeSet: DescribeChangeSetOutput): types.ResourceReplacements {
283285
const replacements: types.ResourceReplacements = {};
284286
for (const resourceChange of changeSet.Changes ?? []) {
285287
const propertiesReplaced: { [propName: string]: types.ChangeSetReplacement } = {};

packages/@aws-cdk/cloudformation-diff/package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
"dependencies": {
2626
"@aws-cdk/aws-service-spec": "^0.0.61",
2727
"@aws-cdk/service-spec-types": "^0.0.61",
28-
"aws-sdk": "2.1586.0",
2928
"chalk": "^4",
3029
"diff": "^5.2.0",
3130
"fast-deep-equal": "^3.1.3",
@@ -35,6 +34,7 @@
3534
"devDependencies": {
3635
"@aws-cdk/cdk-build-tools": "0.0.0",
3736
"@aws-cdk/pkglint": "0.0.0",
37+
"@aws-sdk/client-cloudformation": "^3.529.1",
3838
"@types/jest": "^29.5.12",
3939
"@types/string-width": "^4.0.1",
4040
"fast-check": "^3.17.0",
@@ -64,4 +64,4 @@
6464
"dependencies/cdk-point-dependencies"
6565
]
6666
}
67-
}
67+
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1104,7 +1104,7 @@ describe('changeset', () => {
11041104
ResourceType: 'AWS::S3::Bucket',
11051105
Replacement: 'True',
11061106
Details: [{
1107-
Evaluation: 'Direct',
1107+
Evaluation: 'Static',
11081108
Target: {
11091109
Attribute: 'Properties',
11101110
Name: 'BucketName',
@@ -1153,7 +1153,7 @@ describe('changeset', () => {
11531153
ResourceType: 'AWS::Lambda::Function', // The SAM transform is applied before the changeset is created, so the changeset has a Lambda resource here!
11541154
Replacement: 'False',
11551155
Details: [{
1156-
Evaluation: 'Direct',
1156+
Evaluation: 'Static',
11571157
Target: {
11581158
Attribute: 'Properties',
11591159
Name: 'Code',

packages/aws-cdk/lib/api/util/cloudformation.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
// Temporarily pull this in to avoid creating conflicts with the sdks in this package
2+
import { DescribeChangeSetOutput } from '@aws-cdk/cloudformation-diff';
13
import { SSMPARAM_NO_INVALIDATE } from '@aws-cdk/cx-api';
24
import * as cxapi from '@aws-cdk/cx-api';
35
import { CloudFormation } from 'aws-sdk';
@@ -311,7 +313,7 @@ export type CreateChangeSetOptions = {
311313
/**
312314
* Create a changeset for a diff operation
313315
*/
314-
export async function createDiffChangeSet(options: PrepareChangeSetOptions): Promise<CloudFormation.DescribeChangeSetOutput | undefined> {
316+
export async function createDiffChangeSet(options: PrepareChangeSetOptions): Promise<DescribeChangeSetOutput | undefined> {
315317
// `options.stack` has been modified to include any nested stack templates directly inline with its own template, under a special `NestedTemplate` property.
316318
// Thus the parent template's Resources section contains the nested template's CDK metadata check, which uses Fn::Equals.
317319
// This causes CreateChangeSet to fail with `Template Error: Fn::Equals cannot be partially collapsed`.
@@ -327,7 +329,7 @@ export async function createDiffChangeSet(options: PrepareChangeSetOptions): Pro
327329
return uploadBodyParameterAndCreateChangeSet(options);
328330
}
329331

330-
async function uploadBodyParameterAndCreateChangeSet(options: PrepareChangeSetOptions): Promise<CloudFormation.DescribeChangeSetOutput | undefined> {
332+
async function uploadBodyParameterAndCreateChangeSet(options: PrepareChangeSetOptions): Promise<DescribeChangeSetOutput | undefined> {
331333
try {
332334
const preparedSdk = (await options.deployments.prepareSdkWithDeployRole(options.stack));
333335
const bodyParameter = await makeBodyParameterAndUpload(
@@ -363,7 +365,7 @@ async function uploadBodyParameterAndCreateChangeSet(options: PrepareChangeSetOp
363365
}
364366
}
365367

366-
async function createChangeSet(options: CreateChangeSetOptions): Promise<CloudFormation.DescribeChangeSetOutput> {
368+
async function createChangeSet(options: CreateChangeSetOptions): Promise<DescribeChangeSetOutput> {
367369
await cleanupOldChangeset(options.changeSetName, options.stack.stackName, options.cfn);
368370

369371
debug(`Attempting to create ChangeSet with name ${options.changeSetName} for stack ${options.stack.stackName}`);
@@ -390,7 +392,8 @@ async function createChangeSet(options: CreateChangeSetOptions): Promise<CloudFo
390392
const createdChangeSet = await waitForChangeSet(options.cfn, options.stack.stackName, options.changeSetName, { fetchAll: options.willExecute });
391393
await cleanupOldChangeset(options.changeSetName, options.stack.stackName, options.cfn);
392394

393-
return createdChangeSet;
395+
// TODO: Update this once we remove sdkv2 from the rest of this package
396+
return createdChangeSet as DescribeChangeSetOutput;
394397
}
395398

396399
export async function cleanupOldChangeset(changeSetName: string, stackName: string, cfn: CloudFormation) {

packages/aws-cdk/lib/diff.ts

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,15 @@
11
import { format } from 'util';
22
import * as cxschema from '@aws-cdk/cloud-assembly-schema';
3-
import * as cfnDiff from '@aws-cdk/cloudformation-diff';
3+
import {
4+
type DescribeChangeSetOutput,
5+
type FormatStream,
6+
type TemplateDiff,
7+
formatDifferences,
8+
formatSecurityChanges,
9+
fullDiff,
10+
mangleLikeCloudFormation,
11+
} from '@aws-cdk/cloudformation-diff';
412
import * as cxapi from '@aws-cdk/cx-api';
5-
import { CloudFormation } from 'aws-sdk';
613
import * as chalk from 'chalk';
714
import { NestedStackTemplates } from './api/nested-stack-helpers';
815
import { print, warning } from './logging';
@@ -24,18 +31,18 @@ export function printStackDiff(
2431
strict: boolean,
2532
context: number,
2633
quiet: boolean,
27-
changeSet?: CloudFormation.DescribeChangeSetOutput,
34+
changeSet?: DescribeChangeSetOutput,
2835
isImport?: boolean,
29-
stream: cfnDiff.FormatStream = process.stderr,
36+
stream: FormatStream = process.stderr,
3037
nestedStackTemplates?: { [nestedStackLogicalId: string]: NestedStackTemplates }): number {
3138

32-
let diff = cfnDiff.fullDiff(oldTemplate, newTemplate.template, changeSet, isImport);
39+
let diff = fullDiff(oldTemplate, newTemplate.template, changeSet, isImport);
3340

3441
// detect and filter out mangled characters from the diff
3542
let filteredChangesCount = 0;
3643
if (diff.differenceCount && !strict) {
37-
const mangledNewTemplate = JSON.parse(cfnDiff.mangleLikeCloudFormation(JSON.stringify(newTemplate.template)));
38-
const mangledDiff = cfnDiff.fullDiff(oldTemplate, mangledNewTemplate, changeSet);
44+
const mangledNewTemplate = JSON.parse(mangleLikeCloudFormation(JSON.stringify(newTemplate.template)));
45+
const mangledDiff = fullDiff(oldTemplate, mangledNewTemplate, changeSet);
3946
filteredChangesCount = Math.max(0, diff.differenceCount - mangledDiff.differenceCount);
4047
if (filteredChangesCount > 0) {
4148
diff = mangledDiff;
@@ -55,7 +62,7 @@ export function printStackDiff(
5562
let stackDiffCount = 0;
5663
if (!diff.isEmpty) {
5764
stackDiffCount++;
58-
cfnDiff.formatDifferences(stream, diff, {
65+
formatDifferences(stream, diff, {
5966
...logicalIdMapFromTemplate(oldTemplate),
6067
...buildLogicalToPathMap(newTemplate),
6168
}, context);
@@ -109,16 +116,16 @@ export function printSecurityDiff(
109116
oldTemplate: any,
110117
newTemplate: cxapi.CloudFormationStackArtifact,
111118
requireApproval: RequireApproval,
112-
changeSet?: CloudFormation.DescribeChangeSetOutput,
119+
changeSet?: DescribeChangeSetOutput,
113120
): boolean {
114-
const diff = cfnDiff.fullDiff(oldTemplate, newTemplate.template, changeSet);
121+
const diff = fullDiff(oldTemplate, newTemplate.template, changeSet);
115122

116123
if (difRequiresApproval(diff, requireApproval)) {
117124
// eslint-disable-next-line max-len
118125
warning(`This deployment will make potentially sensitive changes according to your current security approval level (--require-approval ${requireApproval}).`);
119126
warning('Please confirm you intend to make the following modifications:\n');
120127

121-
cfnDiff.formatSecurityChanges(process.stdout, diff, buildLogicalToPathMap(newTemplate));
128+
formatSecurityChanges(process.stdout, diff, buildLogicalToPathMap(newTemplate));
122129
return true;
123130
}
124131
return false;
@@ -130,7 +137,7 @@ export function printSecurityDiff(
130137
* TODO: Filter the security impact determination based off of an enum that allows
131138
* us to pick minimum "severities" to alert on.
132139
*/
133-
function difRequiresApproval(diff: cfnDiff.TemplateDiff, requireApproval: RequireApproval) {
140+
function difRequiresApproval(diff: TemplateDiff, requireApproval: RequireApproval) {
134141
switch (requireApproval) {
135142
case RequireApproval.Never: return false;
136143
case RequireApproval.AnyChange: return diff.permissionsAnyChanges;

packages/aws-cdk/test/diff.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ describe('stack exists checks', () => {
405405
Changes: [
406406
{
407407
ResourceChange: {
408-
Action: 'Dummy',
408+
Action: 'Add',
409409
LogicalResourceId: 'Object',
410410
},
411411
},

0 commit comments

Comments
 (0)