Skip to content

Commit 0cd06e9

Browse files
authored
chore: DeletePolicy should default to retain (#25216)
Considering the still volatile nature of the `import` to Cfn and CDK, the default for the required `DeletePolicy` should be to `Retain`. Any mistake that would create new resources can easily be removed with the stack, but we should not jeopardize existing, possible stateful ones. [CONTRIBUTING GUIDE]: https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md [DESIGN GUIDELINES]: https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md Additional typos corrected. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 27da99e commit 0cd06e9

File tree

4 files changed

+13
-11
lines changed

4 files changed

+13
-11
lines changed

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ export interface BuildStackAssetsOptions {
284284
readonly roleArn?: string;
285285

286286
/**
287-
* Options to pass on to `buildAsests()` function
287+
* Options to pass on to `buildAssets()` function
288288
*/
289289
readonly buildOptions?: BuildAssetsOptions;
290290
}

packages/aws-cdk/lib/cdk-toolkit.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ export class CdkToolkit {
239239
'but terminal (TTY) is not attached so we are unable to get a confirmation from the user');
240240
}
241241

242-
// only talk to user if concurreny is 1 (otherwise, fail)
242+
// only talk to user if concurrency is 1 (otherwise, fail)
243243
if (concurrency > 1) {
244244
throw new Error(
245245
'"--require-approval" is enabled and stack includes security-sensitive updates, ' +
@@ -570,7 +570,7 @@ export class CdkToolkit {
570570
* Synthesize the given set of stacks (called when the user runs 'cdk synth')
571571
*
572572
* INPUT: Stack names can be supplied using a glob filter. If no stacks are
573-
* given, all stacks from the application are implictly selected.
573+
* given, all stacks from the application are implicitly selected.
574574
*
575575
* OUTPUT: If more than one stack ends up being selected, an output directory
576576
* should be supplied, where the templates will be written.
@@ -1159,7 +1159,7 @@ function roundPercentage(num: number): number {
11591159
}
11601160

11611161
/**
1162-
* Given a time in miliseconds, return an equivalent amount in seconds.
1162+
* Given a time in milliseconds, return an equivalent amount in seconds.
11631163
*/
11641164
function millisecondsToSeconds(num: number): number {
11651165
return num / 1000;

packages/aws-cdk/lib/cli.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,7 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
485485
});
486486

487487
case 'bootstrap':
488-
const source: BootstrapSource = determineBootsrapVersion(args, configuration);
488+
const source: BootstrapSource = determineBootstrapVersion(args, configuration);
489489

490490
const bootstrapper = new Bootstrapper(source);
491491

@@ -663,7 +663,7 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
663663
* Determine which version of bootstrapping
664664
* (legacy, or "new") should be used.
665665
*/
666-
function determineBootsrapVersion(args: { template?: string }, configuration: Configuration): BootstrapSource {
666+
function determineBootstrapVersion(args: { template?: string }, configuration: Configuration): BootstrapSource {
667667
const isV1 = version.DISPLAY_VERSION.startsWith('1.');
668668
return isV1 ? determineV1BootstrapSource(args, configuration) : determineV2BootstrapSource(args);
669669
}

packages/aws-cdk/lib/import.ts

+7-5
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ export class ResourceImporter {
145145
}
146146

147147
/**
148-
* Perform a diff between the currently running and the new template, enusre that it is valid
148+
* Perform a diff between the currently running and the new template, ensure that it is valid
149149
* for importing and return a list of resources that are being added in the new version
150150
*
151151
* @return mapping logicalResourceId -> resourceDifference
@@ -171,7 +171,7 @@ export class ResourceImporter {
171171
warning(`Ignoring updated/deleted resources (--force): ${offendingResources.join(', ')}`);
172172
} else {
173173
throw new Error('No resource updates or deletes are allowed on import operation. Make sure to resolve pending changes ' +
174-
`to existing resources, before attempting an import. Updated/deleted resources: ${offendingResources.join(', ')} (--force to override)`);
174+
`to existing resources, before attempting an import. Updated/deleted resources: ${offendingResources.join(', ')} (--force to override)`);
175175
}
176176
}
177177

@@ -199,7 +199,7 @@ export class ResourceImporter {
199199
}
200200

201201
/**
202-
* Return teh current template, with the given resources added to it
202+
* Return the current template, with the given resources added to it
203203
*/
204204
private async currentTemplateWithAdditions(additions: ImportableResource[]): Promise<any> {
205205
const template = await this.currentTemplate();
@@ -397,14 +397,16 @@ function fmtdict<A>(xs: Record<string, A>) {
397397
}
398398

399399
/**
400-
* Add a default 'Delete' policy, which is required to make the import succeed
400+
* Add a default `DeletionPolicy` policy.
401+
* The default value is set to 'Retain', to lower risk of unintentionally
402+
* deleting stateful resources in the process of importing to CDK.
401403
*/
402404
function addDefaultDeletionPolicy(resource: any): any {
403405
if (resource.DeletionPolicy) { return resource; }
404406

405407
return {
406408
...resource,
407-
DeletionPolicy: 'Delete',
409+
DeletionPolicy: 'Retain',
408410
};
409411
}
410412

0 commit comments

Comments
 (0)