fix(iam): service principals use unnecessary exceptions (under feature flag)

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

@@ -1,4 +1,5 @@
 import * as cdk from '@aws-cdk/core';
+import * as cxapi from '@aws-cdk/cx-api';
 import { Default, FactName, RegionInfo } from '@aws-cdk/region-info';
 import { IDependable } from 'constructs';
 import { IOpenIdConnectProvider } from './oidc-provider';
@@ -479,10 +480,26 @@ export class AccountPrincipal extends ArnPrincipal {
  */
 export interface ServicePrincipalOpts {
   /**
-   * The region in which the service is operating.
+   * The region in which you want to reference the service
    *
-   * @default - the current Stack's region.
-   * @deprecated You should not need to set this. The stack's region is always correct.
+   * This is only necessary for *cross-region* references to *opt-in* regions. In those
+   * cases, the region name needs to be included to reference the correct service principal.
+   * In all other cases, the global service principal name is sufficient.
+   *
+   * This field behaves differently depending on whether the `@aws-cdk/aws-iam:standardizedServicePrincipals`
+   * flag is set or not:
+   *
+   * - If the flag is set, the input service principal is assumed to be of the form `SERVICE.amazonaws.com`.
+   *   That value will always be returned, unless the given region is an opt-in region and the service
+   *   principal is rendered in a stack in a different region, in which case `SERVICE.REGION.amazonaws.com`
+   *   will be rendered. Under this regime, there is no downside to always specifying the region property:
+   *   it will be rendered only if necessary.
+   * - If the flag is not set, the service principal will resolve to a single principal
+   *   whose name comes from the `@aws-cdk/region-info` package, using the region to override
+   *   the stack region. If there is no entry for this service principal in the database,, the input
+   *   service name is returned literally. This is legacy behavior and is not recommended.
+   *
+   * @default - the resolving Stack's region.
    */
   readonly region?: string;
 
@@ -514,6 +531,7 @@ export class ServicePrincipal extends PrincipalBase {
   }
 
   /**
+   * Reference an AWS service, optionally in a given region
    *
    * @param service AWS service (i.e. sqs.amazonaws.com)
    */
@@ -523,9 +541,7 @@ export class ServicePrincipal extends PrincipalBase {
 
   public get policyFragment(): PrincipalPolicyFragment {
     return new PrincipalPolicyFragment({
-      Service: [
-        new ServicePrincipalToken(this.service, this.opts).toString(),
-      ],
+      Service: [new ServicePrincipalToken(this.service, this.opts).toString()],
     }, this.opts.conditions);
   }
 
@@ -895,6 +911,33 @@ class ServicePrincipalToken implements cdk.IResolvable {
   }
 
   public resolve(ctx: cdk.IResolveContext) {
+    return cdk.FeatureFlags.of(ctx.scope).isEnabled(cxapi.IAM_STANDARDIZED_SERVICE_PRINCIPALS)
+      ? this.newStandardizedBehavior(ctx)
+      : this.legacyBehavior(ctx);
+
+    // The correct behavior is to always use the global service principal
+  }
+
+  /**
+   * Return the global (original) service principal, and a second one if region is given and points to an opt-in region
+   */
+  private newStandardizedBehavior(ctx: cdk.IResolveContext) {
+    const stack = cdk.Stack.of(ctx.scope);
+    if (
+      this.opts.region &&
+      !cdk.Token.isUnresolved(this.opts.region) &&
+      stack.region !== this.opts.region &&
+      RegionInfo.get(this.opts.region).isOptInRegion
+    ) {
+      return this.service.replace(/\.amazonaws\.com$/, `.${this.opts.region}.amazonaws.com`);
+    }
+    return this.service;
+  }
+
+  /**
+   * Do a single lookup
+   */
+  private legacyBehavior(ctx: cdk.IResolveContext) {
     if (this.opts.region) {
       // Special case, handle it separately to not break legacy behavior.
       return RegionInfo.get(this.opts.region).servicePrincipal(this.service) ??

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

@@ -1,6 +1,8 @@
 import { Template } from '@aws-cdk/assertions';
 import { App, CfnOutput, Stack } from '@aws-cdk/core';
+import * as cxapi from '@aws-cdk/cx-api';
 import * as iam from '../lib';
+import { ServicePrincipal } from '../lib';
 
 test('use of cross-stack role reference does not lead to URLSuffix being exported', () => {
   // GIVEN
@@ -307,34 +309,67 @@ test('AccountPrincipal can specify an organization', () => {
   });
 });
 
-test('ServicePrincipalName returns just a string representing the principal', () => {
-  // GIVEN
-  const usEastStack = new Stack(undefined, undefined, { env: { region: 'us-east-1' } });
-  const afSouthStack = new Stack(undefined, undefined, { env: { region: 'af-south-1' } });
-  const principalName = iam.ServicePrincipal.servicePrincipalName('states.amazonaws.com');
+describe('deprecated ServicePrincipal behavior', () => {
+  // This behavior makes use of deprecated region-info lookup tables
 
-  expect(usEastStack.resolve(principalName)).toEqual('states.us-east-1.amazonaws.com');
-  expect(afSouthStack.resolve(principalName)).toEqual('states.af-south-1.amazonaws.com');
-});
+  test('ServicePrincipalName returns just a string representing the principal', () => {
+    // GIVEN
+    const usEastStack = new Stack(undefined, undefined, { env: { region: 'us-east-1' } });
+    const afSouthStack = new Stack(undefined, undefined, { env: { region: 'af-south-1' } });
+    const principalName = iam.ServicePrincipal.servicePrincipalName('states.amazonaws.com');
+
+    expect(usEastStack.resolve(principalName)).toEqual('states.us-east-1.amazonaws.com');
+    expect(afSouthStack.resolve(principalName)).toEqual('states.af-south-1.amazonaws.com');
+  });
+
+  test('Passing non-string as accountId parameter in AccountPrincipal constructor should throw error', () => {
+    expect(() => new iam.AccountPrincipal(1234)).toThrowError('accountId should be of type string');
+  });
+
+  test('ServicePrincipal in agnostic stack generates lookup table', () => {
+    // GIVEN
+    const stack = new Stack();
+
+    // WHEN
+    new iam.Role(stack, 'Role', {
+      assumedBy: new iam.ServicePrincipal('states.amazonaws.com'),
+    });
 
-test('Passing non-string as accountId parameter in AccountPrincipal constructor should throw error', () => {
-  expect(() => new iam.AccountPrincipal(1234)).toThrowError('accountId should be of type string');
+    // THEN
+    const template = Template.fromStack(stack);
+    const mappings = template.findMappings('ServiceprincipalMap');
+    expect(mappings.ServiceprincipalMap['af-south-1']?.states).toEqual('states.af-south-1.amazonaws.com');
+    expect(mappings.ServiceprincipalMap['us-east-1']?.states).toEqual('states.us-east-1.amazonaws.com');
+  });
 });
 
-test('ServicePrincipal in agnostic stack generates lookup table', () => {
-  // GIVEN
-  const stack = new Stack();
+describe('standardized Service Principal behavior', () => {
+  const agnosticStatesPrincipal = new ServicePrincipal('states.amazonaws.com');
+  const afSouth1StatesPrincipal = new ServicePrincipal('states.amazonaws.com', { region: 'af-south-1' });
+  // af-south-1 is an opt-in region
 
-  // WHEN
-  new iam.Role(stack, 'Role', {
-    assumedBy: new iam.ServicePrincipal('states.amazonaws.com'),
+  let app: App;
+  beforeEach(() => {
+    app = new App({
+      postCliContext: { [cxapi.IAM_STANDARDIZED_SERVICE_PRINCIPALS]: true },
+    });
+  });
+
+  test('no more regional service principals by default', () => {
+    const stack = new Stack(app, 'Stack', { env: { region: 'us-east-1' } });
+    expect(stack.resolve(agnosticStatesPrincipal.policyFragment).principalJson).toEqual({ Service: ['states.amazonaws.com'] });
+  });
+
+  test('regional service principal is added for cross-region reference to opt-in region', () => {
+    const stack = new Stack(app, 'Stack', { env: { region: 'us-east-1' } });
+    expect(stack.resolve(afSouth1StatesPrincipal.policyFragment).principalJson).toEqual({ Service: ['states.af-south-1.amazonaws.com'] });
+  });
+
+  test('regional service principal is not added for same-region reference in opt-in region', () => {
+    const stack = new Stack(app, 'Stack', { env: { region: 'af-south-1' } });
+    expect(stack.resolve(afSouth1StatesPrincipal.policyFragment).principalJson).toEqual({ Service: ['states.amazonaws.com'] });
   });
 
-  // THEN
-  const template = Template.fromStack(stack);
-  const mappings = template.findMappings('ServiceprincipalMap');
-  expect(mappings.ServiceprincipalMap['af-south-1']?.states).toEqual('states.af-south-1.amazonaws.com');
-  expect(mappings.ServiceprincipalMap['us-east-1']?.states).toEqual('states.us-east-1.amazonaws.com');
 });
 
 test('Can enable session tags', () => {

packages/@aws-cdk/cx-api/FEATURE_FLAGS.md

@@ -37,6 +37,7 @@ Flags come in three types:
 | @aws-cdk/aws-apigateway:disableCloudWatchRole | Make default CloudWatch Role behavior safe for multiple API Gateways in one environment | 2.38.0 | (fix) | `true` |
 | @aws-cdk/core:enablePartitionLiterals | Make ARNs concrete if AWS partition is known | 2.38.0 | (fix) | `true` |
 | @aws-cdk/aws-events:eventsTargetQueueSameAccount | Event Rules may only push to encrypted SQS queues in the same account | 2.51.0 | (fix) | `true` |
+| @aws-cdk/aws-iam:standardizedServicePrincipals | Use standardized (global) service principals everywhere | 2.51.0 | (fix) | `true` |
 
 <!-- END table -->
 
@@ -300,11 +301,22 @@ Introduced in **2.38.0**, recommended value `true`.
 Event Rules may only push to encrypted SQS queues in the same account (fix)
 
 This flag applies to SQS Queues that are used as the target of event Rules. When enabled, only principals
-from the same account as the Queue can send messages. If a queue is unencrypted, this restriction will
+from the same account as the Rule can send messages. If a queue is unencrypted, this restriction will
 always apply, regardless of the value of this flag.
 
 Introduced in **2.51.0**, recommended value `true`.
 
+### @aws-cdk/aws-iam:standardizedServicePrincipals
+
+Use standardized (global) service principals everywhere (fix)
+
+We used to maintain a database of exceptions to Service Principal names in various regions. This database
+is no longer necessary: all service principals names have been standardized to their global form (`SERVICE.amazonaws.com`).
+
+This flag disables use of that exceptions database and always uses the global service principal.
+
+Introduced in **2.51.0**, recommended value `true`.
+
 <!-- END details -->
 
 ## Currently recommended cdk.json
@@ -331,7 +343,8 @@ The following json shows the current recommended set of flags, as `cdk init` wou
     "@aws-cdk/aws-sns-subscriptions:restrictSqsDescryption": true,
     "@aws-cdk/aws-apigateway:disableCloudWatchRole": true,
     "@aws-cdk/core:enablePartitionLiterals": true,
-    "@aws-cdk/aws-events:eventsTargetQueueSameAccount": true
+    "@aws-cdk/aws-events:eventsTargetQueueSameAccount": true,
+    "@aws-cdk/aws-iam:standardizedServicePrincipals": true
   }
 }
 ```

packages/@aws-cdk/cx-api/lib/features.ts

@@ -59,6 +59,7 @@ export const SNS_SUBSCRIPTIONS_SQS_DECRYPTION_POLICY = '@aws-cdk/aws-sns-subscri
 export const APIGATEWAY_DISABLE_CLOUDWATCH_ROLE = '@aws-cdk/aws-apigateway:disableCloudWatchRole';
 export const ENABLE_PARTITION_LITERALS = '@aws-cdk/core:enablePartitionLiterals';
 export const EVENTS_TARGET_QUEUE_SAME_ACCOUNT = '@aws-cdk/aws-events:eventsTargetQueueSameAccount';
+export const IAM_STANDARDIZED_SERVICE_PRINCIPALS = '@aws-cdk/aws-iam:standardizedServicePrincipals';
 
 export const FLAGS: Record<string, FlagInfo> = {
   //////////////////////////////////////////////////////////////////////
@@ -504,6 +505,20 @@ export const FLAGS: Record<string, FlagInfo> = {
     introducedIn: { v2: '2.51.0' },
     recommendedValue: true,
   },
+
+  //////////////////////////////////////////////////////////////////////
+  [IAM_STANDARDIZED_SERVICE_PRINCIPALS]: {
+    type: FlagType.BugFix,
+    summary: 'Use standardized (global) service principals everywhere',
+    details: `
+      We used to maintain a database of exceptions to Service Principal names in various regions. This database
+      is no longer necessary: all service principals names have been standardized to their global form (\`SERVICE.amazonaws.com\`).
+
+      This flag disables use of that exceptions database and always uses the global service principal.
+      `,
+    introducedIn: { v2: '2.51.0' },
+    recommendedValue: true,
+  },
 };
 
 const CURRENT_MV = 'v2';

packages/@aws-cdk/region-info/build-tools/generate-static-data.ts

@@ -5,6 +5,7 @@ import {
   AWS_SERVICES,
   before,
   RULE_S3_WEBSITE_REGIONAL_SUBDOMAIN,
+  RULE_CLASSIC_PARTITION_BECOMES_OPT_IN,
 } from '../lib/aws-entities';
 import { Default } from '../lib/default';
 import {
@@ -56,6 +57,8 @@ async function main(): Promise<void> {
 
     registerFact(region, 'CDK_METADATA_RESOURCE_AVAILABLE', AWS_CDK_METADATA.has(region) ? 'YES' : 'NO');
 
+    registerFact(region, 'IS_OPT_IN_REGION', partition === 'aws' && after(region, RULE_CLASSIC_PARTITION_BECOMES_OPT_IN) ? 'YES' : 'NO');
+
     registerFact(region, 'S3_STATIC_WEBSITE_ENDPOINT', before(region, RULE_S3_WEBSITE_REGIONAL_SUBDOMAIN)
       ? `s3-website-${region}.${domainSuffix}`
       : `s3-website.${region}.${domainSuffix}`);
@@ -133,6 +136,10 @@ function checkRegionsSubMap(map: Record<string, Record<string, Record<string, un
   }
 }
 
+export function after(region: string, ruleOrRegion: string | symbol) {
+  return region !== ruleOrRegion && !before(region, ruleOrRegion);
+}
+
 main().catch(e => {
   // eslint-disable-next-line no-console
   console.error(e);

packages/@aws-cdk/region-info/lib/aws-entities.ts

@@ -5,6 +5,11 @@
  */
 export const RULE_S3_WEBSITE_REGIONAL_SUBDOMAIN = Symbol('S3_WEBSITE_REGIONAL_SUBDOMAIN');
 
+/**
+ * After this point, all regions in the 'aws' partition are opt-in.
+ */
+export const RULE_CLASSIC_PARTITION_BECOMES_OPT_IN = Symbol('CLASSIC_PARTITION_BECOMES_OPT_IN');
+
 /**
  * List of AWS region, ordered by launch date (oldest to newest)
  *
@@ -44,13 +49,15 @@ export const AWS_REGIONS_AND_RULES: readonly (string | symbol)[] = [
   'ap-northeast-3', // Asia Pacific (Osaka)
   'us-gov-east-1', // AWS GovCloud (US-East)
   'eu-north-1', // Europe (Stockholm)
+  RULE_CLASSIC_PARTITION_BECOMES_OPT_IN,
   'ap-east-1', // Asia Pacific (Hong Kong)
   'me-south-1', // Middle East (Bahrain)
   'eu-south-1', // Europe (Milan)
   'af-south-1', // Africa (Cape Town)
   'us-iso-west-1', // US ISO West
   'eu-south-2', // Europe (Spain)
   'ap-southeast-3', // Asia Pacific (Jakarta)
+  'me-central-1', // Middle East (UAE)
 ];
 
 /**

packages/@aws-cdk/region-info/lib/default.ts

@@ -21,6 +21,14 @@ export class Default {
    * @param urlSuffix deprecated and ignored.
    */
   public static servicePrincipal(serviceFqn: string, region: string, urlSuffix: string): string {
+    // NOTE: this whole method is deprecated, and should not be used or updated anymore. The global service
+    // principal is always correct, when referenced from within a region.
+    // (As a note, regional principals (`<SERVICE>.<REGION>.amazonaws.com`) are required in
+    // case of a cross-region reference to an opt-in region, but that's the only case, and that is not
+    // controlled here).
+    //
+    // (It cannot be actually @deprecated since many of our tests use it :D)
+
     const serviceName = extractSimpleName(serviceFqn);
     if (!serviceName) {
       // Return "service" if it does not look like any of the following:

packages/@aws-cdk/region-info/lib/fact.ts

@@ -121,6 +121,12 @@ export class FactName {
    */
   public static readonly CDK_METADATA_RESOURCE_AVAILABLE = 'cdk:metadata-resource:available';
 
+  /**
+   * Whether the given region is an opt-in region or not. The value is a boolean
+   * modelled as `YES` or `NO`.
+   */
+  public static readonly IS_OPT_IN_REGION = 'aws:is-opt-in-region';
+
   /**
    * The endpoint used for hosting S3 static websites
    */

packages/@aws-cdk/region-info/lib/region-info.ts

@@ -70,6 +70,13 @@ export class RegionInfo {
     return Fact.find(this.name, FactName.CDK_METADATA_RESOURCE_AVAILABLE) === 'YES';
   }
 
+  /**
+   * Whether the given region is an opt-in region
+   */
+  public get isOptInRegion(): boolean {
+    return Fact.find(this.name, FactName.IS_OPT_IN_REGION) === 'YES';
+  }
+
   /**
    * The domain name suffix (e.g: amazonaws.com) for this region.
    */

packages/@aws-cdk/region-info/test/default.test.ts

@@ -81,4 +81,4 @@ describe('spot-check some service principals', () => {
   function expectServicePrincipals(principal: string, regionMap: Record<string, string>) {
     expect(Object.fromEntries(Object.keys(regionMap).map(reg => [reg, Default.servicePrincipal(principal, reg, 'EXTENSION')]))).toEqual(regionMap);
   }
-});
+});

packages/@aws-cdk/region-info/test/region-info.test.ts

@@ -53,3 +53,12 @@ test('limitedRegionMap only returns information for certain regions', () => {
   expect(map2['us-east-1']).not.toBeDefined();
   expect(map2['cn-north-1']).toBeDefined();
 });
+
+
+test.each([
+  ['us-east-1', false],
+  ['me-south-1', true],
+  ['us-iso-west-1', false],
+])('%p should be opt-in: %p', (region, expected) => {
+  expect(RegionInfo.get(region).isOptInRegion).toEqual(expected);
+});