Skip to content

Commit

Permalink
fix(ssm): Generate correct SSM Parameter ARN (#1726)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
RomainMuller committed Feb 12, 2019
1 parent 5c11680 commit 39df456
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 6 deletions.
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-ssm/lib/parameter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
{
"Ref": "AWS::AccountId"
},
":parameter/",
":parameter",
{
"Ref": "StringParameter472EED0E"
}
Expand Down
21 changes: 21 additions & 0 deletions packages/@aws-cdk/aws-ssm/test/test.parameter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
};
10 changes: 5 additions & 5 deletions packages/@aws-cdk/cdk/lib/cloudformation/arn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
17 changes: 17 additions & 0 deletions packages/@aws-cdk/cdk/test/cloudformation/test.arn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down

0 comments on commit 39df456

Please sign in to comment.