From c321f88c923416afa218c8ef67846d25af118477 Mon Sep 17 00:00:00 2001 From: Evan Louie Date: Wed, 18 Mar 2020 11:33:45 -0700 Subject: [PATCH] Default Routing for isDefault Ring - When a ring is marked as `isDefault: true`, `spk hld reconcile` will now generate an additional IngressRoute and Middleware. - The IngressRoute will not have a `Ring` route match rule - The IngressRoute points to the same backend service as its ringed counterpart. - Only one ring can be marked `isDefault: true` -- validation is run at `spk hld reconcile` execute time, throwing an error if more than one `isDefault`. - Refactored IngressRoute tests - Added compatibility configuration for eslint and prettier. closes https://github.com/microsoft/bedrock/issues/1084 --- jest.config.js | 7 +- src/commands/hld/reconcile-unit.test.ts | 151 ++++++++ src/commands/hld/reconcile.test.ts | 4 +- src/commands/hld/reconcile.ts | 141 ++++--- src/config.ts | 2 +- src/lib/bedrockYaml.test.ts | 48 +++ src/lib/bedrockYaml.ts | 30 +- src/lib/traefik/ingress-route.test.ts | 472 ++++++++++++++---------- src/lib/traefik/ingress-route.ts | 18 +- src/lib/traefik/middleware.test.ts | 10 +- src/lib/traefik/middleware.ts | 2 +- webpack.config.js | 1 + 12 files changed, 634 insertions(+), 252 deletions(-) create mode 100644 src/commands/hld/reconcile-unit.test.ts diff --git a/jest.config.js b/jest.config.js index 91a2d2c0d..a29301a23 100644 --- a/jest.config.js +++ b/jest.config.js @@ -1,4 +1,5 @@ module.exports = { - preset: 'ts-jest', - testEnvironment: 'node', -}; \ No newline at end of file + preset: "ts-jest", + testEnvironment: "node", + rootDir: "src" +}; diff --git a/src/commands/hld/reconcile-unit.test.ts b/src/commands/hld/reconcile-unit.test.ts new file mode 100644 index 000000000..4b20b4354 --- /dev/null +++ b/src/commands/hld/reconcile-unit.test.ts @@ -0,0 +1,151 @@ +import * as fs from "fs"; +import * as mocks from "../../test/mockFactory"; +import { BedrockFile } from "../../types"; +import * as reconcile from "./reconcile"; + +jest.mock("fs"); + +beforeEach(() => { + jest.resetAllMocks(); +}); + +describe("createMiddlewareForRing", () => { + const tests: { + name: string; + actual: () => unknown; + expected: unknown; + effects?: (() => void)[]; + }[] = [ + { + name: "isDefault === false; creates one (1) middleware", + actual: (): unknown => + reconcile.createMiddlewareForRing( + "path-to-ring", + "my-service", + "my-ring", + "v1", + false + ), + expected: { ringed: expect.anything(), default: undefined }, + effects: [ + (): void => { + expect(fs.writeFileSync).toHaveBeenCalledTimes(1); + expect(fs.writeFileSync).toHaveBeenCalledWith( + expect.anything(), + expect.not.stringContaining("\n---\n") + ); + }, + ], + }, + + { + name: "isDefault === true; creates two (2) middleware", + actual: (): unknown => + reconcile.createMiddlewareForRing( + "path-to-ring", + "my-service", + "my-ring", + "v1", + true + ), + expected: { + ringed: expect.objectContaining({ + metadata: { name: "my-service-my-ring" }, + }), + default: expect.objectContaining({ metadata: { name: "my-service" } }), + }, + effects: [ + (): void => { + expect(fs.writeFileSync).toHaveBeenCalledTimes(1); + expect(fs.writeFileSync).toHaveBeenCalledWith( + expect.anything(), + expect.stringContaining("\n---\n") + ); + }, + ], + }, + ]; + + for (const { name, actual, expected, effects } of tests) { + it(name, () => { + expect(actual()).toStrictEqual(expected); + for (const effect of effects ?? []) { + effect(); + } + }); + } +}); + +describe("createIngressRouteForRing", () => { + const { services } = mocks.createTestBedrockYaml(false) as BedrockFile; + const tests: { + name: string; + actual: () => unknown; + expected: unknown; + effects?: (() => void)[]; + }[] = [ + { + name: "isDefault === false; creates one (1) IngressRoute", + actual: (): unknown => + reconcile.createIngressRouteForRing( + "path-to-ring", + "my-service", + Object.values(services)[0], + { ringed: { metadata: { name: "my-service-my-ring" } } }, + "my-ring", + "version-path", + false + ), + expected: [ + expect.objectContaining({ metadata: { name: "my-service-my-ring" } }), + ], + effects: [ + (): void => { + // Should write out one yaml document (no "---") + expect(fs.writeFileSync).toHaveBeenCalledTimes(1); + expect(fs.writeFileSync).toHaveBeenCalledWith( + expect.anything(), + expect.not.stringContaining("\n---\n") + ); + }, + ], + }, + + { + name: "isDefault === true; creates two (2) IngressRoute", + actual: (): unknown => + reconcile.createIngressRouteForRing( + "foo", + "my-service", + Object.values(services)[0], + { ringed: { metadata: { name: "my-service-my-ring" } } }, + "my-ring", + "version-path", + true + ), + expected: [ + expect.objectContaining({ metadata: { name: "my-service-my-ring" } }), + expect.objectContaining({ metadata: { name: "my-service" } }), + ], + effects: [ + (): void => { + // Should write out two yaml documents (with "---") + expect(fs.writeFileSync).toHaveBeenCalledTimes(1); + expect(fs.writeFileSync).toHaveBeenCalledWith( + expect.anything(), + expect.stringContaining("\n---\n") + ); + }, + ], + }, + ]; + + for (const { name, actual, expected, effects } of tests) { + it(name, () => { + expect(actual()).toStrictEqual(expected); + for (const effect of effects ?? []) { + effect(); + } + }); + } +}); diff --git a/src/commands/hld/reconcile.test.ts b/src/commands/hld/reconcile.test.ts index 7d69341df..e159eb5f6 100644 --- a/src/commands/hld/reconcile.test.ts +++ b/src/commands/hld/reconcile.test.ts @@ -2,6 +2,7 @@ import path from "path"; import { create as createBedrockYaml } from "../../lib/bedrockYaml"; import { disableVerboseLogging, enableVerboseLogging } from "../../logger"; import { BedrockFile, BedrockServiceConfig } from "../../types"; +import * as reconcile from "./reconcile"; import { addChartToRing, checkForFabrikate, @@ -14,13 +15,12 @@ import { execAndLog, execute, getFullPathPrefix, - ReconcileDependencies, normalizedName, + ReconcileDependencies, reconcileHld, testAndGetAbsPath, validateInputs, } from "./reconcile"; -import * as reconcile from "./reconcile"; beforeAll(() => { enableVerboseLogging(); diff --git a/src/commands/hld/reconcile.ts b/src/commands/hld/reconcile.ts index c48503f63..7169d7395 100644 --- a/src/commands/hld/reconcile.ts +++ b/src/commands/hld/reconcile.ts @@ -2,19 +2,20 @@ /* eslint-disable @typescript-eslint/camelcase */ import child_process from "child_process"; import commander from "commander"; -import { writeFileSync } from "fs"; +import fs from "fs"; import yaml from "js-yaml"; import path from "path"; import process from "process"; import shelljs, { TestOptions } from "shelljs"; import { Bedrock } from "../../config"; import { assertIsStringWithContent } from "../../lib/assertions"; +import * as bedrock from "../../lib/bedrockYaml"; import { build as buildCmd, exit as exitCmd } from "../../lib/commandBuilder"; import { generateAccessYaml } from "../../lib/fileutils"; import { tryGetGitOrigin } from "../../lib/gitutils"; import * as dns from "../../lib/net/dns"; -import { TraefikIngressRoute } from "../../lib/traefik/ingress-route"; -import { TraefikMiddleware } from "../../lib/traefik/middleware"; +import * as ingressRoute from "../../lib/traefik/ingress-route"; +import * as middleware from "../../lib/traefik/middleware"; import { logger } from "../../logger"; import { BedrockFile, BedrockServiceConfig } from "../../types"; import decorator from "./reconcile.decorator.json"; @@ -57,7 +58,7 @@ const exec = async (cmd: string, pipeIO = false): Promise => { export interface ReconcileDependencies { exec: typeof execAndLog; - writeFile: typeof writeFileSync; + writeFile: typeof fs.writeFileSync; getGitOrigin: typeof tryGetGitOrigin; generateAccessYaml: typeof generateAccessYaml; createAccessYaml: typeof createAccessYaml; @@ -105,6 +106,7 @@ export const execute = async ( ); const bedrockConfig = Bedrock(absBedrockPath); + bedrock.validateRings(bedrockConfig); logger.info( `Attempting to reconcile HLD with services tracked in bedrock.yaml` @@ -123,7 +125,7 @@ export const execute = async ( exec: execAndLog, generateAccessYaml, getGitOrigin: tryGetGitOrigin, - writeFile: writeFileSync, + writeFile: fs.writeFileSync, }; await reconcileHld( @@ -238,22 +240,23 @@ export const reconcileHld = async ( normalizedSvcName ); - const ringsToCreate = Object.keys(managedRings).map((ring) => { - const normalizedRingName = normalizedName(ring); - return { - normalizedRingName, - normalizedRingPathInHld: path.join( - normalizedSvcPathInHld, - normalizedRingName - ), - }; - }); + const ringsToCreate = Object.entries(managedRings).map( + ([ring, { isDefault }]) => { + const normalizedRingName = normalizedName(ring); + return { + isDefault: !!isDefault, + normalizedRingName, + normalizedRingPathInHld: path.join( + normalizedSvcPathInHld, + normalizedRingName + ), + }; + } + ); // Will only loop over _existing_ rings in bedrock.yaml - does not cover the deletion case, ie: i remove a ring from bedrock.yaml - for (const { - normalizedRingName, - normalizedRingPathInHld, - } of ringsToCreate) { + for (const ring of ringsToCreate) { + const { normalizedRingName, normalizedRingPathInHld, isDefault } = ring; // Create the ring in the service. await dependencies.createRingComponent( dependencies.exec, @@ -302,7 +305,8 @@ export const reconcileHld = async ( normalizedRingPathInHld, normalizedSvcName, normalizedRingName, - ingressVersionAndPath + ingressVersionAndPath, + isDefault ); // Create Ingress Route. @@ -312,7 +316,8 @@ export const reconcileHld = async ( serviceConfig, middlewares, normalizedRingName, - ingressVersionAndPath + ingressVersionAndPath, + isDefault ); } } @@ -369,50 +374,89 @@ export const createAccessYaml = async ( writeAccessYaml(absRepositoryPathInHldPath, originUrl); }; -const createIngressRouteForRing = ( +type MiddlewareMap>> = { + ringed: T; + default?: T; +}; + +export const createIngressRouteForRing = ( ringPathInHld: string, serviceName: string, serviceConfig: BedrockServiceConfig, - middlewares: TraefikMiddleware, + middlewares: MiddlewareMap, ring: string, - ingressVersionAndPath: string -): void => { + ingressVersionAndPath: string, + ringIsDefault = false +): ReturnType[] => { const staticComponentPathInRing = path.join(ringPathInHld, "static"); const ingressRoutePathInStaticComponent = path.join( staticComponentPathInRing, "ingress-route.yaml" ); - const ingressRoute = TraefikIngressRoute( + + // Push the default ingress route with ring header + const ingressRoutes = []; + const ringedRoute = ingressRoute.create( serviceName, ring, serviceConfig.k8sBackendPort, ingressVersionAndPath, { + isDefault: false, k8sBackend: serviceConfig.k8sBackend, middlewares: [ - middlewares.metadata.name, + middlewares.ringed.metadata?.name, ...(serviceConfig.middlewares ?? []), - ], + ].filter((e): e is NonNullable => !!e), } ); + ingressRoutes.push(ringedRoute); + + // If ring isDefault, scaffold an additional ingress route without the ring + // header -- i.e with an empty string ring name + if (ringIsDefault) { + const defaultRingRoute = ingressRoute.create( + serviceName, + ring, + serviceConfig.k8sBackendPort, + ingressVersionAndPath, + { + isDefault: ringIsDefault, + k8sBackend: serviceConfig.k8sBackend, + middlewares: [ + middlewares.default?.metadata?.name, + ...(serviceConfig.middlewares ?? []), + ].filter((e): e is NonNullable => !!e), + } + ); + ingressRoutes.push(defaultRingRoute); + } - const routeYaml = yaml.safeDump(ingressRoute, { - lineWidth: Number.MAX_SAFE_INTEGER, - }); + // serialize to routes to yaml separately and join them with `---` to specify + // multiple yaml documents in a single string + const routeYaml = ingressRoutes + .map((str) => { + return yaml.safeDump(str, { + lineWidth: Number.MAX_SAFE_INTEGER, + }); + }) + .join("\n---\n"); logger.info( `Writing IngressRoute YAML to '${ingressRoutePathInStaticComponent}'` ); - writeFileSync(ingressRoutePathInStaticComponent, routeYaml); + fs.writeFileSync(ingressRoutePathInStaticComponent, routeYaml); + return ingressRoutes; }; -const createMiddlewareForRing = ( +export const createMiddlewareForRing = ( ringPathInHld: string, serviceName: string, ring: string, - ingressVersionAndPath: string -): TraefikMiddleware => { + ingressVersionAndPath: string, + ringIsDefault = false +): MiddlewareMap => { // Create Middlewares const staticComponentPathInRing = path.join(ringPathInHld, "static"); const middlewaresPathInStaticComponent = path.join( @@ -420,17 +464,30 @@ const createMiddlewareForRing = ( "middlewares.yaml" ); - const middlewares = TraefikMiddleware(serviceName, ring, [ - ingressVersionAndPath, - ]); - const middlewareYaml = yaml.safeDump(middlewares, { - lineWidth: Number.MAX_SAFE_INTEGER, - }); + // Create the standard ringed middleware as well as one without the ring in + // the name if the ring isDefault + const middlewares = { + ringed: middleware.create(serviceName, ring, [ingressVersionAndPath]), + default: ringIsDefault + ? middleware.create(serviceName, "", [ingressVersionAndPath]) + : undefined, + }; + + // Serialize all the middlewares to yaml separately and join the strings with + // '---' to specify multiple yaml docs in a single string + const middlewareYaml = Object.values(middlewares) + .filter((e): e is NonNullable => !!e) + .map((str) => + yaml.safeDump(str, { + lineWidth: Number.MAX_SAFE_INTEGER, + }) + ) + .join("\n---\n"); logger.info( `Writing Middlewares YAML to '${middlewaresPathInStaticComponent}'` ); - writeFileSync(middlewaresPathInStaticComponent, middlewareYaml); + fs.writeFileSync(middlewaresPathInStaticComponent, middlewareYaml); return middlewares; }; diff --git a/src/config.ts b/src/config.ts index abac56778..ef0835cac 100644 --- a/src/config.ts +++ b/src/config.ts @@ -267,7 +267,7 @@ export const saveConfiguration = ( ): void => { try { const data = yaml.safeDump(readYaml(sourceFilePath), { - lineWidth: Number.MAX_SAFE_INTEGER + lineWidth: Number.MAX_SAFE_INTEGER, }); const targetFile = path.join(targetDir, "config.yaml"); fs.writeFileSync(targetFile, data); diff --git a/src/lib/bedrockYaml.test.ts b/src/lib/bedrockYaml.test.ts index 47c954e94..486d8b53f 100644 --- a/src/lib/bedrockYaml.test.ts +++ b/src/lib/bedrockYaml.test.ts @@ -12,6 +12,7 @@ import { read, removeRing, setDefaultRing, + validateRings, } from "./bedrockYaml"; describe("Creation and Existence test on bedrock.yaml", () => { @@ -226,3 +227,50 @@ describe("removeRing", () => { expect(() => removeRing(original, ringToRemove as string)).toThrow(); }); }); + +describe("validateRings", () => { + const bedrockFile = createTestBedrockYaml(false) as BedrockFile; + const tests: { + name: string; + actual: () => unknown; + throws: boolean; + }[] = [ + { + name: "no default ring: does not throw", + actual: (): unknown => + validateRings({ + ...bedrockFile, + rings: { master: { isDefault: false }, qa: {} }, + }), + throws: false, + }, + { + name: "one default ring: does not throw", + actual: (): unknown => + validateRings({ + ...bedrockFile, + rings: { master: { isDefault: true }, qa: {} }, + }), + throws: false, + }, + { + name: "multiple default ring: throws", + actual: (): unknown => + validateRings({ + ...bedrockFile, + rings: { master: { isDefault: true }, qa: { isDefault: true } }, + }), + throws: true, + }, + ]; + + for (const { name, actual, throws } of tests) { + it(name, () => { + if (throws) { + expect(() => actual()).toThrow(); + } else { + expect(() => actual()).not.toThrow(); + } + }); + } +}); diff --git a/src/lib/bedrockYaml.ts b/src/lib/bedrockYaml.ts index 9f8221ee0..361ee36e2 100644 --- a/src/lib/bedrockYaml.ts +++ b/src/lib/bedrockYaml.ts @@ -3,7 +3,13 @@ import yaml from "js-yaml"; import path from "path"; import { createTempDir } from "../lib/ioUtil"; import { logger } from "../logger"; -import { BedrockFile, BedrockFileInfo, HelmConfig, Rings } from "../types"; +import { + BedrockFile, + BedrockFileInfo, + HelmConfig, + Rings, + RingConfig, +} from "../types"; import { writeVersion, getVersion } from "./fileutils"; export const YAML_NAME = "bedrock.yaml"; @@ -233,3 +239,25 @@ export const removeRing = ( return bedrockWithoutRing; }; + +/** + * Validates that the rings in `bedrock` are valid and throws an Error if not. + * + * Throws when: + * - More than one ring is marked `isDefault` + * + * @param bedrock file to validate the rings of + */ +export const validateRings = (bedrock: BedrockFile): void => { + const rings = Object.entries(bedrock.rings).reduce( + (carry, [name, config]) => carry.concat({ ...config, name }), + [] as (RingConfig & { name: string })[] + ); + const defaultRings = rings.filter((ring) => ring.isDefault); + if (defaultRings.length > 1) { + const defaultRingsNames = defaultRings.map((ring) => ring.name).join(", "); + throw Error( + `only one ring may be set as 'isDefault: true' -- found [${defaultRingsNames}] ` + ); + } +}; diff --git a/src/lib/traefik/ingress-route.test.ts b/src/lib/traefik/ingress-route.test.ts index 9fcbbd647..eb713df0e 100644 --- a/src/lib/traefik/ingress-route.test.ts +++ b/src/lib/traefik/ingress-route.test.ts @@ -1,193 +1,285 @@ -import uuid = require("uuid/v4"); -import { TraefikIngressRoute } from "./ingress-route"; - -describe("TraefikIngressRoute", () => { - test("the right object name and service name is created", () => { - const routeWithoutRing = TraefikIngressRoute( - "my-service", - "", - 80, - "/version/and/Path" - ); - expect(routeWithoutRing.metadata.name).toBe("my-service"); - expect(routeWithoutRing.spec.routes[0].services[0].name).toBe("my-service"); - const routeWithRing = TraefikIngressRoute( - "my-service", - "prod", - 80, - "/version/and/Path" - ); - expect(routeWithRing.metadata.name).toBe("my-service-prod"); - expect(routeWithRing.spec.routes[0].services[0].name).toBe( - "my-service-prod" - ); - }); - - test("service backend maps to service name and port", () => { - const routeWithK8sBackendAndRing = TraefikIngressRoute( - "my-service", - "master", - 80, - "/version/and/path", - { - k8sBackend: "my-k8s-svc", - } - ); - - expect(routeWithK8sBackendAndRing.spec.routes[0].services[0].name).toBe( - "my-k8s-svc-master" - ); - }); - - test("manifest namespace gets injected properly", () => { - const randomNamespaces = Array.from({ length: 10 }, () => uuid()); - for (const namespace of randomNamespaces) { - const withoutNamespace = TraefikIngressRoute( - "foo", - "bar", - 80, - "/version/and/Path", - { - namespace, - } - ); - expect(withoutNamespace.metadata.namespace).toBe(namespace); - } - }); - - test("the service port gets properly injected", () => { - const randomPorts = Array.from({ length: 10 }, () => - Math.floor(Math.random() * 1000) - ); - for (const servicePort of randomPorts) { - const route = TraefikIngressRoute( - "foo", - "", - servicePort, - "/version/and/Path" - ); - expect(route.spec.routes[0].services[0].port).toBe(servicePort); +import { create } from "./ingress-route"; + +describe("TraefikIngressRoute - Unit Tests", () => { + type PartialIngressRoute = Partial>; + const tests: { + name: string; + actual: () => unknown; + expected: unknown; + effects?: (() => void)[]; + }[] = [ + { + name: + "correct object metadata.name, service name/port, namespace is created: no ring", + actual: (): unknown => + create("my-service", "", 80, "/version/and/Path", { + namespace: "my-namespace", + }), + expected: expect.objectContaining({ + metadata: { name: "my-service", namespace: "my-namespace" }, + spec: { + routes: expect.arrayContaining([ + expect.objectContaining({ + services: [{ name: "my-service", port: 80 }], + }), + ]), + }, + }), + }, + + { + name: + "correct object metadata.name, service name/port, namespace is created: with ring", + actual: (): unknown => + create("my-service", "prod", 1337, "/version/and/Path", { + namespace: "my-namespace", + }), + expected: expect.objectContaining({ + metadata: { name: "my-service-prod", namespace: "my-namespace" }, + spec: { + routes: expect.arrayContaining([ + expect.objectContaining({ + services: [{ name: "my-service-prod", port: 1337 }], + }), + ]), + }, + }), + }, + + { + name: "service backend maps to service name and port", + actual: (): unknown => + create("my-service", "master", 80, "/version/and/path", { + k8sBackend: "my-k8s-svc", + }), + expected: expect.objectContaining({ + spec: { + routes: expect.arrayContaining([ + expect.objectContaining({ + services: [{ name: "my-k8s-svc-master", port: 80 }], + }), + ]), + }, + }), + }, + + { + name: "middleware are added", + actual: (): unknown => + create("foo", "bar", 80, "/version/and/Path", { + middlewares: ["mw1", "mw2", "mw3"], + }), + expected: expect.objectContaining({ + spec: { + routes: expect.arrayContaining([ + expect.objectContaining({ + middlewares: [{ name: "mw1" }, { name: "mw2" }, { name: "mw3" }], + }), + ]), + }, + }), + }, + + { + name: "endpoints are injected properly: no entry-points", + actual: (): unknown => create("foo", "bar", 80, "/version/and/Path"), + expected: expect.objectContaining({ + spec: expect.not.objectContaining({ entryPoints: expect.anything() }), + }), + }, + + { + name: "endpoints are injected properly: web", + actual: (): unknown => + create("foo", "bar", 80, "/version/and/Path", { + entryPoints: ["web"], + }), + expected: expect.objectContaining({ + spec: expect.objectContaining({ entryPoints: ["web"] }), + }), + }, + + { + name: "endpoints are injected properly: web-secure", + actual: (): unknown => + create("foo", "bar", 80, "/version/and/Path", { + entryPoints: ["web-secure"], + }), + expected: expect.objectContaining({ + spec: expect.objectContaining({ entryPoints: ["web-secure"] }), + }), + }, + + { + name: "endpoints are injected properly: [web,web-secure]", + actual: (): unknown => + create("foo", "bar", 80, "/version/and/Path", { + entryPoints: ["web", "web-secure"], + }), + expected: expect.objectContaining({ + spec: expect.objectContaining({ entryPoints: ["web", "web-secure"] }), + }), + }, + + { + name: "path prefix and ring headers are created: no ring", + actual: (): unknown => create("my-service", "", 80, "/version/and/Path"), + expected: expect.objectContaining({ + metadata: { name: "my-service" }, + spec: { + routes: expect.arrayContaining([ + expect.objectContaining({ + match: "PathPrefix(`/version/and/Path`)", + services: expect.arrayContaining([ + expect.objectContaining({ name: "my-service" }), + ]), + }), + ]), + }, + }), + }, + + { + name: "path prefix and ring headers are created: with ring", + actual: (): unknown => + create("my-service", "prod", 80, "/version/and/Path"), + expected: expect.objectContaining({ + metadata: { name: "my-service-prod" }, + spec: { + routes: expect.arrayContaining([ + expect.objectContaining({ + match: + "PathPrefix(`/version/and/Path`) && Headers(`Ring`, `prod`)", + services: expect.arrayContaining([ + expect.objectContaining({ name: "my-service-prod" }), + ]), + }), + ]), + }, + }), + }, + + { + name: "configured correctly when isDefault === false", + actual: (): unknown => + create("my-service", "master", 80, "/version/and/path", { + k8sBackend: "my-k8s-svc", + isDefault: false, + }), + expected: expect.objectContaining({ + metadata: { name: "my-service-master" }, + spec: { + routes: expect.arrayContaining([ + expect.objectContaining({ + match: expect.stringMatching(/.*ring.*master/i), + services: expect.arrayContaining([ + expect.objectContaining({ + name: expect.stringContaining("master"), + }), + ]), + }), + ]), + }, + }), + }, + + { + name: "configured correctly when isDefault === true", + actual: (): unknown => + create("my-service", "master", 80, "/version/and/path", { + k8sBackend: "my-k8s-svc", + isDefault: true, + }), + expected: expect.objectContaining({ + metadata: { name: "my-service" }, + spec: { + routes: expect.arrayContaining([ + expect.objectContaining({ + match: expect.not.stringMatching(/.*ring.*master/i), + services: expect.arrayContaining([ + expect.objectContaining({ + name: expect.stringContaining("master"), + }), + ]), + }), + ]), + }, + }), + }, + ]; + + for (const { name, actual, expected, effects } of tests) { + test(name, () => { + expect(actual()).toStrictEqual(expected); + }); + for (const effect of effects ?? []) { + effect(); } - }); - - test("entryPoints gets injected properly", () => { - const withoutEntryPoints = TraefikIngressRoute( - "foo", - "bar", - 80, - "/version/and/Path" - ); - expect(typeof withoutEntryPoints.spec.entryPoints).toBe("undefined"); - const withJustWeb = TraefikIngressRoute( - "foo", - "bar", - 80, - "/version/and/Path", - { - entryPoints: ["web"], - } - ); - expect(withJustWeb.spec.entryPoints).toStrictEqual(["web"]); - const withJustWebSecure = TraefikIngressRoute( - "foo", - "bar", - 80, - "/version/and/Path", - { - entryPoints: ["web-secure"], - } - ); - expect(withJustWebSecure.spec.entryPoints).toStrictEqual(["web-secure"]); - const withBoth = TraefikIngressRoute( - "foo", - "bar", - 80, - "/version/and/Path", - { - entryPoints: ["web", "web-secure"], - } - ); - expect(withBoth.spec.entryPoints).toStrictEqual(["web", "web-secure"]); - }); - - test("middleware gets added properly", () => { - const middlewares = Array.from({ length: 10 }, () => "/" + uuid()); - const middlewaresNameArray = [ - ...middlewares.map((middlewareName) => ({ name: middlewareName })), - ]; - - const withMiddlewares = TraefikIngressRoute( - "foo", - "bar", - 80, - "/version/and/Path", - { - middlewares, + } +}); + +describe("TraefikIngressRoute - Throwable", () => { + const testsThatThrow: { + name: string; + actual: () => unknown; + throws: boolean; + }[] = [ + { + name: "throws when meta.name is invalid: dash (-) prefix", + actual: (): unknown => + create("-invalid-serivce&name", "valid-ring", 80, "v1"), + throws: true, + }, + + { + name: "throws when meta.name is invalid: invalid characters", + actual: (): unknown => + create("valid-service-name", "invalid-ring-!@#", 80, "v1"), + throws: true, + }, + + { + name: + "throws when spec.routes[].services[].name is invalid: dash (-) prefix in service name", + actual: (): unknown => create("-invalid-service", "valid-ring", 80, "v1"), + throws: true, + }, + + { + name: + "throws when spec.routes[].services[].name is invalid: dash (-) prefix in k8sBackend", + actual: (): unknown => + create("valid-service", "valid-ring", 80, "v1", { + k8sBackend: "-invalid", + }), + throws: true, + }, + + { + name: + "does not throw when meta.name and spec.routes[].services[].name is valid: no k8sBackend", + actual: (): unknown => create("valid-service", "valid-ring", 80, "v1"), + throws: false, + }, + + { + name: + "does not throw when meta.name and spec.routes[].services[].name is valid: with k8sBackend", + actual: (): unknown => + create("valid-service", "valid-ring", 80, "v1", { + k8sBackend: "my.valid.service", + }), + throws: false, + }, + ]; + + for (const { name, actual, throws } of testsThatThrow) { + test(name, () => { + if (throws) { + expect(() => actual()).toThrow(); + } else { + expect(() => actual()).not.toThrow(); } - ); - - const middlewaresValues = withMiddlewares.spec.routes[0].middlewares; - expect(middlewaresValues && middlewaresValues.length).toBe( - middlewares.length - ); - expect(middlewaresValues).toMatchObject(middlewaresNameArray); - }); - - test("the path prefix and ring headers are created.", () => { - const routeWithoutRing = TraefikIngressRoute( - "my-service", - "", - 80, - "/version/and/Path" - ); - expect(routeWithoutRing.metadata.name).toBe("my-service"); - expect(routeWithoutRing.spec.routes[0].services[0].name).toBe("my-service"); - expect(routeWithoutRing.spec.routes[0].match).toBe( - "PathPrefix(`/version/and/Path`)" - ); - const routeWithRing = TraefikIngressRoute( - "my-service", - "prod", - 80, - "/version/and/Path" - ); - expect(routeWithRing.metadata.name).toBe("my-service-prod"); - expect(routeWithRing.spec.routes[0].services[0].name).toBe( - "my-service-prod" - ); - expect(routeWithRing.spec.routes[0].match).toBe( - "PathPrefix(`/version/and/Path`) && Headers(`Ring`, `prod`)" - ); - }); - - test("does not throw when meta.name and spec.routes[].services[].name is valid", () => { - expect(() => - TraefikIngressRoute("valid-service", "valid-ring", 80, "v1") - ).not.toThrow(); - expect(() => - TraefikIngressRoute("valid-service", "valid-ring", 80, "v1", { - k8sBackend: "my.valid.service", - }) - ).not.toThrow(); - }); - - test("throws when meta.name is invalid", () => { - expect(() => - TraefikIngressRoute("-invalid-serivce&name", "valid-ring", 80, "v1") - ).toThrow(); - expect(() => - TraefikIngressRoute("valid-service-name", "invalid-ring-!@#", 80, "v1") - ).toThrow(); - }); - - test("throws when spec.routes[].services[].name is invalid", () => { - expect(() => - TraefikIngressRoute("-invalid-service", "valid-ring", 80, "v1") - ).toThrow(); - expect(() => - TraefikIngressRoute("valid-service", "valid-ring", 80, "v1", { - k8sBackend: "-invalid", - }) - ).toThrow(); - }); + }); + } }); diff --git a/src/lib/traefik/ingress-route.ts b/src/lib/traefik/ingress-route.ts index c4bac1b33..810ce0a58 100644 --- a/src/lib/traefik/ingress-route.ts +++ b/src/lib/traefik/ingress-route.ts @@ -53,9 +53,10 @@ interface TraefikIngressRoute { * * @param serviceName name of the service to create the IngressRoute for * @param ringName name of the ring to which the service belongs - * @param opts options to specify the manifest namespace, IngressRoute entryPoints, pathPrefix, backend service, and version + * @param opts options to specify the manifest namespace, IngressRoute + * entryPoints, pathPrefix, backend service, and version */ -export const TraefikIngressRoute = ( +export const create = ( serviceName: string, ringName: string, servicePort: number, @@ -65,15 +66,20 @@ export const TraefikIngressRoute = ( k8sBackend?: string; middlewares?: string[]; namespace?: string; + isDefault?: boolean; } ): TraefikIngressRoute => { - const { entryPoints, k8sBackend, middlewares = [], namespace } = opts ?? {}; - const name = ringName ? `${serviceName}-${ringName}` : serviceName; + const { entryPoints, k8sBackend, middlewares = [], namespace, isDefault } = + opts ?? {}; + const name = + ringName && !isDefault ? `${serviceName}-${ringName}` : serviceName; const routeMatchPathPrefix = `PathPrefix(\`${versionAndPath}\`)`; - const routeMatchHeaders = ringName && `Headers(\`Ring\`, \`${ringName}\`)`; // no 'X-' prefix for header: https://tools.ietf.org/html/rfc6648 + const routeMatchHeaders = isDefault + ? undefined + : ringName && `Headers(\`Ring\`, \`${ringName}\`)`; // no 'X-' prefix for header: https://tools.ietf.org/html/rfc6648 const routeMatch = [routeMatchPathPrefix, routeMatchHeaders] - .filter((matchRule) => !!matchRule) + .filter((rule): rule is NonNullable => !!rule) .join(" && "); const backendService = diff --git a/src/lib/traefik/middleware.test.ts b/src/lib/traefik/middleware.test.ts index 57e01b844..160aa7f0e 100644 --- a/src/lib/traefik/middleware.test.ts +++ b/src/lib/traefik/middleware.test.ts @@ -1,8 +1,8 @@ -import { TraefikMiddleware } from "./middleware"; +import { create } from "./middleware"; describe("TraefikIngressRoute", () => { test("the right name with service name is created", () => { - const middlewareWithoutRing = TraefikMiddleware("my-service", "", [ + const middlewareWithoutRing = create("my-service", "", [ "/home", "/info", "/data", @@ -15,16 +15,14 @@ describe("TraefikIngressRoute", () => { expect(middlewareWithoutRing.spec.stripPrefix.prefixes[1]).toBe("/info"); expect(middlewareWithoutRing.spec.stripPrefix.prefixes[2]).toBe("/data"); - const middlewareWithRing = TraefikMiddleware("my-service", "prod", [ - "/home", - ]); + const middlewareWithRing = create("my-service", "prod", ["/home"]); expect(middlewareWithRing.metadata.name).toBe("my-service-prod"); expect(middlewareWithRing.spec.stripPrefix.prefixes.length).toBe(1); expect(middlewareWithRing.spec.stripPrefix.prefixes[0]).toBe("/home"); }); test("optional parameters", () => { - const middlewareWithRing = TraefikMiddleware( + const middlewareWithRing = create( "my-service", "prod", ["/home", "/away"], diff --git a/src/lib/traefik/middleware.ts b/src/lib/traefik/middleware.ts index 8937cfa78..a2b54598e 100644 --- a/src/lib/traefik/middleware.ts +++ b/src/lib/traefik/middleware.ts @@ -18,7 +18,7 @@ export interface TraefikMiddleware { }; } -export const TraefikMiddleware = ( +export const create = ( serviceName: string, ringName: string, prefixes: string[], diff --git a/webpack.config.js b/webpack.config.js index 4d93d09fa..91d514045 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -1,3 +1,4 @@ +// eslint-disable-next-line @typescript-eslint/no-var-requires const path = require("path"); module.exports = {