Skip to content

Commit

Permalink
fix(kms): aliasName references alias itself (under feature flag) (#25822
Browse files Browse the repository at this point in the history
)

Fixes #25761

The existing behavior was that `alias.aliasName` was always a raw string, so properties like `keyId` and `keyArn` depended on a raw string and not a reference to the alias itself. This behavior is preserved.

Under a new feature flag, `const KMS_ALIAS_NAME_REF = '@aws-cdk/aws-kms:aliasNameRef'`, we instead use a reference to the `aliasName` output itself, which means that properties that depend on `aliasName` now depend on the `alias`. In turn, the `alias` depends on the `key`. This allows the expected behavior where specifying something like `alias.keyArn()` depends on the key.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
kaizencc committed Jun 2, 2023
1 parent 674ec01 commit 45734e3
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 35 deletions.
2 changes: 0 additions & 2 deletions packages/aws-cdk-lib/aws-codepipeline/test/artifacts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,6 @@ describe('artifacts', () => {
});
});

/* eslint-disable @aws-cdk/no-core-construct */
function validate(construct: IConstruct): string[] {
try {
(construct.node.root as cdk.App).synth();
Expand All @@ -276,4 +275,3 @@ function validate(construct: IConstruct): string[] {
return err.message.split('\n').slice(1);
}
}
/* eslint-enable @aws-cdk/no-core-construct */
9 changes: 7 additions & 2 deletions packages/aws-cdk-lib/aws-kms/lib/alias.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import { Construct } from 'constructs';
import { IKey } from './key';
import { CfnAlias } from './kms.generated';
import * as iam from '../../aws-iam';
import { RemovalPolicy, Resource, Stack, Token, Tokenization } from '../../core';
import { FeatureFlags, RemovalPolicy, Resource, Stack, Token, Tokenization } from '../../core';
import { KMS_ALIAS_NAME_REF } from '../../cx-api';

const REQUIRED_ALIAS_PREFIX = 'alias/';
const DISALLOWED_PREFIX = REQUIRED_ALIAS_PREFIX + 'aws/';
Expand Down Expand Up @@ -224,7 +225,11 @@ export class Alias extends AliasBase {
targetKeyId: this.aliasTargetKey.keyArn,
});

this.aliasName = this.getResourceNameAttribute(resource.aliasName);
if (FeatureFlags.of(this).isEnabled(KMS_ALIAS_NAME_REF)) {
this.aliasName = this.getResourceNameAttribute(resource.ref);
} else {
this.aliasName = this.getResourceNameAttribute(resource.aliasName);
}

if (props.removalPolicy) {
resource.applyRemovalPolicy(props.removalPolicy);
Expand Down
71 changes: 41 additions & 30 deletions packages/aws-cdk-lib/aws-kms/test/alias.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Template } from '../../assertions';
import * as iam from '../../aws-iam';
import { ArnPrincipal, PolicyStatement } from '../../aws-iam';
import { App, Aws, CfnOutput, Stack } from '../../core';
import { KMS_ALIAS_NAME_REF } from '../../cx-api';
import { Alias } from '../lib/alias';
import { IKey, Key } from '../lib/key';

Expand Down Expand Up @@ -110,8 +111,34 @@ test('fails if alias starts with "alias/aws/"', () => {
})).toThrow(/Alias cannot start with alias\/aws\/: alias\/AWS\/awesome/);
});

test('keyId includes reference to alias under feature flag', () => {
// GIVEN
const stack = new Stack();
stack.node.setContext(KMS_ALIAS_NAME_REF, true);

const myKey = new Key(stack, 'MyKey', {
enableKeyRotation: true,
enabled: true,
});
const myAlias = new Alias(stack, 'MyAlias', {
targetKey: myKey,
aliasName: 'alias/myAlias',
});

// WHEN
new AliasOutputsConstruct(stack, 'AliasOutputsConstruct', myAlias);

// THEN - keyId includes reference to the alias itself
Template.fromStack(stack).hasOutput('OutId', {
Value: {
Ref: 'MyAlias9A08CB8C',
},
});
});

test('can be used wherever a key is expected', () => {
const stack = new Stack();
stack.node.setContext(KMS_ALIAS_NAME_REF, false);

const myKey = new Key(stack, 'MyKey', {
enableKeyRotation: true,
Expand All @@ -122,21 +149,7 @@ test('can be used wherever a key is expected', () => {
aliasName: 'alias/myAlias',
});

/* eslint-disable @aws-cdk/no-core-construct */
class MyConstruct extends Construct {
constructor(scope: Construct, id: string, key: IKey) {
super(scope, id);

new CfnOutput(stack, 'OutId', {
value: key.keyId,
});
new CfnOutput(stack, 'OutArn', {
value: key.keyArn,
});
}
}
new MyConstruct(stack, 'MyConstruct', myAlias);
/* eslint-enable @aws-cdk/no-core-construct */
new AliasOutputsConstruct(stack, 'AliasOutputsConstruct', myAlias);

Template.fromStack(stack).hasOutput('OutId', {
Value: 'alias/myAlias',
Expand All @@ -161,21 +174,7 @@ test('imported alias by name - can be used where a key is expected', () => {

const myAlias = Alias.fromAliasName(stack, 'MyAlias', 'alias/myAlias');

/* eslint-disable @aws-cdk/no-core-construct */
class MyConstruct extends Construct {
constructor(scope: Construct, id: string, key: IKey) {
super(scope, id);

new CfnOutput(stack, 'OutId', {
value: key.keyId,
});
new CfnOutput(stack, 'OutArn', {
value: key.keyArn,
});
}
}
new MyConstruct(stack, 'MyConstruct', myAlias);
/* eslint-enable @aws-cdk/no-core-construct */
new AliasOutputsConstruct(stack, 'AliasOutputsConstruct', myAlias);

Template.fromStack(stack).hasOutput('OutId', {
Value: 'alias/myAlias',
Expand Down Expand Up @@ -358,3 +357,15 @@ test('does not add alias if starts with token', () => {
});
});

class AliasOutputsConstruct extends Construct {
constructor(scope: Construct, id: string, key: IKey) {
super(scope, id);

new CfnOutput(scope, 'OutId', {
value: key.keyId,
});
new CfnOutput(scope, 'OutArn', {
value: key.keyArn,
});
}
}
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/core/lib/feature-flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as cxapi from '../../cx-api';
/**
* Features that are implemented behind a flag in order to preserve backwards
* compatibility for existing apps. The list of flags are available in the
* `@aws-cdk/cx-api` module.
* `aws-cdk-lib/cx-api` module.
*
* The state of the flag for this application is stored as a CDK context variable.
*/
Expand Down
16 changes: 16 additions & 0 deletions packages/aws-cdk-lib/cx-api/lib/features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ export const REDSHIFT_COLUMN_ID = '@aws-cdk/aws-redshift:columnId';
export const ENABLE_EMR_SERVICE_POLICY_V2 = '@aws-cdk/aws-stepfunctions-tasks:enableEmrServicePolicyV2';
export const EC2_RESTRICT_DEFAULT_SECURITY_GROUP = '@aws-cdk/aws-ec2:restrictDefaultSecurityGroup';
export const APIGATEWAY_REQUEST_VALIDATOR_UNIQUE_ID = '@aws-cdk/aws-apigateway:requestValidatorUniqueId';
export const KMS_ALIAS_NAME_REF = '@aws-cdk/aws-kms:aliasNameRef';

export const FLAGS: Record<string, FlagInfo> = {
//////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -788,6 +789,21 @@ export const FLAGS: Record<string, FlagInfo> = {
introducedIn: { v2: 'V2·NEXT' },
recommendedValue: true,
},

//////////////////////////////////////////////////////////////////////
[KMS_ALIAS_NAME_REF]: {
type: FlagType.BugFix,
summary: 'KMS Alias name and keyArn will have implicit reference to KMS Key',
detailsMd: `
This flag allows an implicit dependency to be created between KMS Alias and KMS Key
when referencing key.aliasName or key.keyArn.
If the flag is not set then a raw string is passed as the Alias name and no
implicit dependencies will be set.
`,
introducedIn: { v2: 'V2·NEXT' },
recommendedValue: true,
},
};

const CURRENT_MV = 'v2';
Expand Down

0 comments on commit 45734e3

Please sign in to comment.