fix(ssm): invalid parameter arn (#4685)

* fix(ssm): invalid parameter arn We were missing a "/" as a prefix to the parameter name when rendering parameter ARNs. Fixes #4672 * fix test
aws · Oct 25, 2019 · e26a36c · e26a36c
1 parent 012eeed
commit e26a36c
Show file tree

Hide file tree

Showing 5 changed files with 87 additions and 12 deletions.
diff --git a/packages/@aws-cdk/aws-ec2/test/test.instance.ts b/packages/@aws-cdk/aws-ec2/test/test.instance.ts
@@ -87,7 +87,7 @@ export = {
                   {
                     Ref: "AWS::AccountId"
                   },
-                  ":parameter",
+                  ":parameter/",
                   {
                     Ref: "Param165332EC"
                   }

diff --git a/packages/@aws-cdk/aws-ssm/lib/parameter.ts b/packages/@aws-cdk/aws-ssm/lib/parameter.ts
@@ -444,10 +444,18 @@ function makeIdentityForImportedValue(parameterName: string) {
 }
 
 function arnForParameterName(scope: IConstruct, parameterName: string): string {
+
+  // remove trailing "/" if we can resolve parameter name.
+  if (!Token.isUnresolved(parameterName)) {
+    if (parameterName.startsWith('/')) {
+      parameterName = parameterName.substr(1);
+    }
+  }
+
   return Stack.of(scope).formatArn({
     service: 'ssm',
     resource: 'parameter',
-    sep: '', // Sep is empty because this.parameterName starts with a / already!
+    sep: '/', // Sep is empty because this.parameterName starts with a / already!
     resourceName: 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
@@ -61,7 +61,7 @@
                     {
                       "Ref": "AWS::AccountId"
                     },
-                    ":parameter",
+                    ":parameter/",
                     {
                       "Ref": "StringParameter472EED0E"
                     }
@@ -113,6 +113,31 @@
           }
         ]
       }
+    },
+    "ParamArn": {
+      "Value": {
+        "Fn::Join": [
+          "",
+          [
+            "arn:",
+            {
+              "Ref": "AWS::Partition"
+            },
+            ":ssm:",
+            {
+              "Ref": "AWS::Region"
+            },
+            ":",
+            {
+              "Ref": "AWS::AccountId"
+            },
+            ":parameter/",
+            {
+              "Ref": "StringParameter472EED0E"
+            }
+          ]
+        ]
+      }
     }
   }
-}
+}
diff --git a/packages/@aws-cdk/aws-ssm/test/integ.parameter.lit.ts b/packages/@aws-cdk/aws-ssm/test/integ.parameter.lit.ts
@@ -34,4 +34,8 @@ new cdk.CfnOutput(stack, 'StringListOutput', {
   value: cdk.Fn.join('+', listParameter.stringListValue),
 });
 
+new cdk.CfnOutput(stack, 'ParamArn', {
+  value: param.parameterArn
+});
+
 app.synth();
diff --git a/packages/@aws-cdk/aws-ssm/test/test.parameter.ts b/packages/@aws-cdk/aws-ssm/test/test.parameter.ts
@@ -2,7 +2,7 @@ import { expect, haveResource } from '@aws-cdk/assert';
 import iam = require('@aws-cdk/aws-iam');
 import kms = require('@aws-cdk/aws-kms');
 import cdk = require('@aws-cdk/core');
-import { App, Stack } from '@aws-cdk/core';
+import { App, CfnParameter, Stack } from '@aws-cdk/core';
 import { Test } from 'nodeunit';
 import ssm = require('../lib');
 
@@ -123,7 +123,7 @@ export = {
         { Ref: 'AWS::Region' },
         ':',
         { Ref: 'AWS::AccountId' },
-        ':parameter',
+        ':parameter/',
         { Ref: 'Parameter9E1B4FBA' }
       ]]
     });
@@ -146,7 +146,7 @@ export = {
         { Ref: 'AWS::Region' },
         ':',
         { Ref: 'AWS::AccountId' },
-        ':parameterMyParamName' ] ]
+        ':parameter/MyParamName' ] ]
     });
     test.deepEqual(stack.resolve(param.parameterName), 'MyParamName');
     test.deepEqual(stack.resolve(param.parameterType), 'String');
@@ -181,7 +181,7 @@ export = {
         { Ref: 'AWS::Region' },
         ':',
         { Ref: 'AWS::AccountId' },
-        ':parameterMyParamName' ] ]
+        ':parameter/MyParamName' ] ]
     });
     test.deepEqual(stack.resolve(param.parameterName), 'MyParamName');
     test.deepEqual(stack.resolve(param.parameterType), 'String');
@@ -208,7 +208,7 @@ export = {
         { Ref: 'AWS::Region' },
         ':',
         { Ref: 'AWS::AccountId' },
-        ':parameterMyParamName' ] ]
+        ':parameter/MyParamName' ] ]
     });
     test.deepEqual(stack.resolve(param.parameterName), 'MyParamName');
     test.deepEqual(stack.resolve(param.parameterType), 'SecureString');
@@ -265,7 +265,7 @@ export = {
                   {
                     Ref: 'AWS::AccountId'
                   },
-                  ':parameterMyParamName'
+                  ':parameter/MyParamName'
                 ]
               ]
             }
@@ -326,7 +326,7 @@ export = {
                   {
                     Ref: 'AWS::AccountId'
                   },
-                  ':parameterMyParamName'
+                  ':parameter/MyParamName'
                 ]
               ]
             }
@@ -355,7 +355,7 @@ export = {
         { Ref: 'AWS::Region' },
         ':',
         { Ref: 'AWS::AccountId' },
-        ':parameterMyParamName' ] ]
+        ':parameter/MyParamName' ] ]
     });
     test.deepEqual(stack.resolve(param.parameterName), 'MyParamName');
     test.deepEqual(stack.resolve(param.parameterType), 'StringList');
@@ -445,5 +445,43 @@ export = {
 
       test.done();
     },
+  },
+
+  'rendering of parameter arns'(test: Test) {
+    const stack = new Stack();
+    const param = new CfnParameter(stack, 'param');
+    const expectedA = { 'Fn::Join': [ '', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter/bam' ] ] };
+    const expectedB = { 'Fn::Join': [ '', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter/', { Ref: 'param' } ] ] };
+    let i = 0;
+
+    // WHEN
+    const case1 = ssm.StringParameter.fromStringParameterName(stack, `p${i++}`, 'bam');
+    const case2 = ssm.StringParameter.fromStringParameterName(stack, `p${i++}`, '/bam');
+    const case3 = ssm.StringParameter.fromStringParameterName(stack, `p${i++}`, param.valueAsString);
+    const case4 = ssm.StringParameter.fromStringParameterAttributes(stack, `p${i++}`, { parameterName: 'bam' });
+    const case5 = ssm.StringParameter.fromStringParameterAttributes(stack, `p${i++}`, { parameterName: '/bam' });
+    const case6 = ssm.StringParameter.fromStringParameterAttributes(stack, `p${i++}`, { parameterName: param.valueAsString });
+    const case7 = ssm.StringParameter.fromSecureStringParameterAttributes(stack, `p${i++}`, { parameterName: 'bam', version: 10 });
+    const case8 = ssm.StringParameter.fromSecureStringParameterAttributes(stack, `p${i++}`, { parameterName: '/bam', version: 10 });
+    const case9 = ssm.StringParameter.fromSecureStringParameterAttributes(stack, `p${i++}`, { parameterName: param.valueAsString, version: 10 });
+    const case10 = new ssm.StringParameter(stack, `p${i++}`, { stringValue: 'value' });
+
+    // THEN
+    test.deepEqual(stack.resolve(case1.parameterArn), expectedA);
+    test.deepEqual(stack.resolve(case2.parameterArn), expectedA);
+    test.deepEqual(stack.resolve(case3.parameterArn), expectedB);
+    test.deepEqual(stack.resolve(case4.parameterArn), expectedA);
+    test.deepEqual(stack.resolve(case5.parameterArn), expectedA);
+    test.deepEqual(stack.resolve(case6.parameterArn), expectedB);
+    test.deepEqual(stack.resolve(case7.parameterArn), expectedA);
+    test.deepEqual(stack.resolve(case8.parameterArn), expectedA);
+    test.deepEqual(stack.resolve(case9.parameterArn), expectedB);
+    test.deepEqual(stack.resolve(case10.parameterArn), {
+      'Fn::Join': [ '', [
+        'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter/', { Ref: 'p97A508212' }
+      ]
+    ] });
+
+    test.done();
   }
 };