Skip to content

Commit

Permalink
fix(cli): cross account asset upload no longer works (#12155)
Browse files Browse the repository at this point in the history
cdk_asset asset handlers use IAws to make calls to AWS APIs to discover
information about target environment: account id, region, partition.

Each asset is described by its manifest in a Cloud Assembly. This manifest
can contain placeholders to resolve by asset handlers when publishing
assets.

Previously `${Aws::Partition}` placeholder was derived from a code path used
to resolve `${Aws::AccountId}`, which was introducing a cyclic dependency for
cross account deployments:

- to replace partition placeholder it was assuming role in a target account
  to discover partition
- to assume role in a target account it needs to know full role ARN to assume
- role ARN contains partition placeholder

It was working for same account deployments and for non environment aware deployments,
because SdkProvider was always using current default (ambient) credentials without making
`AssumeRole` call, thus it was able to replace placeholders in asset manifest without
introducing a cyclic dependency.

To fix cross account deployments we introduce `IAWS.discoverPartition()` method to return
partition of default (ambient) credentials `cdk deploy` is called with. This works, because
cross partition `AssumeRole` calls are not possible, therefore it's enough to know our
default credentials partition.


Fixes #12151

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
redbaron committed Jan 6, 2021
1 parent 4da50e5 commit 1c8cb11
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 1 deletion.
4 changes: 4 additions & 0 deletions packages/aws-cdk/lib/util/asset-publishing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ class PublishingAws implements cdk_assets.IAws {
private readonly targetEnv: cxapi.Environment) {
}

public async discoverPartition(): Promise<string> {
return (await this.aws.baseCredentialsPartition(this.targetEnv, Mode.ForWriting)) ?? 'aws';
}

public async discoverDefaultRegion(): Promise<string> {
return this.targetEnv.region;
}
Expand Down
4 changes: 4 additions & 0 deletions packages/cdk-assets/bin/publish.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ class DefaultAwsClient implements IAws {
return new this.AWS.ECR(await this.awsOptions(options));
}

public async discoverPartition(): Promise<string> {
return (await this.discoverCurrentAccount()).partition;
}

public async discoverDefaultRegion(): Promise<string> {
return this.AWS.config.region || 'us-east-1';
}
Expand Down
1 change: 1 addition & 0 deletions packages/cdk-assets/lib/aws.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as AWS from 'aws-sdk';
* AWS SDK operations required by Asset Publishing
*/
export interface IAws {
discoverPartition(): Promise<string>;
discoverDefaultRegion(): Promise<string>;
discoverCurrentAccount(): Promise<Account>;

Expand Down
8 changes: 7 additions & 1 deletion packages/cdk-assets/lib/private/placeholders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ import { IAws } from '../aws';
* (they're nominally independent tools).
*/
export async function replaceAwsPlaceholders<A extends { region?: string }>(object: A, aws: IAws): Promise<A> {
let partition = async () => {
const p = await aws.discoverPartition();
partition = () => Promise.resolve(p);
return p;
};

let account = async () => {
const a = await aws.discoverCurrentAccount();
account = () => Promise.resolve(a);
Expand All @@ -22,7 +28,7 @@ export async function replaceAwsPlaceholders<A extends { region?: string }>(obje
return (await account()).accountId;
},
async partition() {
return (await account()).partition;
return partition();
},
});
}
1 change: 1 addition & 0 deletions packages/cdk-assets/test/mock-aws.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export function mockAws() {
return {
mockEcr,
mockS3,
discoverPartition: jest.fn(() => Promise.resolve('swa')),
discoverCurrentAccount: jest.fn(() => Promise.resolve({ accountId: 'current_account', partition: 'swa' })),
discoverDefaultRegion: jest.fn(() => Promise.resolve('current_region')),
ecrClient: jest.fn(() => Promise.resolve(mockEcr)),
Expand Down

0 comments on commit 1c8cb11

Please sign in to comment.