Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cli): cdk diff falsely reports resource replacements on trivial template changes #28336

Merged
merged 51 commits into from
Jan 4, 2024
Merged
Changes from 1 commit
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
b299958
can now create changeset during diff
comcalvi Dec 1, 2023
9f73241
working changeset diff for resource replacement, but not for property…
comcalvi Dec 4, 2023
b59e97f
property-level diff
comcalvi Dec 4, 2023
850fd0f
removed resource metadata from diff, we now consider evaluation: dyna…
comcalvi Dec 6, 2023
858abcc
Merge branch 'main' of github.com:aws/aws-cdk into comcalvi/changeset…
comcalvi Dec 6, 2023
edd8dc3
diff now evaluates fn::getAtt short form and long form equivalence
comcalvi Dec 6, 2023
7f5cec0
refactor
comcalvi Dec 6, 2023
f7f8d15
dependson is another problem...
comcalvi Dec 11, 2023
49e890c
tests, pt 1
comcalvi Dec 12, 2023
5129e95
cleanup
comcalvi Dec 12, 2023
4c66a1e
move to devDeps
comcalvi Dec 12, 2023
d373680
yarn.lock
comcalvi Dec 12, 2023
03a79f2
clean
comcalvi Dec 12, 2023
7c6287b
refactor
comcalvi Dec 13, 2023
da00e1a
CLI options
comcalvi Dec 13, 2023
212f603
remove metadata, but only when the flag is passed
comcalvi Dec 13, 2023
9d01051
not sure who uses this, but someone might...
comcalvi Dec 13, 2023
73c4685
console
comcalvi Dec 13, 2023
29d9094
conflicts
comcalvi Dec 13, 2023
3096aec
cleanup
comcalvi Dec 13, 2023
a106c7d
added the SDK comment
comcalvi Dec 13, 2023
e6e770d
style
comcalvi Dec 13, 2023
054e432
test comment cleanup
comcalvi Dec 13, 2023
561663d
tests
comcalvi Dec 13, 2023
5d44f9a
TS
comcalvi Dec 13, 2023
64e918b
bleh
comcalvi Dec 13, 2023
4686b37
cleanup
comcalvi Dec 14, 2023
d3edce4
lefotver
comcalvi Dec 14, 2023
76d9621
resource to import/...
comcalvi Dec 14, 2023
645feb2
more resources to import
comcalvi Dec 14, 2023
f90ba59
naming
comcalvi Dec 14, 2023
d186cc9
rework
comcalvi Dec 16, 2023
a81a5dd
testing
comcalvi Dec 16, 2023
52f86a1
error handling
comcalvi Dec 16, 2023
7d0d38f
dependsOn
comcalvi Dec 16, 2023
dec2261
rename
comcalvi Dec 16, 2023
d1b8533
nested stacks
comcalvi Dec 20, 2023
c9341ea
depends on improvements + comments
comcalvi Dec 20, 2023
ca152bb
1,1
comcalvi Dec 20, 2023
abfe661
done
comcalvi Dec 20, 2023
44041b1
unit test
comcalvi Dec 22, 2023
7b4d7e1
rename
comcalvi Jan 2, 2024
5c654da
cloudformation wowww
comcalvi Jan 3, 2024
3bd3b03
disable it, just for testing...
comcalvi Jan 3, 2024
0956232
Merge branch 'main' of github.com:aws/aws-cdk into comcalvi/changeset…
comcalvi Jan 3, 2024
b07e9ac
Merge branch 'main' of github.com:aws/aws-cdk into comcalvi/changeset…
comcalvi Jan 3, 2024
f0984a6
removing this fix, for now...
comcalvi Jan 3, 2024
c264294
comments
comcalvi Jan 3, 2024
7ebc5ce
nested stack test
comcalvi Jan 3, 2024
b35e538
idk what this is doing here...
comcalvi Jan 3, 2024
4f12072
Merge branch 'main' into comcalvi/changeset-diff
mergify[bot] Jan 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
refactor
comcalvi committed Dec 13, 2023
commit 7c6287bddc8b0880a88eb573779e95c8a15c19ce
39 changes: 36 additions & 3 deletions packages/aws-cdk/lib/api/util/cloudformation.ts
Original file line number Diff line number Diff line change
@@ -4,7 +4,8 @@ import { CloudFormation } from 'aws-sdk';
import { StackStatus } from './cloudformation/stack-status';
import { debug } from '../../logging';
import { deserializeStructure } from '../../serialize';
import { TemplateBodyParameter } from '../deploy-stack';
import { TemplateBodyParameter, makeBodyParameterAndUpload } from '../deploy-stack';
import { Deployments } from '../deployments';

export type Template = {
Parameters?: Record<string, TemplateParameter>;
@@ -282,8 +283,16 @@ export async function waitForChangeSet(
return ret;
}

export type PrepareChangeSetOptions = {
stack: cxapi.CloudFormationStackArtifact,
deployments: Deployments,
uuid: string,
resourcesToImport?: ResourcesToImport,
willExecute: boolean,
}

export type CreateChangeSetOptions = {
cfn: CloudFormation
cfn: CloudFormation,
changeSetName: string,
resourcesToImport?: ResourcesToImport,
willExecute: boolean,
@@ -293,7 +302,31 @@ export type CreateChangeSetOptions = {
bodyParameter: TemplateBodyParameter,
}

export async function createChangeSet(options: CreateChangeSetOptions): Promise<CloudFormation.DescribeChangeSetOutput> {
export async function prepareAndCreateChangeSet(options: PrepareChangeSetOptions) {
const preparedSdk = (await options.deployments.prepareSdkWithDeployRole(options.stack));
const bodyParameter = await makeBodyParameterAndUpload(
options.stack,
preparedSdk.resolvedEnvironment,
undefined as any,
undefined as any,
preparedSdk.stackSdk,
);
const cfn = preparedSdk.stackSdk.cloudFormation();
const exists = (await CloudFormationStack.lookup(cfn, options.stack.stackName, false)).exists;

return createChangeSet({
cfn,
changeSetName: 'some-name',
resourcesToImport: options.resourcesToImport,
stack: options.stack,
exists,
uuid: options.uuid,
willExecute: options.willExecute,
bodyParameter,
});
}

async function createChangeSet(options: CreateChangeSetOptions): Promise<CloudFormation.DescribeChangeSetOutput> {
await cleanupOldChangeset(options.exists, options.changeSetName, options.stack.stackName, options.cfn);

//debug(`Attempting to create ChangeSet with name ${options.changeSetName} to ${verb} stack ${options.stack.stackName}`);
46 changes: 8 additions & 38 deletions packages/aws-cdk/lib/cdk-toolkit.ts
Original file line number Diff line number Diff line change
@@ -6,7 +6,7 @@ import * as chokidar from 'chokidar';
import * as fs from 'fs-extra';
import * as promptly from 'promptly';
import * as uuid from 'uuid';
import { DeploymentMethod, makeBodyParameterAndUpload } from './api';
import { DeploymentMethod } from './api';
import { SdkProvider } from './api/aws-auth';
import { Bootstrapper, BootstrapEnvironmentOptions } from './api/bootstrap';
import { CloudAssembly, DefaultSelection, ExtendedStackSelection, StackCollection, StackSelector } from './api/cxapp/cloud-assembly';
@@ -15,7 +15,7 @@ import { Deployments } from './api/deployments';
import { HotswapMode } from './api/hotswap/common';
import { findCloudWatchLogGroups } from './api/logs/find-cloudwatch-logs';
import { CloudWatchLogEventMonitor } from './api/logs/logs-monitor';
import { CloudFormationStack, ResourcesToImport, createChangeSet } from './api/util/cloudformation';
import { ResourcesToImport, prepareAndCreateChangeSet } from './api/util/cloudformation';
import { StackActivityProgress } from './api/util/cloudformation/stack-activity-monitor';
import { generateCdkApp, generateStack, readFromPath, readFromStack, setEnvironment, validateSourceOptions } from './commands/migrate';
import { printSecurityDiff, printStackDiff, RequireApproval } from './diff';
@@ -134,27 +134,12 @@ export class CdkToolkit {
}

///////////////////////////////
const preparedSdk = (await this.props.deployments.prepareSdkWithDeployRole(stacks.firstStack));
const bodyParameter = await makeBodyParameterAndUpload(
stacks.firstStack,
preparedSdk.resolvedEnvironment,
undefined as any,
undefined as any,
preparedSdk.stackSdk,
);
const cfn = preparedSdk.stackSdk.cloudFormation();
const exists = (await CloudFormationStack.lookup(cfn, stacks.firstStack.stackName, false)).exists;
const diffChangeSetUuid = uuid.v4();

const changeSet = await createChangeSet({
cfn,
changeSetName: 'some-name',
const changeSet = await prepareAndCreateChangeSet({
resourcesToImport: options.resourcesToImport,
stack: stacks.firstStack,
exists,
uuid: diffChangeSetUuid,
uuid: uuid.v4(),
willExecute: false,
bodyParameter,
deployments: this.props.deployments,
});
///////////////////////////////

@@ -175,27 +160,12 @@ export class CdkToolkit {
const currentTemplate = templateWithNames.deployedTemplate;
const nestedStackCount = templateWithNames.nestedStackCount;

const preparedSdk = (await this.props.deployments.prepareSdkWithDeployRole(stack));
const bodyParameter = await makeBodyParameterAndUpload(
stack,
preparedSdk.resolvedEnvironment,
undefined as any,
undefined as any,
preparedSdk.stackSdk,
);
const cfn = preparedSdk.stackSdk.cloudFormation();
const exists = (await CloudFormationStack.lookup(cfn, stack.stackName, false)).exists;
const diffChangeSetUuid = uuid.v4();

const changeSet = await createChangeSet({
cfn,
changeSetName: 'some-name',
const changeSet = await prepareAndCreateChangeSet({
resourcesToImport: options.resourcesToImport,
stack,
exists,
uuid: diffChangeSetUuid,
uuid: uuid.v4(),
deployments: this.props.deployments,
willExecute: false,
bodyParameter,
});

const stackCount =