From 52ad5fafc88c6bc102f37ef9068d09b22d9ec318 Mon Sep 17 00:00:00 2001 From: Jon Edvald Date: Sat, 22 Jun 2019 16:33:03 +0200 Subject: [PATCH] improvement(config): allow non-string values to be output directly This makes it possible to use template strings to assign numeric, boolean and null values to config keys, instead of always being cast to strings. --- docs/using-garden/configuration-files.md | 43 +++++ .../src/template-string-parser.pegjs | 89 ++++++--- garden-service/src/template-string.ts | 19 +- .../non-string-template-values/garden.yml | 8 + .../module-a/garden.yml | 4 + garden-service/test/unit/src/garden.ts | 10 + .../test/unit/src/template-string.ts | 172 +++++++++++++++--- 7 files changed, 288 insertions(+), 57 deletions(-) create mode 100644 garden-service/test/unit/data/test-projects/non-string-template-values/garden.yml create mode 100644 garden-service/test/unit/data/test-projects/non-string-template-values/module-a/garden.yml diff --git a/docs/using-garden/configuration-files.md b/docs/using-garden/configuration-files.md index c29c1a10d6..094fb4066a 100644 --- a/docs/using-garden/configuration-files.md +++ b/docs/using-garden/configuration-files.md @@ -387,6 +387,49 @@ You can do conditional statements in template strings, using the `||` operator. This allows you to easily set default values when certain template keys are not available, and to configure your project based on varying context. +### Numbers, booleans and null values + +When a template string key resolves to a number, boolean or null, the output is handled in two different ways, +depending on whether the template string is part of surrounding string or not. + +If the template string is the whole string being interpolated, we assign the number, boolean or null directly to the +key: + +```yaml +kind: Project +... +variables: + global-memory-limit: 100 +--- +kind: Module +... +services: + - name: my-service + ... + limits: + memory: ${var.global-memory-limit} # <- resolves to a number, as opposed to "100" as a string +``` + +If however the template string is part of a surrounding string, the value is formatted into the string, as you would +expect: + +```yaml +kind: Project +... +variables: + project-id: 123 + some-key: null +--- +kind: Module +... +services: + - name: my-service + ... + env: + CONTEXT: project-${project-id} # <- resolves to "project-123" + SOME_VAR: foo-${var.some-key} # <- resolves to "foo-null" +``` + ## Next steps We highly recommend browsing through the [Example projects](../examples/README.md) to see different examples of how projects and modules can be configured. diff --git a/garden-service/src/template-string-parser.pegjs b/garden-service/src/template-string-parser.pegjs index 0aed0854a3..d3f507c0a9 100644 --- a/garden-service/src/template-string-parser.pegjs +++ b/garden-service/src/template-string-parser.pegjs @@ -10,18 +10,21 @@ TemplateString = a:(FormatString)+ b:TemplateString? { return [...a, ...(b || [])] } / a:Prefix b:(FormatString)+ c:TemplateString? { return [a, ...b, ...(c || [])] } / InvalidFormatString - / $(.*) { return [text()] } + / $(.*) { return text() === "" ? [] : [text()] } FormatString - = FormatStart key:Key FormatEnd { - return options.getKey(key) + = FormatStart v:Literal FormatEnd { + return v } - / FormatStart head:Key tail:(Or (Key / StringLiteral))+ FormatEnd { + / FormatStart v:Key FormatEnd { + return options.getKey(v) + } + / FormatStart head:LiteralOrKey tail:(Or LiteralOrKey)* FormatEnd { const keys = [head, ...tail.map(t => t[1])] // Resolve all the keys return Promise.all(keys.map(key => - typeof key === "string" ? key : options.getKey(key, { allowUndefined: true }) + options.lodash.isArray(key) ? options.getKey(key, { allowUndefined: true }) : key, )) .then(candidates => { // Return the first non-undefined value @@ -34,19 +37,10 @@ FormatString throw new options.ConfigurationError("None of the keys could be resolved in the conditional: " + text()) }) } - / FormatStart a:Key Or b:StringLiteral FormatEnd { - return options.getKey(a, { allowUndefined: true }) - .then(result => { - return result || b - }) - } - // These would be odd in configuration, but there's no reason to throw if it comes up. - / FormatStart a:StringLiteral Or b:StringLiteral FormatEnd { - return a || b - } - / FormatStart a:StringLiteral FormatEnd { - return a - } + +LiteralOrKey + = Literal + / Key InvalidFormatString = Prefix? FormatStart .* { @@ -73,9 +67,60 @@ Key Or = ws "||" ws -// Some of the below is based on https://github.com/pegjs/pegjs/blob/master/examples/json.pegjs +Prefix + = !FormatStart (. ! FormatStart)* . { return text() } + +Suffix + = !FormatEnd (. ! FormatEnd)* . { return text() } + +// Much of the below is based on https://github.com/pegjs/pegjs/blob/master/examples/json.pegjs ws "whitespace" = [ \t\n\r]* +// ----- Literals ----- + +Literal + = BooleanLiteral + / NullLiteral + / NumberLiteral + / StringLiteral + +BooleanLiteral + = ws "true" ws { return true } + / ws "false" ws { return false } + +NullLiteral + = ws "null" ws { return null } + +NumberLiteral + = ws Minus? Int Frac? Exp? ws { return parseFloat(text()); } + +DecimalPoint + = "." + +Digit1_9 + = [1-9] + +E + = [eE] + +Exp + = E (Minus / Plus)? DIGIT+ + +Frac + = DecimalPoint DIGIT+ + +Int + = Zero / (Digit1_9 DIGIT*) + +Minus + = "-" + +Plus + = "+" + +Zero + = "0" + StringLiteral = ws '"' chars:DoubleQuotedChar* '"' ws { return chars.join(""); } / ws "'" chars:SingleQuotedChar* "'" ws { return chars.join(""); } @@ -119,12 +164,6 @@ SingleQuotedChar ) { return sequence; } -Prefix - = !FormatStart (. ! FormatStart)* . { return text() } - -Suffix - = !FormatEnd (. ! FormatEnd)* . { return text() } - // ----- Core ABNF Rules ----- // See RFC 4234, Appendix B (http://tools.ietf.org/html/rfc4234). diff --git a/garden-service/src/template-string.ts b/garden-service/src/template-string.ts index 2643fd7fe7..527511ef66 100644 --- a/garden-service/src/template-string.ts +++ b/garden-service/src/template-string.ts @@ -6,12 +6,14 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +import lodash = require("lodash") import Bluebird = require("bluebird") import { asyncDeepMap } from "./util/util" import { GardenBaseError, ConfigurationError } from "./exceptions" import { ConfigContext, ContextResolveOpts, ContextResolveParams } from "./config/config-context" import { KeyedSet } from "./util/keyed-set" import { uniq } from "lodash" +import { Primitive } from "./config/common" export type StringOrStringPromise = Promise | string @@ -37,7 +39,9 @@ async function getParser() { * The context should be a ConfigContext instance. The optional `stack` parameter is used to detect circular * dependencies when resolving context variables. */ -export async function resolveTemplateString(string: string, context: ConfigContext, opts: ContextResolveOpts = {}) { +export async function resolveTemplateString( + string: string, context: ConfigContext, opts: ContextResolveOpts = {}, +): Promise { const parser = await getParser() const parsed = parser.parse(string, { getKey: async (key: string[], resolveOpts?: ContextResolveOpts) => { @@ -48,12 +52,21 @@ export async function resolveTemplateString(string: string, context: ConfigConte const s = (await Bluebird.all(parts)).join("") return resolveTemplateString(`\$\{${s}\}`, context, { ...opts, ...resolveOpts || {} }) }, + // Some utilities to pass to the parser + lodash, ConfigurationError, TemplateStringError, }) - const resolved = await Bluebird.all(parsed) - return resolved.join("") + const resolved: (Primitive | undefined)[] = await Bluebird.all(parsed) + + const result = resolved.length === 1 + // Return value directly if there is only one value in the output + ? resolved[0] + // Else join together all the parts as a string. Output null as a literal string and not an empty string. + : resolved.map(v => v === null ? "null" : v).join("") + + return result } /** diff --git a/garden-service/test/unit/data/test-projects/non-string-template-values/garden.yml b/garden-service/test/unit/data/test-projects/non-string-template-values/garden.yml new file mode 100644 index 0000000000..ec0d40127d --- /dev/null +++ b/garden-service/test/unit/data/test-projects/non-string-template-values/garden.yml @@ -0,0 +1,8 @@ +project: + name: non-string-template-values + environments: + - name: local + providers: + - name: test-plugin + variables: + allow-publish: false diff --git a/garden-service/test/unit/data/test-projects/non-string-template-values/module-a/garden.yml b/garden-service/test/unit/data/test-projects/non-string-template-values/module-a/garden.yml new file mode 100644 index 0000000000..b4dcc75dcf --- /dev/null +++ b/garden-service/test/unit/data/test-projects/non-string-template-values/module-a/garden.yml @@ -0,0 +1,4 @@ +kind: Module +name: module-a +type: test +allowPublish: ${var.allow-publish} diff --git a/garden-service/test/unit/src/garden.ts b/garden-service/test/unit/src/garden.ts index 788ff7faaa..e38cf85dae 100644 --- a/garden-service/test/unit/src/garden.ts +++ b/garden-service/test/unit/src/garden.ts @@ -749,6 +749,16 @@ describe("Garden", () => { it.skip("should set default values properly", async () => { throw new Error("TODO") }) + + it("should handle template variables for non-string fields", async () => { + const projectRoot = getDataDir("test-projects", "non-string-template-values") + const garden = await makeTestGarden(projectRoot) + + const module = await garden.resolveModuleConfig("module-a") + + // We template in the value for the module's allowPublish field to test this + expect(module.allowPublish).to.equal(false) + }) }) describe("resolveVersion", () => { diff --git a/garden-service/test/unit/src/template-string.ts b/garden-service/test/unit/src/template-string.ts index 87f66201bb..58ef9d407a 100644 --- a/garden-service/test/unit/src/template-string.ts +++ b/garden-service/test/unit/src/template-string.ts @@ -25,7 +25,7 @@ describe("resolveTemplateString", async () => { it("should optionally allow undefined values", async () => { const res = await resolveTemplateString("${some}", new TestContext({}), { allowUndefined: true }) - expect(res).to.equal("") + expect(res).to.equal(undefined) }) it("should interpolate a format string with a prefix", async () => { @@ -53,16 +53,6 @@ describe("resolveTemplateString", async () => { expect(res).to.equal("valueother") }) - it("should interpolate a simple format string that resolves to a number", async () => { - const res = await resolveTemplateString("${some}", new TestContext({ some: 123 })) - expect(res).to.equal("123") - }) - - it("should interpolate a simple format string that resolves to a boolean", async () => { - const res = await resolveTemplateString("${some}", new TestContext({ some: false })) - expect(res).to.equal("false") - }) - it("should throw when a key is not found", async () => { try { await resolveTemplateString("${some}", new TestContext({})) @@ -127,6 +117,70 @@ describe("resolveTemplateString", async () => { expect(res).to.equal("foo") }) + it("should handle a numeric literal and return it directly", async () => { + const res = await resolveTemplateString( + "${123}", + new TestContext({}), + ) + expect(res).to.equal(123) + }) + + it("should handle a boolean true literal and return it directly", async () => { + const res = await resolveTemplateString( + "${true}", + new TestContext({}), + ) + expect(res).to.equal(true) + }) + + it("should handle a boolean false literal and return it directly", async () => { + const res = await resolveTemplateString( + "${false}", + new TestContext({}), + ) + expect(res).to.equal(false) + }) + + it("should handle a null literal and return it directly", async () => { + const res = await resolveTemplateString( + "${null}", + new TestContext({}), + ) + expect(res).to.equal(null) + }) + + it("should handle a numeric literal in a conditional and return it directly", async () => { + const res = await resolveTemplateString( + "${a || 123}", + new TestContext({}), + ) + expect(res).to.equal(123) + }) + + it("should handle a boolean true literal in a conditional and return it directly", async () => { + const res = await resolveTemplateString( + "${a || true}", + new TestContext({}), + ) + expect(res).to.equal(true) + }) + + it("should handle a boolean false literal in a conditional and return it directly", async () => { + const res = await resolveTemplateString( + "${a || false}", + new TestContext({}), + ) + expect(res).to.equal(false) + }) + + it("should handle a null literal in a conditional and return it directly", async () => { + const res = await resolveTemplateString( + "${a || null}", + new TestContext({}), + ) + expect(res).to.equal(null) + }) + it("should handle a double-quoted string", async () => { const res = await resolveTemplateString( "${\"foo\"}", @@ -158,9 +212,9 @@ describe("resolveTemplateString", async () => { it("should handle a conditional between two identifiers", async () => { const res = await resolveTemplateString( "${a || b}", - new TestContext({ a: undefined, b: 123 }), + new TestContext({ a: undefined, b: "abc" }), ) - expect(res).to.equal("123") + expect(res).to.equal("abc") }) it("should handle a conditional between two nested identifiers", async () => { @@ -168,29 +222,29 @@ describe("resolveTemplateString", async () => { "${a.b || c.d}", new TestContext({ a: { b: undefined }, - c: { d: "123" }, + c: { d: "abc" }, }), ) - expect(res).to.equal("123") + expect(res).to.equal("abc") }) it("should handle a conditional between two nested identifiers where the first resolves", async () => { const res = await resolveTemplateString( "${a.b || c.d}", new TestContext({ - a: { b: "123" }, + a: { b: "abc" }, c: { d: undefined }, }), ) - expect(res).to.equal("123") + expect(res).to.equal("abc") }) it("should handle a conditional between two identifiers without spaces with first value undefined", async () => { const res = await resolveTemplateString( "${a||b}", - new TestContext({ a: undefined, b: 123 }), + new TestContext({ a: undefined, b: "abc" }), ) - expect(res).to.equal("123") + expect(res).to.equal("abc") }) it("should handle a conditional between two identifiers with first value undefined and string fallback", async () => { @@ -220,9 +274,9 @@ describe("resolveTemplateString", async () => { it("should handle a conditional between two identifiers without spaces with first value set", async () => { const res = await resolveTemplateString( "${a||b}", - new TestContext({ a: 123, b: undefined }), + new TestContext({ a: "abc", b: undefined }), ) - expect(res).to.equal("123") + expect(res).to.equal("abc") }) it("should throw if neither key in conditional is valid", async () => { @@ -245,14 +299,6 @@ describe("resolveTemplateString", async () => { ) }) - it("should handle a conditional between an identifier and a string", async () => { - const res = await resolveTemplateString( - "${a || 'b'}", - new TestContext({ a: undefined }), - ) - expect(res).to.equal("b") - }) - it("should handle a conditional between a string and a string", async () => { const res = await resolveTemplateString( "${'a' || 'b'}", @@ -260,6 +306,74 @@ describe("resolveTemplateString", async () => { ) expect(res).to.equal("a") }) + + context("when the template string is the full input string", () => { + it("should return a resolved number directly", async () => { + const res = await resolveTemplateString( + "${a}", + new TestContext({ a: 100 }), + ) + expect(res).to.equal(100) + }) + + it("should return a resolved boolean true directly", async () => { + const res = await resolveTemplateString( + "${a}", + new TestContext({ a: true }), + ) + expect(res).to.equal(true) + }) + + it("should return a resolved boolean false directly", async () => { + const res = await resolveTemplateString( + "${a}", + new TestContext({ a: false }), + ) + expect(res).to.equal(false) + }) + + it("should return a resolved null directly", async () => { + const res = await resolveTemplateString( + "${a}", + new TestContext({ a: null }), + ) + expect(res).to.equal(null) + }) + }) + + context("when the template string is a part of a string", () => { + it("should format a resolved number into the string", async () => { + const res = await resolveTemplateString( + "foo-${a}", + new TestContext({ a: 100 }), + ) + expect(res).to.equal("foo-100") + }) + + it("should format a resolved boolean true into the string", async () => { + const res = await resolveTemplateString( + "foo-${a}", + new TestContext({ a: true }), + ) + expect(res).to.equal("foo-true") + }) + + it("should format a resolved boolean false into the string", async () => { + const res = await resolveTemplateString( + "foo-${a}", + new TestContext({ a: false }), + ) + expect(res).to.equal("foo-false") + }) + + it("should format a resolved null into the string", async () => { + const res = await resolveTemplateString( + "foo-${a}", + new TestContext({ a: null }), + ) + expect(res).to.equal("foo-null") + }) + }) }) describe("resolveTemplateStrings", () => {