Merge branch 'main' into 29362-updateGrantDataApiAccess

packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts

@@ -37,7 +37,6 @@ const DIFF_HANDLERS: HandlerRegistry = {
  * @param currentTemplate the current state of the stack.
  * @param newTemplate     the target state of the stack.
  * @param changeSet       the change set for this stack.
- * @param isImport        if the stack is importing resources (a migrate stack).
  *
  * @returns a +types.TemplateDiff+ object that represents the changes that will happen if
  *      a stack which current state is described by +currentTemplate+ is updated with
@@ -47,7 +46,6 @@ export function fullDiff(
   currentTemplate: { [key: string]: any },
   newTemplate: { [key: string]: any },
   changeSet?: CloudFormation.DescribeChangeSetOutput,
-  isImport?: boolean,
 ): types.TemplateDiff {
 
   normalize(currentTemplate);
@@ -57,9 +55,6 @@ export function fullDiff(
     filterFalsePositives(theDiff, changeSet);
     addImportInformation(theDiff, changeSet);
   }
-  if (isImport) {
-    addImportInformation(theDiff);
-  }
 
   return theDiff;
 }
@@ -214,25 +209,13 @@ function deepCopy(x: any): any {
   return x;
 }
 
-/**
- * Sets import flag to true for resource imports.
- * When the changeset parameter is not set, the stack is a new migrate stack,
- * so all resource changes are imports.
- */
-function addImportInformation(diff: types.TemplateDiff, changeSet?: CloudFormation.DescribeChangeSetOutput) {
-  if (changeSet) {
-    const imports = findResourceImports(changeSet);
-    diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => {
-      if (imports.includes(logicalId)) {
-        change.isImport = true;
-      }
-    });
-  } else {
-    diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => {
-      logicalId; // dont know how to get past warning that this variable is not used.
+function addImportInformation(diff: types.TemplateDiff, changeSet: CloudFormation.DescribeChangeSetOutput) {
+  const imports = findResourceImports(changeSet);
+  diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => {
+    if (imports.includes(logicalId)) {
       change.isImport = true;
-    });
-  }
+    }
+  });
 }
 
 function filterFalsePositives(diff: types.TemplateDiff, changeSet: CloudFormation.DescribeChangeSetOutput) {

packages/aws-cdk-lib/aws-kms/lib/key.ts

@@ -262,6 +262,12 @@ abstract class KeyBase extends Resource implements IKey {
     }
     const bucketStack = Stack.of(this);
     const identityStack = Stack.of(grantee.grantPrincipal);
+
+    if (FeatureFlags.of(this).isEnabled(cxapi.KMS_REDUCE_CROSS_ACCOUNT_REGION_POLICY_SCOPE)) {
+      // if two compared stacks have the same region, this should return 'false' since it's from the
+      // same region; if two stacks have different region, then compare env.region
+      return bucketStack.region !== identityStack.region && this.env.region !== identityStack.region;
+    }
     return bucketStack.region !== identityStack.region;
   }
 
@@ -271,6 +277,12 @@ abstract class KeyBase extends Resource implements IKey {
     }
     const bucketStack = Stack.of(this);
     const identityStack = Stack.of(grantee.grantPrincipal);
+
+    if (FeatureFlags.of(this).isEnabled(cxapi.KMS_REDUCE_CROSS_ACCOUNT_REGION_POLICY_SCOPE)) {
+      // if two compared stacks have the same region, this should return 'false' since it's from the
+      // same region; if two stacks have different region, then compare env.account
+      return bucketStack.account !== identityStack.account && this.env.account !== identityStack.account;
+    }
     return bucketStack.account !== identityStack.account;
   }
 }

packages/aws-cdk-lib/aws-kms/test/key.test.ts

@@ -2,6 +2,7 @@ import { describeDeprecated } from '@aws-cdk/cdk-build-tools';
 import { Match, Template } from '../../assertions';
 import * as iam from '../../aws-iam';
 import * as cdk from '../../core';
+import * as cxapi from '../../cx-api';
 import * as kms from '../lib';
 import { KeySpec, KeyUsage } from '../lib';
 
@@ -81,6 +82,66 @@ describe('key policies', () => {
     });
   });
 
+  test('cross region key with iam role grant', () => {
+    const app = new cdk.App({ context: { [cxapi.KMS_REDUCE_CROSS_ACCOUNT_REGION_POLICY_SCOPE]: true } });
+    const stack = new cdk.Stack(app, 'test-stack', { env: { account: '000000000000', region: 'us-west-2' } });
+    const key = kms.Key.fromKeyArn(
+      stack,
+      'Key',
+      'arn:aws:kms:eu-north-1:000000000000:key/e3ab59e5-3dc3-4bc4-9c3f-c790231d2287',
+    );
+
+    const roleStack = new cdk.Stack(app, 'RoleStack', {
+      env: { account: '000000000000', region: 'eu-north-1' },
+    });
+    const role = new iam.Role(roleStack, 'Role', {
+      assumedBy: new iam.AccountPrincipal('000000000000'),
+    });
+    key.grantEncryptDecrypt(role);
+
+    Template.fromStack(roleStack).hasResourceProperties('AWS::IAM::Policy', {
+      PolicyDocument: {
+        Statement: [
+          {
+            Effect: 'Allow',
+            Resource: 'arn:aws:kms:eu-north-1:000000000000:key/e3ab59e5-3dc3-4bc4-9c3f-c790231d2287',
+          },
+        ],
+        Version: '2012-10-17',
+      },
+    });
+  });
+
+  test('cross region key with iam role grant when feature flag is disabled', () => {
+    const app = new cdk.App({ context: { [cxapi.KMS_REDUCE_CROSS_ACCOUNT_REGION_POLICY_SCOPE]: false } });
+    const stack = new cdk.Stack(app, 'test-stack', { env: { account: '000000000000', region: 'us-west-2' } });
+    const key = kms.Key.fromKeyArn(
+      stack,
+      'Key',
+      'arn:aws:kms:eu-north-1:000000000000:key/e3ab59e5-3dc3-4bc4-9c3f-c790231d2287',
+    );
+
+    const roleStack = new cdk.Stack(app, 'RoleStack', {
+      env: { account: '000000000000', region: 'eu-north-1' },
+    });
+    const role = new iam.Role(roleStack, 'Role', {
+      assumedBy: new iam.AccountPrincipal('000000000000'),
+    });
+    key.grantEncryptDecrypt(role);
+
+    Template.fromStack(roleStack).hasResourceProperties('AWS::IAM::Policy', {
+      PolicyDocument: {
+        Statement: [
+          {
+            Effect: 'Allow',
+            Resource: '*',
+          },
+        ],
+        Version: '2012-10-17',
+      },
+    });
+  });
+
   test('can append to the default key policy', () => {
     const stack = new cdk.Stack();
     const statement = new iam.PolicyStatement({ resources: ['*'], actions: ['kms:Put*'] });

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

@@ -66,6 +66,7 @@ Flags come in three types:
 | [@aws-cdk/aws-cloudwatch-actions:changeLambdaPermissionLogicalIdForLambdaAction](#aws-cdkaws-cloudwatch-actionschangelambdapermissionlogicalidforlambdaaction) | When enabled, the logical ID of a Lambda permission for a Lambda action includes an alarm ID. | 2.124.0 | (fix) |
 | [@aws-cdk/aws-codepipeline:crossAccountKeysDefaultValueToFalse](#aws-cdkaws-codepipelinecrossaccountkeysdefaultvaluetofalse) | Enables Pipeline to set the default value for crossAccountKeys to false. | 2.127.0 | (default) |
 | [@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2](#aws-cdkaws-codepipelinedefaultpipelinetypetov2) | Enables Pipeline to set the default pipeline type to V2. | V2NEXT | (default) |
+| [@aws-cdk/aws-kms:reduceCrossAccountRegionPolicyScope](#aws-cdkaws-kmsreducecrossaccountregionpolicyscope) | When enabled, IAM Policy created from KMS key grant will reduce the resource scope to this key only. | V2NEXT | (fix) |
 
 <!-- END table -->
 
@@ -122,7 +123,8 @@ The following json shows the current recommended set of flags, as `cdk init` wou
     "@aws-cdk/aws-codepipeline-actions:useNewDefaultBranchForCodeCommitSource": true,
     "@aws-cdk/aws-cloudwatch-actions:changeLambdaPermissionLogicalIdForLambdaAction": true,
     "@aws-cdk/aws-codepipeline:crossAccountKeysDefaultValueToFalse": true,
-    "@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2": true
+    "@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2": true,
+    "@aws-cdk/aws-kms:reduceCrossAccountRegionPolicyScope": true
   }
 }
 ```
@@ -1249,4 +1251,18 @@ construct, the construct automatically defaults the value of this property to `P
 **Compatibility with old behavior:** Pass `pipelineType: PipelineType.V1` to `Pipeline` construct to restore the previous behavior.
 
 
+### @aws-cdk/aws-kms:reduceCrossAccountRegionPolicyScope
+
+*When enabled, IAM Policy created from KMS key grant will reduce the resource scope to this key only.* (fix)
+
+When this feature flag is enabled and calling KMS key grant method, the created IAM policy will reduce the resource scope from
+'*' to this specific granting KMS key.
+
+
+| Since | Default | Recommended |
+| ----- | ----- | ----- |
+| (not in v1) |  |  |
+| V2NEXT | `false` | `true` |
+
+
 <!-- END details -->

packages/aws-cdk-lib/cx-api/README.md

@@ -292,3 +292,20 @@ _cdk.json_
   }
 }
 ```
+
+* `@aws-cdk/aws-kms:reduceCrossAccountRegionPolicyScope`
+
+Reduce resource scope of the IAM Policy created from KMS key grant to granting key only.
+
+When this feature flag is enabled and calling KMS key grant method, the created IAM policy will reduce the resource scope from
+'*' to this specific granting KMS key.
+
+_cdk.json_
+
+```json
+{
+  "context": {
+    "@aws-cdk/aws-kms:reduceCrossAccountRegionPolicyScope": true
+  }
+}
+```

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

@@ -100,6 +100,7 @@ export const CODECOMMIT_SOURCE_ACTION_DEFAULT_BRANCH_NAME = '@aws-cdk/aws-codepi
 export const LAMBDA_PERMISSION_LOGICAL_ID_FOR_LAMBDA_ACTION = '@aws-cdk/aws-cloudwatch-actions:changeLambdaPermissionLogicalIdForLambdaAction';
 export const CODEPIPELINE_CROSS_ACCOUNT_KEYS_DEFAULT_VALUE_TO_FALSE = '@aws-cdk/aws-codepipeline:crossAccountKeysDefaultValueToFalse';
 export const CODEPIPELINE_DEFAULT_PIPELINE_TYPE_TO_V2 = '@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2';
+export const KMS_REDUCE_CROSS_ACCOUNT_REGION_POLICY_SCOPE = '@aws-cdk/aws-kms:reduceCrossAccountRegionPolicyScope';
 
 export const FLAGS: Record<string, FlagInfo> = {
   //////////////////////////////////////////////////////////////////////
@@ -1021,6 +1022,18 @@ export const FLAGS: Record<string, FlagInfo> = {
     recommendedValue: true,
     compatibilityWithOldBehaviorMd: 'Pass `pipelineType: PipelineType.V1` to `Pipeline` construct to restore the previous behavior.',
   },
+
+  //////////////////////////////////////////////////////////////////////
+  [KMS_REDUCE_CROSS_ACCOUNT_REGION_POLICY_SCOPE]: {
+    type: FlagType.BugFix,
+    summary: 'When enabled, IAM Policy created from KMS key grant will reduce the resource scope to this key only.',
+    detailsMd: `
+      When this feature flag is enabled and calling KMS key grant method, the created IAM policy will reduce the resource scope from
+      '*' to this specific granting KMS key.
+    `,
+    introducedIn: { v2: 'V2NEXT' },
+    recommendedValue: true,
+  },
 };
 
 const CURRENT_MV = 'v2';

packages/aws-cdk/lib/cdk-toolkit.ts

@@ -136,14 +136,7 @@ export class CdkToolkit {
         throw new Error(`There is no file at ${options.templatePath}`);
       }
 
-      const stackExistsOptions = {
-        stack: stacks.firstStack,
-        deployName: stacks.firstStack.stackName,
-      };
-
-      const stackExists = await this.props.deployments.stackExists(stackExistsOptions);
-
-      const changeSet = (stackExists && options.changeSet) ? await createDiffChangeSet({
+      const changeSet = options.changeSet ? await createDiffChangeSet({
         stack: stacks.firstStack,
         uuid: uuid.v4(),
         willExecute: false,
@@ -175,23 +168,13 @@ export class CdkToolkit {
           removeNonImportResources(stack);
         }
 
-        const stackExistsOptions = {
-          stack,
-          deployName: stack.stackName,
-        };
-
-        const stackExists = await this.props.deployments.stackExists(stackExistsOptions);
-
-        // if the stack does not already exist, do not do a changeset
-        // this prevents race conditions between deleting the dummy changeset stack and deploying the real changeset stack
-        // migrate stacks that import resources will not previously exist and default to old diff logic
-        const changeSet = (stackExists && options.changeSet) ? await createDiffChangeSet({
+        const changeSet = options.changeSet ? await createDiffChangeSet({
           stack,
           uuid: uuid.v4(),
           deployments: this.props.deployments,
           willExecute: false,
           sdkProvider: this.props.sdkProvider,
-          parameters: Object.assign({}, parameterMap['*'], parameterMap[stacks.firstStack.stackName]), // should this be stack?
+          parameters: Object.assign({}, parameterMap['*'], parameterMap[stacks.firstStack.stackName]),
           resourcesToImport,
           stream,
         }) : undefined;
@@ -200,11 +183,10 @@ export class CdkToolkit {
           stream.write('Parameters and rules created during migration do not affect resource configuration.\n');
         }
 
-        // pass a boolean to print if the stack is a migrate stack in order to set all resource diffs to import
         const stackCount =
         options.securityOnly
           ? (numberFromBool(printSecurityDiff(currentTemplate, stack, RequireApproval.Broadening, changeSet)))
-          : (printStackDiff(currentTemplate, stack, strict, contextLines, quiet, changeSet, stream, nestedStacks, !!resourcesToImport));
+          : (printStackDiff(currentTemplate, stack, strict, contextLines, quiet, changeSet, stream, nestedStacks));
 
         diffs += stackCount;
       }

packages/aws-cdk/lib/diff.ts

@@ -26,10 +26,9 @@ export function printStackDiff(
   quiet: boolean,
   changeSet?: CloudFormation.DescribeChangeSetOutput,
   stream: cfnDiff.FormatStream = process.stderr,
-  nestedStackTemplates?: { [nestedStackLogicalId: string]: NestedStackTemplates },
-  isImport?: boolean): number {
+  nestedStackTemplates?: { [nestedStackLogicalId: string]: NestedStackTemplates }): number {
 
-  let diff = cfnDiff.fullDiff(oldTemplate, newTemplate.template, changeSet, isImport);
+  let diff = cfnDiff.fullDiff(oldTemplate, newTemplate.template, changeSet);
 
   // detect and filter out mangled characters from the diff
   let filteredChangesCount = 0;