From d30b85674087716e37a801998f21b1a96e558406 Mon Sep 17 00:00:00 2001 From: Jon Edvald Date: Wed, 30 Sep 2020 17:32:38 +0200 Subject: [PATCH] improvement(template): return partially resolved conditionals unchanged Previously, we would collapse conditionals. This could cause issues upon later resolution with a broader context, because the reference to the intended key could be lost. --- core/src/config/config-context.ts | 49 +---- core/src/tasks/resolve-module.ts | 2 +- core/src/template-string-parser.pegjs | 87 +++++--- core/src/template-string.ts | 15 +- core/test/unit/src/config/config-context.ts | 227 +++++--------------- core/test/unit/src/garden.ts | 2 +- core/test/unit/src/template-string.ts | 36 +++- 7 files changed, 165 insertions(+), 253 deletions(-) diff --git a/core/src/config/config-context.ts b/core/src/config/config-context.ts index 4b88a66c13..f39896bae3 100644 --- a/core/src/config/config-context.ts +++ b/core/src/config/config-context.ts @@ -12,7 +12,7 @@ import { isString, mapValues } from "lodash" import { PrimitiveMap, joiIdentifierMap, joiStringMap, joiPrimitive, DeepPrimitiveMap, joiVariables } from "./common" import { Provider, GenericProviderConfig, ProviderMap } from "./provider" import { ConfigurationError } from "../exceptions" -import { resolveTemplateString } from "../template-string" +import { resolveTemplateString, TemplateStringMissingKeyError } from "../template-string" import { Garden } from "../garden" import { joi } from "../config/common" import { KeyedSet } from "../util/keyed-set" @@ -28,8 +28,6 @@ export type ContextKey = ContextKeySegment[] export interface ContextResolveOpts { // Allow templates to be partially resolved (used to defer runtime template resolution, for example) allowPartial?: boolean - // Allow undefined values to be returned without throwing an error - allowUndefined?: boolean // a list of previously resolved paths, used to detect circular references stack?: string[] } @@ -178,14 +176,18 @@ export abstract class ConfigContext { } } - if (opts.allowUndefined) { - return { resolved: undefined, message } - } else { - throw new ConfigurationError(message, { + if (opts.allowPartial) { + // If we're allowing partial strings, we throw the error immediately to end the resolution flow. The error + // is caught in the surrounding template resolution code. + throw new TemplateStringMissingKeyError(message, { nodePath, fullPath, opts, }) + } else { + // Otherwise we return the undefined value, so that any logical expressions can be evaluated appropriately. + // The template resolver will throw the error later if appropriate. + return { resolved: undefined, message } } } @@ -450,9 +452,6 @@ export class WorkflowConfigContext extends EnvironmentConfigContext { ) public secrets: PrimitiveMap - // We ignore step references here, and keep for later resolution - public steps: Map | PassthroughContext - constructor(garden: Garden) { super({ projectName: garden.projectName, @@ -465,7 +464,6 @@ export class WorkflowConfigContext extends EnvironmentConfigContext { const fullEnvName = garden.namespace ? `${garden.namespace}.${garden.environmentName}` : garden.environmentName this.environment = new EnvironmentContext(this, garden.environmentName, fullEnvName, garden.namespace) this.project = new ProjectContext(this, garden.projectName) - this.steps = new PassthroughContext() } } @@ -695,34 +693,7 @@ export class TaskRuntimeContext extends ServiceRuntimeContext { public outputs: PrimitiveMap } -/** - * Used to defer and return the template string back, when allowPartial=true. - */ -export class PassthroughContext extends ConfigContext { - resolve(params: ContextResolveParams): ContextResolveOutput { - const opts = { ...(params.opts || {}), allowUndefined: params.opts.allowPartial || params.opts.allowUndefined } - const res = super.resolve({ ...params, opts }) - - if (res.resolved === undefined) { - if (params.opts.allowPartial) { - // If value can't be resolved and allowPartial is set, we defer the resolution by returning another template - // string, that can be resolved later. - const { key, nodePath } = params - const fullKey = nodePath.concat(key) - return { resolved: renderTemplateString(fullKey), partial: true } - } else { - // If undefined values are allowed, we simply return undefined (We know allowUndefined is set here, because - // otherwise an error would have been thrown by `super.resolve()` above). - return res - } - } else { - // Value successfully resolved - return res - } - } -} - -class RuntimeConfigContext extends PassthroughContext { +class RuntimeConfigContext extends ConfigContext { @schema( joiIdentifierMap(ServiceRuntimeContext.getSchema()) .required() diff --git a/core/src/tasks/resolve-module.ts b/core/src/tasks/resolve-module.ts index aa7bc9a14c..94295d254c 100644 --- a/core/src/tasks/resolve-module.ts +++ b/core/src/tasks/resolve-module.ts @@ -54,7 +54,7 @@ export class ResolveModuleConfigTask extends BaseTask { async resolveDependencies() { const rawConfigs = keyBy(await this.garden.getRawModuleConfigs(), "name") - const templateRefs = await getModuleTemplateReferences(this.moduleConfig) + const templateRefs = getModuleTemplateReferences(this.moduleConfig) const deps = templateRefs.filter((d) => d[1] !== this.moduleConfig.name) return deps.map((d) => { diff --git a/core/src/template-string-parser.pegjs b/core/src/template-string-parser.pegjs index 3a9cd38228..6d7ac4c606 100644 --- a/core/src/template-string-parser.pegjs +++ b/core/src/template-string-parser.pegjs @@ -6,6 +6,21 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +{ + const { + buildBinaryExpression, + buildLogicalExpression, + getKey, + getValue, + isArray, + isPrimitive, + optionalSuffix, + missingKeyErrorType, + resolveNested, + TemplateStringError, + } = options +} + TemplateString = a:(FormatString)+ b:TemplateString? { return [...a, ...(b || [])] } / a:Prefix b:(FormatString)+ c:TemplateString? { return [a, ...b, ...(c || [])] } @@ -14,25 +29,37 @@ TemplateString FormatString = FormatStart e:Expression end:FormatEnd { - if (e && e._error) { + if (e && e._error && e._error.type !== missingKeyErrorType) { return e } // Need to provide the optional suffix as a variable because of a parsing bug in pegjs - const allowUndefined = options.allowUndefined || end[1] === options.optionalSuffix - - if (options.getValue(e) === undefined && !allowUndefined) { - const _error = new options.TemplateStringError(e.message || "Unable to resolve one or more keys.", { - text: text(), - }) - return { _error } + const allowUndefined = end[1] === optionalSuffix + + if (getValue(e) === undefined) { + if (allowUndefined) { + if (e && e._error) { + return { ...e, _error: undefined } + } else { + return e + } + } else if (options.allowPartial) { + return text() + } else if (e && e._error) { + return e + } else { + const _error = new TemplateStringError(e.message || "Unable to resolve one or more keys.", { + text: text(), + }) + return { _error } + } } return e } InvalidFormatString = Prefix? FormatStart .* { - throw new options.TemplateStringError("Unable to parse as valid template string.") + throw new TemplateStringError("Unable to parse as valid template string.") } FormatStart @@ -54,8 +81,8 @@ MemberExpression = head:Identifier tail:( "[" __ e:PrimaryExpression __ "]" { - if (e.resolved && !options.isPrimitive(e.resolved)) { - const _error = new options.TemplateStringError( + if (e.resolved && !isPrimitive(e.resolved)) { + const _error = new TemplateStringError( `Expression in bracket must resolve to a primitive (got ${typeof e}).`, { text: e.resolved } ) @@ -77,7 +104,7 @@ PrimaryExpression } / v:StringLiteral { // Allow nested template strings in literals - return options.resolveNested(v) + return resolveNested(v) } / key:MemberExpression { for (const part of key) { @@ -86,7 +113,11 @@ PrimaryExpression } } key = key.map((part) => part.resolved || part) - return options.getKey(key, { allowUndefined: true }) + try { + return getKey(key, { allowPartial: options.allowPartial }) + } catch (err) { + return { _error: err } + } } / "(" __ e:Expression __ ")" { return e @@ -102,9 +133,9 @@ UnaryExpression } if (operator === "typeof") { - return typeof options.getValue(v) + return typeof getValue(v) } else if (operator === "!") { - return !options.getValue(v) + return !getValue(v) } } @@ -121,12 +152,12 @@ ContainsExpression return tail } - head = options.getValue(head) - tail = options.getValue(tail) + head = getValue(head) + tail = getValue(tail) - if (!options.isPrimitive(tail)) { + if (!isPrimitive(tail)) { return { - _error: new options.TemplateStringError( + _error: new TemplateStringError( `The right-hand side of a 'contains' operator must be a string, number, boolean or null (got ${typeof tail}).` ) } @@ -135,7 +166,7 @@ ContainsExpression const headType = head === null ? "null" : typeof head if (headType === "object") { - if (options.lodash.isArray(head)) { + if (isArray(head)) { return head.includes(tail) } else { return head.hasOwnProperty(tail) @@ -144,7 +175,7 @@ ContainsExpression return head.includes(tail.toString()) } else { return { - _error: new options.TemplateStringError( + _error: new TemplateStringError( `The left-hand side of a 'contains' operator must be a string, array or object (got ${headType}).` ) } @@ -158,7 +189,7 @@ ContainsOperator MultiplicativeExpression = head:ContainsExpression tail:(__ MultiplicativeOperator __ ContainsExpression)* - { return options.buildBinaryExpression(head, tail); } + { return buildBinaryExpression(head, tail); } MultiplicativeOperator = $("*" !"=") @@ -169,7 +200,7 @@ AdditiveExpression = head:MultiplicativeExpression // Note: We require a whitespace around these operators to disambiguate from identifiers with dashes tail:(WhiteSpace+ AdditiveOperator WhiteSpace+ MultiplicativeExpression)* - { return options.buildBinaryExpression(head, tail); } + { return buildBinaryExpression(head, tail); } AdditiveOperator = $("+" ![+=]) @@ -178,7 +209,7 @@ AdditiveOperator RelationalExpression = head:AdditiveExpression tail:(__ RelationalOperator __ AdditiveExpression)* - { return options.buildBinaryExpression(head, tail); } + { return buildBinaryExpression(head, tail); } RelationalOperator = "<=" @@ -189,7 +220,7 @@ RelationalOperator EqualityExpression = head:RelationalExpression tail:(__ EqualityOperator __ RelationalExpression)* - { return options.buildBinaryExpression(head, tail); } + { return buildBinaryExpression(head, tail); } EqualityOperator = "==" @@ -198,7 +229,7 @@ EqualityOperator LogicalANDExpression = head:EqualityExpression tail:(__ LogicalANDOperator __ EqualityExpression)* - { return options.buildLogicalExpression(head, tail); } + { return buildLogicalExpression(head, tail); } LogicalANDOperator = "&&" @@ -206,7 +237,7 @@ LogicalANDOperator LogicalORExpression = head:LogicalANDExpression tail:(__ LogicalOROperator __ LogicalANDExpression)* - { return options.buildLogicalExpression(head, tail); } + { return buildLogicalExpression(head, tail); } LogicalOROperator = "||" @@ -226,7 +257,7 @@ ConditionalExpression return alternate } - return options.getValue(test) ? consequent : alternate + return getValue(test) ? consequent : alternate } / LogicalORExpression diff --git a/core/src/template-string.ts b/core/src/template-string.ts index eaf792ec73..42a3edcfff 100644 --- a/core/src/template-string.ts +++ b/core/src/template-string.ts @@ -6,7 +6,6 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import lodash from "lodash" import { GardenBaseError, ConfigurationError } from "./exceptions" import { ConfigContext, @@ -23,10 +22,16 @@ import { isArray } from "util" export type StringOrStringPromise = Promise | string +const missingKeyErrorType = "template-string-missing-key" + class TemplateStringError extends GardenBaseError { type = "template-string" } +export class TemplateStringMissingKeyError extends GardenBaseError { + type = missingKeyErrorType +} + let _parser: any function getParser() { @@ -64,13 +69,13 @@ export function resolveTemplateString(string: string, context: ConfigContext, op }, getValue, resolveNested: (nested: string) => resolveTemplateString(nested, context, opts), - // Some utilities to pass to the parser buildBinaryExpression, buildLogicalExpression, - lodash, + isArray, ConfigurationError, TemplateStringError, - allowUndefined: opts.allowUndefined, + missingKeyErrorType, + allowPartial: !!opts.allowPartial, optionalSuffix: "}?", isPrimitive, }) @@ -158,7 +163,7 @@ export const resolveTemplateStrings = profile(function $resolveTemplateStrings(obj: T): ContextKeySegment[][] { const context = new ScanContext() - resolveTemplateStrings(obj, context, { allowPartial: true, allowUndefined: true }) + resolveTemplateStrings(obj, context, { allowPartial: true }) return uniq(context.foundKeys.entries()).sort() } diff --git a/core/test/unit/src/config/config-context.ts b/core/test/unit/src/config/config-context.ts index 4259af6f06..56ec8ba802 100644 --- a/core/test/unit/src/config/config-context.ts +++ b/core/test/unit/src/config/config-context.ts @@ -18,7 +18,6 @@ import { WorkflowConfigContext, WorkflowStepConfigContext, ScanContext, - PassthroughContext, } from "../../../../src/config/config-context" import { expectError, makeTestGardenA, TestGarden, projectRootA, makeTestGarden } from "../../../helpers" import { join } from "path" @@ -49,8 +48,8 @@ describe("ConfigContext", () => { describe("resolve", () => { // just a shorthand to aid in testing - function resolveKey(c: ConfigContext, key: ContextKey) { - return c.resolve({ key, nodePath: [], opts: {} }) + function resolveKey(c: ConfigContext, key: ContextKey, opts = {}) { + return c.resolve({ key, nodePath: [], opts }) } it("should resolve simple keys", async () => { @@ -58,14 +57,36 @@ describe("ConfigContext", () => { expect(resolveKey(c, ["basic"])).to.eql({ resolved: "value" }) }) - it("should throw on missing key", async () => { + it("should return undefined for missing key", async () => { const c = new TestContext({}) - await expectError(() => resolveKey(c, ["basic"]), "configuration") + const { resolved, message } = resolveKey(c, ["basic"]) + expect(resolved).to.be.undefined + expect(stripAnsi(message!)).to.equal("Could not find key basic.") + }) + + context("allowPartial=true", () => { + it("should throw on missing key when allowPartial=true", async () => { + const c = new TestContext({}) + expectError( + () => resolveKey(c, ["basic"], { allowPartial: true }), + (err) => expect(stripAnsi(err.message)).to.equal("Could not find key basic.") + ) + }) + + it("should throw on missing key on nested context", async () => { + const c = new TestContext({ + nested: new TestContext({ key: "value" }), + }) + expectError( + () => resolveKey(c, ["nested", "bla"], { allowPartial: true }), + (err) => expect(stripAnsi(err.message)).to.equal("Could not find key bla under nested. Available keys: key.") + ) + }) }) it("should throw when looking for nested value on primitive", async () => { const c = new TestContext({ basic: "value" }) - await expectError(() => resolveKey(c, ["basic", "nested"]), "configuration") + expectError(() => resolveKey(c, ["basic", "nested"]), "configuration") }) it("should resolve nested keys", async () => { @@ -80,11 +101,13 @@ describe("ConfigContext", () => { expect(resolveKey(c, ["nested", "key"])).eql({ resolved: "value" }) }) - it("should throw on missing key on nested context", async () => { + it("should return undefined for missing keys on nested context", async () => { const c = new TestContext({ nested: new TestContext({ key: "value" }), }) - await expectError(() => resolveKey(c, ["nested", "bla"]), "configuration") + const { resolved, message } = resolveKey(c, ["basic", "bla"]) + expect(resolved).to.be.undefined + expect(stripAnsi(message!)).to.equal("Could not find key basic. Available keys: nested.") }) it("should resolve keys with value behind callable", async () => { @@ -117,12 +140,12 @@ describe("ConfigContext", () => { }) const key = ["nested", "key"] const stack = [key.join(".")] - await expectError(() => c.resolve({ key, nodePath: [], opts: { stack } }), "configuration") + expectError(() => c.resolve({ key, nodePath: [], opts: { stack } }), "configuration") }) it("should detect a circular reference from a nested context", async () => { class NestedContext extends ConfigContext { - async resolve({ key, nodePath, opts }: ContextResolveParams) { + resolve({ key, nodePath, opts }: ContextResolveParams) { const circularKey = nodePath.concat(key) opts.stack!.push(circularKey.join(".")) return c.resolve({ key: circularKey, nodePath: [], opts }) @@ -131,10 +154,10 @@ describe("ConfigContext", () => { const c = new TestContext({ nested: new NestedContext(), }) - await expectError(async () => resolveKey(c, ["nested", "bla"]), "configuration") + expectError(() => resolveKey(c, ["nested", "bla"]), "configuration") }) - it("should show helpful error when unable to resolve nested key in map", async () => { + it("should return helpful message when unable to resolve nested key in map", async () => { class Context extends ConfigContext { nested: Map @@ -144,10 +167,8 @@ describe("ConfigContext", () => { } } const c = new Context() - await expectError( - () => resolveKey(c, ["nested", "bla"]), - (err) => expect(stripAnsi(err.message)).to.equal("Could not find key bla under nested.") - ) + const { message } = resolveKey(c, ["nested", "bla"]) + expect(stripAnsi(message!)).to.equal("Could not find key bla under nested.") }) it("should show helpful error when unable to resolve nested key in object", async () => { @@ -160,10 +181,8 @@ describe("ConfigContext", () => { } } const c = new Context() - await expectError( - () => resolveKey(c, ["nested", "bla"]), - (err) => expect(stripAnsi(err.message)).to.equal("Could not find key bla under nested.") - ) + const { message } = resolveKey(c, ["nested", "bla"]) + expect(stripAnsi(message!)).to.equal("Could not find key bla under nested.") }) it("should show helpful error when unable to resolve two-level nested key in object", async () => { @@ -176,10 +195,8 @@ describe("ConfigContext", () => { } } const c = new Context() - await expectError( - () => resolveKey(c, ["nested", "deeper", "bla"]), - (err) => expect(stripAnsi(err.message)).to.equal("Could not find key bla under nested.deeper.") - ) + const { message } = resolveKey(c, ["nested", "deeper", "bla"]) + expect(stripAnsi(message!)).to.equal("Could not find key bla under nested.deeper.") }) it("should show helpful error when unable to resolve in nested context", async () => { @@ -193,10 +210,8 @@ describe("ConfigContext", () => { } } const c = new Context() - await expectError( - () => resolveKey(c, ["nested", "bla"]), - (err) => expect(stripAnsi(err.message)).to.equal("Could not find key bla under nested.") - ) + const { message } = resolveKey(c, ["nested", "bla"]) + expect(stripAnsi(message!)).to.equal("Could not find key bla under nested.") }) it("should resolve template strings", async () => { @@ -219,7 +234,7 @@ describe("ConfigContext", () => { it("should detect a self-reference when resolving a template string", async () => { const c = new TestContext({ key: "${key}" }) - await expectError(() => resolveKey(c, ["key"]), "template-string") + expectError(() => resolveKey(c, ["key"]), "template-string") }) it("should detect a nested self-reference when resolving a template string", async () => { @@ -228,7 +243,7 @@ describe("ConfigContext", () => { }) const nested = new TestContext({ key: "${nested.key}" }, c) c.addValues({ nested }) - await expectError(() => resolveKey(c, ["nested", "key"]), "template-string") + expectError(() => resolveKey(c, ["nested", "key"]), "template-string") }) it("should detect a circular reference when resolving a template string", async () => { @@ -237,7 +252,7 @@ describe("ConfigContext", () => { }) const nested: any = new TestContext({ key: "${nested.foo}", foo: "${nested.key}" }, c) c.addValues({ nested }) - await expectError(() => resolveKey(c, ["nested", "key"]), "template-string") + expectError(() => resolveKey(c, ["nested", "key"]), "template-string") }) it("should detect a circular reference when resolving a nested template string", async () => { @@ -246,7 +261,7 @@ describe("ConfigContext", () => { }) const nested: any = new TestContext({ key: "${nested.foo}", foo: "${'${nested.key}'}" }, c) c.addValues({ nested }) - await expectError(() => resolveKey(c, ["nested", "key"]), "template-string") + expectError(() => resolveKey(c, ["nested", "key"]), "template-string") }) it("should detect a circular reference when nested template string resolves to self", async () => { @@ -255,7 +270,7 @@ describe("ConfigContext", () => { }) const nested: any = new TestContext({ key: "${'${nested.key}'}" }, c) c.addValues({ nested }) - await expectError( + expectError( () => resolveKey(c, ["nested", "key"]), (err) => expect(err.message).to.equal( @@ -329,7 +344,7 @@ describe("ProjectConfigContext", () => { }) }) - it("should throw helpful error when resolving missing env variable", () => { + it("should return helpful message when resolving missing env variable", () => { const c = new ProjectConfigContext({ projectName: "some-project", artifactsPath: "/tmp", @@ -338,12 +353,10 @@ describe("ProjectConfigContext", () => { }) const key = "fiaogsyecgbsjyawecygaewbxrbxajyrgew" - expectError( - () => c.resolve({ key: ["local", "env", key], nodePath: [], opts: {} }), - (err) => - expect(stripAnsi(err.message)).to.match( - /Could not find key fiaogsyecgbsjyawecygaewbxrbxajyrgew under local.env. Available keys: / - ) + const { message } = c.resolve({ key: ["local", "env", key], nodePath: [], opts: {} }) + + expect(stripAnsi(message!)).to.match( + /Could not find key fiaogsyecgbsjyawecygaewbxrbxajyrgew under local.env. Available keys: / ) }) @@ -448,46 +461,6 @@ describe("ModuleConfigContext", () => { resolved: "someSecretValue", }) }) - - it("should throw when resolving a secret with a missing key", async () => { - await expectError( - () => c.resolve({ key: ["secrets", "missingSecret"], nodePath: [], opts: {} }), - (err) => - expect(stripAnsi(err.message)).to.equal( - "Could not find key missingSecret under secrets. Available keys: someSecret." - ) - ) - }) - }) - - context("runtimeContext is not set", () => { - it("should return runtime template strings unchanged if allowPartial=true", async () => { - expect( - c.resolve({ key: ["runtime", "some", "key.with.dots"], nodePath: [], opts: { allowPartial: true } }) - ).to.eql({ - resolved: '${runtime.some["key.with.dots"]}', - }) - }) - - it("should throw if resolving missing runtime key with allowPartial=false", async () => { - await expectError( - () => c.resolve({ key: ["runtime", "some", "key"], nodePath: [], opts: {} }), - (err) => - expect(stripAnsi(err.message)).to.equal( - "Could not find key some under runtime. Available keys: services and tasks." - ) - ) - }) - - it("should allow using a missing runtime key as a test in a conditional", async () => { - const result = resolveTemplateString("${runtime.foo || 'default'}", c) - expect(result).to.equal("default") - }) - - it("should allow using a missing runtime key as a test in a ternary (negative)", async () => { - const result = resolveTemplateString("${runtime.foo ? runtime.foo.bar : 'default'}", c) - expect(result).to.equal("default") - }) }) context("runtimeContext is set", () => { @@ -559,15 +532,6 @@ describe("ModuleConfigContext", () => { expect(result).to.eql({ resolved: "boo" }) }) - it("should return the template string back if allowPartial=true and outputs haven't been resolved ", async () => { - const result = withRuntime.resolve({ - key: ["runtime", "services", "not-ready", "outputs", "foo"], - nodePath: [], - opts: { allowPartial: true }, - }) - expect(result).to.eql({ resolved: "${runtime.services.not-ready.outputs.foo}" }) - }) - it("should allow using a runtime key as a test in a ternary (positive)", async () => { const result = resolveTemplateString( "${runtime.tasks.task-b ? runtime.tasks.task-b.outputs.moo : 'default'}", @@ -575,21 +539,6 @@ describe("ModuleConfigContext", () => { ) expect(result).to.equal("boo") }) - - it("should throw when a service's outputs have been resolved but an output key is not found", async () => { - await expectError( - () => - withRuntime.resolve({ - key: ["runtime", "services", "service-b", "outputs", "boo"], - nodePath: [], - opts: {}, - }), - (err) => - expect(stripAnsi(err.message)).to.equal( - "Could not find key boo under runtime.services.service-b.outputs. Available keys: foo." - ) - ) - }) }) }) @@ -631,28 +580,12 @@ describe("WorkflowConfigContext", () => { expect(c.resolve({ key: ["var", "some"], nodePath: [], opts: {} })).to.eql({ resolved: "variable" }) }) - it("should return a step reference back with allowPartial=true", async () => { - expect(c.resolve({ key: ["steps", "step-1", "foo"], nodePath: [], opts: { allowPartial: true } })).to.eql({ - resolved: "${steps.step-1.foo}", - }) - }) - context("secrets", () => { it("should resolve a secret", async () => { expect(c.resolve({ key: ["secrets", "someSecret"], nodePath: [], opts: {} })).to.eql({ resolved: "someSecretValue", }) }) - - it("should throw when resolving a secret with a missing key", async () => { - await expectError( - () => c.resolve({ key: ["secrets", "missingSecret"], nodePath: [], opts: {} }), - (err) => - expect(stripAnsi(err.message)).to.equal( - "Could not find key missingSecret under secrets. Available keys: someSecret." - ) - ) - }) }) }) @@ -713,22 +646,6 @@ describe("WorkflowStepConfigContext", () => { ) }) - it("should throw error when attempting to reference a missing step", () => { - const c = new WorkflowStepConfigContext({ - garden, - allStepNames: ["step-1", "step-2"], - resolvedSteps: {}, - stepName: "step-1", - }) - expectError( - () => c.resolve({ key: ["steps", "step-foo", "log"], nodePath: [], opts: {} }), - (err) => - expect(stripAnsi(err.message)).to.equal( - "Could not find key step-foo under steps. Available keys: step-1 and step-2." - ) - ) - }) - it("should throw error when attempting to reference current step", () => { const c = new WorkflowStepConfigContext({ garden, @@ -773,41 +690,3 @@ describe("ScanContext", () => { ]) }) }) - -describe("PassthroughContext", () => { - it("should return the template key back unresolved when allowPartial=true", () => { - const context = new PassthroughContext() - const obj = { - a: "some ${templated.string}", - b: "${more.stuff}", - } - const result = resolveTemplateStrings(obj, context, { allowPartial: true }) - expect(result).to.eql(obj) - }) - - it("should handle keys with dots correctly", () => { - const context = new PassthroughContext() - const obj = { - a: "some ${templated['key.with.dots']}", - b: "${more.stuff}", - } - const result = resolveTemplateStrings(obj, context, { allowPartial: true }) - expect(result).to.eql({ - a: 'some ${templated["key.with.dots"]}', - b: "${more.stuff}", - }) - }) - - it("should handle nested template keys correctly", () => { - const context = new PassthroughContext() - const obj = { - a: "some ${templated['${nested.key}']}", - b: "${more.stuff}", - } - const result = resolveTemplateStrings(obj, context, { allowPartial: true }) - expect(result).to.eql({ - a: 'some ${templated["${nested.key}"]}', - b: "${more.stuff}", - }) - }) -}) diff --git a/core/test/unit/src/garden.ts b/core/test/unit/src/garden.ts index e8cb01c367..f0b5359c8d 100644 --- a/core/test/unit/src/garden.ts +++ b/core/test/unit/src/garden.ts @@ -1518,7 +1518,7 @@ describe("Garden", () => { (err) => { expect(err.message).to.equal("Failed resolving one or more providers:\n" + "- test") expect(stripAnsi(err.detail.messages[0])).to.equal( - "- test: Invalid template string ${bla.ble}: Could not find key bla. Available keys: environment, local, project, providers, secrets, steps, var and variables." + "- test: Invalid template string ${bla.ble}: Could not find key bla. Available keys: environment, local, project, providers, secrets, var and variables." ) } ) diff --git a/core/test/unit/src/template-string.ts b/core/test/unit/src/template-string.ts index 96f9cb4478..13bd58ca30 100644 --- a/core/test/unit/src/template-string.ts +++ b/core/test/unit/src/template-string.ts @@ -42,11 +42,6 @@ describe("resolveTemplateString", async () => { expect(res).to.equal("value") }) - it("should optionally allow undefined values", async () => { - const res = resolveTemplateString("${some}", new TestContext({}), { allowUndefined: true }) - expect(res).to.equal(undefined) - }) - it("should allow undefined values if ? suffix is present", async () => { const res = resolveTemplateString("${foo}?", new TestContext({})) expect(res).to.equal(undefined) @@ -627,6 +622,23 @@ describe("resolveTemplateString", async () => { ) }) + context("allowPartial=true", () => { + it("passes through template strings with missing key", () => { + const res = resolveTemplateString("${a}", new TestContext({}), { allowPartial: true }) + expect(res).to.equal("${a}") + }) + + it("passes through a template string with a missing key in an optional clause", () => { + const res = resolveTemplateString("${a || b}", new TestContext({ b: 123 }), { allowPartial: true }) + expect(res).to.equal("${a || b}") + }) + + it("passes through a template string with a missing key in a ternary", () => { + const res = resolveTemplateString("${a ? b : 123}", new TestContext({ b: 123 }), { allowPartial: true }) + expect(res).to.equal("${a ? b : 123}") + }) + }) + context("when the template string is the full input string", () => { it("should return a resolved number directly", async () => { const res = resolveTemplateString("${a}", new TestContext({ a: 100 })) @@ -679,6 +691,20 @@ describe("resolveTemplateString", async () => { const res = resolveTemplateString("foo-${a}", new TestContext({ a: null })) expect(res).to.equal("foo-null") }) + + context("allowPartial=true", () => { + it("passes through template strings with missing key", () => { + const res = resolveTemplateString("${a}-${b}", new TestContext({ b: "foo" }), { allowPartial: true }) + expect(res).to.equal("${a}-foo") + }) + + it("passes through a template string with a missing key in an optional clause", () => { + const res = resolveTemplateString("${a || b}-${c}", new TestContext({ b: 123, c: "foo" }), { + allowPartial: true, + }) + expect(res).to.equal("${a || b}-foo") + }) + }) }) context("contains operator", () => {