Skip to content

Commit

Permalink
feat(bootstrap): prevent accidental bootstrap overwrites (#24302)
Browse files Browse the repository at this point in the history
A problem that sometimes happens today is that an account has a customized bootstrap stack setup, but a developer is not aware and they `cdk bootstrap` the customizations away with the CDK default template.

This change introduces a new parameter, `BootstrapFlavor`, that an organization can change to a different value. The CDK CLI will refuse to overwrite existing bootstrap stacks of one flavor, with a different one.

This string could have been in the template metadata, but making it a parameter puts it front and center, making it more obvious that template customizers should change it.

Does not change the bootstrap stack version itself, since nothing about the resources inside changed.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
rix0rrr authored Feb 28, 2023
1 parent dee02a6 commit 3b251a5
Show file tree
Hide file tree
Showing 9 changed files with 109 additions and 15 deletions.
1 change: 1 addition & 0 deletions packages/@aws-cdk-testing/cli-integ/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
"p-queue": "^6.6.2",
"semver": "^7.3.8",
"ts-mock-imports": "^1.3.8",
"yaml": "1.10.2",
"yargs": "^17.7.0"
},
"repository": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as fs from 'fs';
import * as path from 'path';
import * as yaml from 'yaml';
import { integTest, randomString, withoutBootstrap } from '../../lib';

jest.setTimeout(2 * 60 * 60_000); // Includes the time to acquire locks, worst-case single-threaded runtime
Expand Down Expand Up @@ -196,6 +197,41 @@ integTest('can dump the template, modify and use it to deploy a custom bootstrap
});
}));

integTest('a customized template vendor will not overwrite the default template', withoutBootstrap(async (fixture) => {
// Initial bootstrap
const toolkitStackName = fixture.bootstrapStackName;
await fixture.cdkBootstrapModern({
toolkitStackName,
cfnExecutionPolicy: 'arn:aws:iam::aws:policy/AdministratorAccess',
});

// Customize template
const templateStr = await fixture.cdkBootstrapModern({
// toolkitStackName doesn't matter for this particular invocation
toolkitStackName,
showTemplate: true,
cliOptions: {
captureStderr: false,
},
});

const template = yaml.parse(templateStr, { schema: 'core' });
template.Parameters.BootstrapVariant.Default = 'CustomizedVendor';
const filename = path.join(fixture.integTestDir, `${fixture.qualifier}-template.yaml`);
fs.writeFileSync(filename, yaml.stringify(template, { schema: 'yaml-1.1' }), { encoding: 'utf-8' });

// Rebootstrap. For some reason, this doesn't cause a failure, it's a successful no-op.
const output = await fixture.cdkBootstrapModern({
toolkitStackName,
template: filename,
cfnExecutionPolicy: 'arn:aws:iam::aws:policy/AdministratorAccess',
cliOptions: {
captureStderr: true,
},
});
expect(output).toContain('Not overwriting it with a template containing');
}));

integTest('can use the default permissions boundary to bootstrap', withoutBootstrap(async (fixture) => {
let template = await fixture.cdkBootstrapModern({
// toolkitStackName doesn't matter for this particular invocation
Expand Down
6 changes: 6 additions & 0 deletions packages/aws-cdk/lib/api/bootstrap/bootstrap-props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ export const REPOSITORY_NAME_OUTPUT = 'RepositoryName';
export const BUCKET_DOMAIN_NAME_OUTPUT = 'BucketDomainName';
export const BOOTSTRAP_VERSION_OUTPUT = 'BootstrapVersion';
export const BOOTSTRAP_VERSION_RESOURCE = 'CdkBootstrapVersion';
export const BOOTSTRAP_VARIANT_PARAMETER = 'BootstrapVariant';

/**
* The assumed vendor of a template in case it is not set
*/
export const DEFAULT_BOOTSTRAP_VARIANT = 'AWS CDK: Default Resources';

/**
* Options for the bootstrapEnvironment operation(s)
Expand Down
6 changes: 6 additions & 0 deletions packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ Parameters:
Default: 'false'
AllowedValues: [ 'true', 'false' ]
Type: String
BootstrapVariant:
Type: String
Default: 'AWS CDK: Default Resources'
Description: Describe the provenance of the resources in this bootstrap
stack. Change this when you customize the template. To prevent accidents,
the CDK CLI will not overwrite bootstrap stacks with a different variant.
Conditions:
HasTrustedAccounts:
Fn::Not:
Expand Down
41 changes: 29 additions & 12 deletions packages/aws-cdk/lib/api/bootstrap/deploy-bootstrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as path from 'path';
import * as cxschema from '@aws-cdk/cloud-assembly-schema';
import * as cxapi from '@aws-cdk/cx-api';
import * as fs from 'fs-extra';
import { BOOTSTRAP_VERSION_OUTPUT, BootstrapEnvironmentOptions, BOOTSTRAP_VERSION_RESOURCE } from './bootstrap-props';
import { BOOTSTRAP_VERSION_OUTPUT, BootstrapEnvironmentOptions, BOOTSTRAP_VERSION_RESOURCE, BOOTSTRAP_VARIANT_PARAMETER, DEFAULT_BOOTSTRAP_VARIANT } from './bootstrap-props';
import * as logging from '../../logging';
import { Mode, SdkProvider, ISDK } from '../aws-auth';
import { deployStack, DeployStackResult } from '../deploy-stack';
Expand Down Expand Up @@ -63,21 +63,34 @@ export class BootstrapStack {
parameters: Record<string, string | undefined>,
options: Omit<BootstrapEnvironmentOptions, 'parameters'>,
): Promise<DeployStackResult> {

const newVersion = bootstrapVersionFromTemplate(template);
if (this.currentToolkitInfo.found && newVersion < this.currentToolkitInfo.version && !options.force) {
logging.warning(`Bootstrap stack already at version '${this.currentToolkitInfo.version}'. Not downgrading it to version '${newVersion}' (use --force if you intend to downgrade)`);
if (newVersion === 0) {
// A downgrade with 0 as target version means we probably have a new-style bootstrap in the account,
// and an old-style bootstrap as current target, which means the user probably forgot to put this flag in.
logging.warning('(Did you set the \'@aws-cdk/core:newStyleStackSynthesis\' feature flag in cdk.json?)');
}

return {
if (this.currentToolkitInfo.found && !options.force) {
// Safety checks
const abortResponse = {
noOp: true,
outputs: {},
stackArn: this.currentToolkitInfo.bootstrapStack.stackId,
};

// Validate that the bootstrap stack we're trying to replace is from the same variant as the one we're trying to deploy
const currentVariant = this.currentToolkitInfo.variant;
const newVariant = bootstrapVariantFromTemplate(template);
if (currentVariant !== newVariant) {
logging.warning(`Bootstrap stack already exists, containing '${currentVariant}'. Not overwriting it with a template containing '${newVariant}' (use --force if you intend to overwrite)`);
return abortResponse;
}

// Validate that we're not downgrading the bootstrap stack
const newVersion = bootstrapVersionFromTemplate(template);
const currentVersion = this.currentToolkitInfo.version;
if (newVersion < currentVersion) {
logging.warning(`Bootstrap stack already at version ${currentVersion}. Not downgrading it to version ${newVersion} (use --force if you intend to downgrade)`);
if (newVersion === 0) {
// A downgrade with 0 as target version means we probably have a new-style bootstrap in the account,
// and an old-style bootstrap as current target, which means the user probably forgot to put this flag in.
logging.warning('(Did you set the \'@aws-cdk/core:newStyleStackSynthesis\' feature flag in cdk.json?)');
}
return abortResponse;
}
}

const outdir = await fs.mkdtemp(path.join(os.tmpdir(), 'cdk-bootstrap'));
Expand Down Expand Up @@ -127,3 +140,7 @@ export function bootstrapVersionFromTemplate(template: any): number {
}
return 0;
}

export function bootstrapVariantFromTemplate(template: any): string {
return template.Parameters?.[BOOTSTRAP_VARIANT_PARAMETER]?.Default ?? DEFAULT_BOOTSTRAP_VARIANT;
}
11 changes: 10 additions & 1 deletion packages/aws-cdk/lib/api/toolkit-info.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as cxapi from '@aws-cdk/cx-api';
import * as chalk from 'chalk';
import { ISDK } from './aws-auth';
import { BOOTSTRAP_VERSION_OUTPUT, BUCKET_DOMAIN_NAME_OUTPUT, BUCKET_NAME_OUTPUT } from './bootstrap/bootstrap-props';
import { BOOTSTRAP_VERSION_OUTPUT, BUCKET_DOMAIN_NAME_OUTPUT, BUCKET_NAME_OUTPUT, BOOTSTRAP_VARIANT_PARAMETER, DEFAULT_BOOTSTRAP_VARIANT } from './bootstrap/bootstrap-props';
import { stabilizeStack, CloudFormationStack } from './util/cloudformation';
import { debug, warning } from '../logging';

Expand Down Expand Up @@ -102,6 +102,7 @@ export abstract class ToolkitInfo {
public abstract readonly bucketUrl: string;
public abstract readonly bucketName: string;
public abstract readonly version: number;
public abstract readonly variant: string;
public abstract readonly bootstrapStack: CloudFormationStack;

constructor(protected readonly sdk: ISDK) {
Expand Down Expand Up @@ -132,6 +133,10 @@ class ExistingToolkitInfo extends ToolkitInfo {
return parseInt(this.bootstrapStack.outputs[BOOTSTRAP_VERSION_OUTPUT] ?? '0', 10);
}

public get variant() {
return this.bootstrapStack.parameters[BOOTSTRAP_VARIANT_PARAMETER] ?? DEFAULT_BOOTSTRAP_VARIANT;
}

public get parameters(): Record<string, string> {
return this.bootstrapStack.parameters ?? {};
}
Expand Down Expand Up @@ -258,6 +263,10 @@ class BootstrapStackNotFoundInfo extends ToolkitInfo {
throw new Error(this.errorMessage);
}

public get variant(): string {
throw new Error(this.errorMessage);
}

public async validateVersion(expectedVersion: number, ssmParameterName: string | undefined): Promise<void> {
if (ssmParameterName === undefined) {
throw new Error(this.errorMessage);
Expand Down
15 changes: 15 additions & 0 deletions packages/aws-cdk/test/api/bootstrap2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,21 @@ describe('Bootstrapping v2', () => {
})).resolves.toEqual(expect.objectContaining({ noOp: true }));
});

test('Do not allow overwriting bootstrap stack from a different vendor', async () => {
// GIVEN
mockTheToolkitInfo({
Parameters: [
{
ParameterKey: 'BootstrapVariant',
ParameterValue: 'JoeSchmoe',
},
],
});

await expect(bootstrapper.bootstrapEnvironment(env, sdk, {
})).resolves.toEqual(expect.objectContaining({ noOp: true }));
});

test('bootstrap template has the right exports', async () => {
let template: any;
mockDeployStack.mockImplementation((args: DeployStackOptions) => {
Expand Down
4 changes: 3 additions & 1 deletion packages/aws-cdk/test/api/cloudformation-deployments.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import * as cxschema from '@aws-cdk/cloud-assembly-schema';
import * as cxapi from '@aws-cdk/cx-api';
import { CloudFormation } from 'aws-sdk';
import { FakeCloudformationStack } from './fake-cloudformation-stack';
import { DEFAULT_BOOTSTRAP_VARIANT } from '../../lib';
import { CloudFormationDeployments } from '../../lib/api/cloudformation-deployments';
import { deployStack } from '../../lib/api/deploy-stack';
import { HotswapMode } from '../../lib/api/hotswap/common';
Expand Down Expand Up @@ -931,6 +932,7 @@ function testStackWithAssetManifest() {
public found: boolean = true;
public bucketUrl: string = 's3://fake/here';
public bucketName: string = 'fake';
public variant: string = DEFAULT_BOOTSTRAP_VARIANT;
public version: number = 1234;
public get bootstrapStack(): CloudFormationStack {
throw new Error('This should never happen');
Expand Down Expand Up @@ -1000,4 +1002,4 @@ function testStackWithAssetManifest() {

const assembly = builder.buildAssembly();
return assembly.getStackArtifact('stack');
}
}
4 changes: 3 additions & 1 deletion packages/aws-cdk/test/util/mock-toolkitinfo.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ISDK, ToolkitInfo } from '../../lib/api';
import { ISDK, ToolkitInfo, DEFAULT_BOOTSTRAP_VARIANT } from '../../lib/api';
import { CloudFormationStack } from '../../lib/api/util/cloudformation';

export interface MockToolkitInfoProps {
Expand All @@ -17,6 +17,7 @@ export class MockToolkitInfo extends ToolkitInfo {
public readonly bucketUrl: string;
public readonly bucketName: string;
public readonly version: number;
public readonly variant: string;
public readonly prepareEcrRepository = mockLike<typeof ToolkitInfo.prototype.prepareEcrRepository>();

private readonly _bootstrapStack?: CloudFormationStack;
Expand All @@ -27,6 +28,7 @@ export class MockToolkitInfo extends ToolkitInfo {
this.bucketName = props.bucketName ?? 'MockToolkitBucketName';
this.bucketUrl = props.bucketUrl ?? `https://${this.bucketName}.s3.amazonaws.com/`;
this.version = props.version ?? 1;
this.variant = DEFAULT_BOOTSTRAP_VARIANT;
this._bootstrapStack = props.bootstrapStack;
}

Expand Down

0 comments on commit 3b251a5

Please sign in to comment.