improve(config): allow most template strings in templated module inputs

Basically we now defer the full resolution on input template strings, with the caveat that input values needed for module identifiers need to be resolvable immediately. This involved quite a bit of added complexity, which is prompting me to seriously a refactor in our template string resolution logic. This does the job for now, but I think we'll soon want to implement at least partial lazy+recursive resolution of template strings, instead of all this fine-grained logic to control what's resolved when.
garden-io · Jan 20, 2021 · d61e244 · d61e244
1 parent 882e15b
commit d61e244
Show file tree

Hide file tree

Showing 14 changed files with 254 additions and 76 deletions.
diff --git a/core/src/config/config-context.ts b/core/src/config/config-context.ts
@@ -225,6 +225,20 @@ export abstract class ConfigContext {
   }
 }
 
+/**
+ * A generic context that just wraps an object.
+ */
+export class GenericContext extends ConfigContext {
+  constructor(obj: any) {
+    super()
+    Object.assign(this, obj)
+  }
+
+  static getSchema() {
+    return joi.object()
+  }
+}
+
 export class ScanContext extends ConfigContext {
   foundKeys: KeyedSet<ContextKeySegment[]>
 

diff --git a/core/src/config/module-template.ts b/core/src/config/module-template.ts
@@ -11,7 +11,7 @@ import { baseModuleSpecSchema, BaseModuleSpec, ModuleConfig } from "./module"
 import { dedent, deline } from "../util/string"
 import { GardenResource, prepareModuleResource } from "./base"
 import { DOCS_BASE_URL } from "../constants"
-import { ProjectConfigContext, ModuleTemplateConfigContext } from "./config-context"
+import { ProjectConfigContext, ModuleTemplateConfigContext, EnvironmentConfigContext } from "./config-context"
 import { resolveTemplateStrings } from "../template-string"
 import { validateWithPath } from "./validation"
 import { Garden } from "../garden"
@@ -108,8 +108,22 @@ export async function resolveTemplatedModule(
   config: TemplatedModuleConfig,
   templates: { [name: string]: ModuleTemplateConfig }
 ) {
-  // Resolve template strings for fields
-  const resolved = resolveTemplateStrings(config, new ProjectConfigContext({ ...garden, branch: garden.vcsBranch }))
+  // Resolve template strings for fields. Note that inputs are partially resolved, and will be fully resolved later
+  // when resolving the resolving the resulting modules. Inputs that are used in module names must however be resolvable
+  // immediately.
+  const templateContext = new EnvironmentConfigContext({ ...garden, branch: garden.vcsBranch })
+  const resolvedWithoutInputs = resolveTemplateStrings(
+    { ...config, spec: omit(config.spec, "inputs") },
+    templateContext
+  )
+  const partiallyResolvedInputs = resolveTemplateStrings(config.spec.inputs || {}, templateContext, {
+    allowPartial: true,
+  })
+  const resolved = {
+    ...resolvedWithoutInputs,
+    spec: { ...resolvedWithoutInputs.spec, inputs: partiallyResolvedInputs },
+  }
+
   const configType = "templated module " + resolved.name
 
   let resolvedSpec = omit(resolved.spec, "build")
@@ -119,9 +133,9 @@ export async function resolveTemplatedModule(
     return { resolvedSpec, modules: [] }
   }
 
-  // Validate
+  // Validate the module spec
   resolvedSpec = validateWithPath({
-    config: omit(resolved.spec, "build"),
+    config: resolvedSpec,
     configType,
     path: resolved.configPath || resolved.path,
     schema: templatedModuleSpecSchema(),
@@ -141,37 +155,33 @@ export async function resolveTemplatedModule(
     )
   }
 
-  // Validate template inputs
-  resolvedSpec = validateWithPath({
-    config: resolvedSpec,
-    configType,
-    path: resolved.configPath || resolved.path,
-    schema: templatedModuleSpecSchema().keys({ inputs: template.inputsSchema }),
-    projectRoot: garden.projectRoot,
-  })
-
-  const inputs = resolvedSpec.inputs || {}
-
   // Prepare modules and resolve templated names
   const context = new ModuleTemplateConfigContext({
     ...garden,
     branch: garden.vcsBranch,
     parentName: resolved.name,
     templateName: template.name,
-    inputs,
+    inputs: partiallyResolvedInputs,
   })
 
   const modules = await Bluebird.map(template.modules || [], async (m) => {
-    // Run a partial template resolution with the parent+template info and inputs
+    // Run a partial template resolution with the parent+template info
     const spec = resolveTemplateStrings(m, context, { allowPartial: true })
 
     let moduleConfig: ModuleConfig
 
     try {
       moduleConfig = prepareModuleResource(spec, resolved.configPath || resolved.path, garden.projectRoot)
     } catch (error) {
+      let msg = error.message
+
+      if (spec.name && spec.name.includes && spec.name.includes("${")) {
+        msg +=
+          ". Note that if a template string is used in the name of a module in a template, then the template string must be fully resolvable at the time of module scanning. This means that e.g. references to other modules or runtime outputs cannot be used."
+      }
+
       throw new ConfigurationError(
-        `${templateKind} ${template.name} returned an invalid module (named ${spec.name}) for templated module ${resolved.name}: ${error.message}`,
+        `${templateKind} ${template.name} returned an invalid module (named ${spec.name}) for templated module ${resolved.name}: ${msg}`,
         {
           moduleSpec: spec,
           parent: resolvedSpec,
@@ -195,7 +205,7 @@ export async function resolveTemplatedModule(
     // Attach metadata
     moduleConfig.parentName = resolved.name
     moduleConfig.templateName = template.name
-    moduleConfig.inputs = inputs
+    moduleConfig.inputs = partiallyResolvedInputs
 
     return moduleConfig
   })

diff --git a/core/src/garden.ts b/core/src/garden.ts
@@ -86,6 +86,7 @@ import {
   resolveModuleTemplate,
   resolveTemplatedModule,
   templateKind,
+  ModuleTemplateConfig,
 } from "./config/module-template"
 import { TemplatedModuleConfig } from "./plugins/templated"
 import { BuildDirRsync } from "./build-staging/rsync"
@@ -184,6 +185,7 @@ export class Garden {
   private actionHelper: ActionRouter
   public readonly events: EventBus
   private tools: { [key: string]: PluginTool }
+  public moduleTemplates: { [name: string]: ModuleTemplateConfig }
 
   public readonly production: boolean
   public readonly projectRoot: string
@@ -1056,6 +1058,7 @@ export class Garden {
       this.log.silly(`Scanned and found ${rawModuleConfigs.length} modules and ${rawWorkflowConfigs.length} workflows`)
 
       this.configsScanned = true
+      this.moduleTemplates = keyBy(moduleTemplates, "name")
     })
   }
 

diff --git a/core/src/plugins/templated.ts b/core/src/plugins/templated.ts
@@ -12,6 +12,7 @@ import { templateKind } from "../config/module-template"
 import { joiIdentifier, joi, DeepPrimitiveMap } from "../config/common"
 import { dedent, naturalList } from "../util/string"
 import { omit } from "lodash"
+import { DOCS_BASE_URL } from "../constants"
 
 export interface TemplatedModuleSpec extends ModuleSpec {
   template: string
@@ -31,6 +32,8 @@ export const templatedModuleSpecSchema = () =>
     inputs: joi.object().description(
       dedent`
       A map of inputs to pass to the ${templateKind}. These must match the inputs schema of the ${templateKind}.
+
+      Note: You can use template strings for the inputs, but be aware that inputs that are used to generate the resulting module names and other top-level identifiers must be resolvable when scanning for modules, and thus cannot reference other modules or runtime variables. See the [environment configuration context reference](${DOCS_BASE_URL}/reference/template-strings#environment-configuration-context) to see template strings that are safe to use for inputs used to generate module identifiers.
       `
     ),
   })

diff --git a/core/src/resolve-module.ts b/core/src/resolve-module.ts
@@ -9,7 +9,7 @@
 import { cloneDeep, keyBy } from "lodash"
 import { validateWithPath } from "./config/validation"
 import { resolveTemplateStrings, getModuleTemplateReferences, resolveTemplateString } from "./template-string"
-import { ContextResolveOpts, ModuleConfigContext } from "./config/config-context"
+import { ContextResolveOpts, ModuleConfigContext, GenericContext } from "./config/config-context"
 import { relative, resolve, posix } from "path"
 import { Garden } from "./garden"
 import { ConfigurationError, FilesystemError, PluginError } from "./exceptions"
@@ -80,7 +80,7 @@ export class ModuleResolver {
       }
     }
     for (const rawConfig of this.rawConfigs) {
-      const deps = this.getModuleTemplateDependencies(rawConfig)
+      const deps = this.getModuleDependenciesFromTemplateStrings(rawConfig)
       for (const graph of [fullGraph, processingGraph]) {
         for (const dep of deps) {
           graph.addNode(dep.name)
@@ -178,7 +178,10 @@ export class ModuleResolver {
     return Object.values(resolvedModules)
   }
 
-  private getModuleTemplateDependencies(rawConfig: ModuleConfig) {
+  /**
+   * Returns module configs for each module that is referenced in a ${modules.*} template string in the raw config.
+   */
+  private getModuleDependenciesFromTemplateStrings(rawConfig: ModuleConfig) {
     const configContext = new ModuleConfigContext({
       garden: this.garden,
       resolvedProviders: this.resolvedProviders,
@@ -221,23 +224,63 @@ export class ModuleResolver {
    */
   async resolveModuleConfig(config: ModuleConfig, dependencies: GardenModule[]): Promise<ModuleConfig> {
     const garden = this.garden
-    const configContext = new ModuleConfigContext({
-      garden: this.garden,
+    let inputs = {}
+
+    const templateContextParams = {
+      garden,
       resolvedProviders: this.resolvedProviders,
-      moduleName: config.name,
       dependencies,
       runtimeContext: this.runtimeContext,
       parentName: config.parentName,
       templateName: config.templateName,
-      inputs: config.inputs,
+      inputs,
       partialRuntimeResolution: true,
+    }
+
+    // First resolve and validate the inputs field, because template module inputs may not be fully resolved at this
+    // time.
+    // TODO: This whole complicated procedure could be much improved and simplified by implementing lazy resolution on
+    // values... I'll be looking into that. - JE
+    const templateName = config.templateName
+
+    if (templateName) {
+      const template = this.garden.moduleTemplates[templateName]
+
+      inputs = resolveTemplateStrings(
+        inputs,
+        new ModuleConfigContext(templateContextParams),
+        // Not all inputs may need to be resolvable
+        { allowPartial: true }
+      )
+
+      inputs = validateWithPath({
+        config: cloneDeep(config.inputs || {}),
+        configType: `inputs for module ${config.name}`,
+        path: config.configPath || config.path,
+        schema: template.inputsSchema,
+        projectRoot: garden.projectRoot,
+      })
+    }
+
+    // Now resolve just references to inputs on the config
+    config = resolveTemplateStrings(cloneDeep(config), new GenericContext({ inputs }), {
+      allowPartial: true,
     })
 
-    config = resolveTemplateStrings(cloneDeep(config), configContext, {
+    // And finally fully resolve the config
+    const configContext = new ModuleConfigContext({
+      ...templateContextParams,
+      inputs,
+      moduleName: config.name,
+    })
+
+    config = resolveTemplateStrings({ ...config, inputs: {} }, configContext, {
       allowPartial: false,
     })
 
-    const moduleTypeDefinitions = await this.garden.getModuleTypes()
+    config.inputs = inputs
+
+    const moduleTypeDefinitions = await garden.getModuleTypes()
     const description = moduleTypeDefinitions[config.type]
 
     if (!description) {

diff --git a/core/test/data/test-projects/module-templates/module-templates.json b/core/test/data/test-projects/module-templates/module-templates.json
@@ -1,6 +1,11 @@
 {
   "type": "object",
   "properties": {
-    "foo": { "type": "string" }
+    "name": {
+      "type": "string"
+    },
+    "value": {
+      "type": "string"
+    }
   }
 }
diff --git a/core/test/data/test-projects/module-templates/modules.garden.yml b/core/test/data/test-projects/module-templates/modules.garden.yml
@@ -3,4 +3,5 @@ type: templated
 template: combo
 name: foo
 inputs:
-  foo: bar
+  name: test
+  value: ${providers.test-plugin.outputs.testKey}
diff --git a/core/test/data/test-projects/module-templates/source.txt b/core/test/data/test-projects/module-templates/source.txt
@@ -1,3 +1,3 @@
 Hello I am file!
-input: ${inputs.foo}
-module reference: ${modules["${parent.name}-${inputs.foo}-test-a"].path}
+input: ${inputs.value}
+module reference: ${modules["${parent.name}-${inputs.name}-a"].path}
diff --git a/core/test/data/test-projects/module-templates/templates.garden.yml b/core/test/data/test-projects/module-templates/templates.garden.yml
@@ -3,27 +3,28 @@ name: combo
 inputsSchemaPath: "module-templates.json"
 modules:
   - type: test
-    name: ${parent.name}-${inputs.foo}-test-a
+    name: ${parent.name}-${inputs.name}-a
     include: []
+    extraFlags: ["${inputs.value}"]
     generateFiles:
       - targetPath: module-a.log
         value: "hellow"
   - type: test
-    name: ${parent.name}-${inputs.foo}-test-b
+    name: ${parent.name}-${inputs.name}-b
     build:
-      dependencies: ["${parent.name}-${inputs.foo}-test-a"]
+      dependencies: ["${parent.name}-${inputs.name}-a"]
     include: []
     generateFiles:
       - targetPath: module-b.log
         sourcePath: source.txt
   - type: test
-    name: ${parent.name}-${inputs.foo}-test-c
+    name: ${parent.name}-${inputs.name}-c
     build:
-      dependencies: ["${parent.name}-${inputs.foo}-test-a"]
+      dependencies: ["${parent.name}-${inputs.name}-a"]
     include: []
     generateFiles:
       - targetPath: .garden/subdir/module-c.log
         value: |
           Hello I am string!
-          input: ${inputs.foo}
-          module reference: ${modules["${parent.name}-${inputs.foo}-test-a"].path}
+          input: ${inputs.value}
+          module reference: ${modules["${parent.name}-${inputs.name}-a"].path}
diff --git a/core/test/helpers.ts b/core/test/helpers.ts
@@ -159,8 +159,12 @@ export const testPlugin = createGardenPlugin({
       return { url: `http://localhost:12345/${page.name}` }
     },
 
+    async getEnvironmentStatus() {
+      return { ready: true, outputs: { testKey: "testValue" } }
+    },
+
     async prepareEnvironment() {
-      return { status: { ready: true, outputs: {} } }
+      return { status: { ready: true, outputs: { testKey: "testValue" } } }
     },
 
     async setSecret({ key, value }) {

diff --git a/core/test/unit/src/config/base.ts b/core/test/unit/src/config/base.ts
@@ -182,8 +182,9 @@ describe("loadConfigResources", () => {
         modules: [
           {
             type: "test",
-            name: "${parent.name}-${inputs.foo}-test-a",
+            name: "${parent.name}-${inputs.name}-a",
             include: [],
+            extraFlags: ["${inputs.value}"],
             generateFiles: [
               {
                 targetPath: "module-a.log",
@@ -193,10 +194,10 @@ describe("loadConfigResources", () => {
           },
           {
             type: "test",
-            name: "${parent.name}-${inputs.foo}-test-b",
+            name: "${parent.name}-${inputs.name}-b",
             include: [],
             build: {
-              dependencies: ["${parent.name}-${inputs.foo}-test-a"],
+              dependencies: ["${parent.name}-${inputs.name}-a"],
             },
             generateFiles: [
               {
@@ -207,16 +208,16 @@ describe("loadConfigResources", () => {
           },
           {
             type: "test",
-            name: "${parent.name}-${inputs.foo}-test-c",
+            name: "${parent.name}-${inputs.name}-c",
             include: [],
             build: {
-              dependencies: ["${parent.name}-${inputs.foo}-test-a"],
+              dependencies: ["${parent.name}-${inputs.name}-a"],
             },
             generateFiles: [
               {
                 targetPath: ".garden/subdir/module-c.log",
                 value:
-                  'Hello I am string!\ninput: ${inputs.foo}\nmodule reference: ${modules["${parent.name}-${inputs.foo}-test-a"].path}\n',
+                  'Hello I am string!\ninput: ${inputs.value}\nmodule reference: ${modules["${parent.name}-${inputs.name}-a"].path}\n',
               },
             ],
           },