From df16d40352e56c2d4b33b2066f3fe030792d32d6 Mon Sep 17 00:00:00 2001 From: Reagan Elm <1347066+relm923@users.noreply.github.com> Date: Thu, 3 Jun 2021 01:28:04 -0400 Subject: [PATCH 1/6] fix(lambda-nodejs): pnpm exec command (#14954) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix #14757 Expands on #14772 Currently get the following error when bundling with `pnpm-lock.yaml` ``` ERROR   ERROR  Unknown options: 'bundle', 'target', 'platform', 'outfile', 'external:aws-sdk' ``` ---- Switch from `pnpm run esbuild` to `pnpm exec esbuild --` to properly enable esbuild with `pnpm` `pnpm run` only supports running commands defined in the package's manifest file - [docs](https://pnpm.io/cli/run) `pnpm exec` supports executing a shell command in scope of a project - [docs](https://pnpm.io/cli/exec) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/@aws-cdk/aws-lambda-nodejs/lib/package-manager.ts | 7 ++++++- .../aws-lambda-nodejs/test/package-manager.test.ts | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/@aws-cdk/aws-lambda-nodejs/lib/package-manager.ts b/packages/@aws-cdk/aws-lambda-nodejs/lib/package-manager.ts index a95373bd6d45f..f10f423a4c38b 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/lib/package-manager.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/package-manager.ts @@ -5,6 +5,7 @@ interface PackageManagerProps { readonly lockFile: string; readonly installCommand: string[]; readonly runCommand: string[]; + readonly argsSeparator?: string } /** @@ -26,7 +27,8 @@ export class PackageManager { public static PNPM = new PackageManager({ lockFile: 'pnpm-lock.yaml', installCommand: ['pnpm', 'install'], - runCommand: ['pnpm', 'run'], + runCommand: ['pnpm', 'exec'], + argsSeparator: '--', }); public static fromLockFile(lockFilePath: string): PackageManager { @@ -47,11 +49,13 @@ export class PackageManager { public readonly lockFile: string; public readonly installCommand: string[]; public readonly runCommand: string[]; + public readonly argsSeparator?: string; constructor(props: PackageManagerProps) { this.lockFile = props.lockFile; this.installCommand = props.installCommand; this.runCommand = props.runCommand; + this.argsSeparator = props.argsSeparator; } public runBinCommand(bin: string): string { @@ -60,6 +64,7 @@ export class PackageManager { os.platform() === 'win32' ? `${runCommand}.cmd` : runCommand, ...runArgs, bin, + ...(this.argsSeparator ? [this.argsSeparator] : []), ].join(' '); } } diff --git a/packages/@aws-cdk/aws-lambda-nodejs/test/package-manager.test.ts b/packages/@aws-cdk/aws-lambda-nodejs/test/package-manager.test.ts index e0721bbec3e38..7f64a18d2123f 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/test/package-manager.test.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/test/package-manager.test.ts @@ -19,7 +19,7 @@ test('from a pnpm-lock.yaml', () => { const packageManager = PackageManager.fromLockFile('/path/to/pnpm-lock.yaml'); expect(packageManager).toEqual(PackageManager.PNPM); - expect(packageManager.runBinCommand('my-bin')).toBe('pnpm run my-bin'); + expect(packageManager.runBinCommand('my-bin')).toBe('pnpm exec my-bin --'); }); test('defaults to NPM', () => { From f66f4b80da22b4d24d4419acc3984b56d5690b2e Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Thu, 3 Jun 2021 12:46:52 +0100 Subject: [PATCH 2/6] feat(cli): new bootstrap supports cross-account lookups (#14874) Fixes #8905 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../lib/cloud-assembly/context-queries.ts | 64 ++++++++++++++++++ .../schema/cloud-assembly.schema.json | 36 ++++++++++ .../schema/cloud-assembly.version.json | 2 +- .../stack-synthesizers/default-synthesizer.ts | 20 +++++- packages/@aws-cdk/core/lib/stack.ts | 8 ++- packages/@aws-cdk/core/test/app.test.ts | 6 +- .../new-style-synthesis.test.ts | 25 ++++++- packages/@aws-cdk/pipelines/README.md | 15 +++++ packages/aws-cdk/bin/cdk.ts | 2 + packages/aws-cdk/lib/api/aws-auth/sdk.ts | 3 +- .../api/bootstrap/bootstrap-environment.ts | 6 +- .../lib/api/bootstrap/bootstrap-props.ts | 7 ++ .../lib/api/bootstrap/bootstrap-template.yaml | 65 ++++++++++++++++++- packages/aws-cdk/lib/context-providers/ami.ts | 3 +- .../context-providers/availability-zones.ts | 3 +- .../endpoint-service-availability-zones.ts | 5 +- .../lib/context-providers/hosted-zones.ts | 3 +- .../aws-cdk/lib/context-providers/index.ts | 2 +- .../lib/context-providers/load-balancers.ts | 6 +- .../lib/context-providers/security-groups.ts | 3 +- .../lib/context-providers/ssm-parameters.ts | 9 ++- .../aws-cdk/lib/context-providers/vpcs.ts | 3 +- packages/aws-cdk/test/api/bootstrap2.test.ts | 15 +++++ .../aws-cdk/test/api/sdk-provider.test.ts | 2 +- 24 files changed, 289 insertions(+), 24 deletions(-) diff --git a/packages/@aws-cdk/cloud-assembly-schema/lib/cloud-assembly/context-queries.ts b/packages/@aws-cdk/cloud-assembly-schema/lib/cloud-assembly/context-queries.ts index 56c609a08dd39..a9e5df9bd1d0b 100644 --- a/packages/@aws-cdk/cloud-assembly-schema/lib/cloud-assembly/context-queries.ts +++ b/packages/@aws-cdk/cloud-assembly-schema/lib/cloud-assembly/context-queries.ts @@ -64,6 +64,13 @@ export interface AmiContextQuery { */ readonly region: string; + /** + * The ARN of the role that should be used to look up the missing values + * + * @default - None + */ + readonly lookupRoleArn?: string; + /** * Owners to DescribeImages call * @@ -90,6 +97,14 @@ export interface AvailabilityZonesContextQuery { * Query region */ readonly region: string; + + /** + * The ARN of the role that should be used to look up the missing values + * + * @default - None + */ + readonly lookupRoleArn?: string; + } /** @@ -106,6 +121,13 @@ export interface HostedZoneContextQuery { */ readonly region: string; + /** + * The ARN of the role that should be used to look up the missing values + * + * @default - None + */ + readonly lookupRoleArn?: string; + /** * The domain name e.g. example.com to lookup */ @@ -143,6 +165,13 @@ export interface SSMParameterContextQuery { */ readonly region: string; + /** + * The ARN of the role that should be used to look up the missing values + * + * @default - None + */ + readonly lookupRoleArn?: string; + /** * Parameter name to query */ @@ -163,6 +192,13 @@ export interface VpcContextQuery { */ readonly region: string; + /** + * The ARN of the role that should be used to look up the missing values + * + * @default - None + */ + readonly lookupRoleArn?: string; + /** * Filters to apply to the VPC * @@ -205,6 +241,13 @@ export interface EndpointServiceAvailabilityZonesContextQuery { */ readonly region: string; + /** + * The ARN of the role that should be used to look up the missing values + * + * @default - None + */ + readonly lookupRoleArn?: string; + /** * Query service name */ @@ -261,6 +304,13 @@ export interface LoadBalancerContextQuery extends LoadBalancerFilter { * Query region */ readonly region: string; + + /** + * The ARN of the role that should be used to look up the missing values + * + * @default - None + */ + readonly lookupRoleArn?: string; } /** @@ -312,6 +362,13 @@ export interface LoadBalancerListenerContextQuery extends LoadBalancerFilter { */ readonly region: string; + /** + * The ARN of the role that should be used to look up the missing values + * + * @default - None + */ + readonly lookupRoleArn?: string; + /** * Find by listener's arn * @default - does not find by listener arn @@ -345,6 +402,13 @@ export interface SecurityGroupContextQuery { */ readonly region: string; + /** + * The ARN of the role that should be used to look up the missing values + * + * @default - None + */ + readonly lookupRoleArn?: string; + /** * Security group id */ diff --git a/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.schema.json b/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.schema.json index 77d3117d0aae2..3c0f38f598570 100644 --- a/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.schema.json +++ b/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.schema.json @@ -453,6 +453,10 @@ "description": "Region to query", "type": "string" }, + "lookupRoleArn": { + "description": "The ARN of the role that should be used to look up the missing values (Default - None)", + "type": "string" + }, "owners": { "description": "Owners to DescribeImages call (Default - All owners)", "type": "array", @@ -488,6 +492,10 @@ "region": { "description": "Query region", "type": "string" + }, + "lookupRoleArn": { + "description": "The ARN of the role that should be used to look up the missing values (Default - None)", + "type": "string" } }, "required": [ @@ -507,6 +515,10 @@ "description": "Query region", "type": "string" }, + "lookupRoleArn": { + "description": "The ARN of the role that should be used to look up the missing values (Default - None)", + "type": "string" + }, "domainName": { "description": "The domain name e.g. example.com to lookup", "type": "string" @@ -539,6 +551,10 @@ "description": "Query region", "type": "string" }, + "lookupRoleArn": { + "description": "The ARN of the role that should be used to look up the missing values (Default - None)", + "type": "string" + }, "parameterName": { "description": "Parameter name to query", "type": "string" @@ -562,6 +578,10 @@ "description": "Query region", "type": "string" }, + "lookupRoleArn": { + "description": "The ARN of the role that should be used to look up the missing values (Default - None)", + "type": "string" + }, "filter": { "description": "Filters to apply to the VPC\n\nFilter parameters are the same as passed to DescribeVpcs.", "type": "object", @@ -597,6 +617,10 @@ "description": "Query region", "type": "string" }, + "lookupRoleArn": { + "description": "The ARN of the role that should be used to look up the missing values (Default - None)", + "type": "string" + }, "serviceName": { "description": "Query service name", "type": "string" @@ -620,6 +644,10 @@ "description": "Query region", "type": "string" }, + "lookupRoleArn": { + "description": "The ARN of the role that should be used to look up the missing values (Default - None)", + "type": "string" + }, "loadBalancerType": { "$ref": "#/definitions/LoadBalancerType", "description": "Filter load balancers by their type" @@ -662,6 +690,10 @@ "description": "Query region", "type": "string" }, + "lookupRoleArn": { + "description": "The ARN of the role that should be used to look up the missing values (Default - None)", + "type": "string" + }, "listenerArn": { "description": "Find by listener's arn (Default - does not find by listener arn)", "type": "string" @@ -716,6 +748,10 @@ "description": "Query region", "type": "string" }, + "lookupRoleArn": { + "description": "The ARN of the role that should be used to look up the missing values (Default - None)", + "type": "string" + }, "securityGroupId": { "description": "Security group id", "type": "string" diff --git a/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.version.json b/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.version.json index b056ff69e87b5..42c883f995fd4 100644 --- a/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.version.json +++ b/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.version.json @@ -1 +1 @@ -{"version":"11.0.0"} \ No newline at end of file +{"version":"12.0.0"} \ No newline at end of file diff --git a/packages/@aws-cdk/core/lib/stack-synthesizers/default-synthesizer.ts b/packages/@aws-cdk/core/lib/stack-synthesizers/default-synthesizer.ts index c902e3326d44b..dba94194d6338 100644 --- a/packages/@aws-cdk/core/lib/stack-synthesizers/default-synthesizer.ts +++ b/packages/@aws-cdk/core/lib/stack-synthesizers/default-synthesizer.ts @@ -84,6 +84,13 @@ export interface DefaultStackSynthesizerProps { */ readonly imageAssetPublishingRoleArn?: string; + /** + * The role to use to look up values from the target AWS account during synthesis + * + * @default - None + */ + readonly lookupRoleArn?: string; + /** * External ID to use when assuming role for image asset publishing * @@ -195,6 +202,11 @@ export class DefaultStackSynthesizer extends StackSynthesizer { */ public static readonly DEFAULT_IMAGE_ASSET_PUBLISHING_ROLE_ARN = 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-${Qualifier}-image-publishing-role-${AWS::AccountId}-${AWS::Region}'; + /** + * Default lookup role ARN for missing values. + */ + public static readonly DEFAULT_LOOKUP_ROLE_ARN = 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-${Qualifier}-lookup-role-${AWS::AccountId}-${AWS::Region}'; + /** * Default image assets repository name */ @@ -222,8 +234,9 @@ export class DefaultStackSynthesizer extends StackSynthesizer { private _cloudFormationExecutionRoleArn?: string; private fileAssetPublishingRoleArn?: string; private imageAssetPublishingRoleArn?: string; + private lookupRoleArn?: string; private qualifier?: string; - private bucketPrefix?: string + private bucketPrefix?: string; private readonly files: NonNullable = {}; private readonly dockerImages: NonNullable = {}; @@ -282,6 +295,7 @@ export class DefaultStackSynthesizer extends StackSynthesizer { this._cloudFormationExecutionRoleArn = specialize(this.props.cloudFormationExecutionRole ?? DefaultStackSynthesizer.DEFAULT_CLOUDFORMATION_ROLE_ARN); this.fileAssetPublishingRoleArn = specialize(this.props.fileAssetPublishingRoleArn ?? DefaultStackSynthesizer.DEFAULT_FILE_ASSET_PUBLISHING_ROLE_ARN); this.imageAssetPublishingRoleArn = specialize(this.props.imageAssetPublishingRoleArn ?? DefaultStackSynthesizer.DEFAULT_IMAGE_ASSET_PUBLISHING_ROLE_ARN); + this.lookupRoleArn = specialize(this.props.lookupRoleArn ?? DefaultStackSynthesizer.DEFAULT_LOOKUP_ROLE_ARN); this.bucketPrefix = specialize(this.props.bucketPrefix ?? DefaultStackSynthesizer.DEFAULT_FILE_ASSET_PREFIX); /* eslint-enable max-len */ } @@ -362,6 +376,10 @@ export class DefaultStackSynthesizer extends StackSynthesizer { }; } + protected synthesizeStackTemplate(stack: Stack, session: ISynthesisSession): void { + stack._synthesizeTemplate(session, this.lookupRoleArn); + } + /** * Synthesize the associated stack to the session */ diff --git a/packages/@aws-cdk/core/lib/stack.ts b/packages/@aws-cdk/core/lib/stack.ts index e256f73231714..b32095233b80d 100644 --- a/packages/@aws-cdk/core/lib/stack.ts +++ b/packages/@aws-cdk/core/lib/stack.ts @@ -750,7 +750,7 @@ export class Stack extends CoreConstruct implements ITaggable { * Synthesizes the cloudformation template into a cloud assembly. * @internal */ - public _synthesizeTemplate(session: ISynthesisSession): void { + public _synthesizeTemplate(session: ISynthesisSession, lookupRoleArn?: string): void { // In principle, stack synthesis is delegated to the // StackSynthesis object. // @@ -777,7 +777,11 @@ export class Stack extends CoreConstruct implements ITaggable { fs.writeFileSync(outPath, JSON.stringify(template, undefined, 2)); for (const ctx of this._missingContext) { - builder.addMissing(ctx); + if (lookupRoleArn != null) { + builder.addMissing({ ...ctx, props: { ...ctx.props, lookupRoleArn } }); + } else { + builder.addMissing(ctx); + } } } diff --git a/packages/@aws-cdk/core/test/app.test.ts b/packages/@aws-cdk/core/test/app.test.ts index 199b36dc87465..3bb178edef56a 100644 --- a/packages/@aws-cdk/core/test/app.test.ts +++ b/packages/@aws-cdk/core/test/app.test.ts @@ -1,7 +1,7 @@ import { ContextProvider } from '@aws-cdk/cloud-assembly-schema'; import * as cxapi from '@aws-cdk/cx-api'; import { nodeunitShim, Test } from 'nodeunit-shim'; -import { CfnResource, Construct, Stack, StackProps } from '../lib'; +import { CfnResource, Construct, DefaultStackSynthesizer, Stack, StackProps } from '../lib'; import { Annotations } from '../lib/annotations'; import { App, AppProps } from '../lib/app'; @@ -219,7 +219,7 @@ nodeunitShim({ } const assembly = withApp({}, app => { - new MyStack(app, 'MyStack'); + new MyStack(app, 'MyStack', { synthesizer: new DefaultStackSynthesizer() }); }); test.deepEqual(assembly.manifest.missing, [ @@ -227,6 +227,7 @@ nodeunitShim({ key: 'missing-context-key', provider: ContextProvider.AVAILABILITY_ZONE_PROVIDER, props: { + lookupRoleArn: 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-lookup-role-${AWS::AccountId}-${AWS::Region}', account: '12345689012', region: 'ab-north-1', }, @@ -235,6 +236,7 @@ nodeunitShim({ key: 'missing-context-key-2', provider: ContextProvider.AVAILABILITY_ZONE_PROVIDER, props: { + lookupRoleArn: 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-lookup-role-${AWS::AccountId}-${AWS::Region}', account: '12345689012', region: 'ab-south-1', }, diff --git a/packages/@aws-cdk/core/test/stack-synthesis/new-style-synthesis.test.ts b/packages/@aws-cdk/core/test/stack-synthesis/new-style-synthesis.test.ts index 73f8f185f06ba..5505f6ca9e8fd 100644 --- a/packages/@aws-cdk/core/test/stack-synthesis/new-style-synthesis.test.ts +++ b/packages/@aws-cdk/core/test/stack-synthesis/new-style-synthesis.test.ts @@ -2,7 +2,7 @@ import * as fs from 'fs'; import * as cxschema from '@aws-cdk/cloud-assembly-schema'; import * as cxapi from '@aws-cdk/cx-api'; import { nodeunitShim, Test } from 'nodeunit-shim'; -import { App, Aws, CfnResource, DefaultStackSynthesizer, FileAssetPackaging, Stack } from '../../lib'; +import { App, Aws, CfnResource, ContextProvider, DefaultStackSynthesizer, FileAssetPackaging, Stack } from '../../lib'; import { evaluateCFN } from '../evaluate-cfn'; const CFN_CONTEXT = { @@ -101,6 +101,29 @@ nodeunitShim({ test.done(); }, + 'generates missing context with the lookup role ARN as one of the missing context properties'(test: Test) { + // GIVEN + stack = new Stack(app, 'Stack2', { + synthesizer: new DefaultStackSynthesizer({ + generateBootstrapVersionRule: false, + }), + env: { + account: '111111111111', region: 'us-east-1', + }, + }); + ContextProvider.getValue(stack, { + provider: cxschema.ContextProvider.VPC_PROVIDER, + props: {}, + dummyValue: undefined, + }).value; + + // THEN + const assembly = app.synth(); + test.equal(assembly.manifest.missing![0].props.lookupRoleArn, 'arn:${AWS::Partition}:iam::111111111111:role/cdk-hnb659fds-lookup-role-111111111111-us-east-1'); + + test.done(); + }, + 'add file asset'(test: Test) { // WHEN const location = stack.synthesizer.addFileAsset({ diff --git a/packages/@aws-cdk/pipelines/README.md b/packages/@aws-cdk/pipelines/README.md index 82aded1d69b4b..a99dba1630f3c 100644 --- a/packages/@aws-cdk/pipelines/README.md +++ b/packages/@aws-cdk/pipelines/README.md @@ -632,6 +632,17 @@ $ env CDK_NEW_BOOTSTRAP=1 npx cdk bootstrap \ aws://222222222222/us-east-2 ``` +If you only want to trust an account to do lookups (e.g, when your CDK application has a +`Vpc.fromLookup()` call), use the option `--trust-for-lookup`: + +```console +$ env CDK_NEW_BOOTSTRAP=1 npx cdk bootstrap \ + [--profile admin-profile-2] \ + --cloudformation-execution-policies arn:aws:iam::aws:policy/AdministratorAccess \ + --trust-for-lookup 11111111111 \ + aws://222222222222/us-east-2 +``` + These command lines explained: * `npx`: means to use the CDK CLI from the current NPM install. If you are using @@ -647,6 +658,10 @@ These command lines explained: CDK applications into this account. In this case we indicate the Pipeline's account, but you could also use this for developer accounts (don't do that for production application accounts though!). +* `--trust-for-lookup`: similar to `--trust`, but gives a more limited set of permissions to the + trusted account, allowing it to only look up values, such as availability zones, EC2 images and + VPCs. Note that if you provide an account using `--trust`, that account can also do lookups. + So you only need to pass `--trust-for-lookup` if you need to use a different account. * `aws://222222222222/us-east-2`: the account and region we're bootstrapping. > **Security tip**: we recommend that you use administrative credentials to an diff --git a/packages/aws-cdk/bin/cdk.ts b/packages/aws-cdk/bin/cdk.ts index 7c69bcb43a3f3..b36c01e10969b 100644 --- a/packages/aws-cdk/bin/cdk.ts +++ b/packages/aws-cdk/bin/cdk.ts @@ -79,6 +79,7 @@ async function parseCommandLineArguments() { .option('tags', { type: 'array', alias: 't', desc: 'Tags to add for the stack (KEY=VALUE)', nargs: 1, requiresArg: true, default: [] }) .option('execute', { type: 'boolean', desc: 'Whether to execute ChangeSet (--no-execute will NOT execute the ChangeSet)', default: true }) .option('trust', { type: 'array', desc: 'The AWS account IDs that should be trusted to perform deployments into this environment (may be repeated, modern bootstrapping only)', default: [], nargs: 1, requiresArg: true }) + .option('trust-for-lookup', { type: 'array', desc: 'The AWS account IDs that should be trusted to look up values in this environment (may be repeated, modern bootstrapping only)', default: [], nargs: 1, requiresArg: true }) .option('cloudformation-execution-policies', { type: 'array', desc: 'The Managed Policy ARNs that should be attached to the role performing deployments into this environment (may be repeated, modern bootstrapping only)', default: [], nargs: 1, requiresArg: true }) .option('force', { alias: 'f', type: 'boolean', desc: 'Always bootstrap even if it would downgrade template version', default: false }) .option('termination-protection', { type: 'boolean', default: undefined, desc: 'Toggle CloudFormation termination protection on the bootstrap stacks' }) @@ -279,6 +280,7 @@ async function initCommandLine() { qualifier: args.qualifier, publicAccessBlockConfiguration: args.publicAccessBlockConfiguration, trustedAccounts: arrayFromYargs(args.trust), + trustedAccountsForLookup: arrayFromYargs(args.trustForLookup), cloudFormationExecutionPolicies: arrayFromYargs(args.cloudformationExecutionPolicies), }, }); diff --git a/packages/aws-cdk/lib/api/aws-auth/sdk.ts b/packages/aws-cdk/lib/api/aws-auth/sdk.ts index d5787a258275d..888901a8c33bf 100644 --- a/packages/aws-cdk/lib/api/aws-auth/sdk.ts +++ b/packages/aws-cdk/lib/api/aws-auth/sdk.ts @@ -158,8 +158,9 @@ export class SDK implements ISDK { ...this.sdkOptions.assumeRoleCredentialsSourceDescription ? [`using ${this.sdkOptions.assumeRoleCredentialsSourceDescription}`] : [], - '(did you bootstrap the environment with the right \'--trust\'s?):', e.message, + '. Please make sure that this role exists in the account. If it doesn\'t exist, (re)-bootstrap the environment ' + + 'with the right \'--trust\', using the latest version of the CDK CLI.', ].join(' ')); } } diff --git a/packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts b/packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts index 5350524b0dae3..7877368710c5f 100644 --- a/packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts +++ b/packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts @@ -92,7 +92,10 @@ export class Bootstrapper { // templates doesn't seem to be able to express the conditions that we need // (can't use Fn::Join or reference Conditions) so we do it here instead. const trustedAccounts = params.trustedAccounts ?? splitCfnArray(current.parameters.TrustedAccounts); - info(`Trusted accounts: ${trustedAccounts.length > 0 ? trustedAccounts.join(', ') : '(none)'}`); + info(`Trusted accounts for deployment: ${trustedAccounts.length > 0 ? trustedAccounts.join(', ') : '(none)'}`); + + const trustedAccountsForLookup = params.trustedAccountsForLookup ?? splitCfnArray(current.parameters.TrustedAccountsForLookup); + info(`Trusted accounts for lookup: ${trustedAccountsForLookup.length > 0 ? trustedAccountsForLookup.join(', ') : '(none)'}`); const cloudFormationExecutionPolicies = params.cloudFormationExecutionPolicies ?? splitCfnArray(current.parameters.CloudFormationExecutionPolicies); if (trustedAccounts.length === 0 && cloudFormationExecutionPolicies.length === 0) { @@ -137,6 +140,7 @@ export class Bootstrapper { FileAssetsBucketKmsKeyId: kmsKeyId, // Empty array becomes empty string TrustedAccounts: trustedAccounts.join(','), + TrustedAccountsForLookup: trustedAccountsForLookup.join(','), CloudFormationExecutionPolicies: cloudFormationExecutionPolicies.join(','), Qualifier: params.qualifier, PublicAccessBlockConfiguration: params.publicAccessBlockConfiguration || params.publicAccessBlockConfiguration === undefined ? 'true' : 'false', diff --git a/packages/aws-cdk/lib/api/bootstrap/bootstrap-props.ts b/packages/aws-cdk/lib/api/bootstrap/bootstrap-props.ts index dd36739d4381d..10fc4fc8ff598 100644 --- a/packages/aws-cdk/lib/api/bootstrap/bootstrap-props.ts +++ b/packages/aws-cdk/lib/api/bootstrap/bootstrap-props.ts @@ -71,6 +71,13 @@ export interface BootstrappingParameters { */ readonly trustedAccounts?: string[]; + /** + * The list of AWS account IDs that are trusted to look up values in the environment being bootstrapped. + * + * @default - only the bootstrapped account can look up values in this environment + */ + readonly trustedAccountsForLookup?: string[]; + /** * The ARNs of the IAM managed policies that should be attached to the role performing CloudFormation deployments. * In most cases, this will be the AdministratorAccess policy. diff --git a/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml b/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml index b541401f930e7..6fb29fbbb6d5b 100644 --- a/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml +++ b/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml @@ -6,6 +6,11 @@ Parameters: stacks to this environment Default: '' Type: CommaDelimitedList + TrustedAccountsForLookup: + Description: List of AWS accounts that are trusted to look up values in this + environment + Default: '' + Type: CommaDelimitedList CloudFormationExecutionPolicies: Description: List of the ManagedPolicy ARN(s) to attach to the CloudFormation deployment role @@ -45,6 +50,13 @@ Conditions: - Fn::Join: - '' - Ref: TrustedAccounts + HasTrustedAccountsForLookup: + Fn::Not: + - Fn::Equals: + - '' + - Fn::Join: + - '' + - Ref: TrustedAccountsForLookup HasCloudFormationExecutionPolicies: Fn::Not: - Fn::Equals: @@ -233,6 +245,57 @@ Resources: - Ref: AWS::NoValue RoleName: Fn::Sub: cdk-${Qualifier}-image-publishing-role-${AWS::AccountId}-${AWS::Region} + LookupRole: + Type: AWS::IAM::Role + Properties: + AssumeRolePolicyDocument: + Statement: + - Action: sts:AssumeRole + Effect: Allow + Principal: + AWS: + Ref: AWS::AccountId + - Fn::If: + - HasTrustedAccountsForLookup + - Action: sts:AssumeRole + Effect: Allow + Principal: + AWS: + Ref: TrustedAccountsForLookup + - Ref: AWS::NoValue + - Fn::If: + - HasTrustedAccounts + - Action: sts:AssumeRole + Effect: Allow + Principal: + AWS: + Ref: TrustedAccounts + - Ref: AWS::NoValue + RoleName: + Fn::Sub: cdk-${Qualifier}-lookup-role-${AWS::AccountId}-${AWS::Region} + Policies: + - PolicyDocument: + Statement: + - Action: + - ec2:DescribeVpcs + - ec2:DescribeAvailabilityZones + - ec2:DescribeSubnets + - ec2:DescribeRouteTables + - ec2:DescribeVpnGateways + - ec2:DescribeImages + - ec2:DescribeVpcEndpointServices + - ec2:DescribeSecurityGroups + - elasticloadbalancing:DescribeLoadBalancers + - elasticloadbalancing:DescribeTags + - elasticloadbalancing:DescribeListeners + - route53:ListHostedZonesByName + - route53:GetHostedZone + - ssm:GetParameter + Resource: "*" + Effect: Allow + Version: '2012-10-17' + PolicyName: + Fn::Sub: cdk-${Qualifier}-lookup-role-default-policy-${AWS::AccountId}-${AWS::Region} FilePublishingRoleDefaultPolicy: Type: AWS::IAM::Policy Properties: @@ -393,7 +456,7 @@ Resources: Type: String Name: Fn::Sub: '/cdk-bootstrap/${Qualifier}/version' - Value: '5' + Value: '6' Outputs: BucketName: Description: The name of the S3 bucket owned by the CDK toolkit stack diff --git a/packages/aws-cdk/lib/context-providers/ami.ts b/packages/aws-cdk/lib/context-providers/ami.ts index 050afdb0a1b5f..f49bbef114175 100644 --- a/packages/aws-cdk/lib/context-providers/ami.ts +++ b/packages/aws-cdk/lib/context-providers/ami.ts @@ -20,7 +20,8 @@ export class AmiContextProviderPlugin implements ContextProviderPlugin { print(`Searching for AMI in ${account}:${region}`); debug(`AMI search parameters: ${JSON.stringify(args)}`); - const ec2 = (await this.aws.forEnvironment(cxapi.EnvironmentUtils.make(account, region), Mode.ForReading)).ec2(); + const options = { assumeRoleArn: args.lookupRoleArn }; + const ec2 = (await this.aws.forEnvironment(cxapi.EnvironmentUtils.make(account, region), Mode.ForReading, options)).ec2(); const response = await ec2.describeImages({ Owners: args.owners, Filters: Object.entries(args.filters).map(([key, values]) => ({ diff --git a/packages/aws-cdk/lib/context-providers/availability-zones.ts b/packages/aws-cdk/lib/context-providers/availability-zones.ts index beaa651cf22ba..1f3d656b17c11 100644 --- a/packages/aws-cdk/lib/context-providers/availability-zones.ts +++ b/packages/aws-cdk/lib/context-providers/availability-zones.ts @@ -15,7 +15,8 @@ export class AZContextProviderPlugin implements ContextProviderPlugin { const region = args.region; const account = args.account; debug(`Reading AZs for ${account}:${region}`); - const ec2 = (await this.aws.forEnvironment(cxapi.EnvironmentUtils.make(account, region), Mode.ForReading)).ec2(); + const options = { assumeRoleArn: args.lookupRoleArn }; + const ec2 = (await this.aws.forEnvironment(cxapi.EnvironmentUtils.make(account, region), Mode.ForReading, options)).ec2(); const response = await ec2.describeAvailabilityZones().promise(); if (!response.AvailabilityZones) { return []; } const azs = response.AvailabilityZones.filter(zone => zone.State === 'available').map(zone => zone.ZoneName); diff --git a/packages/aws-cdk/lib/context-providers/endpoint-service-availability-zones.ts b/packages/aws-cdk/lib/context-providers/endpoint-service-availability-zones.ts index f9099154e2407..68147799ff517 100644 --- a/packages/aws-cdk/lib/context-providers/endpoint-service-availability-zones.ts +++ b/packages/aws-cdk/lib/context-providers/endpoint-service-availability-zones.ts @@ -10,12 +10,13 @@ export class EndpointServiceAZContextProviderPlugin implements ContextProviderPl constructor(private readonly aws: SdkProvider) { } - public async getValue(args: {[key: string]: any}) { + public async getValue(args: { [key: string]: any }) { const region = args.region; const account = args.account; const serviceName = args.serviceName; debug(`Reading AZs for ${account}:${region}:${serviceName}`); - const ec2 = (await this.aws.forEnvironment(cxapi.EnvironmentUtils.make(account, region), Mode.ForReading)).ec2(); + const options = { assumeRoleArn: args.lookupRoleArn }; + const ec2 = (await this.aws.forEnvironment(cxapi.EnvironmentUtils.make(account, region), Mode.ForReading, options)).ec2(); const response = await ec2.describeVpcEndpointServices({ ServiceNames: [serviceName] }).promise(); // expect a service in the response diff --git a/packages/aws-cdk/lib/context-providers/hosted-zones.ts b/packages/aws-cdk/lib/context-providers/hosted-zones.ts index 8d959c5c1e142..909475eaccc06 100644 --- a/packages/aws-cdk/lib/context-providers/hosted-zones.ts +++ b/packages/aws-cdk/lib/context-providers/hosted-zones.ts @@ -17,7 +17,8 @@ export class HostedZoneContextProviderPlugin implements ContextProviderPlugin { } const domainName = args.domainName; debug(`Reading hosted zone ${account}:${region}:${domainName}`); - const r53 = (await this.aws.forEnvironment(cxapi.EnvironmentUtils.make(account, region), Mode.ForReading)).route53(); + const options = { assumeRoleArn: args.lookupRoleArn }; + const r53 = (await this.aws.forEnvironment(cxapi.EnvironmentUtils.make(account, region), Mode.ForReading, options)).route53(); const response = await r53.listHostedZonesByName({ DNSName: domainName }).promise(); if (!response.HostedZones) { throw new Error(`Hosted Zone not found in account ${account}, region ${region}: ${domainName}`); diff --git a/packages/aws-cdk/lib/context-providers/index.ts b/packages/aws-cdk/lib/context-providers/index.ts index e60ad6066d280..a87183d3b4e46 100644 --- a/packages/aws-cdk/lib/context-providers/index.ts +++ b/packages/aws-cdk/lib/context-providers/index.ts @@ -13,7 +13,7 @@ import { SecurityGroupContextProviderPlugin } from './security-groups'; import { SSMContextProviderPlugin } from './ssm-parameters'; import { VpcNetworkContextProviderPlugin } from './vpcs'; -type ProviderConstructor = (new (sdk: SdkProvider) => ContextProviderPlugin); +type ProviderConstructor = (new (sdk: SdkProvider, lookupRoleArn?: string) => ContextProviderPlugin); export type ProviderMap = {[name: string]: ProviderConstructor}; /** diff --git a/packages/aws-cdk/lib/context-providers/load-balancers.ts b/packages/aws-cdk/lib/context-providers/load-balancers.ts index 26f00c7746fe3..a36e29c0bec58 100644 --- a/packages/aws-cdk/lib/context-providers/load-balancers.ts +++ b/packages/aws-cdk/lib/context-providers/load-balancers.ts @@ -12,7 +12,8 @@ export class LoadBalancerContextProviderPlugin implements ContextProviderPlugin } async getValue(query: cxschema.LoadBalancerContextQuery): Promise { - const elbv2 = (await this.aws.forEnvironment(cxapi.EnvironmentUtils.make(query.account, query.region), Mode.ForReading)).elbv2(); + const options = { assumeRoleArn: query.lookupRoleArn }; + const elbv2 = (await this.aws.forEnvironment(cxapi.EnvironmentUtils.make(query.account, query.region), Mode.ForReading, options)).elbv2(); if (!query.loadBalancerArn && !query.loadBalancerTags) { throw new Error('The load balancer lookup query must specify either `loadBalancerArn` or `loadBalancerTags`'); @@ -57,7 +58,8 @@ export class LoadBalancerListenerContextProviderPlugin implements ContextProvide } async getValue(query: LoadBalancerListenerQuery): Promise { - const elbv2 = (await this.aws.forEnvironment(cxapi.EnvironmentUtils.make(query.account, query.region), Mode.ForReading)).elbv2(); + const options = { assumeRoleArn: query.lookupRoleArn }; + const elbv2 = (await this.aws.forEnvironment(cxapi.EnvironmentUtils.make(query.account, query.region), Mode.ForReading, options)).elbv2(); if (!query.listenerArn && !query.loadBalancerArn && !query.loadBalancerTags) { throw new Error('The load balancer listener query must specify at least one of: `listenerArn`, `loadBalancerArn` or `loadBalancerTags`'); diff --git a/packages/aws-cdk/lib/context-providers/security-groups.ts b/packages/aws-cdk/lib/context-providers/security-groups.ts index e8f464128b68d..7edde696fba45 100644 --- a/packages/aws-cdk/lib/context-providers/security-groups.ts +++ b/packages/aws-cdk/lib/context-providers/security-groups.ts @@ -12,7 +12,8 @@ export class SecurityGroupContextProviderPlugin implements ContextProviderPlugin const account: string = args.account!; const region: string = args.region!; - const ec2 = (await this.aws.forEnvironment(cxapi.EnvironmentUtils.make(account, region), Mode.ForReading)).ec2(); + const options = { assumeRoleArn: args.lookupRoleArn }; + const ec2 = (await this.aws.forEnvironment(cxapi.EnvironmentUtils.make(account, region), Mode.ForReading, options)).ec2(); const response = await ec2.describeSecurityGroups({ GroupIds: [args.securityGroupId], diff --git a/packages/aws-cdk/lib/context-providers/ssm-parameters.ts b/packages/aws-cdk/lib/context-providers/ssm-parameters.ts index 79558a89ff4bf..150f4f14dad40 100644 --- a/packages/aws-cdk/lib/context-providers/ssm-parameters.ts +++ b/packages/aws-cdk/lib/context-providers/ssm-parameters.ts @@ -21,7 +21,7 @@ export class SSMContextProviderPlugin implements ContextProviderPlugin { const parameterName = args.parameterName; debug(`Reading SSM parameter ${account}:${region}:${parameterName}`); - const response = await this.getSsmParameterValue(account, region, parameterName); + const response = await this.getSsmParameterValue(account, region, parameterName, args.lookupRoleArn); if (!response.Parameter || response.Parameter.Value === undefined) { throw new Error(`SSM parameter not available in account ${account}, region ${region}: ${parameterName}`); } @@ -33,13 +33,16 @@ export class SSMContextProviderPlugin implements ContextProviderPlugin { * @param account the account in which the SSM Parameter is expected to be. * @param region the region in which the SSM Parameter is expected to be. * @param parameterName the name of the SSM Parameter + * @param lookupRoleArn the ARN of the lookup role. * * @returns the result of the ``GetParameter`` operation. * * @throws Error if a service error (other than ``ParameterNotFound``) occurs. */ - private async getSsmParameterValue(account: string, region: string, parameterName: string): Promise { - const ssm = (await this.aws.forEnvironment(cxapi.EnvironmentUtils.make(account, region), Mode.ForReading)).ssm(); + private async getSsmParameterValue(account: string, region: string, parameterName: string, lookupRoleArn?: string) + : Promise { + const options = { assumeRoleArn: lookupRoleArn }; + const ssm = (await this.aws.forEnvironment(cxapi.EnvironmentUtils.make(account, region), Mode.ForReading, options)).ssm(); try { return await ssm.getParameter({ Name: parameterName }).promise(); } catch (e) { diff --git a/packages/aws-cdk/lib/context-providers/vpcs.ts b/packages/aws-cdk/lib/context-providers/vpcs.ts index 33e2e09ff9499..9b37eadee7aef 100644 --- a/packages/aws-cdk/lib/context-providers/vpcs.ts +++ b/packages/aws-cdk/lib/context-providers/vpcs.ts @@ -14,7 +14,8 @@ export class VpcNetworkContextProviderPlugin implements ContextProviderPlugin { const account: string = args.account!; const region: string = args.region!; - const ec2 = (await this.aws.forEnvironment(cxapi.EnvironmentUtils.make(account, region), Mode.ForReading)).ec2(); + const options = { assumeRoleArn: args.lookupRoleArn }; + const ec2 = (await this.aws.forEnvironment(cxapi.EnvironmentUtils.make(account, region), Mode.ForReading, options)).ec2(); const vpcId = await this.findVpc(ec2, args); diff --git a/packages/aws-cdk/test/api/bootstrap2.test.ts b/packages/aws-cdk/test/api/bootstrap2.test.ts index 639f0a6d759bc..bf6f6e836a79a 100644 --- a/packages/aws-cdk/test/api/bootstrap2.test.ts +++ b/packages/aws-cdk/test/api/bootstrap2.test.ts @@ -123,6 +123,21 @@ describe('Bootstrapping v2', () => { })); }); + test('passing trusted accounts for lookup generates the correct stack parameter', async () => { + await bootstrapper.bootstrapEnvironment(env, sdk, { + parameters: { + trustedAccountsForLookup: ['123456789012'], + cloudFormationExecutionPolicies: ['aws://foo'], + }, + }); + + expect(mockDeployStack).toHaveBeenCalledWith(expect.objectContaining({ + parameters: expect.objectContaining({ + TrustedAccountsForLookup: '123456789012', + }), + })); + }); + test('allow adding trusted account if there was already a policy on the stack', async () => { // GIVEN mockTheToolkitInfo({ diff --git a/packages/aws-cdk/test/api/sdk-provider.test.ts b/packages/aws-cdk/test/api/sdk-provider.test.ts index c7dfbbcfeb45c..0e9f0bd01e366 100644 --- a/packages/aws-cdk/test/api/sdk-provider.test.ts +++ b/packages/aws-cdk/test/api/sdk-provider.test.ts @@ -312,7 +312,7 @@ describe('with intercepted network calls', () => { }); // THEN - error message contains both a helpful hint and the underlying AssumeRole message - await expect(promise).rejects.toThrow('did you bootstrap'); + await expect(promise).rejects.toThrow('(re)-bootstrap the environment'); await expect(promise).rejects.toThrow('doesnotexist.role.arn'); }); From 25412a60971d3e332fa22fad4c44122eef9dfd2c Mon Sep 17 00:00:00 2001 From: Kyle Roach Date: Thu, 3 Jun 2021 12:20:12 +0000 Subject: [PATCH 3/6] fix(apigatewayv2): http api - default route does not use the default authorizer (#14904) The default authorizer worked by passing the authorizer config to routes in the api by the addRoutes method. We completely forgot about the use case of the default integration, so currently using default integration + default authorizer does not create an authorizer. This PR fixes the bug and allows using default authorizer + default integration as expected. Reported by https://github.com/aws/aws-cdk/issues/10534#issuecomment-837895317 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../@aws-cdk/aws-apigatewayv2/lib/http/api.ts | 2 ++ .../aws-apigatewayv2/test/http/api.test.ts | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/packages/@aws-cdk/aws-apigatewayv2/lib/http/api.ts b/packages/@aws-cdk/aws-apigatewayv2/lib/http/api.ts index 73abc83c16111..f650d62bd289b 100644 --- a/packages/@aws-cdk/aws-apigatewayv2/lib/http/api.ts +++ b/packages/@aws-cdk/aws-apigatewayv2/lib/http/api.ts @@ -422,6 +422,8 @@ export class HttpApi extends HttpApiBase { httpApi: this, routeKey: HttpRouteKey.DEFAULT, integration: props.defaultIntegration, + authorizer: props.defaultAuthorizer, + authorizationScopes: props.defaultAuthorizationScopes, }); } diff --git a/packages/@aws-cdk/aws-apigatewayv2/test/http/api.test.ts b/packages/@aws-cdk/aws-apigatewayv2/test/http/api.test.ts index 3b07593676c11..c2324412d3396 100644 --- a/packages/@aws-cdk/aws-apigatewayv2/test/http/api.test.ts +++ b/packages/@aws-cdk/aws-apigatewayv2/test/http/api.test.ts @@ -400,6 +400,24 @@ describe('HttpApi', () => { }); }); + test('can add default authorizer when using default integration', () => { + const stack = new Stack(); + + const authorizer = new DummyAuthorizer(); + + new HttpApi(stack, 'api', { + defaultIntegration: new DummyRouteIntegration(), + defaultAuthorizer: authorizer, + defaultAuthorizationScopes: ['read:pets'], + }); + + expect(stack).toHaveResource('AWS::ApiGatewayV2::Route', { + AuthorizerId: 'auth-1234', + AuthorizationType: 'JWT', + AuthorizationScopes: ['read:pets'], + }); + }); + test('can add default authorizer, but remove it for a route', () => { const stack = new Stack(); const authorizer = new DummyAuthorizer(); From 1ff5b9e5b728116171cb1922a861c1ecd4105292 Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Thu, 3 Jun 2021 08:14:07 -0700 Subject: [PATCH 4/6] feat(kms): introduce `fromCfnKey()` method (#14859) This is part 1 of adding support from converting L1 resources to L2 without making them immutable in the process. Next phase after this will be adding support for `Bucket.fromCfnBucket()` (which will use the method from KMS defined here). Related issues: #9719 #14795 #14809 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../@aws-cdk/aws-iam/lib/policy-statement.ts | 10 +- packages/@aws-cdk/aws-kms/lib/key.ts | 51 ++- packages/@aws-cdk/aws-kms/test/key.test.ts | 319 +++++++++++++++++- .../@aws-cdk/cloudformation-include/README.md | 106 +++++- 4 files changed, 479 insertions(+), 7 deletions(-) diff --git a/packages/@aws-cdk/aws-iam/lib/policy-statement.ts b/packages/@aws-cdk/aws-iam/lib/policy-statement.ts index 8777e40d5d7f7..62257140e9c30 100644 --- a/packages/@aws-cdk/aws-iam/lib/policy-statement.ts +++ b/packages/@aws-cdk/aws-iam/lib/policy-statement.ts @@ -30,7 +30,7 @@ export class PolicyStatement { * @param obj the PolicyStatement in object form. */ public static fromJson(obj: any) { - return new PolicyStatement({ + const ret = new PolicyStatement({ sid: obj.Sid, actions: ensureArrayOrUndefined(obj.Action), resources: ensureArrayOrUndefined(obj.Resource), @@ -41,6 +41,14 @@ export class PolicyStatement { principals: obj.Principal ? [new JsonPrincipal(obj.Principal)] : undefined, notPrincipals: obj.NotPrincipal ? [new JsonPrincipal(obj.NotPrincipal)] : undefined, }); + + // validate that the PolicyStatement has the correct shape + const errors = ret.validateForAnyPolicy(); + if (errors.length > 0) { + throw new Error('Incorrect Policy Statement: ' + errors.join('\n')); + } + + return ret; } /** diff --git a/packages/@aws-cdk/aws-kms/lib/key.ts b/packages/@aws-cdk/aws-kms/lib/key.ts index bea78532bbf90..03097d9d49073 100644 --- a/packages/@aws-cdk/aws-kms/lib/key.ts +++ b/packages/@aws-cdk/aws-kms/lib/key.ts @@ -1,5 +1,5 @@ import * as iam from '@aws-cdk/aws-iam'; -import { FeatureFlags, IResource, RemovalPolicy, Resource, Stack, Duration } from '@aws-cdk/core'; +import { FeatureFlags, IResource, Lazy, RemovalPolicy, Resource, Stack, Duration } from '@aws-cdk/core'; import * as cxapi from '@aws-cdk/cx-api'; import { IConstruct, Construct } from 'constructs'; import { Alias } from './alias'; @@ -485,6 +485,55 @@ export class Key extends KeyBase { return new Import(keyResourceName); } + /** + * Create a mutable {@link IKey} based on a low-level {@link CfnKey}. + * This is most useful when combined with the cloudformation-include module. + * This method is different than {@link fromKeyArn()} because the {@link IKey} + * returned from this method is mutable; + * meaning, calling any mutating methods on it, + * like {@link IKey.addToResourcePolicy()}, + * will actually be reflected in the resulting template, + * as opposed to the object returned from {@link fromKeyArn()}, + * on which calling those methods would have no effect. + */ + public static fromCfnKey(cfnKey: CfnKey): IKey { + // use a "weird" id that has a higher chance of being unique + const id = '@FromCfnKey'; + + // if fromCfnKey() was already called on this cfnKey, + // return the same L2 + // (as different L2s would conflict, because of the mutation of the keyPolicy property of the L1 below) + const existing = cfnKey.node.tryFindChild(id); + if (existing) { + return existing; + } + + let keyPolicy: iam.PolicyDocument; + try { + keyPolicy = iam.PolicyDocument.fromJson(cfnKey.keyPolicy); + } catch (e) { + // If the KeyPolicy contains any CloudFormation functions, + // PolicyDocument.fromJson() throws an exception. + // In that case, because we would have to effectively make the returned IKey immutable, + // throw an exception suggesting to use the other importing methods instead. + // We might make this parsing logic smarter later, + // but let's start by erroring out. + throw new Error('Could not parse the PolicyDocument of the passed AWS::KMS::Key resource because it contains CloudFormation functions. ' + + 'This makes it impossible to create a mutable IKey from that Policy. ' + + 'You have to use fromKeyArn instead, passing it the ARN attribute property of the low-level CfnKey'); + } + + // change the key policy of the L1, so that all changes done in the L2 are reflected in the resulting template + cfnKey.keyPolicy = Lazy.any({ produce: () => keyPolicy.toJSON() }); + + return new class extends KeyBase { + public readonly keyArn = cfnKey.attrArn; + public readonly keyId = cfnKey.ref; + protected readonly policy = keyPolicy; + protected readonly trustAccountIdentities = false; + }(cfnKey, id); + } + public readonly keyArn: string; public readonly keyId: string; protected readonly policy?: iam.PolicyDocument; diff --git a/packages/@aws-cdk/aws-kms/test/key.test.ts b/packages/@aws-cdk/aws-kms/test/key.test.ts index 0bdb6c755d7bb..f80dce397b4b1 100644 --- a/packages/@aws-cdk/aws-kms/test/key.test.ts +++ b/packages/@aws-cdk/aws-kms/test/key.test.ts @@ -1,4 +1,4 @@ -import { arrayWith, ResourcePart } from '@aws-cdk/assert-internal'; +import { arrayWith, countResources, expect as expectCdk, haveResource, haveResourceLike, ResourcePart } from '@aws-cdk/assert-internal'; import '@aws-cdk/assert-internal/jest'; import * as iam from '@aws-cdk/aws-iam'; import * as cdk from '@aws-cdk/core'; @@ -582,6 +582,323 @@ describe('imported keys', () => { }); }); +describe('fromCfnKey()', () => { + let stack: cdk.Stack; + let cfnKey: kms.CfnKey; + let key: kms.IKey; + + beforeEach(() => { + stack = new cdk.Stack(); + cfnKey = new kms.CfnKey(stack, 'CfnKey', { + keyPolicy: { + Statement: [ + { + Action: 'kms:*', + Effect: 'Allow', + Principal: { + AWS: cdk.Fn.join('', [ + 'arn:', + cdk.Aws.PARTITION, + ':iam::', + cdk.Aws.ACCOUNT_ID, + ':root', + ]), + }, + Resource: '*', + }, + ], + Version: '2012-10-17', + }, + }); + key = kms.Key.fromCfnKey(cfnKey); + }); + + test("correctly resolves the 'keyId' property", () => { + expect(stack.resolve(key.keyId)).toStrictEqual({ + Ref: 'CfnKey', + }); + }); + + test("correctly resolves the 'keyArn' property", () => { + expect(stack.resolve(key.keyArn)).toStrictEqual({ + 'Fn::GetAtt': ['CfnKey', 'Arn'], + }); + }); + + test('preserves the KMS Key resource', () => { + expectCdk(stack).to(haveResource('AWS::KMS::Key', { + KeyPolicy: { + Statement: [ + { + Action: 'kms:*', + Effect: 'Allow', + Principal: { + AWS: { + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + ':iam::', + { Ref: 'AWS::AccountId' }, + ':root', + ]], + }, + }, + Resource: '*', + }, + ], + Version: '2012-10-17', + }, + })); + + expectCdk(stack).to(countResources('AWS::KMS::Key', 1)); + }); + + describe("calling 'addToResourcePolicy()' on the returned Key", () => { + let addToResourcePolicyResult: iam.AddToResourcePolicyResult; + + beforeEach(() => { + addToResourcePolicyResult = key.addToResourcePolicy(new iam.PolicyStatement({ + actions: ['kms:action'], + resources: ['*'], + principals: [new iam.AnyPrincipal()], + })); + }); + + test("the AddToResourcePolicyResult returned has 'statementAdded' set to 'true'", () => { + expect(addToResourcePolicyResult.statementAdded).toBeTruthy(); + }); + + test('preserves the mutating call in the resulting template', () => { + expectCdk(stack).to(haveResource('AWS::KMS::Key', { + KeyPolicy: { + Statement: [ + { + Action: 'kms:*', + Effect: 'Allow', + Principal: { + AWS: { + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + ':iam::', + { Ref: 'AWS::AccountId' }, + ':root', + ]], + }, + }, + Resource: '*', + }, + { + Action: 'kms:action', + Effect: 'Allow', + Principal: '*', + Resource: '*', + }, + ], + Version: '2012-10-17', + }, + })); + }); + }); + + describe('calling fromCfnKey() again', () => { + beforeEach(() => { + key = kms.Key.fromCfnKey(cfnKey); + }); + + describe('and using it for grantDecrypt() on a Role', function () { + beforeEach(() => { + const role = new iam.Role(stack, 'Role', { + assumedBy: new iam.AnyPrincipal(), + }); + key.grantDecrypt(role); + }); + + test('creates the correct IAM Policy', () => { + expectCdk(stack).to(haveResourceLike('AWS::IAM::Policy', { + PolicyDocument: { + Statement: [ + { + Action: 'kms:Decrypt', + Effect: 'Allow', + Resource: { + 'Fn::GetAtt': ['CfnKey', 'Arn'], + }, + }, + ], + }, + })); + }); + + test('correctly mutates the Policy of the underlying CfnKey', () => { + expectCdk(stack).to(haveResourceLike('AWS::KMS::Key', { + KeyPolicy: { + Statement: [ + { + Action: 'kms:*', + Effect: 'Allow', + Principal: { + AWS: { + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + ':iam::', + { Ref: 'AWS::AccountId' }, + ':root', + ]], + }, + }, + Resource: '*', + }, + { + Action: 'kms:Decrypt', + Effect: 'Allow', + Principal: { + AWS: { + 'Fn::GetAtt': ['Role1ABCC5F0', 'Arn'], + }, + }, + Resource: '*', + }, + ], + Version: '2012-10-17', + }, + })); + }); + }); + }); + + describe("called with a CfnKey that has an 'Fn::If' passed as the KeyPolicy", () => { + beforeEach(() => { + cfnKey = new kms.CfnKey(stack, 'CfnKey2', { + keyPolicy: cdk.Fn.conditionIf( + 'AlwaysTrue', + { + Statement: [ + { + Action: 'kms:action1', + Effect: 'Allow', + Principal: '*', + Resource: '*', + }, + ], + Version: '2012-10-17', + }, + { + Statement: [ + { + Action: 'kms:action2', + Effect: 'Allow', + Principal: '*', + Resource: '*', + }, + ], + Version: '2012-10-17', + }, + ), + }); + }); + + test('throws a descriptive exception', () => { + expect(() => { + kms.Key.fromCfnKey(cfnKey); + }).toThrow(/Could not parse the PolicyDocument of the passed AWS::KMS::Key/); + }); + }); + + describe("called with a CfnKey that has an 'Fn::If' passed as the Statement of a KeyPolicy", () => { + beforeEach(() => { + cfnKey = new kms.CfnKey(stack, 'CfnKey2', { + keyPolicy: { + Statement: cdk.Fn.conditionIf( + 'AlwaysTrue', + [ + { + Action: 'kms:action1', + Effect: 'Allow', + Principal: '*', + Resource: '*', + }, + ], + [ + { + Action: 'kms:action2', + Effect: 'Allow', + Principal: '*', + Resource: '*', + }, + ], + ), + Version: '2012-10-17', + }, + }); + }); + + test('throws a descriptive exception', () => { + expect(() => { + kms.Key.fromCfnKey(cfnKey); + }).toThrow(/Could not parse the PolicyDocument of the passed AWS::KMS::Key/); + }); + }); + + describe("called with a CfnKey that has an 'Fn::If' passed as one of the statements of a KeyPolicy", () => { + beforeEach(() => { + cfnKey = new kms.CfnKey(stack, 'CfnKey2', { + keyPolicy: { + Statement: [ + cdk.Fn.conditionIf( + 'AlwaysTrue', + { + Action: 'kms:action1', + Effect: 'Allow', + Principal: '*', + Resource: '*', + }, + { + Action: 'kms:action2', + Effect: 'Allow', + Principal: '*', + Resource: '*', + }, + ), + ], + Version: '2012-10-17', + }, + }); + }); + + test('throws a descriptive exception', () => { + expect(() => { + kms.Key.fromCfnKey(cfnKey); + }).toThrow(/Could not parse the PolicyDocument of the passed AWS::KMS::Key/); + }); + }); + + describe("called with a CfnKey that has an 'Fn::If' passed for the Action in one of the statements of a KeyPolicy", () => { + beforeEach(() => { + cfnKey = new kms.CfnKey(stack, 'CfnKey2', { + keyPolicy: { + Statement: [ + { + Action: cdk.Fn.conditionIf('AlwaysTrue', 'kms:action1', 'kms:action2'), + Effect: 'Allow', + Principal: '*', + Resource: '*', + }, + ], + Version: '2012-10-17', + }, + }); + }); + + test('throws a descriptive exception', () => { + expect(() => { + key = kms.Key.fromCfnKey(cfnKey); + }).toThrow(/Could not parse the PolicyDocument of the passed AWS::KMS::Key/); + }); + }); +}); + describe('addToResourcePolicy allowNoOp and there is no policy', () => { // eslint-disable-next-line jest/expect-expect testFutureBehavior('succeed if set to true (default)', flags, cdk.App, (app) => { diff --git a/packages/@aws-cdk/cloudformation-include/README.md b/packages/@aws-cdk/cloudformation-include/README.md index 2dcd71edc2b38..2c67aeb4d554b 100644 --- a/packages/@aws-cdk/cloudformation-include/README.md +++ b/packages/@aws-cdk/cloudformation-include/README.md @@ -118,14 +118,112 @@ role.addToPolicy(new iam.PolicyStatement({ })); ``` -If you need, you can also convert the CloudFormation resource to a higher-level -resource by importing it: +### Converting L1 resources to L2 + +The resources the `getResource` method returns are what the CDK calls +[Layer 1 resources](https://docs.aws.amazon.com/cdk/latest/guide/cfn_layer.html#cfn_layer_cfn) +(like `CfnBucket`). +However, in many places in the Construct Library, +the CDK requires so-called Layer 2 resources, like `IBucket`. +There are two ways of going from an L1 to an L2 resource. + +#### Using`fromCfn*()` methods + +This is the preferred method of converting an L1 resource to an L2. +It works by invoking a static method of the class of the L2 resource +whose name starts with `fromCfn` - +for example, for KMS Keys, that would be the `Kms.fromCfnKey()` method - +and passing the L1 instance as an argument: + +```ts +import * as kms from '@aws-cdk/aws-kms'; + +const cfnKey = cfnTemplate.getResource('Key') as kms.CfnKey; +const key = kms.Key.fromCfnKey(cfnKey); +``` + +This returns an instance of the `kms.IKey` type that can be passed anywhere in the CDK an `IKey` is expected. +What is more, that `IKey` instance will be mutable - +which means calling any mutating methods on it, +like `addToResourcePolicy()`, +will be reflected in the resulting template. + +Note that, in some cases, the `fromCfn*()` method might not be able to create an L2 from the underlying L1. +This can happen when the underlying L1 heavily uses CloudFormation functions. +For example, if you tried to create an L2 `IKey` +from an L1 represented as this CloudFormation template: + +```json +{ + "Resources": { + "Key": { + "Type": "AWS::KMS::Key", + "Properties": { + "KeyPolicy": { + "Statement": [ + { + "Fn::If": [ + "Condition", + { + "Action": "kms:if-action", + "Resource": "*", + "Principal": "*", + "Effect": "Allow" + }, + { + "Action": "kms:else-action", + "Resource": "*", + "Principal": "*", + "Effect": "Allow" + } + ] + } + ], + "Version": "2012-10-17" + } + } + } + } +} +``` + +The `Key.fromCfnKey()` method does not know how to translate that into CDK L2 concepts, +and would throw an exception. + +In those cases, you need the use the second method of converting an L1 to an L2. + +#### Using `from*Name/Arn/Attributes()` methods + +If the resource you need does not have a `fromCfn*()` method, +or if it does, but it throws an exception for your particular L1, +you need to use the second method of converting an L1 resource to L2. + +Each L2 class has static factory methods with names like `from*Name()`, +`from*Arn()`, and/or `from*Attributes()`. +You can obtain an L2 resource from an L1 by passing the correct properties of the L1 as the arguments to those methods: ```ts +// using from*Name() const bucket = s3.Bucket.fromBucketName(this, 'L2Bucket', cfnBucket.ref); -// bucket is of type s3.IBucket + +// using from*Arn() +const key = kms.Key.fromKeyArn(this, 'L2Key', cfnKey.attrArn); + +// using from*Attributes() +const vpc = ec2.Vpc.fromVpcAttributes(this, 'L2Vpc', { + vpcId: cfnVpc.ref, + availabilityZones: cdk.Fn.getAzs(), + privateSubnetIds: [privateCfnSubnet1.ref, privateCfnSubnet2.ref], +}); ``` +As long as they just need to be referenced, +and not changed in any way, everything should work; +however, note that resources returned from those methods, +unlike those returned by `fromCfn*()` methods, +are immutable, which means calling any mutating methods on them will have no effect. +You will have to mutate the underlying L1 in order to change them. + ## Non-resource template elements In addition to resources, @@ -215,7 +313,7 @@ new inc.CfnInclude(this, 'includeTemplate', { ``` This will replace all references to `MyParam` with the string `'my-value'`, -and `MyParam` will be removed from the 'Parameters' section of the template. +and `MyParam` will be removed from the 'Parameters' section of the resulting template. ## Nested Stacks From da6f2c6752a5ad0bd602d267480ff05cb919e13b Mon Sep 17 00:00:00 2001 From: Ben Chaimberg Date: Thu, 3 Jun 2021 11:04:50 -0700 Subject: [PATCH 5/6] chore: transfer shivlaks ownership to BenChaimberg (#14976) --- .github/workflows/issue-label-assign.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/issue-label-assign.yml b/.github/workflows/issue-label-assign.yml index d8053118ca4f0..fe09e74b25e2e 100644 --- a/.github/workflows/issue-label-assign.yml +++ b/.github/workflows/issue-label-assign.yml @@ -201,8 +201,8 @@ jobs: {"keywords":["(@aws-cdk/aws-sqs)","(aws-sqs)","(sqs)"],"labels":["@aws-cdk/aws-sqs"],"assignees":["njlynch"]}, {"keywords":["(@aws-cdk/aws-ssm)","(aws-ssm)","(ssm)"],"labels":["@aws-cdk/aws-ssm"],"assignees":["njlynch"]}, {"keywords":["(@aws-cdk/aws-sso)","(aws-sso)","(sso)"],"labels":["@aws-cdk/aws-sso"],"assignees":["skinny85"]}, - {"keywords":["(@aws-cdk/aws-stepfunctions)","(aws-stepfunctions)","(stepfunctions)","(step functions)","(step-functions)"],"labels":["@aws-cdk/aws-stepfunctions"],"assignees":["shivlaks"]}, - {"keywords":["(@aws-cdk/aws-stepfunctions-tasks)","(aws-stepfunctions-tasks)","(stepfunctions-tasks)","(stepfunctions tasks)"],"labels":["@aws-cdk/aws-stepfunctions-tasks"],"assignees":["shivlaks"]}, + {"keywords":["(@aws-cdk/aws-stepfunctions)","(aws-stepfunctions)","(stepfunctions)","(step functions)","(step-functions)"],"labels":["@aws-cdk/aws-stepfunctions"],"assignees":["BenChaimberg"]}, + {"keywords":["(@aws-cdk/aws-stepfunctions-tasks)","(aws-stepfunctions-tasks)","(stepfunctions-tasks)","(stepfunctions tasks)"],"labels":["@aws-cdk/aws-stepfunctions-tasks"],"assignees":["BenChaimberg"]}, {"keywords":["(@aws-cdk/aws-synthetics)","(aws-synthetics)","(synthetics)"],"labels":["@aws-cdk/aws-synthetics"],"assignees":["BenChaimberg"]}, {"keywords":["(@aws-cdk/aws-timestream)","(aws-timestream)","(timestream)"],"labels":["@aws-cdk/aws-timestream"],"assignees":["skinny85"]}, {"keywords":["(@aws-cdk/aws-transfer)","(aws-transfer)","(transfer)"],"labels":["@aws-cdk/aws-transfer"],"assignees":["otaviomacedo"]}, From af6d49f2e245b60ae3bbea3bb2c5d283beedba3f Mon Sep 17 00:00:00 2001 From: Christian Moore Date: Thu, 3 Jun 2021 14:39:17 -0400 Subject: [PATCH 6/6] fix(ec2): add missing entry for XLARGE3 (#14750) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../@aws-cdk/aws-ec2/lib/instance-types.ts | 5 +++ .../@aws-cdk/aws-ec2/test/instance.test.ts | 33 +++++++++++++------ 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/packages/@aws-cdk/aws-ec2/lib/instance-types.ts b/packages/@aws-cdk/aws-ec2/lib/instance-types.ts index fc3b3b411d7b3..a12dfb92061c6 100644 --- a/packages/@aws-cdk/aws-ec2/lib/instance-types.ts +++ b/packages/@aws-cdk/aws-ec2/lib/instance-types.ts @@ -527,6 +527,11 @@ export enum InstanceSize { */ XLARGE2 = '2xlarge', + /** + * Instance size XLARGE3 (3xlarge) + */ + XLARGE3 = '3xlarge', + /** * Instance size XLARGE4 (4xlarge) */ diff --git a/packages/@aws-cdk/aws-ec2/test/instance.test.ts b/packages/@aws-cdk/aws-ec2/test/instance.test.ts index d5b77e01ad0ea..392aeb8ec22d9 100644 --- a/packages/@aws-cdk/aws-ec2/test/instance.test.ts +++ b/packages/@aws-cdk/aws-ec2/test/instance.test.ts @@ -21,17 +21,30 @@ beforeEach(() => { nodeunitShim({ 'instance is created correctly'(test: Test) { - // WHEN - new Instance(stack, 'Instance', { - vpc, - machineImage: new AmazonLinuxImage(), - instanceType: InstanceType.of(InstanceClass.BURSTABLE4_GRAVITON, InstanceSize.LARGE), - }); + // GIVEN + const sampleInstances = [{ + instanceClass: InstanceClass.BURSTABLE4_GRAVITON, + instanceSize: InstanceSize.LARGE, + instanceType: 't4g.large', + }, { + instanceClass: InstanceClass.HIGH_COMPUTE_MEMORY1, + instanceSize: InstanceSize.XLARGE3, + instanceType: 'z1d.3xlarge', + }]; - // THEN - cdkExpect(stack).to(haveResource('AWS::EC2::Instance', { - InstanceType: 't4g.large', - })); + for (const [i, sampleInstance] of sampleInstances.entries()) { + // WHEN + new Instance(stack, `Instance${i}`, { + vpc, + machineImage: new AmazonLinuxImage(), + instanceType: InstanceType.of(sampleInstance.instanceClass, sampleInstance.instanceSize), + }); + + // THEN + cdkExpect(stack).to(haveResource('AWS::EC2::Instance', { + InstanceType: sampleInstance.instanceType, + })); + } test.done(); },