From 921bb6fd6a8d6269b4299501e6e04b3352f22a10 Mon Sep 17 00:00:00 2001 From: Jon Edvald Date: Fri, 7 Aug 2020 16:41:15 +0200 Subject: [PATCH] feat(config): add $merge key for merging maps together in configs Any object or mapping field supports a special `$merge` key, which allows you to merge two objects together. This can be used to avoid repeating a set of commonly repeated values. Example: ```yaml kind: Project ... variables: - commonEnvVars: LOG_LEVEL: info SOME_API_KEY: abcdefg EXTERNAL_API_URL: http://api.example.com ... ``` ```yaml kind: Module type: container name: service-a ... services: env: $merge: ${var.commonEnvVars} OTHER_ENV_VAR: something # <- This overrides the value set in commonEnvVars, because it is # below the $merge key LOG_LEVEL: debug ... ``` ```yaml kind: Module type: container name: service-b ... services: env: # <- Because this is above the $merge key, the API_KEY from # commonEnvVars will override this SOME_API_KEY: default $merge: ${var.commonEnvVars} ... ``` --- core/src/config/common.ts | 16 ++++- core/src/config/workflow.ts | 3 +- .../volumes/persistentvolumeclaim.ts | 2 +- core/src/template-string.ts | 45 ++++++++++-- core/src/util/util.ts | 9 +-- core/test/unit/src/config/common.ts | 10 +-- core/test/unit/src/docs/joi-schema.ts | 4 +- core/test/unit/src/garden.ts | 58 ++++++++++++++- core/test/unit/src/template-string.ts | 71 +++++++++++++++++++ .../module-types/persistentvolumeclaim.md | 6 +- docs/using-garden/variables-and-templating.md | 44 ++++++++++++ 11 files changed, 242 insertions(+), 26 deletions(-) diff --git a/core/src/config/common.ts b/core/src/config/common.ts index 0351bc6be1..603fd800e4 100644 --- a/core/src/config/common.ts +++ b/core/src/config/common.ts @@ -13,6 +13,8 @@ import { deline, dedent } from "../util/string" import { cloneDeep } from "lodash" import { joiPathPlaceholder } from "./validation" +export const objectSpreadKey = "$merge" + const ajv = new Ajv({ allErrors: true, useDefaults: true }) export type Primitive = string | number | boolean | null @@ -94,6 +96,7 @@ declare module "@hapi/joi" { } export interface CustomObjectSchema extends Joi.ObjectSchema { + concat(schema: object): this jsonSchema(schema: object): this } @@ -110,7 +113,7 @@ export interface PosixPathSchema extends Joi.StringSchema { } interface CustomJoi extends Joi.Root { - customObject: () => CustomObjectSchema + object: () => CustomObjectSchema environment: () => Joi.StringSchema gitUrl: () => GitUrlSchema posixPath: () => PosixPathSchema @@ -278,7 +281,7 @@ joi = joi.extend({ }) /** - * Add a joi.customObject() type, which includes additional methods, including one for validating with a + * Extend the joi.object() type with additional methods and minor customizations, including one for validating with a * JSON Schema. * * Note that the jsonSchema() option should generally not be used in conjunction with other options (like keys() @@ -286,7 +289,7 @@ joi = joi.extend({ */ joi = joi.extend({ base: Joi.object(), - type: "customObject", + type: "object", messages: { validation: "", }, @@ -294,6 +297,13 @@ joi = joi.extend({ // validate(value: string, { error }) { // return { value } // }, + args(schema: any, keys: any) { + // Always allow the $merge key, which we resolve and collapse in resolveTemplateStrings() + return schema.keys({ + [objectSpreadKey]: joi.alternatives(joi.object(), joi.string()), + ...(keys || {}), + }) + }, rules: { jsonSchema: { method(jsonSchema: object) { diff --git a/core/src/config/workflow.ts b/core/src/config/workflow.ts index 5102991bd8..8b4a7781c2 100644 --- a/core/src/config/workflow.ts +++ b/core/src/config/workflow.ts @@ -66,7 +66,8 @@ export const workflowConfigSchema = () => .default(48) .description("The number of hours to keep the workflow pod running after completion."), limits: joi - .object({ + .object() + .keys({ cpu: joi .number() .default(defaultContainerLimits.cpu) diff --git a/core/src/plugins/kubernetes/volumes/persistentvolumeclaim.ts b/core/src/plugins/kubernetes/volumes/persistentvolumeclaim.ts index df459c4b60..41a47b65e0 100644 --- a/core/src/plugins/kubernetes/volumes/persistentvolumeclaim.ts +++ b/core/src/plugins/kubernetes/volumes/persistentvolumeclaim.ts @@ -54,7 +54,7 @@ export const pvcModuleDefinition: ModuleTypeDefinition = { "The namespace to deploy the PVC in. Note that any module referencing the PVC must be in the same namespace, so in most cases you should leave this unset." ), spec: joi - .customObject() + .object() .jsonSchema({ ...jsonSchema.properties.spec, type: "object" }) .required() .description( diff --git a/core/src/template-string.ts b/core/src/template-string.ts index b4a73ab1a7..eaf792ec73 100644 --- a/core/src/template-string.ts +++ b/core/src/template-string.ts @@ -7,7 +7,6 @@ */ import lodash from "lodash" -import { deepMap } from "./util/util" import { GardenBaseError, ConfigurationError } from "./exceptions" import { ConfigContext, @@ -17,9 +16,10 @@ import { ContextKeySegment, } from "./config/config-context" import { difference, flatten, uniq, isPlainObject, isNumber } from "lodash" -import { Primitive, StringMap, isPrimitive } from "./config/common" +import { Primitive, StringMap, isPrimitive, objectSpreadKey } from "./config/common" import { profile } from "./util/profiling" import { dedent, deline } from "./util/string" +import { isArray } from "util" export type StringOrStringPromise = Promise | string @@ -112,12 +112,45 @@ export function resolveTemplateString(string: string, context: ConfigContext, op /** * Recursively parses and resolves all templated strings in the given object. */ -export const resolveTemplateStrings = profile(function $resolveTemplateStrings( - obj: T, +export const resolveTemplateStrings = profile(function $resolveTemplateStrings( + value: T, context: ConfigContext, opts: ContextResolveOpts = {} ): T { - return deepMap(obj, (v) => (typeof v === "string" ? resolveTemplateString(v, context, opts) : v)) as T + if (typeof value === "string") { + return resolveTemplateString(value, context, opts) + } else if (isArray(value)) { + return (value.map((v) => resolveTemplateStrings(v, context, opts))) + } else if (isPlainObject(value)) { + // Resolve $merge keys, depth-first, leaves-first + let output = {} + + for (const [k, v] of Object.entries(value)) { + const resolved = resolveTemplateStrings(v, context, opts) + + if (k === objectSpreadKey) { + if (isPlainObject(resolved)) { + output = { ...output, ...resolved } + } else if (opts.allowPartial) { + output[k] = resolved + } else { + throw new ConfigurationError( + `Value of ${objectSpreadKey} key must be (or resolve to) a mapping object (got ${typeof resolved})`, + { + value, + resolved, + } + ) + } + } else { + output[k] = resolved + } + } + + return output + } else { + return value + } }) /** @@ -125,7 +158,7 @@ export const resolveTemplateStrings = profile(function $resolveTemplateStrings(obj: T): ContextKeySegment[][] { const context = new ScanContext() - resolveTemplateStrings(obj, context) + resolveTemplateStrings(obj, context, { allowPartial: true, allowUndefined: true }) return uniq(context.foundKeys.entries()).sort() } diff --git a/core/src/util/util.ts b/core/src/util/util.ts index f6e9154e81..57bd0f1fe6 100644 --- a/core/src/util/util.ts +++ b/core/src/util/util.ts @@ -348,14 +348,15 @@ export function splitLast(s: string, delimiter: string) { */ export function deepMap( value: T | Iterable, - fn: (value: any, key: string | number) => any + fn: (value: any, key: string | number) => any, + key?: number | string ): U | Iterable { if (isArray(value)) { - return value.map((v) => deepMap(v, fn)) + return value.map((v, k) => deepMap(v, fn, k)) } else if (isPlainObject(value)) { - return mapValues(value, (v) => deepMap(v, fn)) + return mapValues(value, (v, k) => deepMap(v, fn, k)) } else { - return fn(value, 0) + return fn(value, key || 0) } } diff --git a/core/test/unit/src/config/common.ts b/core/test/unit/src/config/common.ts index f9a1f2d33c..baae74eb21 100644 --- a/core/test/unit/src/config/common.ts +++ b/core/test/unit/src/config/common.ts @@ -389,20 +389,20 @@ describe("joi.customObject", () => { } it("should validate an object with a JSON Schema", () => { - const joiSchema = joi.customObject().jsonSchema(jsonSchema) + const joiSchema = joi.object().jsonSchema(jsonSchema) const value = { stringProperty: "foo", numberProperty: 123 } const result = validateSchema(value, joiSchema) expect(result).to.eql({ stringProperty: "foo", numberProperty: 123 }) }) it("should apply default values based on the JSON Schema", () => { - const joiSchema = joi.customObject().jsonSchema(jsonSchema) + const joiSchema = joi.object().jsonSchema(jsonSchema) const result = validateSchema({ stringProperty: "foo" }, joiSchema) expect(result).to.eql({ stringProperty: "foo", numberProperty: 999 }) }) it("should give validation error if object doesn't match specified JSON Schema", async () => { - const joiSchema = joi.customObject().jsonSchema(jsonSchema) + const joiSchema = joi.object().jsonSchema(jsonSchema) await expectError( () => validateSchema({ numberProperty: "oops", blarg: "blorg" }, joiSchema), (err) => @@ -414,14 +414,14 @@ describe("joi.customObject", () => { it("should throw if schema with wrong type is passed to .jsonSchema()", async () => { await expectError( - () => joi.customObject().jsonSchema({ type: "number" }), + () => joi.object().jsonSchema({ type: "number" }), (err) => expect(err.message).to.equal("jsonSchema must be a valid JSON Schema with type=object or reference") ) }) it("should throw if invalid schema is passed to .jsonSchema()", async () => { await expectError( - () => joi.customObject().jsonSchema({ type: "banana", blorg: "blarg" }), + () => joi.object().jsonSchema({ type: "banana", blorg: "blarg" }), (err) => expect(err.message).to.equal("jsonSchema must be a valid JSON Schema with type=object or reference") ) }) diff --git a/core/test/unit/src/docs/joi-schema.ts b/core/test/unit/src/docs/joi-schema.ts index 5507b6c272..7ec2fd9a0d 100644 --- a/core/test/unit/src/docs/joi-schema.ts +++ b/core/test/unit/src/docs/joi-schema.ts @@ -13,8 +13,8 @@ import { testJsonSchema } from "./json-schema" import { normalizeJsonSchema } from "../../../../src/docs/json-schema" describe("normalizeJoiSchemaDescription", () => { - it("should correctly handle joi.customObject().jsonSchema() schemas", async () => { - const schema = joi.customObject().jsonSchema(testJsonSchema) + it("should correctly handle joi.object().jsonSchema() schemas", async () => { + const schema = joi.object().jsonSchema(testJsonSchema) const result = normalizeJoiSchemaDescription(schema.describe() as JoiDescription) expect(result).to.eql(normalizeJsonSchema(testJsonSchema)) }) diff --git a/core/test/unit/src/garden.ts b/core/test/unit/src/garden.ts index ad334b4cf1..7138c3607b 100644 --- a/core/test/unit/src/garden.ts +++ b/core/test/unit/src/garden.ts @@ -694,7 +694,7 @@ describe("Garden", () => { it("should inherit config schema from base, if none is specified", async () => { const base = createGardenPlugin({ name: "base", - configSchema: joi.object({ foo: joi.string().default("bar") }), + configSchema: joi.object().keys({ foo: joi.string().default("bar") }), }) const foo = createGardenPlugin({ name: "foo", @@ -2407,6 +2407,62 @@ describe("Garden", () => { expect(module.spec.bla).to.eql({ nested: { key: "my value" } }) }) + it("should correctly resolve template strings with $merge keys", async () => { + const test = createGardenPlugin({ + name: "test", + createModuleTypes: [ + { + name: "test", + docs: "test", + schema: joi.object().keys({ bla: joi.object() }), + handlers: {}, + }, + ], + }) + + const garden = await TestGarden.factory(pathFoo, { + plugins: [test], + config: { + apiVersion: DEFAULT_API_VERSION, + kind: "Project", + name: "test", + path: pathFoo, + defaultEnvironment: "default", + dotIgnoreFiles: [], + environments: [{ name: "default", defaultNamespace, variables: { obj: { b: "B", c: "c" } } }], + providers: [{ name: "test" }], + variables: {}, + }, + }) + + garden.setModuleConfigs([ + { + apiVersion: DEFAULT_API_VERSION, + name: "module-a", + type: "test", + allowPublish: false, + build: { dependencies: [] }, + disabled: false, + outputs: {}, + path: pathFoo, + serviceConfigs: [], + taskConfigs: [], + testConfigs: [], + spec: { + bla: { + a: "a", + b: "b", + $merge: "${var.obj}", + }, + }, + }, + ]) + + const module = await garden.resolveModule("module-a") + + expect(module.spec.bla).to.eql({ a: "a", b: "B", c: "c" }) + }) + it("should handle module references within single file", async () => { const projectRoot = getDataDir("test-projects", "1067-module-ref-within-file") const garden = await makeTestGarden(projectRoot) diff --git a/core/test/unit/src/template-string.ts b/core/test/unit/src/template-string.ts index dcbb0fb423..96f9cb4478 100644 --- a/core/test/unit/src/template-string.ts +++ b/core/test/unit/src/template-string.ts @@ -807,6 +807,77 @@ describe("resolveTemplateStrings", () => { }, }) }) + + it("should collapse $merge keys on objects", () => { + const obj = { + $merge: { a: "a", b: "b" }, + b: "B", + c: "c", + } + const templateContext = new TestContext({}) + + const result = resolveTemplateStrings(obj, templateContext) + + expect(result).to.eql({ + a: "a", + b: "B", + c: "c", + }) + }) + + it("should collapse $merge keys based on position on object", () => { + const obj = { + b: "B", + c: "c", + $merge: { a: "a", b: "b" }, + } + const templateContext = new TestContext({}) + + const result = resolveTemplateStrings(obj, templateContext) + + expect(result).to.eql({ + a: "a", + b: "b", + c: "c", + }) + }) + + it("should resolve $merge keys before collapsing", () => { + const obj = { + $merge: "${obj}", + b: "B", + c: "c", + } + const templateContext = new TestContext({ obj: { a: "a", b: "b" } }) + + const result = resolveTemplateStrings(obj, templateContext) + + expect(result).to.eql({ + a: "a", + b: "B", + c: "c", + }) + }) + + it("should resolve $merge keys depth-first", () => { + const obj = { + b: "B", + c: "c", + $merge: { + $merge: "${obj}", + a: "a", + }, + } + const templateContext = new TestContext({ obj: { b: "b" } }) + + const result = resolveTemplateStrings(obj, templateContext) + + expect(result).to.eql({ + a: "a", + b: "b", + c: "c", + }) + }) }) describe("collectTemplateReferences", () => { diff --git a/docs/reference/module-types/persistentvolumeclaim.md b/docs/reference/module-types/persistentvolumeclaim.md index b75a38e8b2..649e97fc7d 100644 --- a/docs/reference/module-types/persistentvolumeclaim.md +++ b/docs/reference/module-types/persistentvolumeclaim.md @@ -382,9 +382,9 @@ The namespace to deploy the PVC in. Note that any module referencing the PVC mus The spec for the PVC. This is passed directly to the created PersistentVolumeClaim resource. Note that the spec schema may include (or even require) additional fields, depending on the used `storageClass`. See the [PersistentVolumeClaim docs](https://kubernetes.io/docs/concepts/storage/persistent-volumes/#persistentvolumeclaims) for details. -| Type | Required | -| -------------- | -------- | -| `customObject` | Yes | +| Type | Required | +| -------- | -------- | +| `object` | Yes | ### `spec.accessModes[]` diff --git a/docs/using-garden/variables-and-templating.md b/docs/using-garden/variables-and-templating.md index 1f98db07c8..db3655f895 100644 --- a/docs/using-garden/variables-and-templating.md +++ b/docs/using-garden/variables-and-templating.md @@ -141,6 +141,50 @@ When the nested expression is a simple key lookup like above, you can also just You can even use one variable to index another variable, e.g. `${var.a[var.b]}`. +### Merging maps + +Any object or mapping field supports a special `$merge` key, which allows you to merge two objects together. This can be used to avoid repeating a set of commonly repeated values. + +Here's an example where we share a common set of environment variables for two services: + +```yaml +kind: Project +... +variables: + - commonEnvVars: + LOG_LEVEL: info + SOME_API_KEY: abcdefg + EXTERNAL_API_URL: http://api.example.com + ... +``` + +```yaml +kind: Module +type: container +name: service-a +... +services: + env: + $merge: ${var.commonEnvVars} + OTHER_ENV_VAR: something + LOG_LEVEL: debug # <- This overrides the value set in commonEnvVars, because it is below the $merge key + ... +``` + +```yaml +kind: Module +type: container +name: service-b +... +services: + env: + SOME_API_KEY: default # <- Because this is above the $merge key, the API_KEY from commonEnvVars will override this + $merge: ${var.commonEnvVars} + ... +``` + +Notice above that the position of the `$merge` key matters. If the keys being merged overlap between the two objects, the value that's defined later is chosen. + ### Optional values In some cases, you may want to provide configuration values only for certain cases, e.g. only for specific environments. By default, an error is thrown when a template string resolves to an undefined value, but you can explicitly allow that by adding a `?` after the template.