fix(ssm): Generate correct SSM Parameter ARN (#1726)

The SSM parameter name (`resourceName` portion of it's ARN) is required to start with a `/`, which caused the presence of a doubled `//` in the generated ARN caused by the fact `sep` defaults to `/`. This change allows `sep` to be set to an empty string, allowing SSM rendering of correct SSM parameter ARNs.
aws · Feb 12, 2019 · 39df456 · 39df456
1 parent 5c11680
commit 39df456
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 6 deletions.
diff --git a/packages/@aws-cdk/aws-ssm/lib/parameter.ts b/packages/@aws-cdk/aws-ssm/lib/parameter.ts
@@ -119,6 +119,7 @@ export abstract class ParameterBase extends cdk.Construct implements IParameter
     return cdk.Stack.find(this).formatArn({
       service: 'ssm',
       resource: 'parameter',
+      sep: '', // Sep is empty because this.parameterName starts with a / already!
       resourceName: this.parameterName,
     });
   }

diff --git a/packages/@aws-cdk/aws-ssm/test/integ.parameter.lit.expected.json b/packages/@aws-cdk/aws-ssm/test/integ.parameter.lit.expected.json
@@ -60,7 +60,7 @@
                     {
                       "Ref": "AWS::AccountId"
                     },
-                    ":parameter/",
+                    ":parameter",
                     {
                       "Ref": "StringParameter472EED0E"
                     }

diff --git a/packages/@aws-cdk/aws-ssm/test/test.parameter.ts b/packages/@aws-cdk/aws-ssm/test/test.parameter.ts
@@ -100,4 +100,25 @@ export = {
     }));
     test.done();
   },
+
+  'parameterArn is crafted correctly'(test: Test) {
+    // GIVEN
+    const stack = new cdk.Stack();
+    const param = new ssm.StringParameter(stack, 'Parameter', { value: 'Foo' });
+
+    // THEN
+    test.deepEqual(param.node.resolve(param.parameterArn), {
+      'Fn::Join': ['', [
+        'arn:',
+        { Ref: 'AWS::Partition' },
+        ':ssm:',
+        { Ref: 'AWS::Region' },
+        ':',
+        { Ref: 'AWS::AccountId' },
+        ':parameter',
+        { Ref: 'Parameter9E1B4FBA' }
+      ]]
+    });
+    test.done();
+  }
 };
diff --git a/packages/@aws-cdk/cdk/lib/cloudformation/arn.ts b/packages/@aws-cdk/cdk/lib/cloudformation/arn.ts
@@ -13,7 +13,7 @@ import { Stack } from './stack';
  *
  * The ARN will be formatted as follows:
  *
- *   arn:{partition}:{service}:{region}:{account}:{resource}{sep}}{resource-name}
+ *   arn:{partition}:{service}:{region}:{account}:{resource}{sep}{resource-name}
  *
  * The required ARN pieces that are omitted will be taken from the stack that
  * the 'scope' is attached to. If all ARN pieces are supplied, the supplied scope
@@ -23,12 +23,12 @@ export function arnFromComponents(components: ArnComponents, stack: Stack): stri
   const partition = components.partition !== undefined ? components.partition : stack.partition;
   const region = components.region !== undefined ? components.region : stack.region;
   const account = components.account !== undefined ? components.account : stack.accountId;
+  const sep = components.sep !== undefined ? components.sep : '/';
 
   const values = [ 'arn', ':', partition, ':', components.service, ':', region, ':', account, ':', components.resource ];
 
-  const sep = components.sep || '/';
-  if (sep !== '/' && sep !== ':') {
-    throw new Error('resourcePathSep may only be ":" or "/"');
+  if (sep !== '/' && sep !== ':' && sep !== '') {
+    throw new Error('resourcePathSep may only be ":", "/" or an empty string');
   }
 
   if (components.resourceName != null) {
@@ -257,7 +257,7 @@ export interface ArnComponents {
   /**
    * Separator between resource type and the resource.
    *
-   * Can be either '/' or ':'. Will only be used if path is defined.
+   * Can be either '/', ':' or an empty string. Will only be used if resourceName is defined.
    * @default '/'
    */
   sep?: string;

diff --git a/packages/@aws-cdk/cdk/test/cloudformation/test.arn.ts b/packages/@aws-cdk/cdk/test/cloudformation/test.arn.ts
@@ -68,6 +68,23 @@ export = {
     test.done();
   },
 
+  'resourcePathSep can be set to "" instead of the default "/"'(test: Test) {
+    const stack = new Stack();
+
+    const arn = stack.formatArn({
+      service: 'ssm',
+      resource: 'parameter',
+      sep: '',
+      resourceName: '/parameter-name'
+    });
+
+    const pseudo = new Aws(stack);
+
+    test.deepEqual(stack.node.resolve(arn),
+                   stack.node.resolve(`arn:${pseudo.partition}:ssm:${pseudo.region}:${pseudo.accountId}:parameter/parameter-name`));
+    test.done();
+  },
+
   'fails if resourcePathSep is neither ":" nor "/"'(test: Test) {
     const stack = new Stack();