fix(core): some conditional template strings were not resolved correctly

This was introduced in v0.12.7 after some changes in template resolution
logic. Added tests to catch this, should all work as expected now, but
please test to make sure when reviewing.

core/src/actions.ts

@@ -866,6 +866,7 @@ export class ActionRouter implements TypeGuard {
         parentName: module.parentName,
         templateName: module.templateName,
         inputs: module.inputs,
+        partialRuntimeResolution: false,
       })
 
       // Set allowPartial=false to ensure all required strings are resolved.
@@ -928,6 +929,7 @@ export class ActionRouter implements TypeGuard {
         parentName: module.parentName,
         templateName: module.templateName,
         inputs: module.inputs,
+        partialRuntimeResolution: false,
       })
 
       // Set allowPartial=false to ensure all required strings are resolved.

core/src/commands/deploy.ts

@@ -118,7 +118,7 @@ export class DeployCommand extends Command<Args, Opts> {
     }
 
     const initGraph = await garden.getConfigGraph(log)
-    let services = await initGraph.getServices({ names: args.services, includeDisabled: true })
+    let services = initGraph.getServices({ names: args.services, includeDisabled: true })
 
     const disabled = services.filter((s) => s.disabled).map((s) => s.name)
 

core/src/config/config-context.ts

@@ -20,7 +20,11 @@ import {
 } from "./common"
 import { Provider, GenericProviderConfig, ProviderMap } from "./provider"
 import { ConfigurationError } from "../exceptions"
-import { resolveTemplateString, TemplateStringMissingKeyError } from "../template-string"
+import {
+  resolveTemplateString,
+  TemplateStringMissingKeyException,
+  TemplateStringPassthroughException,
+} from "../template-string"
 import { Garden } from "../garden"
 import { joi } from "../config/common"
 import { KeyedSet } from "../util/keyed-set"
@@ -64,9 +68,13 @@ export abstract class ConfigContext {
   private readonly _rootContext: ConfigContext
   private readonly _resolvedValues: { [path: string]: string }
 
+  // This is used for special-casing e.g. runtime.* resolution
+  protected _alwaysAllowPartial: boolean
+
   constructor(rootContext?: ConfigContext) {
     this._rootContext = rootContext || this
     this._resolvedValues = {}
+    this._alwaysAllowPartial = false
   }
 
   static getSchema() {
@@ -185,10 +193,18 @@ export abstract class ConfigContext {
         }
       }
 
-      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, {
+      // 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.
+      if (this._alwaysAllowPartial) {
+        // We use a separate exception type when contexts are specifically indicating that unresolvable keys should
+        // be passed through. This is caught in the template parser code.
+        throw new TemplateStringPassthroughException(message, {
+          nodePath,
+          fullPath,
+          opts,
+        })
+      } else if (opts.allowPartial) {
+        throw new TemplateStringMissingKeyException(message, {
           nodePath,
           fullPath,
           opts,
@@ -716,21 +732,25 @@ class RuntimeConfigContext extends ConfigContext {
   )
   public tasks: Map<string, TaskRuntimeContext>
 
-  constructor(root: ConfigContext, runtimeContext?: RuntimeContext) {
+  constructor(root: ConfigContext, allowPartial: boolean, runtimeContext?: RuntimeContext) {
     super(root)
 
     this.services = new Map()
     this.tasks = new Map()
 
-    const dependencies = runtimeContext ? runtimeContext.dependencies : []
-
-    for (const dep of dependencies) {
-      if (dep.type === "service") {
-        this.services.set(dep.name, new ServiceRuntimeContext(this, dep.outputs))
-      } else if (dep.type === "task") {
-        this.tasks.set(dep.name, new TaskRuntimeContext(this, dep.outputs))
+    if (runtimeContext) {
+      for (const dep of runtimeContext.dependencies) {
+        if (dep.type === "service") {
+          this.services.set(dep.name, new ServiceRuntimeContext(this, dep.outputs))
+        } else if (dep.type === "task") {
+          this.tasks.set(dep.name, new TaskRuntimeContext(this, dep.outputs))
+        }
       }
     }
+
+    // This ensures that any template string containing runtime.* references is returned unchanged when
+    // there is no or limited runtimeContext available.
+    this._alwaysAllowPartial = allowPartial
   }
 }
 
@@ -843,6 +863,7 @@ export class ModuleConfigContext extends ProviderConfigContext {
     parentName,
     templateName,
     inputs,
+    partialRuntimeResolution,
   }: {
     garden: Garden
     resolvedProviders: ProviderMap
@@ -854,6 +875,7 @@ export class ModuleConfigContext extends ProviderConfigContext {
     parentName: string | undefined
     templateName: string | undefined
     inputs: DeepPrimitiveMap | undefined
+    partialRuntimeResolution: boolean
   }) {
     super(garden, resolvedProviders)
 
@@ -866,7 +888,7 @@ export class ModuleConfigContext extends ProviderConfigContext {
       this.modules.set(moduleName, new ErrorContext(`Module ${chalk.white.bold(moduleName)} cannot reference itself.`))
     }
 
-    this.runtime = new RuntimeConfigContext(this, runtimeContext)
+    this.runtime = new RuntimeConfigContext(this, partialRuntimeResolution, runtimeContext)
     if (parentName && templateName) {
       this.parent = new ParentContext(this, parentName)
       this.template = new ModuleTemplateContext(this, templateName)
@@ -898,6 +920,7 @@ export class OutputConfigContext extends ModuleConfigContext {
       parentName: undefined,
       templateName: undefined,
       inputs: {},
+      partialRuntimeResolution: false,
     })
   }
 }

core/src/garden.ts

@@ -843,6 +843,7 @@ export class Garden {
         parentName: undefined,
         templateName: undefined,
         inputs: {},
+        partialRuntimeResolution: true,
       })
 
       // Resolve modules from specs and add to the list

core/src/resolve-module.ts

@@ -29,8 +29,7 @@ export const resolveModuleConfig = profileAsync(async function $resolveModuleCon
   config: ModuleConfig,
   opts: ModuleConfigResolveOpts
 ): Promise<ModuleConfig> {
-  // Allowing partial resolution here, to defer runtime remplate resolution
-  config = resolveTemplateStrings(cloneDeep(config), opts.configContext, { allowPartial: true, ...opts })
+  config = resolveTemplateStrings(cloneDeep(config), opts.configContext, opts)
 
   const moduleTypeDefinitions = await garden.getModuleTypes()
   const description = moduleTypeDefinitions[config.type]

core/src/tasks/deploy.ts

@@ -67,7 +67,7 @@ export class DeployTask extends BaseTask {
     const dg = this.graph
 
     // We filter out service dependencies on services configured for hot reloading (if any)
-    const deps = await dg.getDependencies({
+    const deps = dg.getDependencies({
       nodeType: "deploy",
       name: this.getName(),
       recursive: false,

core/src/tasks/resolve-module.ts

@@ -66,6 +66,7 @@ export class ResolveModuleConfigTask extends BaseTask {
       parentName: this.moduleConfig.parentName,
       templateName: this.moduleConfig.templateName,
       inputs: this.moduleConfig.inputs,
+      partialRuntimeResolution: true,
     })
 
     const templateRefs = getModuleTemplateReferences(this.moduleConfig, configContext)
@@ -116,10 +117,11 @@ export class ResolveModuleConfigTask extends BaseTask {
       parentName: this.moduleConfig.parentName,
       templateName: this.moduleConfig.templateName,
       inputs: this.moduleConfig.inputs,
+      partialRuntimeResolution: true,
     })
 
     return resolveModuleConfig(this.garden, this.moduleConfig, {
-      allowPartial: true,
+      allowPartial: false,
       configContext,
     })
   }
@@ -221,6 +223,7 @@ export class ResolveModuleTask extends BaseTask {
       parentName: this.moduleConfig.parentName,
       templateName: this.moduleConfig.templateName,
       inputs: this.moduleConfig.inputs,
+      partialRuntimeResolution: true,
     })
 
     await Bluebird.map(resolvedConfig.generateFiles || [], async (fileSpec) => {

core/src/template-string-parser.pegjs

@@ -15,7 +15,8 @@
     isArray,
     isPrimitive,
     optionalSuffix,
-    missingKeyErrorType,
+    missingKeyExceptionType,
+    passthroughExceptionType,
     resolveNested,
     TemplateStringError,
   } = options
@@ -29,15 +30,20 @@ TemplateString
 
 FormatString
   = FormatStart e:Expression end:FormatEnd {
-      if (e && e._error && e._error.type !== missingKeyErrorType) {
+      // Any unexpected error is returned immediately. Certain exceptions have special semantics that are caught below.
+      if (e && e._error && e._error.type !== missingKeyExceptionType && e._error.type !== passthroughExceptionType) {
         return e
       }
 
       // Need to provide the optional suffix as a variable because of a parsing bug in pegjs
       const allowUndefined = end[1] === optionalSuffix
 
       if (getValue(e) === undefined) {
-        if (allowUndefined) {
+        if (e && e._error && e._error.type === passthroughExceptionType) {
+          // We allow certain configuration contexts (e.g. placeholders for runtime.*) to indicate that a template
+          // string should be returned partially resolved even if allowPartial=false.
+          return text()
+        } else if (allowUndefined) {
           if (e && e._error) {
             return { ...e, _error: undefined }
           } else {

core/src/template-string.ts

@@ -24,14 +24,19 @@ import { ObjectWithName } from "./util/util"
 
 export type StringOrStringPromise = Promise<string> | string
 
-const missingKeyErrorType = "template-string-missing-key"
+const missingKeyExceptionType = "template-string-missing-key"
+const passthroughExceptionType = "template-string-passthrough"
 
 class TemplateStringError extends GardenBaseError {
   type = "template-string"
 }
 
-export class TemplateStringMissingKeyError extends GardenBaseError {
-  type = missingKeyErrorType
+export class TemplateStringMissingKeyException extends GardenBaseError {
+  type = missingKeyExceptionType
+}
+
+export class TemplateStringPassthroughException extends GardenBaseError {
+  type = passthroughExceptionType
 }
 
 let _parser: any
@@ -76,7 +81,8 @@ export function resolveTemplateString(string: string, context: ConfigContext, op
       isArray,
       ConfigurationError,
       TemplateStringError,
-      missingKeyErrorType,
+      missingKeyExceptionType,
+      passthroughExceptionType,
       allowPartial: !!opts.allowPartial,
       optionalSuffix: "}?",
       isPrimitive,

core/src/util/testing.ts

@@ -109,8 +109,8 @@ export class TestGarden extends Garden {
    * Helper to get a single module. We don't put this on the Garden class because it is highly inefficient
    * and not advisable except for testing.
    */
-  async resolveModule(name: string) {
-    const modules = await this.resolveModules({ log: this.log })
+  async resolveModule(name: string, runtimeContext?: RuntimeContext) {
+    const modules = await this.resolveModules({ log: this.log, runtimeContext })
     const config = findByName(modules, name)
 
     if (!config) {

core/test/unit/src/config/config-context.ts

@@ -408,6 +408,7 @@ describe("ModuleConfigContext", () => {
       parentName: undefined,
       templateName: undefined,
       inputs: {},
+      partialRuntimeResolution: false,
     })
   })
 
@@ -517,6 +518,7 @@ describe("ModuleConfigContext", () => {
         parentName: undefined,
         templateName: undefined,
         inputs: {},
+        partialRuntimeResolution: false,
       })
     })
 

core/test/unit/src/garden.ts

@@ -2560,6 +2560,104 @@ describe("Garden", () => {
       expect(module.spec.bla).to.eql({ nested: { key: "my value" } })
     })
 
+    it("should pass through runtime template strings when no runtimeContext is provider", async () => {
+      const test = createGardenPlugin({
+        name: "test",
+        createModuleTypes: [
+          {
+            name: "test",
+            docs: "test",
+            schema: joi.object().keys({ bla: joi.string() }),
+            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: {} }],
+          providers: [{ name: "test" }],
+          variables: {},
+        },
+      })
+
+      garden.setModuleConfigs([
+        {
+          apiVersion: DEFAULT_API_VERSION,
+          name: "module-a",
+          type: "test",
+          allowPublish: false,
+          build: { dependencies: [] },
+          disabled: false,
+          path: pathFoo,
+          serviceConfigs: [],
+          taskConfigs: [],
+          testConfigs: [],
+          spec: { bla: "${runtime.services.foo.bar || 'default'}" },
+        },
+      ])
+
+      const module = await garden.resolveModule("module-a")
+
+      expect(module.spec.bla).to.equal("${runtime.services.foo.bar || 'default'}")
+    })
+
+    it("should resolve conditional strings with missing variables", async () => {
+      const test = createGardenPlugin({
+        name: "test",
+        createModuleTypes: [
+          {
+            name: "test",
+            docs: "test",
+            schema: joi.object().keys({ bla: joi.string() }),
+            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: {} }],
+          providers: [{ name: "test" }],
+          variables: {},
+        },
+      })
+
+      garden.setModuleConfigs([
+        {
+          apiVersion: DEFAULT_API_VERSION,
+          name: "module-a",
+          type: "test",
+          allowPublish: false,
+          build: { dependencies: [] },
+          disabled: false,
+          path: pathFoo,
+          serviceConfigs: [],
+          taskConfigs: [],
+          testConfigs: [],
+          spec: { bla: "${var.foo || 'default'}" },
+        },
+      ])
+
+      const module = await garden.resolveModule("module-a")
+
+      expect(module.spec.bla).to.equal("default")
+    })
+
     it("should correctly resolve template strings with $merge keys", async () => {
       const test = createGardenPlugin({
         name: "test",