fix(ssm): correctly deduplicate parameter names (#3183)

Parameter names with '/' in them would not be correctly deduplicated,
since the '/' is also used to delimit identifiers in construct paths.

Change findChild() so it no longer traverses into the tree; if we assume it
will only ever find one child deep, we can do the same kind of escaping
there as we apply to construct IDs.

Fixes #3076.

BREAKING CHANGE: `construct.findChild()` now only looks up direct children

Author: rix0rrr

packages/@aws-cdk/aws-ecs/test/test.ecs-cluster.ts

@@ -155,6 +155,22 @@ export = {
       test.done();
     },
 
+    "multiple clusters with default capacity"(test: Test) {
+      // GIVEN
+      const stack =  new cdk.Stack();
+      const vpc = new ec2.Vpc(stack, 'MyVpc', {});
+
+      // WHEN
+      for (let i = 0; i < 2; i++) {
+        const cluster = new ecs.Cluster(stack, `EcsCluster${i}`, { vpc, });
+        cluster.addCapacity('MyCapacity', {
+          instanceType: new ec2.InstanceType('m3.medium'),
+        });
+      }
+
+      test.done();
+    },
+
     'lifecycle hook is automatically added'(test: Test) {
       // GIVEN
       const stack =  new cdk.Stack();

packages/@aws-cdk/aws-ssm/lib/parameter.ts

@@ -248,6 +248,7 @@ export class StringParameter extends ParameterBase implements IStringParameter {
     const stack = Stack.of(scope);
     const id = makeIdentityForImportedValue(parameterName);
     const exists = stack.node.tryFindChild(id) as IStringParameter;
+
     if (exists) { return exists.stringValue; }
 
     return this.fromStringParameterAttributes(stack, id, { parameterName, version }).stringValue;

packages/@aws-cdk/aws-ssm/test/test.parameter.ts

@@ -281,7 +281,17 @@ export = {
         }
       });
       test.done();
-    }
+    },
+
+    'can query actual SSM Parameter Names, multiple times'(test: Test) {
+      // GIVEN
+      const stack = new Stack();
 
+      // WHEN
+      ssm.StringParameter.valueForStringParameter(stack, '/my/param/name');
+      ssm.StringParameter.valueForStringParameter(stack, '/my/param/name');
+
+      test.done();
+    },
   }
 };

packages/@aws-cdk/core/lib/construct.ts

@@ -129,7 +129,7 @@ export class ConstructNode {
   constructor(private readonly host: Construct, scope: IConstruct, id: string) {
     id = id || ''; // if undefined, convert to empty string
 
-    this.id = id;
+    this.id = sanitizeId(id);
     this.scope = scope;
 
     // We say that scope is required, but root scopes will bypass the type
@@ -146,9 +146,6 @@ export class ConstructNode {
       this.id = id;
     }
 
-    // escape any path separators so they don't wreck havoc
-    this.id = this._escapePathSeparator(this.id);
-
     if (Token.isUnresolved(id)) {
       throw new Error(`Cannot use tokens in construct ID: ${id}`);
     }
@@ -174,25 +171,13 @@ export class ConstructNode {
   }
 
   /**
-   * Return a descendant by path, or undefined
+   * Return a direct child by id, or undefined
    *
-   * Note that if the original ID of the construct you are looking for contained
-   * a '/', then it would have been replaced by '--'.
-   *
-   * @param path Relative path of a direct or indirect child
-   * @returns a child by path or undefined if not found.
+   * @param id Identifier of direct child
+   * @returns the child if found, or undefined
    */
-  public tryFindChild(path: string): IConstruct | undefined {
-    if (path.startsWith(ConstructNode.PATH_SEP)) {
-      throw new Error('Path must be relative');
-    }
-    const parts = path.split(ConstructNode.PATH_SEP);
-
-    let curr: IConstruct | undefined = this.host;
-    while (curr != null && parts.length > 0) {
-      curr = curr.node._children[parts.shift()!];
-    }
-    return curr;
+  public tryFindChild(id: string): IConstruct | undefined {
+    return this._children[sanitizeId(id)];
   }
 
   /**
@@ -527,14 +512,6 @@ export class ConstructNode {
       this.invokedAspects.push(aspect);
     }
   }
-
-  /**
-   * If the construct ID contains a path separator, it is replaced by double dash (`--`).
-   */
-  private _escapePathSeparator(id: string) {
-    if (!id) { return id; }
-    return id.split(ConstructNode.PATH_SEP).join('--');
-  }
 }
 
 /**
@@ -715,3 +692,13 @@ function ignore(_x: any) {
 // Import this _after_ everything else to help node work the classes out in the correct order...
 
 import { Reference } from './reference';
+
+const PATH_SEP_REGEX = new RegExp(`${ConstructNode.PATH_SEP}`, 'g');
+
+/**
+ * Return a sanitized version of an arbitrary string, so it can be used as an ID
+ */
+function sanitizeId(id: string) {
+  // Escape path seps as double dashes
+  return id.replace(PATH_SEP_REGEX, '--');
+}

packages/@aws-cdk/core/lib/private/encoding.ts

@@ -78,7 +78,7 @@ export class TokenString {
 /**
  * Quote a string for use in a regex
  */
-function regexQuote(s: string) {
+export function regexQuote(s: string) {
   return s.replace(/[.?*+^$[\]\\(){}|-]/g, "\\$&");
 }