Skip to content

Commit

Permalink
feat(config): add $merge key for merging maps together in configs
Browse files Browse the repository at this point in the history
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}
  ...
```
  • Loading branch information
edvald committed Aug 12, 2020
1 parent b3cc5bd commit 921bb6f
Show file tree
Hide file tree
Showing 11 changed files with 242 additions and 26 deletions.
16 changes: 13 additions & 3 deletions core/src/config/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -94,6 +96,7 @@ declare module "@hapi/joi" {
}

export interface CustomObjectSchema extends Joi.ObjectSchema {
concat(schema: object): this
jsonSchema(schema: object): this
}

Expand All @@ -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
Expand Down Expand Up @@ -278,22 +281,29 @@ 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()
* and unknown()) since the behavior can be confusing. It is meant to facilitate a gradual transition away from Joi.
*/
joi = joi.extend({
base: Joi.object(),
type: "customObject",
type: "object",
messages: {
validation: "<not used>",
},
// TODO: check if jsonSchema() is being used in conjunction with other methods that may be incompatible.
// 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) {
Expand Down
3 changes: 2 additions & 1 deletion core/src/config/workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
45 changes: 39 additions & 6 deletions core/src/template-string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
*/

import lodash from "lodash"
import { deepMap } from "./util/util"
import { GardenBaseError, ConfigurationError } from "./exceptions"
import {
ConfigContext,
Expand All @@ -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> | string

Expand Down Expand Up @@ -112,20 +112,53 @@ 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<T extends object>(
obj: T,
export const resolveTemplateStrings = profile(function $resolveTemplateStrings<T>(
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 <T>resolveTemplateString(value, context, opts)
} else if (isArray(value)) {
return <T>(<unknown>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 <T>output
} else {
return <T>value
}
})

/**
* Scans for all template strings in the given object and lists the referenced keys.
*/
export function collectTemplateReferences<T extends object>(obj: T): ContextKeySegment[][] {
const context = new ScanContext()
resolveTemplateStrings(obj, context)
resolveTemplateStrings(obj, context, { allowPartial: true, allowUndefined: true })
return uniq(context.foundKeys.entries()).sort()
}

Expand Down
9 changes: 5 additions & 4 deletions core/src/util/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,14 +348,15 @@ export function splitLast(s: string, delimiter: string) {
*/
export function deepMap<T extends object, U extends object = T>(
value: T | Iterable<T>,
fn: (value: any, key: string | number) => any
fn: (value: any, key: string | number) => any,
key?: number | string
): U | Iterable<U> {
if (isArray(value)) {
return value.map((v) => <U>deepMap(v, fn))
return value.map((v, k) => <U>deepMap(v, fn, k))
} else if (isPlainObject(value)) {
return <U>mapValues(value, (v) => deepMap(v, fn))
return <U>mapValues(value, (v, k) => deepMap(v, fn, k))
} else {
return <U>fn(value, 0)
return <U>fn(value, key || 0)
}
}

Expand Down
10 changes: 5 additions & 5 deletions core/test/unit/src/config/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) =>
Expand All @@ -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")
)
})
Expand Down
4 changes: 2 additions & 2 deletions core/test/unit/src/docs/joi-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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))
})
Expand Down
58 changes: 57 additions & 1 deletion core/test/unit/src/garden.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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)
Expand Down
71 changes: 71 additions & 0 deletions core/test/unit/src/template-string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
6 changes: 3 additions & 3 deletions docs/reference/module-types/persistentvolumeclaim.md
Original file line number Diff line number Diff line change
Expand Up @@ -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[]`

Expand Down
Loading

0 comments on commit 921bb6f

Please sign in to comment.