Skip to content

Commit

Permalink
chore(kms): convert kms tests from nodeunit to jest (#11911)
Browse files Browse the repository at this point in the history
Straight conversion from nodeunit to jest, with no changes, except:
* Fixed one error message with a typo
* Fixed a throws exception to pass (was erroneously passing before)
* Converted a few output-based tests to use `toHaveOutput` rather than
  synthesizing and doing a partial template match.

As always, each time I convert a module from nodeunit to jest I find at least
one broken test. Two tests this time!


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
njlynch authored Dec 7, 2020
1 parent da4ef80 commit e8c8d77
Show file tree
Hide file tree
Showing 11 changed files with 707 additions and 834 deletions.
3 changes: 2 additions & 1 deletion packages/@aws-cdk/aws-kms/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,5 @@ nyc.config.js
*.snk
!.eslintrc.js

junit.xml
junit.xml
!jest.config.js
3 changes: 2 additions & 1 deletion packages/@aws-cdk/aws-kms/.npmignore
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,5 @@ tsconfig.json
# exclude cdk artifacts
**/cdk.out
junit.xml
test/
test/
jest.config.js
2 changes: 2 additions & 0 deletions packages/@aws-cdk/aws-kms/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
const baseConfig = require('cdk-build-tools/config/jest.config');
module.exports = baseConfig;
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-kms/lib/alias.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ export class Alias extends AliasBase {
public readonly keyArn = Stack.of(this).formatArn({ service: 'kms', resource: aliasName });
public readonly keyId = aliasName;
public readonly aliasName = aliasName;
public get aliasTargetKey(): IKey { throw new Error('Cannot access aliasTargetKey on an Alias imnported by Alias.fromAliasName().'); }
public get aliasTargetKey(): IKey { throw new Error('Cannot access aliasTargetKey on an Alias imported by Alias.fromAliasName().'); }
public addAlias(_alias: string): Alias { throw new Error('Cannot call addAlias on an Alias imported by Alias.fromAliasName().'); }
public addToResourcePolicy(_statement: iam.PolicyStatement, _allowNoOp?: boolean): iam.AddToResourcePolicyResult {
return { statementAdded: false };
Expand Down
3 changes: 1 addition & 2 deletions packages/@aws-cdk/aws-kms/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
},
"cdk-build": {
"cloudformation": "AWS::KMS",
"jest": true,
"env": {
"AWSLINT_BASE_CONSTRUCT": "true"
}
Expand All @@ -73,11 +74,9 @@
"license": "Apache-2.0",
"devDependencies": {
"@aws-cdk/assert": "0.0.0",
"@types/nodeunit": "^0.0.31",
"cdk-build-tools": "0.0.0",
"cdk-integ-tools": "0.0.0",
"cfn2ts": "0.0.0",
"nodeunit": "^0.11.3",
"pkglint": "0.0.0"
},
"dependencies": {
Expand Down
220 changes: 220 additions & 0 deletions packages/@aws-cdk/aws-kms/test/alias.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,220 @@
import '@aws-cdk/assert/jest';
import { ArnPrincipal, PolicyStatement } from '@aws-cdk/aws-iam';
import { App, CfnOutput, Construct, Stack } from '@aws-cdk/core';
import { Alias } from '../lib/alias';
import { IKey, Key } from '../lib/key';

test('default alias', () => {
const app = new App();
const stack = new Stack(app, 'Test');
const key = new Key(stack, 'Key');

new Alias(stack, 'Alias', { targetKey: key, aliasName: 'alias/foo' });

expect(stack).toHaveResource('AWS::KMS::Alias', {
AliasName: 'alias/foo',
TargetKeyId: { 'Fn::GetAtt': ['Key961B73FD', 'Arn'] },
});
});

test('add "alias/" prefix if not given.', () => {
const app = new App();
const stack = new Stack(app, 'Test');

const key = new Key(stack, 'Key', {
enableKeyRotation: true,
enabled: false,
});

new Alias(stack, 'Alias', {
aliasName: 'foo',
targetKey: key,
});

expect(stack).toHaveResource('AWS::KMS::Alias', {
AliasName: 'alias/foo',
TargetKeyId: { 'Fn::GetAtt': ['Key961B73FD', 'Arn'] },
});
});

test('can create alias directly while creating the key', () => {
const app = new App();
const stack = new Stack(app, 'Test');

new Key(stack, 'Key', {
enableKeyRotation: true,
enabled: false,
alias: 'foo',
});

expect(stack).toHaveResource('AWS::KMS::Alias', {
AliasName: 'alias/foo',
TargetKeyId: { 'Fn::GetAtt': ['Key961B73FD', 'Arn'] },
});
});

test('fails if alias is "alias/" (and nothing more)', () => {
const app = new App();
const stack = new Stack(app, 'Test');

const key = new Key(stack, 'MyKey', {
enableKeyRotation: true,
enabled: false,
});

expect(() => new Alias(stack, 'Alias', {
aliasName: 'alias/',
targetKey: key,
})).toThrow(/Alias must include a value after/);
});

test('fails if alias contains illegal characters', () => {
const app = new App();
const stack = new Stack(app, 'Test');

const key = new Key(stack, 'MyKey', {
enableKeyRotation: true,
enabled: false,
});

expect(() => new Alias(stack, 'Alias', {
aliasName: 'alias/@Nope',
targetKey: key,
})).toThrow('a-zA-Z0-9:/_-');
});

test('fails if alias starts with "alias/aws/"', () => {
const app = new App();
const stack = new Stack(app, 'Test');

const key = new Key(stack, 'MyKey', {
enableKeyRotation: true,
enabled: false,
});

expect(() => new Alias(stack, 'Alias1', {
aliasName: 'alias/aws/',
targetKey: key,
})).toThrow(/Alias cannot start with alias\/aws\/: alias\/aws\//);

expect(() => new Alias(stack, 'Alias2', {
aliasName: 'alias/aws/Awesome',
targetKey: key,
})).toThrow(/Alias cannot start with alias\/aws\/: alias\/aws\/Awesome/);

expect(() => new Alias(stack, 'Alias3', {
aliasName: 'alias/AWS/awesome',
targetKey: key,
})).toThrow(/Alias cannot start with alias\/aws\/: alias\/AWS\/awesome/);
});

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

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

/* eslint-disable 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 cdk/no-core-construct */

expect(stack).toHaveOutput({
outputName: 'OutId',
outputValue: 'alias/myAlias',
});
expect(stack).toHaveOutput({
outputName: 'OutArn',
outputValue: {
'Fn::Join': ['', [
'arn:',
{ Ref: 'AWS::Partition' },
':kms:',
{ Ref: 'AWS::Region' },
':',
{ Ref: 'AWS::AccountId' },
':alias/myAlias',
]],
},
});
});

test('imported alias by name - can be used where a key is expected', () => {
const stack = new Stack();

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

/* eslint-disable 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 cdk/no-core-construct */

expect(stack).toHaveOutput({
outputName: 'OutId',
outputValue: 'alias/myAlias',
});
expect(stack).toHaveOutput({
outputName: 'OutArn',
outputValue: {
'Fn::Join': ['', [
'arn:',
{ Ref: 'AWS::Partition' },
':kms:',
{ Ref: 'AWS::Region' },
':',
{ Ref: 'AWS::AccountId' },
':alias/myAlias',
]],
},
});
});

test('imported alias by name - will throw an error when accessing the key', () => {
const stack = new Stack();

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

expect(() => myAlias.aliasTargetKey).toThrow('Cannot access aliasTargetKey on an Alias imported by Alias.fromAliasName().');
});

test('fails if alias policy is invalid', () => {
const app = new App();
const stack = new Stack(app, 'my-stack');
const key = new Key(stack, 'MyKey');
const alias = new Alias(stack, 'Alias', { targetKey: key, aliasName: 'alias/foo' });

alias.addToResourcePolicy(new PolicyStatement({
resources: ['*'],
principals: [new ArnPrincipal('arn')],
}));

expect(() => app.synth()).toThrow(/A PolicyStatement must specify at least one \'action\' or \'notAction\'/);
});
Loading

0 comments on commit e8c8d77

Please sign in to comment.