Exit on invalid DNS user input

- `spk service create` and `spk ring create` now log an error and exit when any
  arguments are inputted which are not RFC1123 compliant.
  - The checks only occur when the input has been explicitly passed by the user
    (i.e. not `undefined` or `""`)

Author: evanlouie

src/commands/ring/create.test.ts

@@ -4,6 +4,7 @@ import {
 } from "../../lib/bedrockYaml";
 import * as fileUtils from "../../lib/fileutils";
 import { createTempDir } from "../../lib/ioUtil";
+import * as dns from "../../lib/net/dns";
 import { disableVerboseLogging, enableVerboseLogging } from "../../logger";
 import { IBedrockFile } from "../../types";
 import { checkDependencies, execute } from "./create";
@@ -60,9 +61,19 @@ describe("test execute function and logic", () => {
     expect(exitFn).toBeCalledTimes(1);
     expect(exitFn.mock.calls).toEqual([[1]]);
   });
+  it("test execute function: invalid ring input", async () => {
+    const exitFn = jest.fn();
+    jest.spyOn(dns, "assertIsValid");
+    await execute("-not!dns@compliant%", "someprojectpath", exitFn);
+    expect(dns.assertIsValid).toHaveReturnedTimes(0); // should never return because it throws
+    expect(exitFn).toBeCalledTimes(1);
+    expect(exitFn.mock.calls).toEqual([[1]]);
+  });
   it("test execute function: missing project path", async () => {
     const exitFn = jest.fn();
+    jest.spyOn(dns, "assertIsValid");
     await execute("ring", "", exitFn);
+    expect(dns.assertIsValid).toHaveReturnedTimes(1);
     expect(exitFn).toBeCalledTimes(1);
     expect(exitFn.mock.calls).toEqual([[1]]);
   });

src/commands/ring/create.ts

@@ -10,6 +10,7 @@ import {
   PROJECT_INIT_DEPENDENCY_ERROR_MESSAGE
 } from "../../lib/constants";
 import { updateTriggerBranchesForServiceBuildAndUpdatePipeline } from "../../lib/fileutils";
+import * as dns from "../../lib/net/dns";
 import { hasValue } from "../../lib/validator";
 import { logger } from "../../logger";
 import { IBedrockFile, IBedrockFileInfo } from "../../types";
@@ -35,6 +36,7 @@ export const execute = async (
   try {
     logger.info(`Project path: ${projectPath}`);
 
+    dns.assertIsValid("<ring-name>", ringName);
     checkDependencies(projectPath, ringName);
 
     // Add ring to bedrock.yaml

src/commands/service/create.test.ts

@@ -1,14 +1,15 @@
-import fs from "fs";
-import path from "path";
+import * as fs from "fs";
+import * as path from "path";
 import { promisify } from "util";
-import uuid from "uuid/v4";
+import uuid = require("uuid/v4");
 import { Bedrock } from "../../config";
 import * as config from "../../config";
 import * as bedrockYaml from "../../lib/bedrockYaml";
 import { DEFAULT_CONTENT as BedrockMockedContent } from "../../lib/bedrockYaml";
 import { SERVICE_PIPELINE_FILENAME } from "../../lib/constants";
 import { checkoutCommitPushCreatePRLink } from "../../lib/gitutils";
 import { createTempDir, removeDir } from "../../lib/ioUtil";
+import * as dns from "../../lib/net/dns";
 import { deepClone } from "../../lib/util";
 import {
   disableVerboseLogging,
@@ -20,12 +21,14 @@ import {
   createTestMaintainersYaml
 } from "../../test/mockFactory";
 import {
+  assertValidDnsInputs,
   createService,
   execute,
   fetchValues,
   ICommandValues,
   validateGitUrl
 } from "./create";
+
 jest.mock("../../lib/gitutils");
 
 beforeAll(() => {
@@ -130,7 +133,69 @@ describe("Test fetchValues function", () => {
   });
 });
 
+describe("isValidDnsInputs", () => {
+  test("valid inputs does not throw", () => {
+    expect(() =>
+      assertValidDnsInputs({
+        displayName: "bar",
+        k8sBackend: "my-service",
+        pathPrefix: "service",
+        pathPrefixMajorVersion: "v1"
+      })
+    ).not.toThrow();
+  });
+
+  test("invalid inputs throws", () => {
+    expect(() =>
+      assertValidDnsInputs({
+        displayName: "-not_dns_compliant",
+        k8sBackend: "",
+        pathPrefix: "",
+        pathPrefixMajorVersion: ""
+      })
+    ).toThrow();
+
+    expect(() =>
+      assertValidDnsInputs({
+        displayName: "",
+        k8sBackend: "-not_dns_compliant",
+        pathPrefix: "",
+        pathPrefixMajorVersion: ""
+      })
+    ).toThrow();
+
+    expect(() =>
+      assertValidDnsInputs({
+        displayName: "",
+        k8sBackend: "",
+        pathPrefix: "-not_dns_compliant",
+        pathPrefixMajorVersion: ""
+      })
+    ).toThrow();
+
+    expect(() =>
+      assertValidDnsInputs({
+        displayName: "",
+        k8sBackend: "",
+        pathPrefix: "",
+        pathPrefixMajorVersion: "-not_dns_compliant"
+      })
+    ).toThrow();
+  });
+});
+
 describe("Test execute function", () => {
+  it("Negative test: with non-dns compliant values", async () => {
+    const exitFn = jest.fn();
+    jest.spyOn(dns, "assertIsValid");
+    await execute(
+      "foo",
+      { ...getMockValues(), displayName: "-non_dns@compliant" },
+      exitFn
+    );
+    expect(dns.assertIsValid).toHaveBeenCalledTimes(1);
+  });
+
   it("Negative test: without service name", async () => {
     const exitFn = jest.fn();
     await execute("", getMockValues(), exitFn);

src/commands/service/create.ts

@@ -21,6 +21,7 @@ import {
   generateServiceBuildAndUpdatePipelineYaml
 } from "../../lib/fileutils";
 import { checkoutCommitPushCreatePRLink } from "../../lib/gitutils";
+import * as dns from "../../lib/net/dns";
 import { isPortNumberString } from "../../lib/validator";
 import { logger } from "../../logger";
 import { IBedrockFileInfo, IHelmConfig, IUser } from "../../types";
@@ -54,7 +55,7 @@ export interface ICommandValues extends ICommandOptions {
 
 export const fetchValues = (opts: ICommandOptions) => {
   if (!isPortNumberString(opts.k8sBackendPort)) {
-    throw new Error("value for --k8s-service-port is not a valid port number");
+    throw Error("value for --k8s-service-port is not a valid port number");
   }
 
   const bedrock = Bedrock();
@@ -97,9 +98,34 @@ export const fetchValues = (opts: ICommandOptions) => {
 export const checkDependencies = (projectPath: string) => {
   const fileInfo: IBedrockFileInfo = bedrockFileInfo(projectPath);
   if (fileInfo.exist === false) {
-    throw new Error(PROJECT_INIT_CVG_DEPENDENCY_ERROR_MESSAGE);
+    throw Error(PROJECT_INIT_CVG_DEPENDENCY_ERROR_MESSAGE);
   } else if (fileInfo.hasVariableGroups === false) {
-    throw new Error(PROJECT_CVG_DEPENDENCY_ERROR_MESSAGE);
+    throw Error(PROJECT_CVG_DEPENDENCY_ERROR_MESSAGE);
+  }
+};
+
+/**
+ * Validate that the provided ICommandOpts contain RFC1123 compliant values if
+ * they are were set by the user
+ *
+ * @throws {Error} when any of displayName, k8sBackend, pathPrefix,
+ *                 pathPrefixMajorVersion are provided and are non-DNS compliant
+ */
+export const assertValidDnsInputs = (opts: Partial<ICommandOptions>): void => {
+  if (opts.displayName) {
+    dns.assertIsValid("--display-name", opts.displayName);
+  }
+  if (opts.k8sBackend) {
+    dns.assertIsValid("--k8s-backend", opts.k8sBackend);
+  }
+  if (opts.pathPrefix) {
+    dns.assertIsValid("--path-prefix", opts.pathPrefix);
+  }
+  if (opts.pathPrefixMajorVersion) {
+    dns.assertIsValid(
+      "--path-prefix-major-version",
+      opts.pathPrefixMajorVersion
+    );
   }
 };
 
@@ -116,12 +142,20 @@ export const execute = async (
 
   if (serviceName === "." && opts.displayName === "") {
     logger.error(
-      `If specifying the current directory as service name, please incluce a display name using '-n'`
+      `If specifying the current directory as service name, please include a display name using '-n'`
     );
     await exitFn(1);
     return;
   }
 
+  // validate user inputs are DNS compliant
+  try {
+    assertValidDnsInputs(opts);
+  } catch (err) {
+    logger.error(err);
+    await exitFn(1);
+  }
+
   // Sanity checking the specified Helm URLs
   await validateGitUrl(opts.helmConfigGit, exitFn);
 
@@ -242,7 +276,7 @@ export const createService = async (
     logger.error(
       `Cannot create service pipeline due to serviceName being '.'. Please include a displayName if you are trying to create a service in your project root directory.`
     );
-    throw new Error(
+    throw Error(
       "Cannot create service pipeline due to serviceName being '.'. Please include a displayName if you are trying to create a service in your project root directory."
     );
   }

src/lib/net/dns.ts

@@ -55,7 +55,7 @@ export function assertIsValid(
 ): asserts dns is string {
   if (!isValid(dns)) {
     throw Error(
-      `Invalid ${fieldName} '${dns}' provided for Traefik IngressRoute. Must be RFC1123 compliant and match regex: ${validDnsRegex}`
+      `Invalid ${fieldName} '${dns}' provided. Must be RFC1123 compliant and match regex: ${validDnsRegex}`
     );
   }
 }