fix(region-info): ssm service principal is wrong in majority of regions

packages/@aws-cdk/aws-codebuild/lib/linux-gpu-build-image.ts

@@ -1,6 +1,6 @@
 import * as ecr from '@aws-cdk/aws-ecr';
 import * as core from '@aws-cdk/core';
-import { FactName, RegionInfo } from '@aws-cdk/region-info';
+import { FactName } from '@aws-cdk/region-info';
 import { BuildSpec } from './build-spec';
 import { runScriptLinuxBuildSpec } from './private/run-script-linux-build-spec';
 import {
@@ -12,8 +12,6 @@ import {
 // eslint-disable-next-line no-duplicate-imports, import/order
 import { Construct } from '@aws-cdk/core';
 
-const mappingName = 'AwsDeepLearningContainersRepositoriesAccounts';
-
 /**
  * A CodeBuild GPU image running Linux.
  *
@@ -109,38 +107,38 @@ export class LinuxGpuBuildImage implements IBindableBuildImage {
 
   public readonly type = 'LINUX_GPU_CONTAINER';
   public readonly defaultComputeType = ComputeType.LARGE;
-  public readonly imageId: string;
   public readonly imagePullPrincipalType?: ImagePullPrincipalType = ImagePullPrincipalType.SERVICE_ROLE;
+  public readonly imageId: string;
 
-  private readonly accountExpression: string;
+  private _imageAccount?: string;
 
   private constructor(private readonly repositoryName: string, tag: string, private readonly account: string | undefined) {
-    this.accountExpression = account ?? core.Fn.findInMap(mappingName, core.Aws.REGION, 'repositoryAccount');
-    this.imageId = `${this.accountExpression}.dkr.ecr.${core.Aws.REGION}.${core.Aws.URL_SUFFIX}/${repositoryName}:${tag}`;
+    const imageAccount = account ?? core.Lazy.string({
+      produce: () => {
+        if (this._imageAccount === undefined) {
+          throw new Error('Make sure this \'LinuxGpuBuildImage\' is used in a CodeBuild Project construct');
+        }
+        return this._imageAccount;
+      },
+    });
+
+    // The value of imageId below *should* have been `Lazy.stringValue(() => repository.repositoryUriForTag(this.tag))`,
+    // but we can't change that anymore because someone somewhere might at this point have written code
+    // to do `image.imageId.includes('pytorch')` and changing this to a full-on token would break them.
+    this.imageId = `${imageAccount}.dkr.ecr.${core.Aws.REGION}.${core.Aws.URL_SUFFIX}/${repositoryName}:${tag}`;
   }
 
   public bind(scope: Construct, project: IProject, _options: BuildImageBindOptions): BuildImageConfig {
-    if (!this.account) {
-      const scopeStack = core.Stack.of(scope);
-      // Unfortunately, the account IDs of the DLC repositories are not the same in all regions.
-      // Because of that, use a (singleton) Mapping to find the correct account
-      if (!scopeStack.node.tryFindChild(mappingName)) {
-        const mapping: { [k1: string]: { [k2: string]: any } } = {};
-        // get the accounts from the region-info module
-        const region2Accounts = RegionInfo.regionMap(FactName.DLC_REPOSITORY_ACCOUNT);
-        for (const [region, account] of Object.entries(region2Accounts)) {
-          mapping[region] = { repositoryAccount: account };
-        }
-        new core.CfnMapping(scopeStack, mappingName, { mapping });
-      }
-    }
-
+    const account = this.account ?? core.Stack.of(scope).regionalFact(FactName.DLC_REPOSITORY_ACCOUNT);
     const repository = ecr.Repository.fromRepositoryAttributes(scope, 'AwsDlcRepositoryCodeBuild', {
       repositoryName: this.repositoryName,
-      repositoryArn: ecr.Repository.arnForLocalRepository(this.repositoryName, scope, this.accountExpression),
+      repositoryArn: ecr.Repository.arnForLocalRepository(this.repositoryName, scope, account),
     });
+
     repository.grantPull(project);
 
+    this._imageAccount = account;
+
     return {
     };
   }

packages/@aws-cdk/aws-codebuild/lib/project.ts

@@ -1138,9 +1138,8 @@ export class Project extends ProjectBase {
     }
 
     // bind
-    const bindFunction = (this.buildImage as any).bind;
-    if (bindFunction) {
-      bindFunction.call(this.buildImage, this, this, {});
+    if (isBindableBuildImage(this.buildImage)) {
+      this.buildImage.bind(this, this, {});
     }
   }
 
@@ -2123,3 +2122,7 @@ export enum ProjectNotificationEvents {
    */
   BUILD_PHASE_SUCCEEDED = 'codebuild-project-build-phase-success',
 }
+
+function isBindableBuildImage(x: unknown): x is IBindableBuildImage {
+  return typeof x === 'object' && !!x && !!(x as any).bind;
+}

packages/@aws-cdk/aws-codebuild/test/integ.aws-deep-learning-container-build-image.expected.json

@@ -135,11 +135,11 @@
                     ":",
                     {
                       "Fn::FindInMap": [
-                        "AwsDeepLearningContainersRepositoriesAccounts",
+                        "DlcRepositoryAccountMap",
                         {
                           "Ref": "AWS::Region"
                         },
-                        "repositoryAccount"
+                        "value"
                       ]
                     },
                     ":repository/mxnet-training"
@@ -177,11 +177,11 @@
               [
                 {
                   "Fn::FindInMap": [
-                    "AwsDeepLearningContainersRepositoriesAccounts",
+                    "DlcRepositoryAccountMap",
                     {
                       "Ref": "AWS::Region"
                     },
-                    "repositoryAccount"
+                    "value"
                   ]
                 },
                 ".dkr.ecr.",
@@ -215,66 +215,66 @@
     }
   },
   "Mappings": {
-    "AwsDeepLearningContainersRepositoriesAccounts": {
+    "DlcRepositoryAccountMap": {
       "ap-east-1": {
-        "repositoryAccount": "871362719292"
+        "value": "871362719292"
       },
       "ap-northeast-1": {
-        "repositoryAccount": "763104351884"
+        "value": "763104351884"
       },
       "ap-northeast-2": {
-        "repositoryAccount": "763104351884"
+        "value": "763104351884"
       },
       "ap-south-1": {
-        "repositoryAccount": "763104351884"
+        "value": "763104351884"
       },
       "ap-southeast-1": {
-        "repositoryAccount": "763104351884"
+        "value": "763104351884"
       },
       "ap-southeast-2": {
-        "repositoryAccount": "763104351884"
+        "value": "763104351884"
       },
       "ca-central-1": {
-        "repositoryAccount": "763104351884"
+        "value": "763104351884"
       },
       "cn-north-1": {
-        "repositoryAccount": "727897471807"
+        "value": "727897471807"
       },
       "cn-northwest-1": {
-        "repositoryAccount": "727897471807"
+        "value": "727897471807"
       },
       "eu-central-1": {
-        "repositoryAccount": "763104351884"
+        "value": "763104351884"
       },
       "eu-north-1": {
-        "repositoryAccount": "763104351884"
+        "value": "763104351884"
       },
       "eu-west-1": {
-        "repositoryAccount": "763104351884"
+        "value": "763104351884"
       },
       "eu-west-2": {
-        "repositoryAccount": "763104351884"
+        "value": "763104351884"
       },
       "eu-west-3": {
-        "repositoryAccount": "763104351884"
+        "value": "763104351884"
       },
       "me-south-1": {
-        "repositoryAccount": "217643126080"
+        "value": "217643126080"
       },
       "sa-east-1": {
-        "repositoryAccount": "763104351884"
+        "value": "763104351884"
       },
       "us-east-1": {
-        "repositoryAccount": "763104351884"
+        "value": "763104351884"
       },
       "us-east-2": {
-        "repositoryAccount": "763104351884"
+        "value": "763104351884"
       },
       "us-west-1": {
-        "repositoryAccount": "763104351884"
+        "value": "763104351884"
       },
       "us-west-2": {
-        "repositoryAccount": "763104351884"
+        "value": "763104351884"
       }
     }
   }

packages/@aws-cdk/aws-iam/lib/principals.ts

@@ -1,5 +1,5 @@
 import * as cdk from '@aws-cdk/core';
-import { Default, RegionInfo } from '@aws-cdk/region-info';
+import { Default, FactName, RegionInfo } from '@aws-cdk/region-info';
 import { IOpenIdConnectProvider } from './oidc-provider';
 import { Condition, Conditions, PolicyStatement } from './policy-statement';
 import { ISamlProvider } from './saml-provider';
@@ -331,6 +331,7 @@ export interface ServicePrincipalOpts {
    * The region in which the service is operating.
    *
    * @default the current Stack's region.
+   * @deprecated You should not need to set this. The stack's region is always correct.
    */
   readonly region?: string;
 
@@ -694,9 +695,23 @@ class ServicePrincipalToken implements cdk.IResolvable {
   }
 
   public resolve(ctx: cdk.IResolveContext) {
-    const region = this.opts.region || cdk.Stack.of(ctx.scope).region;
-    const fact = RegionInfo.get(region).servicePrincipal(this.service);
-    return fact || Default.servicePrincipal(this.service, region, cdk.Aws.URL_SUFFIX);
+    if (this.opts.region) {
+      // Special case, handle it separately to not break legacy behavior.
+      return (
+        RegionInfo.get(this.opts.region).servicePrincipal(this.service) ??
+        Default.servicePrincipal(
+          this.service,
+          this.opts.region,
+          cdk.Aws.URL_SUFFIX,
+        )
+      );
+    }
+
+    const stack = cdk.Stack.of(ctx.scope);
+    return stack.regionalFact(
+      FactName.servicePrincipal(this.service),
+      Default.servicePrincipal(this.service, stack.region, cdk.Aws.URL_SUFFIX),
+    );
   }
 
   public toString() {

packages/@aws-cdk/aws-iam/lib/util.ts

@@ -137,4 +137,4 @@ export class UniqueStringSet implements IResolvable, IPostProcessor {
 
 function isEmptyObject(x: { [key: string]: any }): boolean {
   return Object.keys(x).length === 0;
-}
+}

packages/@aws-cdk/aws-iam/package.json

@@ -80,6 +80,7 @@
   "license": "Apache-2.0",
   "devDependencies": {
     "@aws-cdk/assert-internal": "0.0.0",
+    "@aws-cdk/assertions": "0.0.0",
     "@aws-cdk/cdk-build-tools": "0.0.0",
     "@aws-cdk/cdk-integ-tools": "0.0.0",
     "@aws-cdk/cfn2ts": "0.0.0",

packages/@aws-cdk/aws-iam/test/policy-document.test.ts

@@ -1,4 +1,5 @@
 import '@aws-cdk/assert-internal/jest';
+import { testDeprecated } from '@aws-cdk/cdk-build-tools';
 import { Lazy, Stack, Token } from '@aws-cdk/core';
 import {
   AccountPrincipal, Anyone, AnyPrincipal, ArnPrincipal, CanonicalUserPrincipal, CompositePrincipal,
@@ -444,7 +445,8 @@ describe('IAM policy document', () => {
       });
     });
 
-    test('regional service principals resolve appropriately (with user-set region)', () => {
+    // Deprecated: 'region' parameter to ServicePrincipal shouldn't be used.
+    testDeprecated('regional service principals resolve appropriately (with user-set region)', () => {
       const stack = new Stack(undefined, undefined, { env: { region: 'cn-northeast-1' } });
       const s = new PolicyStatement();
       s.addActions('test:Action');

packages/@aws-cdk/aws-iam/test/principals.test.ts

@@ -1,4 +1,5 @@
 import '@aws-cdk/assert-internal/jest';
+import { Template } from '@aws-cdk/assertions';
 import { App, CfnOutput, Stack } from '@aws-cdk/core';
 import * as iam from '../lib';
 
@@ -243,4 +244,20 @@ test('PrincipalWithConditions inherits principalAccount from AccountPrincipal ',
   // THEN
   expect(accountPrincipal.principalAccount).toStrictEqual('123456789012');
   expect(principalWithConditions.principalAccount).toStrictEqual('123456789012');
+});
+
+test('ServicePrincipal in agnostic stack generates lookup table', () => {
+  // GIVEN
+  const stack = new Stack();
+
+  // WHEN
+  new iam.Role(stack, 'Role', {
+    assumedBy: new iam.ServicePrincipal('ssm.amazonaws.com'),
+  });
+
+  // THEN
+  const template = Template.fromStack(stack);
+  const mappings = template.findMappings('ServiceprincipalMap');
+  expect(mappings.ServiceprincipalMap['af-south-1']?.ssm).toEqual('ssm.af-south-1.amazonaws.com');
+  expect(mappings.ServiceprincipalMap['us-east-1']?.ssm).toEqual('ssm.amazonaws.com');
 });