From 2d96aefdf9e6ed3740c6a538a76139afe4545fe8 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Mon, 4 Nov 2019 17:42:16 +0200 Subject: [PATCH 1/5] fix(ssm): malformed ARNs for parameters with physical names that use path notation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SSM parameter names can have one of two forms: “simpleName” or “/path/name”. This makes it tricky to render an ARN for the parameter is the name is an unresolvable token (such as a “Ref”) because we can’t decide whether a “/“ separator is required in the ARN. The previous implementation assumed "Ref" always returns the name without a "/" prefix, and therefore did not use the "/" separator. This fix will use the physical name itself (if possible) to determine the separator (and also assume that generated names will not use the path notation). The only case where this is impossible is if the physical name is a token (either created or imported), in which case we should be able to synthesize a CloudFormation condition which will parse the token during deployment. This test also adds a validation that verifies that if a physical name is provided and uses path notation, it must begin with a "/". Misc: re-add `install.sh` to call `npx yarn install` --- packages/@aws-cdk/aws-ssm/lib/parameter.ts | 45 ++-- packages/@aws-cdk/aws-ssm/lib/util.ts | 71 ++++++ .../test/integ.parameter-arns.expected.json | 202 ++++++++++++++++++ .../aws-ssm/test/integ.parameter-arns.ts | 30 +++ .../@aws-cdk/aws-ssm/test/test.parameter.ts | 93 +++++--- packages/@aws-cdk/aws-ssm/test/test.util.ts | 87 ++++++++ 6 files changed, 479 insertions(+), 49 deletions(-) create mode 100644 packages/@aws-cdk/aws-ssm/lib/util.ts create mode 100644 packages/@aws-cdk/aws-ssm/test/integ.parameter-arns.expected.json create mode 100644 packages/@aws-cdk/aws-ssm/test/integ.parameter-arns.ts create mode 100644 packages/@aws-cdk/aws-ssm/test/test.util.ts diff --git a/packages/@aws-cdk/aws-ssm/lib/parameter.ts b/packages/@aws-cdk/aws-ssm/lib/parameter.ts index 4cbbabacdcffd..9a91d8d7474b2 100644 --- a/packages/@aws-cdk/aws-ssm/lib/parameter.ts +++ b/packages/@aws-cdk/aws-ssm/lib/parameter.ts @@ -2,10 +2,11 @@ import iam = require('@aws-cdk/aws-iam'); import kms = require('@aws-cdk/aws-kms'); import { CfnDynamicReference, CfnDynamicReferenceService, CfnParameter, - Construct, ContextProvider, Fn, IConstruct, IResource, Resource, Stack, Token + Construct, ContextProvider, Fn, IResource, Resource, Stack, Token } from '@aws-cdk/core'; import cxapi = require('@aws-cdk/cx-api'); import ssm = require('./ssm.generated'); +import { arnForParameterName } from './util'; /** * An SSM Parameter reference. @@ -186,10 +187,25 @@ export enum ParameterType { export interface StringParameterAttributes { /** - * The name of the parameter store value + * The name of the parameter store value. + * + * This value can be a token or a concrete string. If it is a concrete string + * and includes "/" it must also be prefixed with a "/" (fully-qualified). */ readonly parameterName: string; + /** + * Determines the separator used to render the ARN for the SSM parameter. + * Valid values are `"/"` or `""`. + * + * If `parameterName` is a path (i.e. begins with "/"), the separator must be + * `""`. Otherwise, it must be `"/"`. + * + * @default - automatically determined based on the value of `parameterName` + * unless it is a token, in which case this field is required. + */ + readonly parameterArnSeparator?: string; + /** * The version number of the value you wish to retrieve. * @@ -253,7 +269,7 @@ export class StringParameter extends ParameterBase implements IStringParameter { class Import extends ParameterBase { public readonly parameterName = attrs.parameterName; - public readonly parameterArn = arnForParameterName(this, this.parameterName); + public readonly parameterArn = arnForParameterName(this, attrs.parameterName, undefined); public readonly parameterType = type; public readonly stringValue = stringValue; } @@ -269,7 +285,7 @@ export class StringParameter extends ParameterBase implements IStringParameter { class Import extends ParameterBase { public readonly parameterName = attrs.parameterName; - public readonly parameterArn = arnForParameterName(this, this.parameterName); + public readonly parameterArn = arnForParameterName(this, attrs.parameterName, undefined); public readonly parameterType = ParameterType.SECURE_STRING; public readonly stringValue = stringValue; public readonly encryptionKey = attrs.encryptionKey; @@ -360,7 +376,7 @@ export class StringParameter extends ParameterBase implements IStringParameter { }); this.parameterName = this.getResourceNameAttribute(resource.ref); - this.parameterArn = arnForParameterName(this, this.parameterName); + this.parameterArn = arnForParameterName(this, this.parameterName, props.parameterName || 'autogen'); this.parameterType = resource.attrType; this.stringValue = resource.attrValue; @@ -413,7 +429,7 @@ export class StringListParameter extends ParameterBase implements IStringListPar value: props.stringListValue.join(','), }); this.parameterName = this.getResourceNameAttribute(resource.ref); - this.parameterArn = arnForParameterName(this, this.parameterName); + this.parameterArn = arnForParameterName(this, this.parameterName, props.parameterName || 'autogen'); this.parameterType = resource.attrType; this.stringListValue = Fn.split(',', resource.attrValue); @@ -442,20 +458,3 @@ function _assertValidValue(value: string, allowedPattern: string): void { function makeIdentityForImportedValue(parameterName: string) { return `SsmParameterValue:${parameterName}:C96584B6-F00A-464E-AD19-53AFF4B05118`; } - -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! - resourceName: parameterName, - }); -} diff --git a/packages/@aws-cdk/aws-ssm/lib/util.ts b/packages/@aws-cdk/aws-ssm/lib/util.ts new file mode 100644 index 0000000000000..c4ef770e873d9 --- /dev/null +++ b/packages/@aws-cdk/aws-ssm/lib/util.ts @@ -0,0 +1,71 @@ +import { CfnCondition, Construct, Fn, IConstruct, Stack, Token } from "@aws-cdk/core"; + +/** + * Renders an ARN for an SSM parameter given a parameter name. + * @param scope definition scope + * @param parameterName the parameter name to include in the ARN + * @param physicalName optional physical name specified by the user (to auto-detect separator) + */ +export function arnForParameterName(scope: IConstruct, parameterName: string, physicalName?: string): string { + const { sep, resourceName } = determineSepAndResourceName(); + + validateParameterName(physicalName || parameterName); + + return Stack.of(scope).formatArn({ + service: 'ssm', + resource: 'parameter', + sep, + resourceName, + }); + + function validateParameterName(concreteName: string) { + // can't validate tokens + if (Token.isUnresolved(concreteName)) { + return; + } + + if (concreteName.includes('/') && !concreteName.startsWith('/')) { + throw new Error(`Parameter names must be fully qualified (if they include "/" they must also begin with a "/"): ${concreteName}`); + } + } + + function determineSepAndResourceName() { + // if the parameter name is a token + if (Token.isUnresolved(parameterName)) { + + // if we have a concrete physical name, we can use it to determine the separator + if (physicalName && !Token.isUnresolved(physicalName)) { + return { + sep: physicalName.startsWith('/') ? '' : '/', + resourceName: parameterName + }; + } + + // parameterName is a token and physical name is not helping us (either missing or a token itself) + // in this use case we will need to synthesize a CloudFormation condition that will be used to determine + // if the name has a "/" prefix or not. + const startsWithSlash = startsWithCondition(scope as Construct, parameterName, "/"); + return { + sep: '', + resourceName: Token.asString(Fn.conditionIf(startsWithSlash.logicalId, parameterName, `/${parameterName}`)) + }; + } + + // parameterName is concrete, use it to determine the token + return { + sep: parameterName.startsWith('/') ? '' : '/', + resourceName: parameterName + }; + } +} + +/** + * Gets or creates a CloudFormation condition that evaluates to "TRUE" if `parameterName` (treated as an opaque token) + * starts with a "/". + */ +function startsWithCondition(scope: Construct, value: string, startsWith: string) { + const id = `AWS::CDK::StartsWith(${startsWith})`; + return scope.node.tryFindChild(id) as CfnCondition || new CfnCondition(scope, id, { + expression: Fn.conditionEquals(Fn.select(0, Fn.split(startsWith, value)), "") + }); +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-ssm/test/integ.parameter-arns.expected.json b/packages/@aws-cdk/aws-ssm/test/integ.parameter-arns.expected.json new file mode 100644 index 0000000000000..57dda6ed903e5 --- /dev/null +++ b/packages/@aws-cdk/aws-ssm/test/integ.parameter-arns.expected.json @@ -0,0 +1,202 @@ +{ + "Resources": { + "StringAutogenE7E896E4": { + "Type": "AWS::SSM::Parameter", + "Properties": { + "Type": "String", + "Value": "hello, world" + } + }, + "StringSimpleA681514D": { + "Type": "AWS::SSM::Parameter", + "Properties": { + "Type": "String", + "Value": "hello, world", + "Name": "simple-name" + } + }, + "StringPathD8120137": { + "Type": "AWS::SSM::Parameter", + "Properties": { + "Type": "String", + "Value": "hello, world", + "Name": "/path/name/foo/bar" + } + }, + "ListAutogenC5DA1CAE": { + "Type": "AWS::SSM::Parameter", + "Properties": { + "Type": "StringList", + "Value": "hello,world" + } + }, + "ListSimple9DB641CB": { + "Type": "AWS::SSM::Parameter", + "Properties": { + "Type": "StringList", + "Value": "hello,world", + "Name": "list-simple-name" + } + }, + "ListPath120D6FAB": { + "Type": "AWS::SSM::Parameter", + "Properties": { + "Type": "StringList", + "Value": "hello,world", + "Name": "/list/path/name" + } + } + }, + "Outputs": { + "StringAutogenArn": { + "Value": { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":ssm:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":parameter/", + { + "Ref": "StringAutogenE7E896E4" + } + ] + ] + } + }, + "StringSimpleArn": { + "Value": { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":ssm:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":parameter/", + { + "Ref": "StringSimpleA681514D" + } + ] + ] + } + }, + "StringPathArn": { + "Value": { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":ssm:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":parameter", + { + "Ref": "StringPathD8120137" + } + ] + ] + } + }, + "ListAutogenArn": { + "Value": { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":ssm:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":parameter/", + { + "Ref": "ListAutogenC5DA1CAE" + } + ] + ] + } + }, + "ListSimpleArn": { + "Value": { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":ssm:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":parameter/", + { + "Ref": "ListSimple9DB641CB" + } + ] + ] + } + }, + "ListPathArn": { + "Value": { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":ssm:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":parameter", + { + "Ref": "ListPath120D6FAB" + } + ] + ] + } + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-ssm/test/integ.parameter-arns.ts b/packages/@aws-cdk/aws-ssm/test/integ.parameter-arns.ts new file mode 100644 index 0000000000000..59eb0641d2146 --- /dev/null +++ b/packages/@aws-cdk/aws-ssm/test/integ.parameter-arns.ts @@ -0,0 +1,30 @@ +// expected: +// { +// "ListAutogenArn": "arn:aws:ssm:us-east-1:585695036304:parameter/CFN-ListAutogenC5DA1CAE-QmGaUkqhh6Au", +// "ListPathArn": "arn:aws:ssm:us-east-1:585695036304:parameter/list/path/name", +// "ListSimpleArn": "arn:aws:ssm:us-east-1:585695036304:parameter/list-simple-name", +// "StringAutogenArn": "arn:aws:ssm:us-east-1:585695036304:parameter/CFN-StringAutogenE7E896E4-L0BHbfLgtgJT", +// "StringPathArn": "arn:aws:ssm:us-east-1:585695036304:parameter/path/name/foo/bar", +// "StringSimpleArn": "arn:aws:ssm:us-east-1:585695036304:parameter/simple-name", +// } + +import { App, CfnOutput, Stack } from "@aws-cdk/core"; +import ssm = require('../lib'); + +const app = new App(); +const stack = new Stack(app, 'integ-parameter-arns'); + +const params = [ + new ssm.StringParameter(stack, 'StringAutogen', { stringValue: 'hello, world' }), + new ssm.StringParameter(stack, 'StringSimple', { stringValue: 'hello, world', parameterName: 'simple-name' }), + new ssm.StringParameter(stack, 'StringPath', { stringValue: 'hello, world', parameterName: '/path/name/foo/bar' }), + new ssm.StringListParameter(stack, 'ListAutogen', { stringListValue: [ 'hello', 'world' ] }), + new ssm.StringListParameter(stack, 'ListSimple', { stringListValue: [ 'hello', 'world' ], parameterName: 'list-simple-name' }), + new ssm.StringListParameter(stack, 'ListPath', { stringListValue: [ 'hello', 'world' ], parameterName: '/list/path/name' }), +]; + +for (const p of params) { + new CfnOutput(stack, `${p.node.id}Arn`, { value: p.parameterArn }); +} + +app.synth(); \ No newline at end of file diff --git a/packages/@aws-cdk/aws-ssm/test/test.parameter.ts b/packages/@aws-cdk/aws-ssm/test/test.parameter.ts index 6889bcdd743f8..ac9f6e57ea0cf 100644 --- a/packages/@aws-cdk/aws-ssm/test/test.parameter.ts +++ b/packages/@aws-cdk/aws-ssm/test/test.parameter.ts @@ -1,3 +1,5 @@ +// tslint:disable: max-line-length + import { expect, haveResource } from '@aws-cdk/assert'; import iam = require('@aws-cdk/aws-iam'); import kms = require('@aws-cdk/aws-kms'); @@ -36,7 +38,7 @@ export = { // THEN test.throws(() => new ssm.StringParameter(stack, 'Parameter', { allowedPattern: '^Bar$', stringValue: 'FooBar' }), - /does not match the specified allowedPattern/); + /does not match the specified allowedPattern/); test.done(); }, @@ -46,9 +48,9 @@ export = { // THEN test.doesNotThrow(() => { - new ssm.StringParameter(stack, 'Parameter', { - allowedPattern: '^Bar$', - stringValue: cdk.Lazy.stringValue({ produce: () => 'Foo!' }), + new ssm.StringParameter(stack, 'Parameter', { + allowedPattern: '^Bar$', + stringValue: cdk.Lazy.stringValue({ produce: () => 'Foo!' }), }); }); test.done(); @@ -83,7 +85,7 @@ export = { // THEN test.throws(() => new ssm.StringListParameter(stack, 'Parameter', { stringListValue: ['Foo,Bar'] }), - /cannot contain the ',' character/); + /cannot contain the ',' character/); test.done(); }, @@ -93,7 +95,7 @@ export = { // THEN test.throws(() => new ssm.StringListParameter(stack, 'Parameter', { allowedPattern: '^(Foo|Bar)$', stringListValue: ['Foo', 'FooBar'] }), - /does not match the specified allowedPattern/); + /does not match the specified allowedPattern/); test.done(); }, @@ -130,6 +132,24 @@ export = { test.done(); }, + 'parameterName that includes a "/" must be fully qualified (i.e. begin with "/") as well'(test: Test) { + // GIVEN + const stack = new cdk.Stack(); + + // THEN + test.throws(() => new ssm.StringParameter(stack, 'myParam', { + stringValue: 'myValue', + parameterName: 'path/to/parameter', + }), /Parameter names must be fully qualified/); + + test.throws(() => new ssm.StringListParameter(stack, 'myParam2', { + stringListValue: [ 'foo', 'bar' ], + parameterName: 'path/to/parameter2' + }), /Parameter names must be fully qualified \(if they include \"\/\" they must also begin with a \"\/\"\)\: path\/to\/parameter2/); + + test.done(); + }, + 'StringParameter.fromStringParameterName'(test: Test) { // GIVEN const stack = new Stack(); @@ -139,14 +159,14 @@ export = { // THEN test.deepEqual(stack.resolve(param.parameterArn), { - 'Fn::Join': [ '', [ + 'Fn::Join': ['', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, - ':parameter/MyParamName' ] ] + ':parameter/MyParamName']] }); test.deepEqual(stack.resolve(param.parameterName), 'MyParamName'); test.deepEqual(stack.resolve(param.parameterType), 'String'); @@ -174,14 +194,14 @@ export = { // THEN test.deepEqual(stack.resolve(param.parameterArn), { - 'Fn::Join': [ '', [ + 'Fn::Join': ['', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, - ':parameter/MyParamName' ] ] + ':parameter/MyParamName']] }); test.deepEqual(stack.resolve(param.parameterName), 'MyParamName'); test.deepEqual(stack.resolve(param.parameterType), 'String'); @@ -201,14 +221,14 @@ export = { // THEN test.deepEqual(stack.resolve(param.parameterArn), { - 'Fn::Join': [ '', [ + 'Fn::Join': ['', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, - ':parameter/MyParamName' ] ] + ':parameter/MyParamName']] }); test.deepEqual(stack.resolve(param.parameterName), 'MyParamName'); test.deepEqual(stack.resolve(param.parameterType), 'SecureString'); @@ -348,25 +368,25 @@ export = { // THEN test.deepEqual(stack.resolve(param.parameterArn), { - 'Fn::Join': [ '', [ + 'Fn::Join': ['', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, - ':parameter/MyParamName' ] ] + ':parameter/MyParamName']] }); test.deepEqual(stack.resolve(param.parameterName), 'MyParamName'); test.deepEqual(stack.resolve(param.parameterType), 'StringList'); - test.deepEqual(stack.resolve(param.stringListValue), { 'Fn::Split': [ ',', '{{resolve:ssm:MyParamName}}' ] }); + test.deepEqual(stack.resolve(param.stringListValue), { 'Fn::Split': [',', '{{resolve:ssm:MyParamName}}'] }); test.done(); }, 'fromLookup will use the SSM context provider to read value during synthesis'(test: Test) { // GIVEN const app = new App(); - const stack = new Stack(app, 'my-staq', { env: { region: 'us-east-1', account: '12344' }}); + const stack = new Stack(app, 'my-staq', { env: { region: 'us-east-1', account: '12344' } }); // WHEN const value = ssm.StringParameter.valueFromLookup(stack, 'my-param-name'); @@ -450,8 +470,16 @@ export = { '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' } ] ] }; + const expectedA = { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter/bam']] }; + const expectedB = (conditionName: string) => ({ + 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter', { + 'Fn::If': [ + conditionName, + { Ref: 'param' }, + { 'Fn::Join': ['', ['/', { Ref: 'param' }]] } + ] + }]] + }); let i = 0; // WHEN @@ -464,23 +492,36 @@ export = { 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 }); + + // auto-generated name is always generated as a "simple name" (not/a/path) const case10 = new ssm.StringParameter(stack, `p${i++}`, { stringValue: 'value' }); + // explicitly named physical name gives us a hint on how to render the ARN + const case11 = new ssm.StringParameter(stack, `p${i++}`, { parameterName: '/foo/bar', stringValue: 'hello' }); + const case12 = new ssm.StringParameter(stack, `p${i++}`, { parameterName: 'simple-name', stringValue: 'hello' }); + + const case13 = new ssm.StringListParameter(stack, `p${i++}`, { stringListValue: [ 'hello', 'world' ] }); + const case14 = new ssm.StringListParameter(stack, `p${i++}`, { parameterName: '/not/simple', stringListValue: [ 'hello', 'world' ] }); + const case15 = new ssm.StringListParameter(stack, `p${i++}`, { parameterName: 'simple', stringListValue: [ 'hello', 'world' ] }); + // 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(case3.parameterArn), expectedB('p2AWSCDKStartsWith63AFB06B')); 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(case6.parameterArn), expectedB('p5AWSCDKStartsWith33AD432E')); 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.deepEqual(stack.resolve(case9.parameterArn), expectedB('p8AWSCDKStartsWith7339C979')); + + // new ssm.Parameters determine if "/" is needed based on the posture of `parameterName`. + test.deepEqual(stack.resolve(case10.parameterArn), { 'Fn::Join': [ '', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter/', { Ref: 'p97A508212' } ] ] }); + test.deepEqual(stack.resolve(case11.parameterArn), { 'Fn::Join': [ '', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter', { Ref: 'p107D6B8AB0' } ] ] }); + test.deepEqual(stack.resolve(case12.parameterArn), { 'Fn::Join': [ '', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter/', { Ref: 'p118A9CB02C' } ] ] }); + test.deepEqual(stack.resolve(case13.parameterArn), { 'Fn::Join': [ '', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter/', { Ref: 'p129BE4CE91' } ] ] }); + test.deepEqual(stack.resolve(case14.parameterArn), { 'Fn::Join': [ '', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter', { Ref: 'p1326A2AEC4' } ] ] }); + test.deepEqual(stack.resolve(case15.parameterArn), { 'Fn::Join': [ '', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter/', { Ref: 'p14C90B4AB7' } ] ] }); test.done(); } diff --git a/packages/@aws-cdk/aws-ssm/test/test.util.ts b/packages/@aws-cdk/aws-ssm/test/test.util.ts new file mode 100644 index 0000000000000..6324a008532bf --- /dev/null +++ b/packages/@aws-cdk/aws-ssm/test/test.util.ts @@ -0,0 +1,87 @@ +// tslint:disable: max-line-length + +import { expect } from '@aws-cdk/assert'; +import { Stack, Token } from '@aws-cdk/core'; +import { Test } from 'nodeunit'; +import { arnForParameterName } from '../lib/util'; + +export = { + arnForParameterName: { + + 'simple names': { + + 'concrete parameterName and no physical name (sep is "/")'(test: Test) { + const stack = new Stack(); + test.deepEqual(stack.resolve(arnForParameterName(stack, 'myParam', undefined)), { + 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter/myParam']] + }); + test.done(); + }, + + 'token parameterName and concrete physical name (no additional "/")'(test: Test) { + const stack = new Stack(); + test.deepEqual(stack.resolve(arnForParameterName(stack, Token.asString({ Ref: 'Boom' }), 'myParam')), { + 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter/', { Ref: 'Boom' }]] + }); + test.done(); + }, + + }, + + 'path names': { + + 'concrete parameterName and no physical name (sep is "/")'(test: Test) { + const stack = new Stack(); + test.deepEqual(stack.resolve(arnForParameterName(stack, '/foo/bar', undefined)), { + 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter/foo/bar']] + }); + test.done(); + }, + + 'token parameterName and concrete physical name (no sep)'(test: Test) { + const stack = new Stack(); + test.deepEqual(stack.resolve(arnForParameterName(stack, Token.asString({ Ref: 'Boom' }), '/foo/bar')), { + 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter', { Ref: 'Boom' }]] + }); + test.done(); + }, + + }, + + 'token parameterName and no physical name (Fn::If expression)'(test: Test) { + const stack = new Stack(); + + test.deepEqual(stack.resolve(arnForParameterName(stack, Token.asString({ Ref: 'Boom' }), undefined)), { + 'Fn::Join': + ['', + ['arn:', + { Ref: 'AWS::Partition' }, + ':ssm:', + { Ref: 'AWS::Region' }, + ':', + { Ref: 'AWS::AccountId' }, + ':parameter', + { + 'Fn::If': + ['AWSCDKStartsWith', + { Ref: 'Boom' }, + { 'Fn::Join': ['', ['/', { Ref: 'Boom' }]] }] + }]] + }); + + expect(stack).toMatch({ + Conditions: { + AWSCDKStartsWith: { + "Fn::Equals": [ + { "Fn::Select": [ 0, { "Fn::Split": [ "/", { Ref: "Boom" } ] } ] }, + "" + ] + } + } + }); + + test.done(); + } + + } +}; \ No newline at end of file From b0e5fa5222c69a57afa646473a04203943dcaf68 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Mon, 4 Nov 2019 19:51:27 +0200 Subject: [PATCH 2/5] explicit parameterArnSeparator revert attempt to guess parameter name prefix if it's a token since we can't incorporate refs in conditions. Instead, if the parameter name if a token, we expect `parameterArnSeparator` to be explicitly defined and be one of "/" or "". --- install.sh | 3 + packages/@aws-cdk/aws-ssm/lib/parameter.ts | 41 ++++++--- packages/@aws-cdk/aws-ssm/lib/util.ts | 90 +++++++++--------- .../test/integ.parameter-arns.expected.json | 41 +++++++++ .../aws-ssm/test/integ.parameter-arns.ts | 5 +- .../@aws-cdk/aws-ssm/test/test.parameter.ts | 91 ++++++++++++++----- packages/@aws-cdk/aws-ssm/test/test.util.ts | 54 ++++------- 7 files changed, 206 insertions(+), 119 deletions(-) create mode 100755 install.sh diff --git a/install.sh b/install.sh new file mode 100755 index 0000000000000..c8429223ada1f --- /dev/null +++ b/install.sh @@ -0,0 +1,3 @@ +#!/bin/bash +set -euo pipefail +exec npx yarn install diff --git a/packages/@aws-cdk/aws-ssm/lib/parameter.ts b/packages/@aws-cdk/aws-ssm/lib/parameter.ts index 9a91d8d7474b2..6051a6bf13cb5 100644 --- a/packages/@aws-cdk/aws-ssm/lib/parameter.ts +++ b/packages/@aws-cdk/aws-ssm/lib/parameter.ts @@ -95,6 +95,18 @@ export interface ParameterOptions { * @default - a name will be generated by CloudFormation */ readonly parameterName?: string; + + /** + * Determines the separator used to render the ARN for this SSM parameter. + * Valid values are `"/"` or `""`. + * + * If `parameterName` is a path (i.e. begins with "/"), the separator must be + * an empty string `""`, otherwise, it must be `"/"`. + * + * @default - automatically determined based on the value of `parameterName` + * unless it is a token, in which case this field is required. + */ + readonly parameterArnSeparator?: string; } /** @@ -185,7 +197,7 @@ export enum ParameterType { AWS_EC2_IMAGE_ID = 'AWS::EC2::Image::Id', } -export interface StringParameterAttributes { +export interface CommonStringParameterAttributes { /** * The name of the parameter store value. * @@ -195,17 +207,19 @@ export interface StringParameterAttributes { readonly parameterName: string; /** - * Determines the separator used to render the ARN for the SSM parameter. + * Determines the separator used to render the ARN for this SSM parameter. * Valid values are `"/"` or `""`. * * If `parameterName` is a path (i.e. begins with "/"), the separator must be - * `""`. Otherwise, it must be `"/"`. + * an empty string `""`, otherwise, it must be `"/"`. * * @default - automatically determined based on the value of `parameterName` * unless it is a token, in which case this field is required. */ readonly parameterArnSeparator?: string; +} +export interface StringParameterAttributes extends CommonStringParameterAttributes { /** * The version number of the value you wish to retrieve. * @@ -221,12 +235,7 @@ export interface StringParameterAttributes { readonly type?: ParameterType; } -export interface SecureStringParameterAttributes { - /** - * The name of the parameter store value - */ - readonly parameterName: string; - +export interface SecureStringParameterAttributes extends CommonStringParameterAttributes { /** * The version number of the value you wish to retrieve. This is required for secure strings. */ @@ -269,7 +278,7 @@ export class StringParameter extends ParameterBase implements IStringParameter { class Import extends ParameterBase { public readonly parameterName = attrs.parameterName; - public readonly parameterArn = arnForParameterName(this, attrs.parameterName, undefined); + public readonly parameterArn = arnForParameterName(this, attrs.parameterName, { parameterArnSeparator: attrs.parameterArnSeparator }); public readonly parameterType = type; public readonly stringValue = stringValue; } @@ -285,7 +294,7 @@ export class StringParameter extends ParameterBase implements IStringParameter { class Import extends ParameterBase { public readonly parameterName = attrs.parameterName; - public readonly parameterArn = arnForParameterName(this, attrs.parameterName, undefined); + public readonly parameterArn = arnForParameterName(this, attrs.parameterName, { parameterArnSeparator: attrs.parameterArnSeparator }); public readonly parameterType = ParameterType.SECURE_STRING; public readonly stringValue = stringValue; public readonly encryptionKey = attrs.encryptionKey; @@ -376,7 +385,10 @@ export class StringParameter extends ParameterBase implements IStringParameter { }); this.parameterName = this.getResourceNameAttribute(resource.ref); - this.parameterArn = arnForParameterName(this, this.parameterName, props.parameterName || 'autogen'); + this.parameterArn = arnForParameterName(this, this.parameterName, { + physicalName: props.parameterName || 'autogen', + parameterArnSeparator: props.parameterArnSeparator + }); this.parameterType = resource.attrType; this.stringValue = resource.attrValue; @@ -429,7 +441,10 @@ export class StringListParameter extends ParameterBase implements IStringListPar value: props.stringListValue.join(','), }); this.parameterName = this.getResourceNameAttribute(resource.ref); - this.parameterArn = arnForParameterName(this, this.parameterName, props.parameterName || 'autogen'); + this.parameterArn = arnForParameterName(this, this.parameterName, { + physicalName: props.parameterName || 'autogen', + parameterArnSeparator: props.parameterArnSeparator + }); this.parameterType = resource.attrType; this.stringListValue = Fn.split(',', resource.attrValue); diff --git a/packages/@aws-cdk/aws-ssm/lib/util.ts b/packages/@aws-cdk/aws-ssm/lib/util.ts index c4ef770e873d9..5974f9bf15ea5 100644 --- a/packages/@aws-cdk/aws-ssm/lib/util.ts +++ b/packages/@aws-cdk/aws-ssm/lib/util.ts @@ -1,4 +1,9 @@ -import { CfnCondition, Construct, Fn, IConstruct, Stack, Token } from "@aws-cdk/core"; +import { IConstruct, Stack, Token } from "@aws-cdk/core"; + +export interface ArnForParameterNameOptions { + readonly physicalName?: string; + readonly parameterArnSeparator?: string; +} /** * Renders an ARN for an SSM parameter given a parameter name. @@ -6,66 +11,53 @@ import { CfnCondition, Construct, Fn, IConstruct, Stack, Token } from "@aws-cdk/ * @param parameterName the parameter name to include in the ARN * @param physicalName optional physical name specified by the user (to auto-detect separator) */ -export function arnForParameterName(scope: IConstruct, parameterName: string, physicalName?: string): string { - const { sep, resourceName } = determineSepAndResourceName(); +export function arnForParameterName(scope: IConstruct, parameterName: string, options: ArnForParameterNameOptions = { }): string { + const physicalName = options.physicalName; + const nameToValidate = physicalName || parameterName; + + // validate "parameterArnSeparator" (if defined). + if (options.parameterArnSeparator !== undefined) { + if (options.parameterArnSeparator !== '/' && options.parameterArnSeparator !== '') { + throw new Error(`parameterArnSeparator must be either "/" or "". got "${options.parameterArnSeparator}"`); + } + } - validateParameterName(physicalName || parameterName); + if (!Token.isUnresolved(nameToValidate) && nameToValidate.includes('/') && !nameToValidate.startsWith('/')) { + throw new Error(`Parameter names must be fully qualified (if they include "/" they must also begin with a "/"): ${nameToValidate}`); + } return Stack.of(scope).formatArn({ service: 'ssm', resource: 'parameter', - sep, - resourceName, + sep: determineSeperator(), + resourceName: parameterName, }); - function validateParameterName(concreteName: string) { - // can't validate tokens - if (Token.isUnresolved(concreteName)) { - return; - } + /** + * Determines the ARN separator for this parameter: if we have a concrete + * parameter name (or explicitly defined physical name), we will parse them + * and decide whether a "/" is needed or not. Otherwise, users will have to + * explicitly specify `parameterArnSeparator` when they import the ARN. + */ + function determineSeperator() { + // look for a concrete name as a hint for determining the separator + const concreteName = !Token.isUnresolved(parameterName) ? parameterName : physicalName; + if (!concreteName || Token.isUnresolved(concreteName)) { + + if (options.parameterArnSeparator === undefined) { + throw new Error(`Unable to determine ARN separator for SSM parameter since the parameter name is an unresolved token. Use "fromAttributes" and specify "parameterArnSeparator" explicitly`); + } - if (concreteName.includes('/') && !concreteName.startsWith('/')) { - throw new Error(`Parameter names must be fully qualified (if they include "/" they must also begin with a "/"): ${concreteName}`); + return options.parameterArnSeparator; } - } - - function determineSepAndResourceName() { - // if the parameter name is a token - if (Token.isUnresolved(parameterName)) { - // if we have a concrete physical name, we can use it to determine the separator - if (physicalName && !Token.isUnresolved(physicalName)) { - return { - sep: physicalName.startsWith('/') ? '' : '/', - resourceName: parameterName - }; - } + const calculatedSep = concreteName.startsWith('/') ? '' : '/'; - // parameterName is a token and physical name is not helping us (either missing or a token itself) - // in this use case we will need to synthesize a CloudFormation condition that will be used to determine - // if the name has a "/" prefix or not. - const startsWithSlash = startsWithCondition(scope as Construct, parameterName, "/"); - return { - sep: '', - resourceName: Token.asString(Fn.conditionIf(startsWithSlash.logicalId, parameterName, `/${parameterName}`)) - }; + // if users explicitly specify the separator and it conflicts with the one we need, it's an error. + if (options.parameterArnSeparator !== undefined && options.parameterArnSeparator !== calculatedSep) { + throw new Error(`parameterArnSeparator "${options.parameterArnSeparator}" is invalid for SSM parameter with name "${concreteName}". It should be "${calculatedSep}"`); } - // parameterName is concrete, use it to determine the token - return { - sep: parameterName.startsWith('/') ? '' : '/', - resourceName: parameterName - }; + return calculatedSep; } } - -/** - * Gets or creates a CloudFormation condition that evaluates to "TRUE" if `parameterName` (treated as an opaque token) - * starts with a "/". - */ -function startsWithCondition(scope: Construct, value: string, startsWith: string) { - const id = `AWS::CDK::StartsWith(${startsWith})`; - return scope.node.tryFindChild(id) as CfnCondition || new CfnCondition(scope, id, { - expression: Fn.conditionEquals(Fn.select(0, Fn.split(startsWith, value)), "") - }); -} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-ssm/test/integ.parameter-arns.expected.json b/packages/@aws-cdk/aws-ssm/test/integ.parameter-arns.expected.json index 57dda6ed903e5..074f101dd7401 100644 --- a/packages/@aws-cdk/aws-ssm/test/integ.parameter-arns.expected.json +++ b/packages/@aws-cdk/aws-ssm/test/integ.parameter-arns.expected.json @@ -1,4 +1,10 @@ { + "Parameters": { + "ParameterNameParameter": { + "Type": "String", + "Default": "myParamName" + } + }, "Resources": { "StringAutogenE7E896E4": { "Type": "AWS::SSM::Parameter", @@ -45,6 +51,16 @@ "Value": "hello,world", "Name": "/list/path/name" } + }, + "Parameterized6DC5E5E5": { + "Type": "AWS::SSM::Parameter", + "Properties": { + "Type": "String", + "Value": "hello, world", + "Name": { + "Ref": "ParameterNameParameter" + } + } } }, "Outputs": { @@ -197,6 +213,31 @@ ] ] } + }, + "ParameterizedArn": { + "Value": { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":ssm:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":parameter/", + { + "Ref": "Parameterized6DC5E5E5" + } + ] + ] + } } } } \ No newline at end of file diff --git a/packages/@aws-cdk/aws-ssm/test/integ.parameter-arns.ts b/packages/@aws-cdk/aws-ssm/test/integ.parameter-arns.ts index 59eb0641d2146..68076921b834a 100644 --- a/packages/@aws-cdk/aws-ssm/test/integ.parameter-arns.ts +++ b/packages/@aws-cdk/aws-ssm/test/integ.parameter-arns.ts @@ -8,12 +8,14 @@ // "StringSimpleArn": "arn:aws:ssm:us-east-1:585695036304:parameter/simple-name", // } -import { App, CfnOutput, Stack } from "@aws-cdk/core"; +import { App, CfnOutput, CfnParameter, Stack } from "@aws-cdk/core"; import ssm = require('../lib'); const app = new App(); const stack = new Stack(app, 'integ-parameter-arns'); +const input = new CfnParameter(stack, 'ParameterNameParameter', { type: 'String', default: 'myParamName' }); + const params = [ new ssm.StringParameter(stack, 'StringAutogen', { stringValue: 'hello, world' }), new ssm.StringParameter(stack, 'StringSimple', { stringValue: 'hello, world', parameterName: 'simple-name' }), @@ -21,6 +23,7 @@ const params = [ new ssm.StringListParameter(stack, 'ListAutogen', { stringListValue: [ 'hello', 'world' ] }), new ssm.StringListParameter(stack, 'ListSimple', { stringListValue: [ 'hello', 'world' ], parameterName: 'list-simple-name' }), new ssm.StringListParameter(stack, 'ListPath', { stringListValue: [ 'hello', 'world' ], parameterName: '/list/path/name' }), + new ssm.StringParameter(stack, 'Parameterized', { stringValue: 'hello, world', parameterName: input.valueAsString, parameterArnSeparator: '/' }) ]; for (const p of params) { diff --git a/packages/@aws-cdk/aws-ssm/test/test.parameter.ts b/packages/@aws-cdk/aws-ssm/test/test.parameter.ts index ac9f6e57ea0cf..2ef12d21b33ec 100644 --- a/packages/@aws-cdk/aws-ssm/test/test.parameter.ts +++ b/packages/@aws-cdk/aws-ssm/test/test.parameter.ts @@ -470,28 +470,20 @@ export = { '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 = (conditionName: string) => ({ - 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter', { - 'Fn::If': [ - conditionName, - { Ref: 'param' }, - { 'Fn::Join': ['', ['/', { Ref: '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' } ] ] }; + const expectedC = { '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 case6 = ssm.StringParameter.fromStringParameterAttributes(stack, `p${i++}`, { parameterName: param.valueAsString, parameterArnSeparator: '/' }); 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 case9 = ssm.StringParameter.fromSecureStringParameterAttributes(stack, `p${i++}`, { parameterName: param.valueAsString, version: 10, parameterArnSeparator: '' }); // auto-generated name is always generated as a "simple name" (not/a/path) const case10 = new ssm.StringParameter(stack, `p${i++}`, { stringValue: 'value' }); @@ -507,22 +499,77 @@ export = { // THEN test.deepEqual(stack.resolve(case1.parameterArn), expectedA); test.deepEqual(stack.resolve(case2.parameterArn), expectedA); - test.deepEqual(stack.resolve(case3.parameterArn), expectedB('p2AWSCDKStartsWith63AFB06B')); test.deepEqual(stack.resolve(case4.parameterArn), expectedA); test.deepEqual(stack.resolve(case5.parameterArn), expectedA); - test.deepEqual(stack.resolve(case6.parameterArn), expectedB('p5AWSCDKStartsWith33AD432E')); + 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('p8AWSCDKStartsWith7339C979')); + test.deepEqual(stack.resolve(case9.parameterArn), expectedC); // new ssm.Parameters determine if "/" is needed based on the posture of `parameterName`. - test.deepEqual(stack.resolve(case10.parameterArn), { 'Fn::Join': [ '', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter/', { Ref: 'p97A508212' } ] ] }); - test.deepEqual(stack.resolve(case11.parameterArn), { 'Fn::Join': [ '', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter', { Ref: 'p107D6B8AB0' } ] ] }); - test.deepEqual(stack.resolve(case12.parameterArn), { 'Fn::Join': [ '', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter/', { Ref: 'p118A9CB02C' } ] ] }); - test.deepEqual(stack.resolve(case13.parameterArn), { 'Fn::Join': [ '', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter/', { Ref: 'p129BE4CE91' } ] ] }); - test.deepEqual(stack.resolve(case14.parameterArn), { 'Fn::Join': [ '', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter', { Ref: 'p1326A2AEC4' } ] ] }); - test.deepEqual(stack.resolve(case15.parameterArn), { 'Fn::Join': [ '', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter/', { Ref: 'p14C90B4AB7' } ] ] }); + test.deepEqual(stack.resolve(case10.parameterArn), { 'Fn::Join': [ '', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter/', { Ref: 'p81BB0F6FE' } ] ] }); + test.deepEqual(stack.resolve(case11.parameterArn), { 'Fn::Join': [ '', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter', { Ref: 'p97A508212' } ] ] }); + test.deepEqual(stack.resolve(case12.parameterArn), { 'Fn::Join': [ '', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter/', { Ref: 'p107D6B8AB0' } ] ] }); + test.deepEqual(stack.resolve(case13.parameterArn), { 'Fn::Join': [ '', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter/', { Ref: 'p118A9CB02C' } ] ] }); + test.deepEqual(stack.resolve(case14.parameterArn), { 'Fn::Join': [ '', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter', { Ref: 'p129BE4CE91' } ] ] }); + test.deepEqual(stack.resolve(case15.parameterArn), { 'Fn::Join': [ '', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter/', { Ref: 'p1326A2AEC4' } ] ] }); + + test.done(); + }, + + 'if parameterName is a token separator must be specified'(test: Test) { + // GIVEN + const stack = new Stack(); + const param = new CfnParameter(stack, 'param'); + let i = 0; + + // WHEN + const p1 = new ssm.StringParameter(stack, `p${i++}`, { parameterName: param.valueAsString, stringValue: 'foo', parameterArnSeparator: '/' }); + const p2 = new ssm.StringParameter(stack, `p${i++}`, { parameterName: param.valueAsString, stringValue: 'foo', parameterArnSeparator: '' }); + const p3 = new ssm.StringListParameter(stack, `p${i++}`, { parameterName: param.valueAsString, stringListValue: [ 'foo' ], parameterArnSeparator: '' }); + + // THEN + test.deepEqual(stack.resolve(p1.parameterArn), { 'Fn::Join': [ '', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter/', { Ref: 'p0B02A8F65' } ] ] }); + test.deepEqual(stack.resolve(p2.parameterArn), { 'Fn::Join': [ '', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter', { Ref: 'p1E43AD5AC' } ] ] }); + test.deepEqual(stack.resolve(p3.parameterArn), { 'Fn::Join': [ '', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter', { Ref: 'p2C1903AEB' } ] ] }); + + test.done(); + }, + + 'fails if name is a token and no explicit separator'(test: Test) { + // GIVEN + const stack = new Stack(); + const param = new CfnParameter(stack, 'param'); + let i = 0; + + // THEN + const expected = /Unable to determine ARN separator for SSM parameter since the parameter name is an unresolved token. Use "fromAttributes" and specify "parameterArnSeparator" explicitly/; + test.throws(() => ssm.StringParameter.fromStringParameterName(stack, `p${i++}`, param.valueAsString), expected); + test.throws(() => ssm.StringParameter.fromSecureStringParameterAttributes(stack, `p${i++}`, { parameterName: param.valueAsString, version: 1 }), expected); + test.throws(() => new ssm.StringParameter(stack, `p${i++}`, { parameterName: param.valueAsString, stringValue: 'foo' }), expected); + test.throws(() => new ssm.StringParameter(stack, `p${i++}`, { parameterName: param.valueAsString, stringValue: 'foo' }), expected); + test.done(); + }, + + 'fails if parameterArnSeparator is wrong based on a concrete physical name'(test: Test) { + // GIVEN + const stack = new Stack(); + let i = 0; + // THEN + test.throws(() => ssm.StringParameter.fromStringParameterAttributes(stack, `p${i++}`, { parameterName: 'simple', parameterArnSeparator: '' }), /parameterArnSeparator "" is invalid for SSM parameter with name "simple". It should be "\/"/); + test.throws(() => ssm.StringParameter.fromStringParameterAttributes(stack, `p${i++}`, { parameterName: '/foo/bar', parameterArnSeparator: '/' }), /parameterArnSeparator "\/" is invalid for SSM parameter with name \"\/foo\/bar\"\. It should be \"\"/); + test.done(); + }, + + 'fails if parameterArnSeparator is not "/" or ""'(test: Test) { + const stack = new Stack(); + const param = new CfnParameter(stack, 'param'); + let i = 0; + + // THEN + test.throws(() => ssm.StringParameter.fromStringParameterAttributes(stack, `p${i++}`, { parameterName: param.valueAsString, parameterArnSeparator: 'x' }), /parameterArnSeparator must be either "\/" or ""\. got "x"/); + test.throws(() => ssm.StringParameter.fromStringParameterAttributes(stack, `p${i++}`, { parameterName: param.valueAsString, parameterArnSeparator: 'boom' }), /parameterArnSeparator must be either "\/" or ""\. got "boom"/); test.done(); } }; diff --git a/packages/@aws-cdk/aws-ssm/test/test.util.ts b/packages/@aws-cdk/aws-ssm/test/test.util.ts index 6324a008532bf..a6dca9f0fa742 100644 --- a/packages/@aws-cdk/aws-ssm/test/test.util.ts +++ b/packages/@aws-cdk/aws-ssm/test/test.util.ts @@ -1,6 +1,5 @@ // tslint:disable: max-line-length -import { expect } from '@aws-cdk/assert'; import { Stack, Token } from '@aws-cdk/core'; import { Test } from 'nodeunit'; import { arnForParameterName } from '../lib/util'; @@ -20,12 +19,20 @@ export = { 'token parameterName and concrete physical name (no additional "/")'(test: Test) { const stack = new Stack(); - test.deepEqual(stack.resolve(arnForParameterName(stack, Token.asString({ Ref: 'Boom' }), 'myParam')), { + test.deepEqual(stack.resolve(arnForParameterName(stack, Token.asString({ Ref: 'Boom' }), { physicalName: 'myParam' })), { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter/', { Ref: 'Boom' }]] }); test.done(); }, + 'token parameterName, explicit "/" separator'(test: Test) { + const stack = new Stack(); + test.deepEqual(stack.resolve(arnForParameterName(stack, Token.asString({ Ref: 'Boom' }), { parameterArnSeparator: '/' })), { + 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter/', { Ref: 'Boom' }]] + }); + test.done(); + } + }, 'path names': { @@ -40,46 +47,25 @@ export = { 'token parameterName and concrete physical name (no sep)'(test: Test) { const stack = new Stack(); - test.deepEqual(stack.resolve(arnForParameterName(stack, Token.asString({ Ref: 'Boom' }), '/foo/bar')), { + test.deepEqual(stack.resolve(arnForParameterName(stack, Token.asString({ Ref: 'Boom' }), { physicalName: '/foo/bar' })), { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter', { Ref: 'Boom' }]] }); test.done(); }, + 'token parameterName, explicit "" separator'(test: Test) { + const stack = new Stack(); + test.deepEqual(stack.resolve(arnForParameterName(stack, Token.asString({ Ref: 'Boom' }), { parameterArnSeparator: '' })), { + 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter', { Ref: 'Boom' }]] + }); + test.done(); + } + }, - 'token parameterName and no physical name (Fn::If expression)'(test: Test) { + 'fails if explicit separator is not defined and parameterName is a token'(test: Test) { const stack = new Stack(); - - test.deepEqual(stack.resolve(arnForParameterName(stack, Token.asString({ Ref: 'Boom' }), undefined)), { - 'Fn::Join': - ['', - ['arn:', - { Ref: 'AWS::Partition' }, - ':ssm:', - { Ref: 'AWS::Region' }, - ':', - { Ref: 'AWS::AccountId' }, - ':parameter', - { - 'Fn::If': - ['AWSCDKStartsWith', - { Ref: 'Boom' }, - { 'Fn::Join': ['', ['/', { Ref: 'Boom' }]] }] - }]] - }); - - expect(stack).toMatch({ - Conditions: { - AWSCDKStartsWith: { - "Fn::Equals": [ - { "Fn::Select": [ 0, { "Fn::Split": [ "/", { Ref: "Boom" } ] } ] }, - "" - ] - } - } - }); - + test.throws(() => arnForParameterName(stack, Token.asString({ Ref: 'Boom' })), /foo/); test.done(); } From 210d0b084366de1c0fe2415fea911a52c135820b Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Mon, 4 Nov 2019 22:10:39 +0200 Subject: [PATCH 3/5] misc * fix test expectation * add public API doc --- packages/@aws-cdk/aws-ssm/lib/parameter.ts | 3 +++ packages/@aws-cdk/aws-ssm/test/test.util.ts | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-ssm/lib/parameter.ts b/packages/@aws-cdk/aws-ssm/lib/parameter.ts index 6051a6bf13cb5..624eeacb58fb4 100644 --- a/packages/@aws-cdk/aws-ssm/lib/parameter.ts +++ b/packages/@aws-cdk/aws-ssm/lib/parameter.ts @@ -197,6 +197,9 @@ export enum ParameterType { AWS_EC2_IMAGE_ID = 'AWS::EC2::Image::Id', } +/** + * Common attributes for string parameters. + */ export interface CommonStringParameterAttributes { /** * The name of the parameter store value. diff --git a/packages/@aws-cdk/aws-ssm/test/test.util.ts b/packages/@aws-cdk/aws-ssm/test/test.util.ts index a6dca9f0fa742..15033cd0327f8 100644 --- a/packages/@aws-cdk/aws-ssm/test/test.util.ts +++ b/packages/@aws-cdk/aws-ssm/test/test.util.ts @@ -65,7 +65,7 @@ export = { 'fails if explicit separator is not defined and parameterName is a token'(test: Test) { const stack = new Stack(); - test.throws(() => arnForParameterName(stack, Token.asString({ Ref: 'Boom' })), /foo/); + test.throws(() => arnForParameterName(stack, Token.asString({ Ref: 'Boom' })), /Unable to determine ARN separator for SSM parameter since the parameter name is an unresolved token. Use "fromAttributes" and specify "parameterArnSeparator" explicitly/); test.done(); } From 5494c88f5c835aec6ef589443061776e81ede37e Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Tue, 5 Nov 2019 11:09:00 +0200 Subject: [PATCH 4/5] add --frozen-lockfile to install.sh --- install.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/install.sh b/install.sh index c8429223ada1f..e753da03d08e7 100755 --- a/install.sh +++ b/install.sh @@ -1,3 +1,3 @@ #!/bin/bash set -euo pipefail -exec npx yarn install +exec npx yarn install --frozen-lockfile From 8d8ee14a7e5747b887ba9105e9f73d15a4bff677 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Tue, 5 Nov 2019 11:09:29 +0200 Subject: [PATCH 5/5] rename "parameterArnSeparator: string" to "simpleName: boolean" --- packages/@aws-cdk/aws-ssm/lib/parameter.ts | 50 +++++++++++-------- packages/@aws-cdk/aws-ssm/lib/util.ts | 36 ++++++------- .../test/integ.parameter-arns.expected.json | 50 +++++++++++++++++-- .../aws-ssm/test/integ.parameter-arns.ts | 14 ++---- .../@aws-cdk/aws-ssm/test/test.parameter.ts | 26 +++++----- packages/@aws-cdk/aws-ssm/test/test.util.ts | 6 +-- 6 files changed, 112 insertions(+), 70 deletions(-) diff --git a/packages/@aws-cdk/aws-ssm/lib/parameter.ts b/packages/@aws-cdk/aws-ssm/lib/parameter.ts index 624eeacb58fb4..a0994ef843d5a 100644 --- a/packages/@aws-cdk/aws-ssm/lib/parameter.ts +++ b/packages/@aws-cdk/aws-ssm/lib/parameter.ts @@ -6,7 +6,7 @@ import { } from '@aws-cdk/core'; import cxapi = require('@aws-cdk/cx-api'); import ssm = require('./ssm.generated'); -import { arnForParameterName } from './util'; +import { arnForParameterName, AUTOGEN_MARKER } from './util'; /** * An SSM Parameter reference. @@ -97,16 +97,20 @@ export interface ParameterOptions { readonly parameterName?: string; /** - * Determines the separator used to render the ARN for this SSM parameter. - * Valid values are `"/"` or `""`. + * Indicates of the parameter name is a simple name (i.e. does not include "/" + * separators). * - * If `parameterName` is a path (i.e. begins with "/"), the separator must be - * an empty string `""`, otherwise, it must be `"/"`. + * This is only required only if `parameterName` is a token, which means we + * are unable to detect if the name is simple or "path-like" for the purpose + * of rendering SSM parameter ARNs. * - * @default - automatically determined based on the value of `parameterName` - * unless it is a token, in which case this field is required. + * If `parameterName` is not specified, `simpleName` must be `true` (or + * undefined) since the name generated by AWS CloudFormation is always a + * simple name. + * + * @default - auto-detect based on `parameterName` */ - readonly parameterArnSeparator?: string; + readonly simpleName?: boolean; } /** @@ -210,16 +214,20 @@ export interface CommonStringParameterAttributes { readonly parameterName: string; /** - * Determines the separator used to render the ARN for this SSM parameter. - * Valid values are `"/"` or `""`. + * Indicates of the parameter name is a simple name (i.e. does not include "/" + * separators). + * + * This is only required only if `parameterName` is a token, which means we + * are unable to detect if the name is simple or "path-like" for the purpose + * of rendering SSM parameter ARNs. * - * If `parameterName` is a path (i.e. begins with "/"), the separator must be - * an empty string `""`, otherwise, it must be `"/"`. + * If `parameterName` is not specified, `simpleName` must be `true` (or + * undefined) since the name generated by AWS CloudFormation is always a + * simple name. * - * @default - automatically determined based on the value of `parameterName` - * unless it is a token, in which case this field is required. + * @default - auto-detect based on `parameterName` */ - readonly parameterArnSeparator?: string; + readonly simpleName?: boolean; } export interface StringParameterAttributes extends CommonStringParameterAttributes { @@ -281,7 +289,7 @@ export class StringParameter extends ParameterBase implements IStringParameter { class Import extends ParameterBase { public readonly parameterName = attrs.parameterName; - public readonly parameterArn = arnForParameterName(this, attrs.parameterName, { parameterArnSeparator: attrs.parameterArnSeparator }); + public readonly parameterArn = arnForParameterName(this, attrs.parameterName, { simpleName: attrs.simpleName }); public readonly parameterType = type; public readonly stringValue = stringValue; } @@ -297,7 +305,7 @@ export class StringParameter extends ParameterBase implements IStringParameter { class Import extends ParameterBase { public readonly parameterName = attrs.parameterName; - public readonly parameterArn = arnForParameterName(this, attrs.parameterName, { parameterArnSeparator: attrs.parameterArnSeparator }); + public readonly parameterArn = arnForParameterName(this, attrs.parameterName, { simpleName: attrs.simpleName }); public readonly parameterType = ParameterType.SECURE_STRING; public readonly stringValue = stringValue; public readonly encryptionKey = attrs.encryptionKey; @@ -389,8 +397,8 @@ export class StringParameter extends ParameterBase implements IStringParameter { this.parameterName = this.getResourceNameAttribute(resource.ref); this.parameterArn = arnForParameterName(this, this.parameterName, { - physicalName: props.parameterName || 'autogen', - parameterArnSeparator: props.parameterArnSeparator + physicalName: props.parameterName || AUTOGEN_MARKER, + simpleName: props.simpleName }); this.parameterType = resource.attrType; @@ -445,8 +453,8 @@ export class StringListParameter extends ParameterBase implements IStringListPar }); this.parameterName = this.getResourceNameAttribute(resource.ref); this.parameterArn = arnForParameterName(this, this.parameterName, { - physicalName: props.parameterName || 'autogen', - parameterArnSeparator: props.parameterArnSeparator + physicalName: props.parameterName || AUTOGEN_MARKER, + simpleName: props.simpleName }); this.parameterType = resource.attrType; diff --git a/packages/@aws-cdk/aws-ssm/lib/util.ts b/packages/@aws-cdk/aws-ssm/lib/util.ts index 5974f9bf15ea5..660179eedff78 100644 --- a/packages/@aws-cdk/aws-ssm/lib/util.ts +++ b/packages/@aws-cdk/aws-ssm/lib/util.ts @@ -1,8 +1,10 @@ import { IConstruct, Stack, Token } from "@aws-cdk/core"; +export const AUTOGEN_MARKER = '$$autogen$$'; + export interface ArnForParameterNameOptions { readonly physicalName?: string; - readonly parameterArnSeparator?: string; + readonly simpleName?: boolean; } /** @@ -15,13 +17,6 @@ export function arnForParameterName(scope: IConstruct, parameterName: string, op const physicalName = options.physicalName; const nameToValidate = physicalName || parameterName; - // validate "parameterArnSeparator" (if defined). - if (options.parameterArnSeparator !== undefined) { - if (options.parameterArnSeparator !== '/' && options.parameterArnSeparator !== '') { - throw new Error(`parameterArnSeparator must be either "/" or "". got "${options.parameterArnSeparator}"`); - } - } - if (!Token.isUnresolved(nameToValidate) && nameToValidate.includes('/') && !nameToValidate.startsWith('/')) { throw new Error(`Parameter names must be fully qualified (if they include "/" they must also begin with a "/"): ${nameToValidate}`); } @@ -29,7 +24,7 @@ export function arnForParameterName(scope: IConstruct, parameterName: string, op return Stack.of(scope).formatArn({ service: 'ssm', resource: 'parameter', - sep: determineSeperator(), + sep: isSimpleName() ? '/' : '', resourceName: parameterName, }); @@ -37,27 +32,32 @@ export function arnForParameterName(scope: IConstruct, parameterName: string, op * Determines the ARN separator for this parameter: if we have a concrete * parameter name (or explicitly defined physical name), we will parse them * and decide whether a "/" is needed or not. Otherwise, users will have to - * explicitly specify `parameterArnSeparator` when they import the ARN. + * explicitly specify `simpleName` when they import the ARN. */ - function determineSeperator() { + function isSimpleName(): boolean { // look for a concrete name as a hint for determining the separator const concreteName = !Token.isUnresolved(parameterName) ? parameterName : physicalName; if (!concreteName || Token.isUnresolved(concreteName)) { - if (options.parameterArnSeparator === undefined) { - throw new Error(`Unable to determine ARN separator for SSM parameter since the parameter name is an unresolved token. Use "fromAttributes" and specify "parameterArnSeparator" explicitly`); + if (options.simpleName === undefined) { + throw new Error(`Unable to determine ARN separator for SSM parameter since the parameter name is an unresolved token. Use "fromAttributes" and specify "simpleName" explicitly`); } - return options.parameterArnSeparator; + return options.simpleName; } - const calculatedSep = concreteName.startsWith('/') ? '' : '/'; + const result = !concreteName.startsWith('/'); // if users explicitly specify the separator and it conflicts with the one we need, it's an error. - if (options.parameterArnSeparator !== undefined && options.parameterArnSeparator !== calculatedSep) { - throw new Error(`parameterArnSeparator "${options.parameterArnSeparator}" is invalid for SSM parameter with name "${concreteName}". It should be "${calculatedSep}"`); + if (options.simpleName !== undefined && options.simpleName !== result) { + + if (concreteName === AUTOGEN_MARKER) { + throw new Error(`If "parameterName" is not explicitly defined, "simpleName" must be "true" or undefined since auto-generated parameter names always have simple names`); + } + + throw new Error(`Parameter name "${concreteName}" is ${result ? 'a simple name' : 'not a simple name'}, but "simpleName" was explicitly set to ${options.simpleName}. Either omit it or set it to ${result}`); } - return calculatedSep; + return result; } } diff --git a/packages/@aws-cdk/aws-ssm/test/integ.parameter-arns.expected.json b/packages/@aws-cdk/aws-ssm/test/integ.parameter-arns.expected.json index 074f101dd7401..1c4b0348b7888 100644 --- a/packages/@aws-cdk/aws-ssm/test/integ.parameter-arns.expected.json +++ b/packages/@aws-cdk/aws-ssm/test/integ.parameter-arns.expected.json @@ -52,7 +52,7 @@ "Name": "/list/path/name" } }, - "Parameterized6DC5E5E5": { + "ParameterizedSimpleB6311859": { "Type": "AWS::SSM::Parameter", "Properties": { "Type": "String", @@ -61,6 +61,25 @@ "Ref": "ParameterNameParameter" } } + }, + "ParameterizedNonSimple23C44BF6": { + "Type": "AWS::SSM::Parameter", + "Properties": { + "Type": "String", + "Value": "hello, world", + "Name": { + "Fn::Join": [ + "", + [ + "/", + { + "Ref": "ParameterNameParameter" + }, + "/non/simple" + ] + ] + } + } } }, "Outputs": { @@ -214,7 +233,7 @@ ] } }, - "ParameterizedArn": { + "ParameterizedSimpleArn": { "Value": { "Fn::Join": [ "", @@ -233,7 +252,32 @@ }, ":parameter/", { - "Ref": "Parameterized6DC5E5E5" + "Ref": "ParameterizedSimpleB6311859" + } + ] + ] + } + }, + "ParameterizedNonSimpleArn": { + "Value": { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":ssm:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":parameter", + { + "Ref": "ParameterizedNonSimple23C44BF6" } ] ] diff --git a/packages/@aws-cdk/aws-ssm/test/integ.parameter-arns.ts b/packages/@aws-cdk/aws-ssm/test/integ.parameter-arns.ts index 68076921b834a..b5c99e2395d40 100644 --- a/packages/@aws-cdk/aws-ssm/test/integ.parameter-arns.ts +++ b/packages/@aws-cdk/aws-ssm/test/integ.parameter-arns.ts @@ -1,13 +1,4 @@ -// expected: -// { -// "ListAutogenArn": "arn:aws:ssm:us-east-1:585695036304:parameter/CFN-ListAutogenC5DA1CAE-QmGaUkqhh6Au", -// "ListPathArn": "arn:aws:ssm:us-east-1:585695036304:parameter/list/path/name", -// "ListSimpleArn": "arn:aws:ssm:us-east-1:585695036304:parameter/list-simple-name", -// "StringAutogenArn": "arn:aws:ssm:us-east-1:585695036304:parameter/CFN-StringAutogenE7E896E4-L0BHbfLgtgJT", -// "StringPathArn": "arn:aws:ssm:us-east-1:585695036304:parameter/path/name/foo/bar", -// "StringSimpleArn": "arn:aws:ssm:us-east-1:585695036304:parameter/simple-name", -// } - +// tslint:disable: max-line-length import { App, CfnOutput, CfnParameter, Stack } from "@aws-cdk/core"; import ssm = require('../lib'); @@ -23,7 +14,8 @@ const params = [ new ssm.StringListParameter(stack, 'ListAutogen', { stringListValue: [ 'hello', 'world' ] }), new ssm.StringListParameter(stack, 'ListSimple', { stringListValue: [ 'hello', 'world' ], parameterName: 'list-simple-name' }), new ssm.StringListParameter(stack, 'ListPath', { stringListValue: [ 'hello', 'world' ], parameterName: '/list/path/name' }), - new ssm.StringParameter(stack, 'Parameterized', { stringValue: 'hello, world', parameterName: input.valueAsString, parameterArnSeparator: '/' }) + new ssm.StringParameter(stack, 'ParameterizedSimple', { stringValue: 'hello, world', parameterName: input.valueAsString, simpleName: true }), + new ssm.StringParameter(stack, 'ParameterizedNonSimple', { stringValue: 'hello, world', parameterName: `/${input.valueAsString}/non/simple`, simpleName: false }), ]; for (const p of params) { diff --git a/packages/@aws-cdk/aws-ssm/test/test.parameter.ts b/packages/@aws-cdk/aws-ssm/test/test.parameter.ts index 2ef12d21b33ec..b59dcf9781f3d 100644 --- a/packages/@aws-cdk/aws-ssm/test/test.parameter.ts +++ b/packages/@aws-cdk/aws-ssm/test/test.parameter.ts @@ -480,10 +480,10 @@ export = { const case2 = ssm.StringParameter.fromStringParameterName(stack, `p${i++}`, '/bam'); 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, parameterArnSeparator: '/' }); + const case6 = ssm.StringParameter.fromStringParameterAttributes(stack, `p${i++}`, { parameterName: param.valueAsString, simpleName: true }); 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, parameterArnSeparator: '' }); + const case9 = ssm.StringParameter.fromSecureStringParameterAttributes(stack, `p${i++}`, { parameterName: param.valueAsString, version: 10, simpleName: false }); // auto-generated name is always generated as a "simple name" (not/a/path) const case10 = new ssm.StringParameter(stack, `p${i++}`, { stringValue: 'value' }); @@ -524,9 +524,9 @@ export = { let i = 0; // WHEN - const p1 = new ssm.StringParameter(stack, `p${i++}`, { parameterName: param.valueAsString, stringValue: 'foo', parameterArnSeparator: '/' }); - const p2 = new ssm.StringParameter(stack, `p${i++}`, { parameterName: param.valueAsString, stringValue: 'foo', parameterArnSeparator: '' }); - const p3 = new ssm.StringListParameter(stack, `p${i++}`, { parameterName: param.valueAsString, stringListValue: [ 'foo' ], parameterArnSeparator: '' }); + const p1 = new ssm.StringParameter(stack, `p${i++}`, { parameterName: param.valueAsString, stringValue: 'foo', simpleName: true }); + const p2 = new ssm.StringParameter(stack, `p${i++}`, { parameterName: param.valueAsString, stringValue: 'foo', simpleName: false }); + const p3 = new ssm.StringListParameter(stack, `p${i++}`, { parameterName: param.valueAsString, stringListValue: [ 'foo' ], simpleName: false }); // THEN test.deepEqual(stack.resolve(p1.parameterArn), { 'Fn::Join': [ '', [ 'arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter/', { Ref: 'p0B02A8F65' } ] ] }); @@ -543,7 +543,7 @@ export = { let i = 0; // THEN - const expected = /Unable to determine ARN separator for SSM parameter since the parameter name is an unresolved token. Use "fromAttributes" and specify "parameterArnSeparator" explicitly/; + const expected = /Unable to determine ARN separator for SSM parameter since the parameter name is an unresolved token. Use "fromAttributes" and specify "simpleName" explicitly/; test.throws(() => ssm.StringParameter.fromStringParameterName(stack, `p${i++}`, param.valueAsString), expected); test.throws(() => ssm.StringParameter.fromSecureStringParameterAttributes(stack, `p${i++}`, { parameterName: param.valueAsString, version: 1 }), expected); test.throws(() => new ssm.StringParameter(stack, `p${i++}`, { parameterName: param.valueAsString, stringValue: 'foo' }), expected); @@ -551,25 +551,23 @@ export = { test.done(); }, - 'fails if parameterArnSeparator is wrong based on a concrete physical name'(test: Test) { + 'fails if simpleName is wrong based on a concrete physical name'(test: Test) { // GIVEN const stack = new Stack(); let i = 0; // THEN - test.throws(() => ssm.StringParameter.fromStringParameterAttributes(stack, `p${i++}`, { parameterName: 'simple', parameterArnSeparator: '' }), /parameterArnSeparator "" is invalid for SSM parameter with name "simple". It should be "\/"/); - test.throws(() => ssm.StringParameter.fromStringParameterAttributes(stack, `p${i++}`, { parameterName: '/foo/bar', parameterArnSeparator: '/' }), /parameterArnSeparator "\/" is invalid for SSM parameter with name \"\/foo\/bar\"\. It should be \"\"/); + test.throws(() => ssm.StringParameter.fromStringParameterAttributes(stack, `p${i++}`, { parameterName: 'simple', simpleName: false }), /Parameter name "simple" is a simple name, but "simpleName" was explicitly set to false. Either omit it or set it to true/); + test.throws(() => ssm.StringParameter.fromStringParameterAttributes(stack, `p${i++}`, { parameterName: '/foo/bar', simpleName: true }), /Parameter name "\/foo\/bar" is not a simple name, but "simpleName" was explicitly set to true. Either omit it or set it to false/); test.done(); }, - 'fails if parameterArnSeparator is not "/" or ""'(test: Test) { + 'fails if parameterName is undefined and simpleName is "false"'(test: Test) { + // GIVEN const stack = new Stack(); - const param = new CfnParameter(stack, 'param'); - let i = 0; // THEN - test.throws(() => ssm.StringParameter.fromStringParameterAttributes(stack, `p${i++}`, { parameterName: param.valueAsString, parameterArnSeparator: 'x' }), /parameterArnSeparator must be either "\/" or ""\. got "x"/); - test.throws(() => ssm.StringParameter.fromStringParameterAttributes(stack, `p${i++}`, { parameterName: param.valueAsString, parameterArnSeparator: 'boom' }), /parameterArnSeparator must be either "\/" or ""\. got "boom"/); + test.throws(() => new ssm.StringParameter(stack, 'p', { simpleName: false, stringValue: 'foo' }), /If "parameterName" is not explicitly defined, "simpleName" must be "true" or undefined since auto-generated parameter names always have simple names/); test.done(); } }; diff --git a/packages/@aws-cdk/aws-ssm/test/test.util.ts b/packages/@aws-cdk/aws-ssm/test/test.util.ts index 15033cd0327f8..2f11117384415 100644 --- a/packages/@aws-cdk/aws-ssm/test/test.util.ts +++ b/packages/@aws-cdk/aws-ssm/test/test.util.ts @@ -27,7 +27,7 @@ export = { 'token parameterName, explicit "/" separator'(test: Test) { const stack = new Stack(); - test.deepEqual(stack.resolve(arnForParameterName(stack, Token.asString({ Ref: 'Boom' }), { parameterArnSeparator: '/' })), { + test.deepEqual(stack.resolve(arnForParameterName(stack, Token.asString({ Ref: 'Boom' }), { simpleName: true })), { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter/', { Ref: 'Boom' }]] }); test.done(); @@ -55,7 +55,7 @@ export = { 'token parameterName, explicit "" separator'(test: Test) { const stack = new Stack(); - test.deepEqual(stack.resolve(arnForParameterName(stack, Token.asString({ Ref: 'Boom' }), { parameterArnSeparator: '' })), { + test.deepEqual(stack.resolve(arnForParameterName(stack, Token.asString({ Ref: 'Boom' }), { simpleName: false })), { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':ssm:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':parameter', { Ref: 'Boom' }]] }); test.done(); @@ -65,7 +65,7 @@ export = { 'fails if explicit separator is not defined and parameterName is a token'(test: Test) { const stack = new Stack(); - test.throws(() => arnForParameterName(stack, Token.asString({ Ref: 'Boom' })), /Unable to determine ARN separator for SSM parameter since the parameter name is an unresolved token. Use "fromAttributes" and specify "parameterArnSeparator" explicitly/); + test.throws(() => arnForParameterName(stack, Token.asString({ Ref: 'Boom' })), /Unable to determine ARN separator for SSM parameter since the parameter name is an unresolved token. Use "fromAttributes" and specify "simpleName" explicitly/); test.done(); }