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(core): Access Denied using legacy synthesizer with new bootstrap #9831

Merged
merged 10 commits into from
Aug 20, 2020
2 changes: 2 additions & 0 deletions packages/@aws-cdk/core/lib/assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,8 @@ export interface FileAssetLocation {
* can be used as an example for how to configure the key properly.
*
* @default - Asset bucket is not encrypted
* @deprecated Since bootstrap bucket v4, the key policy properly allows use of the
* key via the bucket and no additional parameters have to be granted anymore.
*/
readonly kmsKeyArn?: string;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import * as cxschema from '@aws-cdk/cloud-assembly-schema';
import * as cxapi from '@aws-cdk/cx-api';
import { DockerImageAssetLocation, DockerImageAssetSource, FileAssetLocation, FileAssetPackaging, FileAssetSource } from '../assets';
import { Fn } from '../cfn-fn';
import { CfnParameter } from '../cfn-parameter';
import { CfnRule } from '../cfn-rule';
import { ISynthesisSession } from '../construct-compat';
import { Stack } from '../stack';
import { Token } from '../token';
Expand All @@ -17,7 +19,7 @@ export const BOOTSTRAP_QUALIFIER_CONTEXT = '@aws-cdk/core:bootstrapQualifier';
/**
* The minimum bootstrap stack version required by this app.
*/
const MIN_BOOTSTRAP_STACK_VERSION = 3;
const MIN_BOOTSTRAP_STACK_VERSION = 4;

/**
* Configuration properties for DefaultStackSynthesizer
Expand Down Expand Up @@ -233,6 +235,8 @@ export class DefaultStackSynthesizer implements IStackSynthesizer {
this.imageAssetPublishingRoleArn = specialize(this.props.imageAssetPublishingRoleArn ?? DefaultStackSynthesizer.DEFAULT_IMAGE_ASSET_PUBLISHING_ROLE_ARN);
this._kmsKeyArnExportName = specialize(this.props.fileAssetKeyArnExportName ?? DefaultStackSynthesizer.DEFAULT_FILE_ASSET_KEY_ARN_EXPORT_NAME);
/* eslint-enable max-len */

addBootstrapVersionRule(stack, MIN_BOOTSTRAP_STACK_VERSION, qualifier);
}

public addFileAsset(asset: FileAssetSource): FileAssetLocation {
Expand Down Expand Up @@ -270,7 +274,6 @@ export class DefaultStackSynthesizer implements IStackSynthesizer {
httpUrl,
s3ObjectUrl,
s3Url: httpUrl,
kmsKeyArn: Fn.importValue(cfnify(this._kmsKeyArnExportName)),
};
}

Expand Down Expand Up @@ -462,3 +465,38 @@ function stackLocationOrInstrinsics(stack: Stack) {
urlSuffix: resolvedOr(stack.urlSuffix, '${AWS::URLSuffix}'),
};
}

/**
* Add a CfnRule to the Stack which checks the current version of the bootstrap stack this template is targeting
*
* The CLI normally checks this, but in a pipeline the CLI is not involved
* so we encode this rule into the template in a way that CloudFormation will check it.
*/
function addBootstrapVersionRule(stack: Stack, requiredVersion: number, qualifier: string) {
const param = new CfnParameter(stack, 'BootstrapVersion', {
type: 'AWS::SSM::Parameter::Value<String>',
description: 'Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store.',
default: `/CdkBootstrap/${qualifier}/Version`,
});

// There is no >= check in CloudFormation, so we have to check the number
// is NOT in [1, 2, 3, ... <required> - 1]
const oldVersions = range(1, requiredVersion).map(n => `${n}`);

new CfnRule(stack, 'CheckBootstrapVersion', {
assertions: [
{
assert: Fn.conditionNot(Fn.conditionContains(oldVersions, param.valueAsString)),
assertDescription: `CDK bootstrap stack version ${requiredVersion} required. Please run 'cdk bootstrap' with a recent version of the CDK CLI.`,
},
],
});
}

function range(startIncl: number, endExcl: number) {
const ret = new Array<number>();
for (let i = startIncl; i < endExcl; i++) {
ret.push(i);
}
return ret;
}
30 changes: 22 additions & 8 deletions packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ Conditions:
UsePublicAccessBlockConfiguration:
Fn::Equals:
- 'true'
- Ref: PublicAccessBlockConfiguration
- Ref: PublicAccessBlockConfiguration
Resources:
FileAssetsBucketEncryptionKey:
Type: AWS::KMS::Key
Expand Down Expand Up @@ -99,11 +99,14 @@ Resources:
- kms:GenerateDataKey*
Effect: Allow
Principal:
AWS:
Ref: AWS::AccountId
# Not actually everyone -- see below for Conditions
AWS: "*"
Resource: "*"
Condition:
StringEquals:
# See https://docs.aws.amazon.com/kms/latest/developerguide/policy-conditions.html#conditions-kms-caller-account
kms:CallerAccount:
Ref: AWS::AccountId
kms:ViaService:
- Fn::Sub: s3.${AWS::Region}.amazonaws.com
- Action:
Expand Down Expand Up @@ -143,7 +146,7 @@ Resources:
BlockPublicPolicy: true
IgnorePublicAcls: true
RestrictPublicBuckets: true
- Ref: AWS::NoValue
- Ref: AWS::NoValue
UpdateReplacePolicy: Retain
StagingBucketPolicy:
Type: 'AWS::S3::BucketPolicy'
Expand Down Expand Up @@ -350,6 +353,15 @@ Resources:
- Ref: AWS::NoValue
RoleName:
Fn::Sub: cdk-${Qualifier}-cfn-exec-role-${AWS::AccountId}-${AWS::Region}
# The SSM parameter is used in pipeline-deployed templates to verify the version
# of the bootstrap resources.
CdkBootstrapVersion:
Type: AWS::SSM::Parameter
Properties:
Type: String
Name:
Fn::Sub: '/CdkBootstrap/${Qualifier}/Version'
Value: 4
Outputs:
BucketName:
Description: The name of the S3 bucket owned by the CDK toolkit stack
Expand All @@ -359,6 +371,9 @@ Outputs:
Description: The domain name of the S3 bucket owned by the CDK toolkit stack
Value:
Fn::Sub: "${StagingBucket.RegionalDomainName}"
# @deprecated - This Export can be removed at some future point in time.
# We can't do it today because if there are stacks that use it, the bootstrap
# stack cannot be updated. Not used anymore by apps >= 1.60.0
FileAssetKeyArn:
Description: The ARN of the KMS key used to encrypt the asset bucket
Value:
Expand All @@ -373,10 +388,9 @@ Outputs:
Description: The name of the ECR repository which hosts docker image assets
Value:
Fn::Sub: "${ContainerAssetsRepository}"
# The Output is used by the CLI to verify the version of the bootstrap resources.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need this to be validated both by CFN and the CLI? Maybe we can omit the CLI now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CLI also does the check before uploading assets and, in the future, before querying context.

Even if we start reading the SSM parameter today (which is not a bad idea) we'll still need the the CLI to support old bootstrap stacks.

BootstrapVersion:
Description: The version of the bootstrap resources that are currently mastered
in this stack
Value: '3'
Export:
Name:
Fn::Sub: CdkBootstrap-${Qualifier}-Version
Value:
Fn::GetAtt: [CdkBootstrapVersion, Value]
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ abstract class ActivityPrinterBase implements IActivityPrinter {
this.resourcesInProgress[activity.event.LogicalResourceId] = activity;
}

if (status.endsWith('_FAILED')) {
if (hasErrorMessage(status)) {
const isCancelled = (activity.event.ResourceStatusReason ?? '').indexOf('cancelled') > -1;

// Cancelled is not an interesting failure reason
Expand Down Expand Up @@ -601,7 +601,7 @@ export class CurrentActivityPrinter extends ActivityPrinterBase {
}

private failureReasonOnNextLine(activity: StackActivity) {
return (activity.event.ResourceStatus ?? '').endsWith('_FAILED')
return hasErrorMessage(activity.event.ResourceStatus ?? '')
? `\n${' '.repeat(TIMESTAMP_WIDTH + STATUS_WIDTH + 6)}${colors.red(activity.event.ResourceStatusReason ?? '')}`
: '';
}
Expand All @@ -613,6 +613,10 @@ const MAX_PROGRESSBAR_WIDTH = 60;
const MIN_PROGRESSBAR_WIDTH = 10;
const PROGRESSBAR_EXTRA_SPACE = 2 /* leading spaces */ + 2 /* brackets */ + 4 /* progress number decoration */ + 6 /* 2 progress numbers up to 999 */;

function hasErrorMessage(status: string) {
return status.endsWith('_FAILED') || status === 'ROLLBACK_IN_PROGRESS' || status === 'UPDATE_ROLLBACK_IN_PROGRESS';
}

function colorFromStatusResult(status?: string) {
if (!status) {
return colors.reset;
Expand Down Expand Up @@ -643,7 +647,8 @@ function colorFromStatusActivity(status?: string) {
if (status.startsWith('CREATE_') || status.startsWith('UPDATE_')) {
return colors.green;
}
if (status.startsWith('ROLLBACK_')) {
// For stacks, it may also be 'UPDDATE_ROLLBACK_IN_PROGRESS'
if (status.indexOf('ROLLBACK_') !== -1) {
return colors.yellow;
}
if (status.startsWith('DELETE_')) {
Expand Down