Skip to content

Commit

Permalink
fix(ssm): malformed ARNs for parameters with physical names that use …
Browse files Browse the repository at this point in the history
…path notation (#4842)

* fix(ssm): malformed ARNs for parameters with physical names that use path notation

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`

* 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 "".

* misc

* fix test expectation
* add public API doc

* add --frozen-lockfile to install.sh

* rename "parameterArnSeparator: string" to "simpleName: boolean"
  • Loading branch information
Elad Ben-Israel authored and mergify[bot] committed Nov 5, 2019
1 parent df6fc58 commit 43f276a
Show file tree
Hide file tree
Showing 7 changed files with 620 additions and 58 deletions.
3 changes: 3 additions & 0 deletions install.sh
@@ -0,0 +1,3 @@
#!/bin/bash
set -euo pipefail
exec npx yarn install --frozen-lockfile
85 changes: 55 additions & 30 deletions packages/@aws-cdk/aws-ssm/lib/parameter.ts
Expand Up @@ -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, AUTOGEN_MARKER } from './util';

/**
* An SSM Parameter reference.
Expand Down Expand Up @@ -94,6 +95,22 @@ export interface ParameterOptions {
* @default - a name will be generated by CloudFormation
*/
readonly parameterName?: string;

/**
* 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 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 simpleName?: boolean;
}

/**
Expand Down Expand Up @@ -184,12 +201,36 @@ export enum ParameterType {
AWS_EC2_IMAGE_ID = 'AWS::EC2::Image::Id',
}

export interface StringParameterAttributes {
/**
* Common attributes for string parameters.
*/
export interface CommonStringParameterAttributes {
/**
* 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;

/**
* 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 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 simpleName?: boolean;
}

export interface StringParameterAttributes extends CommonStringParameterAttributes {
/**
* The version number of the value you wish to retrieve.
*
Expand All @@ -205,12 +246,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.
*/
Expand Down Expand Up @@ -253,7 +289,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, { simpleName: attrs.simpleName });
public readonly parameterType = type;
public readonly stringValue = stringValue;
}
Expand All @@ -269,7 +305,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, { simpleName: attrs.simpleName });
public readonly parameterType = ParameterType.SECURE_STRING;
public readonly stringValue = stringValue;
public readonly encryptionKey = attrs.encryptionKey;
Expand Down Expand Up @@ -360,7 +396,10 @@ 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, {
physicalName: props.parameterName || AUTOGEN_MARKER,
simpleName: props.simpleName
});

this.parameterType = resource.attrType;
this.stringValue = resource.attrValue;
Expand Down Expand Up @@ -413,7 +452,10 @@ 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, {
physicalName: props.parameterName || AUTOGEN_MARKER,
simpleName: props.simpleName
});

this.parameterType = resource.attrType;
this.stringListValue = Fn.split(',', resource.attrValue);
Expand Down Expand Up @@ -442,20 +484,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,
});
}
63 changes: 63 additions & 0 deletions packages/@aws-cdk/aws-ssm/lib/util.ts
@@ -0,0 +1,63 @@
import { IConstruct, Stack, Token } from "@aws-cdk/core";

export const AUTOGEN_MARKER = '$$autogen$$';

export interface ArnForParameterNameOptions {
readonly physicalName?: string;
readonly simpleName?: boolean;
}

/**
* 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, options: ArnForParameterNameOptions = { }): string {
const physicalName = options.physicalName;
const nameToValidate = 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: isSimpleName() ? '/' : '',
resourceName: parameterName,
});

/**
* 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 `simpleName` when they import the ARN.
*/
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.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.simpleName;
}

const result = !concreteName.startsWith('/');

// if users explicitly specify the separator and it conflicts with the one we need, it's an error.
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 result;
}
}

0 comments on commit 43f276a

Please sign in to comment.