fix(kms): allow multiple addAlias calls on single key (aws#3596)

* fix(kms): allow multiple addAlias calls on single key * Add prefix+constructor feature, fix tests
sjagvi · Aug 9, 2019 · 54f8ea9 · 54f8ea9
1 parent 4681d01
commit 54f8ea9
Show file tree

Hide file tree

Showing 7 changed files with 182 additions and 31 deletions.
diff --git a/packages/@aws-cdk/aws-kms/lib/alias.ts b/packages/@aws-cdk/aws-kms/lib/alias.ts
@@ -81,26 +81,28 @@ export class Alias extends AliasBase {
   constructor(scope: Construct, id: string, props: AliasProps) {
     super(scope, id);
 
-    if (!Token.isUnresolved(props.aliasName)) {
-      if (!props.aliasName.startsWith(REQUIRED_ALIAS_PREFIX)) {
-        throw new Error(`Alias must start with the prefix "${REQUIRED_ALIAS_PREFIX}": ${props.aliasName}`);
+    let aliasName = props.aliasName;
+
+    if (!Token.isUnresolved(aliasName)) {
+      if (!aliasName.startsWith(REQUIRED_ALIAS_PREFIX)) {
+        aliasName = REQUIRED_ALIAS_PREFIX + aliasName;
       }
 
-      if (props.aliasName === REQUIRED_ALIAS_PREFIX) {
-        throw new Error(`Alias must include a value after "${REQUIRED_ALIAS_PREFIX}": ${props.aliasName}`);
+      if (aliasName === REQUIRED_ALIAS_PREFIX) {
+        throw new Error(`Alias must include a value after "${REQUIRED_ALIAS_PREFIX}": ${aliasName}`);
       }
 
-      if (props.aliasName.startsWith(DISALLOWED_PREFIX)) {
-        throw new Error(`Alias cannot start with ${DISALLOWED_PREFIX}: ${props.aliasName}`);
+      if (aliasName.startsWith(DISALLOWED_PREFIX)) {
+        throw new Error(`Alias cannot start with ${DISALLOWED_PREFIX}: ${aliasName}`);
       }
 
-      if (!props.aliasName.match(/^[a-zA-Z0-9:/_-]{1,256}$/)) {
+      if (!aliasName.match(/^[a-zA-Z0-9:/_-]{1,256}$/)) {
         throw new Error(`Alias name must be between 1 and 256 characters in a-zA-Z0-9:/_-`);
       }
     }
 
     const resource = new CfnAlias(this, 'Resource', {
-      aliasName: props.aliasName,
+      aliasName,
       targetKeyId: props.targetKey.keyArn
     });
 

diff --git a/packages/@aws-cdk/aws-kms/lib/key.ts b/packages/@aws-cdk/aws-kms/lib/key.ts
@@ -68,7 +68,7 @@ abstract class KeyBase extends Resource implements IKey {
    * Defines a new alias for the key.
    */
   public addAlias(alias: string): Alias {
-    return new Alias(this, 'Alias', { aliasName: alias, targetKey: this });
+    return new Alias(this, `Alias${alias}`, { aliasName: alias, targetKey: this });
   }
 
   /**
@@ -151,6 +151,15 @@ export interface KeyProps {
    */
   readonly description?: string;
 
+  /**
+   * Initial alias to add to the key
+   *
+   * More aliases can be added later by calling `addAlias`.
+   *
+   * @default - No alias is added for the key.
+   */
+  readonly alias?: string;
+
   /**
    * Indicates whether AWS KMS rotates the key.
    *
@@ -226,6 +235,10 @@ export class Key extends KeyBase {
 
     this.keyArn = resource.attrArn;
     resource.applyRemovalPolicy(props.removalPolicy);
+
+    if (props.alias !== undefined) {
+      this.addAlias(props.alias);
+    }
   }
 
   /**

diff --git a/packages/@aws-cdk/aws-kms/test/integ.key-sharing.lit.expected.json b/packages/@aws-cdk/aws-kms/test/integ.key-sharing.lit.expected.json
@@ -47,22 +47,35 @@
             "Version": "2012-10-17"
           }
         },
-        "DeletionPolicy": "Delete",
-        "UpdateReplacePolicy": "Delete"
-      },
-      "MyKeyAlias1B45D9DA": {
+        "UpdateReplacePolicy": "Delete",
+        "DeletionPolicy": "Delete"
+      }
+    },
+    "Outputs": {
+      "ExportsOutputFnGetAttMyKey6AB29FA6Arn4FA82736": {
+        "Value": {
+          "Fn::GetAtt": [
+            "MyKey6AB29FA6",
+            "Arn"
+          ]
+        },
+        "Export": {
+          "Name": "KeyStack:ExportsOutputFnGetAttMyKey6AB29FA6Arn4FA82736"
+        }
+      }
+    }
+  },
+  {
+    "Resources": {
+      "Alias325C5727": {
         "Type": "AWS::KMS::Alias",
         "Properties": {
           "AliasName": "alias/foo",
           "TargetKeyId": {
-            "Fn::GetAtt": [
-              "MyKey6AB29FA6",
-              "Arn"
-            ]
+            "Fn::ImportValue": "KeyStack:ExportsOutputFnGetAttMyKey6AB29FA6Arn4FA82736"
           }
         }
       }
     }
-  },
-  {}
-]
+  }
+]
diff --git a/packages/@aws-cdk/aws-kms/test/integ.key-sharing.lit.ts b/packages/@aws-cdk/aws-kms/test/integ.key-sharing.lit.ts
@@ -31,7 +31,10 @@ class UseStack extends cdk.Stack {
     super(scope, id, props);
 
     // Use the IKey object here.
-    props.key.addAlias('alias/foo');
+    new kms.Alias(this, 'Alias', {
+      aliasName: 'alias/foo',
+      targetKey: props.key
+    });
   }
 }
 

diff --git a/packages/@aws-cdk/aws-kms/test/integ.key.expected.json b/packages/@aws-cdk/aws-kms/test/integ.key.expected.json
@@ -56,10 +56,10 @@
           "Version": "2012-10-17"
         }
       },
-      "DeletionPolicy": "Delete",
-      "UpdateReplacePolicy": "Delete"
+      "UpdateReplacePolicy": "Delete",
+      "DeletionPolicy": "Delete"
     },
-    "MyKeyAlias1B45D9DA": {
+    "MyKeyAliasaliasbarD0D50DB8": {
       "Type": "AWS::KMS::Alias",
       "Properties": {
         "AliasName": "alias/bar",
@@ -72,4 +72,4 @@
       }
     }
   }
-}
+}
diff --git a/packages/@aws-cdk/aws-kms/test/test.alias.ts b/packages/@aws-cdk/aws-kms/test/test.alias.ts
@@ -20,18 +20,41 @@ export = {
     test.done();
   },
 
-  'fails if alias name does\'t start with "alias/"'(test: Test) {
+  'add "alias/" prefix if not given.'(test: Test) {
     const app = new App();
     const stack = new Stack(app, 'Test');
 
-    const key = new Key(stack, 'MyKey', {
+    const key = new Key(stack, 'Key', {
       enableKeyRotation: true,
       enabled: false
     });
 
-    test.throws(() => new Alias(stack, 'Alias', {
+    new Alias(stack, 'Alias', {
       aliasName: 'foo',
       targetKey: key
+    });
+
+    expect(stack).to(haveResource('AWS::KMS::Alias', {
+      AliasName: 'alias/foo',
+      TargetKeyId: { 'Fn::GetAtt': [ 'Key961B73FD', 'Arn' ] }
+    }));
+
+    test.done();
+  },
+
+  'can create alias directly while creating the key'(test: Test) {
+    const app = new App();
+    const stack = new Stack(app, 'Test');
+
+    new Key(stack, 'Key', {
+      enableKeyRotation: true,
+      enabled: false,
+      alias: 'foo',
+    });
+
+    expect(stack).to(haveResource('AWS::KMS::Alias', {
+      AliasName: 'alias/foo',
+      TargetKeyId: { 'Fn::GetAtt': [ 'Key961B73FD', 'Arn' ] }
     }));
 
     test.done();

diff --git a/packages/@aws-cdk/aws-kms/test/test.key.ts b/packages/@aws-cdk/aws-kms/test/test.key.ts
@@ -311,7 +311,7 @@ export = {
           DeletionPolicy: "Retain",
           UpdateReplacePolicy: "Retain",
         },
-        MyKeyAlias1B45D9DA: {
+        MyKeyAliasaliasxoo9464EB1C: {
           Type: "AWS::KMS::Alias",
           Properties: {
             AliasName: "alias/xoo",
@@ -329,6 +329,103 @@ export = {
     test.done();
   },
 
+  'can run multiple addAlias'(test: Test) {
+    const app = new App();
+    const stack = new Stack(app, 'Test');
+
+    const key = new Key(stack, 'MyKey', {
+      enableKeyRotation: true,
+      enabled: false
+    });
+
+    const alias1 = key.addAlias('alias/alias1');
+    const alias2 = key.addAlias('alias/alias2');
+    test.ok(alias1.aliasName);
+    test.ok(alias2.aliasName);
+
+    expect(stack).toMatch({
+      Resources: {
+        MyKey6AB29FA6: {
+          Type: "AWS::KMS::Key",
+          Properties: {
+            EnableKeyRotation: true,
+            Enabled: false,
+            KeyPolicy: {
+              Statement: [
+                {
+                  Action: [
+                    "kms:Create*",
+                    "kms:Describe*",
+                    "kms:Enable*",
+                    "kms:List*",
+                    "kms:Put*",
+                    "kms:Update*",
+                    "kms:Revoke*",
+                    "kms:Disable*",
+                    "kms:Get*",
+                    "kms:Delete*",
+                    "kms:ScheduleKeyDeletion",
+                    "kms:CancelKeyDeletion",
+                    "kms:GenerateDataKey"
+                  ],
+                  Effect: "Allow",
+                  Principal: {
+                    AWS: {
+                      "Fn::Join": [
+                        "",
+                        [
+                          "arn:",
+                          {
+                            Ref: "AWS::Partition"
+                          },
+                          ":iam::",
+                          {
+                            Ref: "AWS::AccountId"
+                          },
+                          ":root"
+                        ]
+                      ]
+                    }
+                  },
+                  Resource: "*"
+                }
+              ],
+              Version: "2012-10-17"
+            }
+          },
+          DeletionPolicy: "Retain",
+          UpdateReplacePolicy: "Retain",
+        },
+        MyKeyAliasaliasalias1668D31D7: {
+          Type: "AWS::KMS::Alias",
+          Properties: {
+            AliasName: "alias/alias1",
+            TargetKeyId: {
+              "Fn::GetAtt": [
+                "MyKey6AB29FA6",
+                "Arn"
+              ]
+            }
+          }
+        },
+        MyKeyAliasaliasalias2EC56BD3E: {
+          Type: "AWS::KMS::Alias",
+          Properties: {
+            AliasName: "alias/alias2",
+            TargetKeyId: {
+              "Fn::GetAtt": [
+                "MyKey6AB29FA6",
+                "Arn"
+              ]
+            }
+          }
+        }
+      }
+    });
+
+    test.done();
+  },
+
   'grant decrypt on a key'(test: Test) {
     // GIVEN
     const stack = new Stack();
@@ -387,7 +484,7 @@ export = {
 
     expect(stack2).toMatch({
       Resources: {
-        MyKeyImportedAliasB1C5269F: {
+        MyKeyImportedAliasaliashelloD41FEB6C: {
           Type: "AWS::KMS::Alias",
           Properties: {
             AliasName: "alias/hello",