fix(cli): can't use credential providers for stacks with assets (#7022)

* fix(cli): can't use credential providers for stacks with assets

Stacks with assets were broken for environments that completely
used credential providers (and no other sources of AWS credentials).

Not because the wrong credentials were being used (as you would expect
it to be broken). Instead, an error was being thrown because the
"current" account lookup was incorrectly forwarding to the "default"
account lookup (fix: we know what the current account is. Just return
that instead).

What's worse, this value wasn't even being used! It was being looked up
so that if something was wrong finding the target bucket, we could
format a nice error message containing the account id. Even happy
paths were failing due to the premature lookup though (fix: only
look up when we actually need the value).

Fixes #7005.

* Properly look up partition so this also works in aws-cn

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

packages/aws-cdk/lib/api/aws-auth/sdk.ts

@@ -14,6 +14,8 @@ export interface ISDK {
   route53(): AWS.Route53;
 
   ecr(): AWS.ECR;
+
+  sts(): AWS.STS;
 }
 
 /**
@@ -66,4 +68,8 @@ export class SDK implements ISDK {
   public ecr(): AWS.ECR {
     return new AWS.ECR(this.config);
   }
+
+  public sts(): AWS.STS {
+    return new AWS.STS(this.config);
+  }
 }

packages/aws-cdk/lib/util/asset-publishing.ts

@@ -8,6 +8,13 @@ import { debug, error, print } from '../logging';
  * Use cdk-assets to publish all assets in the given manifest.
  */
 export async function publishAssets(manifest: cdk_assets.AssetManifest, sdk: SdkProvider, targetEnv: cxapi.Environment) {
+  // This shouldn't really happen (it's a programming error), but we don't have
+  // the types here to guide us. Do an runtime validation to be super super sure.
+  if (targetEnv.account === undefined || targetEnv.account === cxapi.UNKNOWN_ACCOUNT
+    || targetEnv.region === undefined || targetEnv.account === cxapi.UNKNOWN_REGION) {
+    throw new Error(`Asset publishing requires resolved account and region, got ${JSON.stringify(targetEnv)}`);
+  }
+
   const publisher = new cdk_assets.AssetPublishing(manifest, {
     aws: new PublishingAws(sdk, targetEnv),
     progressListener: new PublishingProgressListener(),
@@ -33,15 +40,21 @@ class PublishingAws implements cdk_assets.IAws {
   }
 
   public async discoverDefaultRegion(): Promise<string> {
-    return this.aws.defaultRegion;
+    return this.targetEnv.region;
   }
 
   public async discoverCurrentAccount(): Promise<cdk_assets.Account> {
-    const account = await this.aws.defaultAccount();
-    if (!account) {
-      throw new Error('AWS credentials are required to upload assets. Please configure environment variables or ~/.aws/credentials.');
+    // Discover the current partition given current credentials. We already
+    // have the target environment, but it doesn't contain the partition yet.
+    //
+    // Until it does, we need to do a lookup here.
+    const sts = (await this.sdk({})).sts();
+    const response = await sts.getCallerIdentity().promise();
+    if (!response.Arn) {
+      throw new Error(`Unexpected STS response: ${JSON.stringify(response)}`);
     }
-    return account;
+    const partition = response.Arn!.split(':')[1];
+    return { accountId: this.targetEnv.account, partition };
   }
 
   public async s3Client(options: cdk_assets.ClientOptions): Promise<AWS.S3> {

packages/aws-cdk/test/assets.test.ts

@@ -1,8 +1,11 @@
 import { AssetMetadataEntry } from '@aws-cdk/cx-api';
+import { AssetManifest } from 'cdk-assets';
 import { ToolkitInfo } from '../lib';
 import { addMetadataAssetsToManifest } from '../lib/assets';
 import { AssetManifestBuilder } from '../lib/util/asset-manifest-builder';
+import { publishAssets } from '../lib/util/asset-publishing';
 import { testStack } from './util';
+import { MockSDK } from './util/mock-sdk';
 
 let toolkit: ToolkitInfo;
 let assets: AssetManifestBuilder;
@@ -218,6 +221,29 @@ describe('docker assets', () => {
   });
 });
 
+test('publishing does not fail if there is no "default" account', () => {
+  // GIVEN
+  const manifest = new AssetManifest('.', {
+    version: '0',
+    files: {
+      assetId: {
+        source: { path: __filename },
+        destinations: {
+          theDestination: {
+            bucketName: '${AWS::AccountId}-bucket',
+            objectKey: 'key',
+          },
+        }
+      },
+    },
+  });
+  const provider = new MockSDK();
+  provider.defaultAccount = jest.fn(() => Promise.resolve(undefined));
+
+  // WHEN
+  publishAssets(manifest, provider, { account: '12345678', region: 'aa-south-1', name: 'main' });
+});
+
 function stackWithAssets(assetEntries: AssetMetadataEntry[]) {
   return testStack({
     stackName: 'SomeStack',

packages/cdk-assets/lib/private/handlers/files.ts

@@ -25,14 +25,16 @@ export class FileAssetHandler implements IAssetHandler {
     const s3 = await this.host.aws.s3Client(destination);
     this.host.emitMessage(EventType.CHECK, `Check ${s3Url}`);
 
-    const account = await this.host.aws.discoverCurrentAccount();
+    // A thunk for describing the current account. Used when we need to format an error
+    // message, not in the success case.
+    const account = async () => (await this.host.aws.discoverCurrentAccount())?.accountId;
     switch (await bucketOwnership(s3, destination.bucketName)) {
       case BucketOwnership.MINE:
         break;
       case BucketOwnership.DOES_NOT_EXIST:
-        throw new Error(`No bucket named '${destination.bucketName}'. Is account ${account} bootstrapped?`);
+        throw new Error(`No bucket named '${destination.bucketName}'. Is account ${await account()} bootstrapped?`);
       case BucketOwnership.SOMEONE_ELSES_OR_NO_ACCESS:
-        throw new Error(`Bucket named '${destination.bucketName}' exists, but not in account ${account}. Wrong account?`);
+        throw new Error(`Bucket named '${destination.bucketName}' exists, but not in account ${await account()}. Wrong account?`);
     }
 
     if (await objectExists(s3, destination.bucketName, destination.objectKey)) {

packages/cdk-assets/test/files.test.ts

@@ -92,6 +92,17 @@ test('upload file if new', async () => {
   // We'll just have to assume the contents are correct
 });
 
+test('successful run does not need to query account ID', async () => {
+  const pub = new AssetPublishing(AssetManifest.fromPath('/simple/cdk.out'), { aws });
+
+  aws.mockS3.headObject = mockedApiFailure('NotFound', 'File does not exist');
+  aws.mockS3.upload = mockUpload('FILE_CONTENTS');
+
+  await pub.publish();
+
+  expect(aws.discoverCurrentAccount).not.toHaveBeenCalled();
+});
+
 test('correctly identify asset path if path is absolute', async () => {
   const pub = new AssetPublishing(AssetManifest.fromPath('/abs/cdk.out'), { aws });