Skip to content

Commit

Permalink
Add feature flags
Browse files Browse the repository at this point in the history
  • Loading branch information
GavinZZ committed Mar 14, 2024
1 parent efa6690 commit 9c79ad7
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 8 deletions.
20 changes: 14 additions & 6 deletions packages/aws-cdk-lib/aws-kms/lib/key.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,9 +262,13 @@ abstract class KeyBase extends Resource implements IKey {
}
const bucketStack = Stack.of(this);
const identityStack = Stack.of(grantee.grantPrincipal);
// 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;

if (FeatureFlags.of(this).isEnabled(cxapi.KMS_CROSS_ACCOUNT_REGION_KMS_KEY_POLICY)) {
// 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;
}

private isGranteeFromAnotherAccount(grantee: iam.IGrantable): boolean {
Expand All @@ -273,9 +277,13 @@ abstract class KeyBase extends Resource implements IKey {
}
const bucketStack = Stack.of(this);
const identityStack = Stack.of(grantee.grantPrincipal);
// 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;

if (FeatureFlags.of(this).isEnabled(cxapi.KMS_CROSS_ACCOUNT_REGION_KMS_KEY_POLICY)) {
// 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;
}
}

Expand Down
33 changes: 32 additions & 1 deletion packages/aws-cdk-lib/aws-kms/test/key.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -82,7 +83,7 @@ describe('key policies', () => {
});

test('cross region key with iam role grant', () => {
const app = new cdk.App();
const app = new cdk.App({ context: { [cxapi.KMS_CROSS_ACCOUNT_REGION_KMS_KEY_POLICY]: true } });
const stack = new cdk.Stack(app, 'test-stack', { env: { account: '000000000000', region: 'us-west-2' } });
const key = kms.Key.fromKeyArn(
stack,
Expand Down Expand Up @@ -111,6 +112,36 @@ describe('key policies', () => {
});
});

test('cross region key with iam role grant when feature flag is disabled', () => {
const app = new cdk.App({ context: { [cxapi.KMS_CROSS_ACCOUNT_REGION_KMS_KEY_POLICY]: 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*'] });
Expand Down
18 changes: 17 additions & 1 deletion packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:crossAccountRegionKmsKeyPolicy](#aws-cdkaws-kmscrossaccountregionkmskeypolicy) | When enabled, KMS key grant should create policy with only one resource. | V2NEXT | (fix) |

<!-- END table -->

Expand Down Expand Up @@ -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:crossAccountRegionKmsKeyPolicy": true
}
}
```
Expand Down Expand Up @@ -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:crossAccountRegionKmsKeyPolicy

*When enabled, KMS key grant should create policy with only one resource.* (fix)

When this feature flag is enabled and calling KMS key grant method, the created IAM policy should correctly resolve to this
granting KMS key instead of a * resource property.


| Since | Default | Recommended |
| ----- | ----- | ----- |
| (not in v1) | | |
| V2NEXT | `false` | `true` |


<!-- END details -->
17 changes: 17 additions & 0 deletions packages/aws-cdk-lib/cx-api/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -292,3 +292,20 @@ _cdk.json_
}
}
```

* `@aws-cdk/aws-kms:crossAccountRegionKmsKeyPolicy`

Enables KMS key grant to correctly set 'Resoruce' property of IAM policy to the key itself.

When this feature flag is enabled and calling KMS key grant method, the created IAM policy should correctly resolve to this
granting KMS key instead of a * resource property.

_cdk.json_

```json
{
"context": {
"@aws-cdk/aws-kms:crossAccountRegionKmsKeyPolicy": true
}
}
```
13 changes: 13 additions & 0 deletions packages/aws-cdk-lib/cx-api/lib/features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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_CROSS_ACCOUNT_REGION_KMS_KEY_POLICY = '@aws-cdk/aws-kms:crossAccountRegionKmsKeyPolicy';

export const FLAGS: Record<string, FlagInfo> = {
//////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -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_CROSS_ACCOUNT_REGION_KMS_KEY_POLICY]: {
type: FlagType.BugFix,
summary: 'When enabled, KMS key grant should create policy with only one resource.',
detailsMd: `
When this feature flag is enabled and calling KMS key grant method, the created IAM policy should correctly resolve to this
granting KMS key instead of a * resource property.
`,
introducedIn: { v2: 'V2NEXT' },
recommendedValue: true,
},
};

const CURRENT_MV = 'v2';
Expand Down

0 comments on commit 9c79ad7

Please sign in to comment.