From 8c4e43513e6b3b419e37e8358f303b0e518cbea5 Mon Sep 17 00:00:00 2001
From: Maxim Ivanov <hi+github@yamlcoder.me>
Date: Fri, 18 Dec 2020 12:15:41 +0000
Subject: [PATCH] fix(cli): cross account asset upload no longer works

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 resolved 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.
---
 packages/aws-cdk/lib/util/asset-publishing.ts   | 4 ++++
 packages/cdk-assets/bin/publish.ts              | 4 ++++
 packages/cdk-assets/lib/aws.ts                  | 1 +
 packages/cdk-assets/lib/private/placeholders.ts | 8 +++++++-
 packages/cdk-assets/test/mock-aws.ts            | 1 +
 5 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/packages/aws-cdk/lib/util/asset-publishing.ts b/packages/aws-cdk/lib/util/asset-publishing.ts
index 4d5ef96362ddc..eb929bf03b4f3 100644
--- a/packages/aws-cdk/lib/util/asset-publishing.ts
+++ b/packages/aws-cdk/lib/util/asset-publishing.ts
@@ -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;
   }
diff --git a/packages/cdk-assets/bin/publish.ts b/packages/cdk-assets/bin/publish.ts
index 20b7a609bfdd0..e8d251cf82b97 100644
--- a/packages/cdk-assets/bin/publish.ts
+++ b/packages/cdk-assets/bin/publish.ts
@@ -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';
   }
diff --git a/packages/cdk-assets/lib/aws.ts b/packages/cdk-assets/lib/aws.ts
index 62c886885137f..40609eb155af7 100644
--- a/packages/cdk-assets/lib/aws.ts
+++ b/packages/cdk-assets/lib/aws.ts
@@ -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>;
 
diff --git a/packages/cdk-assets/lib/private/placeholders.ts b/packages/cdk-assets/lib/private/placeholders.ts
index 44ac4b80c36c3..50f76dfd3a7a6 100644
--- a/packages/cdk-assets/lib/private/placeholders.ts
+++ b/packages/cdk-assets/lib/private/placeholders.ts
@@ -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);
@@ -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();
     },
   });
 }
\ No newline at end of file
diff --git a/packages/cdk-assets/test/mock-aws.ts b/packages/cdk-assets/test/mock-aws.ts
index e331f40179cc6..262ab495bd430 100644
--- a/packages/cdk-assets/test/mock-aws.ts
+++ b/packages/cdk-assets/test/mock-aws.ts
@@ -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)),