From 1309960e62d13b00dc964f6c88e15d74390e09a2 Mon Sep 17 00:00:00 2001
From: Elad Ben-Israel <benisrae@amazon.com>
Date: Tue, 10 Mar 2020 15:24:15 +0200
Subject: [PATCH] feat: allow arbitrary construct names (#64)

To improve interoperability of cdk8s with other frameworks, do not impose DNS_LABEL constraints on construct names. Instead, normalize them to DNS_LABEL in case they don't comply.

Related to #48
---
 packages/cdk8s/lib/names.ts       | 27 +++++++++++++--------------
 packages/cdk8s/test/names.test.ts | 17 ++++++++---------
 2 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/packages/cdk8s/lib/names.ts b/packages/cdk8s/lib/names.ts
index 4ee15e4629..ae176202e3 100644
--- a/packages/cdk8s/lib/names.ts
+++ b/packages/cdk8s/lib/names.ts
@@ -39,24 +39,16 @@ export class Names {
       throw new Error(`minimum max length for object names is ${HASH_LEN} (required for hash)`);
     }
 
-    const components = path.split('/');
+    let components = path.split('/');
 
-    // verify components only use allowed chars.
-    for (const comp of components) {
-      if (!VALIDATE.test(comp)) {
-        throw new Error(`"${comp}" is not a valid object name. The characters allowed in names are: digits (0-9), lower case letters (a-z), -, and .`);
-      }
-
-      if (comp.length > maxLen) {
-        throw new Error(`Object name "${comp}" is too long. Maximum allowed length is ${maxLen}`);
-      }
-    }
-
-    // special case: if we only have one component in our path, we don't decorate it
-    if (components.length === 1) {
+    // special case: if we only have one component in our path and it adheres to DNS_NAME, we don't decorate it
+    if (components.length === 1 && VALIDATE.test(components[0]) && components[0].length <= maxLen) {
       return components[0];
     }
 
+    // okay, now we need to normalize all components to adhere to DNS_NAME and append the hash of the full path.
+    components = components.map(c => normalizeToDnsName(c, maxLen));
+
     components.push(calcHash(path, HASH_LEN));
 
     return components
@@ -75,6 +67,13 @@ export class Names {
   }
 }
 
+function normalizeToDnsName(c: string, maxLen: number) {
+  return c
+    .toLocaleLowerCase()        // lower case
+    .replace(/[^0-9a-z-]/g, '') // remove non-allowed characters
+    .substr(0, maxLen)          // trim to maxLength
+}
+
 function calcHash(path: string, maxLen: number) {
   const hash = crypto.createHash('sha256');
   hash.update(path);
diff --git a/packages/cdk8s/test/names.test.ts b/packages/cdk8s/test/names.test.ts
index 6b19a15717..c0d6d30e67 100644
--- a/packages/cdk8s/test/names.test.ts
+++ b/packages/cdk8s/test/names.test.ts
@@ -2,18 +2,17 @@ import { Names } from "../lib/names";
 
 const toDnsName = Names.toDnsLabel;
 
-test('allowed characters', () => {
-  const expected = /is not a valid object name/;
-  expect(() => toDnsName(' ')).toThrow(expected);
-  expect(() => toDnsName('')).toThrow(expected);
-  expect(() => toDnsName('Hello')).toThrow(expected);
-  expect(() => toDnsName('hey*')).toThrow(expected);
-  expect(() => toDnsName('not allowed')).toThrow(expected);
+test('normalize to dns_name', () => {
+  expect(toDnsName(' ')).toEqual('36a9e7f1');
+  expect(toDnsName('')).toEqual('e3b0c442');
+  expect(toDnsName('Hello')).toEqual('hello-185f8db3');
+  expect(toDnsName('hey*')).toEqual('hey-96c05e6c');
+  expect(toDnsName('not allowed')).toEqual('notallowed-a26075ed');
 });
 
 test('maximum length for a single term', () => {
-  expect(() => toDnsName('1234567890a', 10)).toThrow(/Maximum allowed length is 10/);
-  expect(() => toDnsName('a'.repeat(64))).toThrow(/Maximum allowed length is 63/);
+  expect(toDnsName('1234567890abcdef', 15)).toEqual('123456-8e9916c5');
+  expect(toDnsName('x' + 'a'.repeat(64))).toEqual('xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-f69f4ba1');
 });
 
 test('single term is not decorated with a hash', () => {