feat(core): module varfiles

Added support for module varfiles. These are located at or under the
module root, and take precedence over module variables (thus having the
second-highest precedence behind CLI vars).

The path to the module varfile is set via the optional `module.varfile`
field.

Also fixed an issue where CLI variables were overridden by module
variables. CLI variables now correctly have the highest precedence.

core/src/config/base.ts

@@ -187,6 +187,7 @@ export function prepareModuleResource(spec: any, configPath: string, projectRoot
     type: spec.type,
     taskConfigs: [],
     variables: spec.variables,
+    varfile: spec.varfile,
   }
 
   validateWithPath({

core/src/config/module.ts

@@ -23,6 +23,7 @@ import { TestConfig, testConfigSchema } from "./test"
 import { TaskConfig, taskConfigSchema } from "./task"
 import { dedent, stableStringify } from "../util/string"
 import { templateKind } from "./module-template"
+import { varfileDescription } from "./project"
 
 export const defaultBuildTimeout = 1200
 
@@ -91,6 +92,7 @@ interface ModuleSpecCommon {
   repositoryUrl?: string
   type: string
   variables?: DeepPrimitiveMap
+  varfile?: string
 }
 
 export interface AddModuleSpec extends ModuleSpecCommon {
@@ -230,6 +232,21 @@ export const baseModuleSpecKeys = () => ({
   variables: joiVariables().default(() => undefined).description(dedent`
     A map of variables scoped to this particular module. These are resolved before any other parts of the module configuration and take precedence over project-scoped variables. They may reference project-scoped variables, and generally use any template strings normally allowed when resolving modules.
   `),
+  varfile: joi
+    .posixPath()
+    .description(
+      dedent`
+      Specify a path (relative to the module root) to a file containing variables, that we apply on top of the
+      module-level \`variables\` field.
+
+      ${varfileDescription}
+
+      To use different module-level varfiles in different environments, you can template in the environment name
+      to the varfile name, e.g. \`varfile: "my-module.\$\{environment.name\}.env\` (this assumes that the corresponding
+      varfiles exist).
+    `
+    )
+    .example("my-module.env"),
 })
 
 export const baseModuleSpecSchema = () => coreModuleSpecSchema().keys(baseModuleSpecKeys())

core/src/config/project.ts

@@ -63,7 +63,7 @@ export interface EnvironmentConfig {
   production?: boolean
 }
 
-const varfileDescription = `
+export const varfileDescription = `
 The format of the files is determined by the configured file's extension:
 
 * \`.env\` - Standard "dotenv" format, as defined by [dotenv](https://github.com/motdotla/dotenv#rules).
@@ -492,8 +492,12 @@ export function resolveProjectConfig({
  * For project variables, we apply the variables specified to the selected environment on the global variables
  * specified on the top-level `variables` key using a JSON Merge Patch (https://tools.ietf.org/html/rfc7396).
  * We also attempt to load the configured varfiles, and include those in the merge. The precedence order is as follows:
+ *
  *   environment.varfile > environment.variables > project.varfile > project.variables
  *
+ * Variables passed through the `--var` CLI option have the highest precedence, and are merged in later in the flow
+ * (see `resolveGardenParams`).
+ *
  * For provider configuration, we filter down to the providers that are enabled for all environments (no `environments`
  * key specified) and those that explicitly list the specified environments. Then we merge any provider configs with
  * the same provider name, again using JSON Merge Patching, with later entries in the list taking precedence over
@@ -543,7 +547,11 @@ export async function pickEnvironment({
     })
   }
 
-  const projectVarfileVars = await loadVarfile(projectConfig.path, projectConfig.varfile, defaultVarfilePath)
+  const projectVarfileVars = await loadVarfile({
+    configRoot: projectConfig.path,
+    path: projectConfig.varfile,
+    defaultPath: defaultVarfilePath,
+  })
   const projectVariables: DeepPrimitiveMap = <any>merge(projectConfig.variables, projectVarfileVars)
 
   const envProviders = environmentConfig.providers || []
@@ -593,11 +601,11 @@ export async function pickEnvironment({
     }
   }
 
-  const envVarfileVars = await loadVarfile(
-    projectConfig.path,
-    environmentConfig.varfile,
-    defaultEnvVarfilePath(environment)
-  )
+  const envVarfileVars = await loadVarfile({
+    configRoot: projectConfig.path,
+    path: environmentConfig.varfile,
+    defaultPath: defaultEnvVarfilePath(environment),
+  })
 
   const variables: DeepPrimitiveMap = <any>merge(projectVariables, merge(environmentConfig.variables, envVarfileVars))
 
@@ -653,8 +661,20 @@ export function parseEnvironment(env: string): ParsedEnvironment {
   }
 }
 
-async function loadVarfile(projectRoot: string, path: string | undefined, defaultPath: string): Promise<PrimitiveMap> {
-  const resolvedPath = resolve(projectRoot, path || defaultPath)
+export async function loadVarfile({
+  configRoot,
+  path,
+  defaultPath,
+}: {
+  // project root (when resolving project config) or module root (when resolving module config)
+  configRoot: string
+  path: string | undefined
+  defaultPath: string | undefined
+}): Promise<PrimitiveMap> {
+  if (!path && !defaultPath) {
+    throw new ParameterError(`Neither a path nor a defaultPath was provided.`, { configRoot, path, defaultPath })
+  }
+  const resolvedPath = resolve(configRoot, <string>(path || defaultPath))
   const exists = await pathExists(resolvedPath)
 
   if (!exists && path && path !== defaultPath) {
@@ -670,7 +690,7 @@ async function loadVarfile(projectRoot: string, path: string | undefined, defaul
 
   try {
     const data = await readFile(resolvedPath)
-    const relPath = relative(projectRoot, resolvedPath)
+    const relPath = relative(configRoot, resolvedPath)
     const filename = basename(resolvedPath.toLowerCase())
 
     if (filename.endsWith(".json")) {

core/src/garden.ts

@@ -174,6 +174,7 @@ export interface GardenParams {
   projectSources?: SourceConfig[]
   providerConfigs: GenericProviderConfig[]
   variables: DeepPrimitiveMap
+  cliVariables: DeepPrimitiveMap
   secrets: StringMap
   sessionId: string
   username: string | undefined
@@ -215,6 +216,9 @@ export class Garden {
   public readonly environmentConfigs: EnvironmentConfig[]
   public readonly namespace: string
   public readonly variables: DeepPrimitiveMap
+  // Any variables passed via the `--var` CLI option (maintained here so that they can be used during module resolution
+  // to override module variables and module varfiles).
+  public readonly cliVariables: DeepPrimitiveMap
   public readonly secrets: StringMap
   private readonly projectSources: SourceConfig[]
   public readonly buildStaging: BuildStaging
@@ -257,6 +261,7 @@ export class Garden {
     this.projectSources = params.projectSources || []
     this.providerConfigs = params.providerConfigs
     this.variables = params.variables
+    this.cliVariables = params.cliVariables
     this.secrets = params.secrets
     this.workingCopyId = params.workingCopyId
     this.dotIgnoreFiles = params.dotIgnoreFiles
@@ -1321,7 +1326,8 @@ export async function resolveGardenParams(currentDirectory: string, opts: Garden
   })
 
   // Allow overriding variables
-  variables = { ...variables, ...(opts.variables || {}) }
+  const cliVariables = opts.variables || {}
+  variables = { ...variables, ...cliVariables }
 
   // Use the legacy build sync mode if
   // A) GARDEN_LEGACY_BUILD_STAGE=true is set or
@@ -1356,6 +1362,7 @@ export async function resolveGardenParams(currentDirectory: string, opts: Garden
     environmentConfigs: config.environments,
     namespace,
     variables,
+    cliVariables,
     secrets,
     projectSources,
     buildStaging: buildDir,

core/src/resolve-module.ts

@@ -23,7 +23,7 @@ import { getModuleTypeBases } from "./plugins"
 import { ModuleConfig, moduleConfigSchema } from "./config/module"
 import { Profile } from "./util/profiling"
 import { getLinkedSources } from "./util/ext-source-util"
-import { allowUnknown } from "./config/common"
+import { allowUnknown, DeepPrimitiveMap } from "./config/common"
 import { ProviderMap } from "./config/provider"
 import { RuntimeContext } from "./runtime-context"
 import chalk from "chalk"
@@ -33,6 +33,8 @@ import { readFile, mkdirp, writeFile } from "fs-extra"
 import { LogEntry } from "./logger/log-entry"
 import { ModuleConfigContext, ModuleConfigContextParams } from "./config/template-contexts/module"
 import { pathToCacheContext } from "./cache"
+import { loadVarfile } from "./config/project"
+import { merge } from "json-merge-patch"
 
 // This limit is fairly arbitrary, but we need to have some cap on concurrent processing.
 export const moduleResolutionConcurrencyLimit = 40
@@ -279,13 +281,8 @@ export class ModuleResolver {
       config.inputs = inputs
     }
 
-    // Resolve the variables field before resolving everything else
-    const rawVariables = config.variables
-    const resolvedVariables = resolveTemplateStrings(
-      cloneDeep(rawVariables || {}),
-      new ModuleConfigContext(templateContextParams),
-      { allowPartial: false }
-    )
+    // Resolve the variables field before resolving everything else (overriding with module varfiles if present)
+    const resolvedModuleVariables = await this.resolveVariables(config, templateContextParams)
 
     // Now resolve just references to inputs on the config
     config = resolveTemplateStrings(cloneDeep(config), new GenericContext({ inputs }), {
@@ -296,14 +293,14 @@ export class ModuleResolver {
     const configContext = new ModuleConfigContext({
       ...templateContextParams,
       moduleConfig: config,
-      variables: { ...garden.variables, ...resolvedVariables },
+      variables: { ...garden.variables, ...resolvedModuleVariables },
     })
 
     config = resolveTemplateStrings({ ...config, inputs: {}, variables: {} }, configContext, {
       allowPartial: false,
     })
 
-    config.variables = rawVariables ? resolvedVariables : undefined
+    config.variables = resolvedModuleVariables
     config.inputs = inputs
 
     const moduleTypeDefinitions = await garden.getModuleTypes()
@@ -509,9 +506,35 @@ export class ModuleResolver {
         })
       }
     }
-
     return module
   }
+
+  /**
+   * Resolves module variables with the following precedence order:
+   *
+   *   garden.cliVariables > module varfile > config.variables
+   */
+  private async resolveVariables(
+    config: ModuleConfig,
+    templateContextParams: ModuleConfigContextParams
+  ): Promise<DeepPrimitiveMap> {
+    const moduleConfigContext = new ModuleConfigContext(templateContextParams)
+    const resolveOpts = { allowPartial: false }
+    let varfileVars: DeepPrimitiveMap = {}
+    if (config.varfile) {
+      const varfilePath = resolveTemplateString(config.varfile, moduleConfigContext, resolveOpts)
+      varfileVars = await loadVarfile({
+        configRoot: config.path,
+        path: varfilePath,
+        defaultPath: undefined,
+      })
+    }
+
+    const rawVariables = config.variables
+    const moduleVariables = resolveTemplateStrings(cloneDeep(rawVariables || {}), moduleConfigContext, resolveOpts)
+    const mergedVariables: DeepPrimitiveMap = <any>merge(moduleVariables, merge(varfileVars, this.garden.cliVariables))
+    return mergedVariables
+  }
 }
 
 export interface ModuleConfigResolveOpts extends ContextResolveOpts {

core/test/data/test-projects/module-varfiles/garden.module-a.default.env

@@ -0,0 +1,2 @@
+c=from-module-varfile
+d=from-module-varfile

core/test/data/test-projects/module-varfiles/garden.project.env

@@ -0,0 +1,4 @@
+a=from-project-varfile
+b=from-project-varfile
+c=from-project-varfile
+d=from-project-varfile

core/test/data/test-projects/module-varfiles/garden.yml

@@ -0,0 +1,20 @@
+kind: Project
+name: module-varfiles
+varfile: garden.project.env
+environments:
+  - name: default
+providers:
+  - name: test-plugin
+
+---
+
+kind: Module
+name: module-a
+type: test
+varfile: "garden.module-a.${environment.name}.env"
+variables:
+  b: from-module-vars
+  c: from-module-vars # should be overridden by module-level varfile
+  d: from-module-vars # should be overridden by var passed as a CLI option
+build:
+  command: [echo, A]

core/test/integ/src/plugins/kubernetes/helm/config.ts

@@ -119,7 +119,8 @@ describe("configureHelmModule", () => {
       testConfigs: [],
       type: "helm",
       taskConfigs: [],
-      variables: undefined,
+      variables: {},
+      varfile: undefined,
     })
   })
 

core/test/unit/src/config/base.ts

@@ -133,6 +133,7 @@ describe("loadConfigResources", () => {
         build: { dependencies: [] },
         path: modulePathA,
         variables: { msg: "OK" },
+        varfile: undefined,
 
         spec: {
           build: {
@@ -276,6 +277,7 @@ describe("loadConfigResources", () => {
         testConfigs: [],
         taskConfigs: [],
         variables: undefined,
+        varfile: undefined,
       },
     ])
   })
@@ -315,6 +317,7 @@ describe("loadConfigResources", () => {
         testConfigs: [],
         taskConfigs: [],
         variables: undefined,
+        varfile: undefined,
       },
       {
         apiVersion: "garden.io/v0",
@@ -344,6 +347,7 @@ describe("loadConfigResources", () => {
         testConfigs: [],
         taskConfigs: [],
         variables: undefined,
+        varfile: undefined,
       },
     ])
   })

core/test/unit/src/garden.ts

@@ -271,6 +271,22 @@ describe("Garden", () => {
       })
     })
 
+    it("should respect the module variables < module varfile < CLI var precedence order", async () => {
+      const projectRoot = join(dataDir, "test-projects", "module-varfiles")
+
+      const garden = await makeTestGarden(projectRoot)
+      // In the normal flow, `garden.cliVariables` is populated with variables passed via the `--var` CLI option.
+      garden.cliVariables["d"] = "from-cli-var"
+      const graph = await garden.getConfigGraph({ log: garden.log, emit: false })
+      const module = graph.getModule("module-a")
+      expect({ ...garden.variables, ...module.variables }).to.eql({
+        a: "from-project-varfile",
+        b: "from-module-vars",
+        c: "from-module-varfile",
+        d: "from-cli-var",
+      })
+    })
+
     it("should throw if project root is not in a git repo root", async () => {
       const dir = await tmp.dir({ unsafeCleanup: true })
 
@@ -4462,9 +4478,9 @@ describe("Garden", () => {
     })
 
     context("test against fixed version hashes", async () => {
-      const moduleAVersionString = "v-f6743d2423"
-      const moduleBVersionString = "v-97a77e8414"
-      const moduleCVersionString = "v-afb847fa9a"
+      const moduleAVersionString = "v-3b072717eb"
+      const moduleBVersionString = "v-b9e3153900"
+      const moduleCVersionString = "v-371a6bbdec"
 
       it("should return the same module versions between runtimes", async () => {
         const projectRoot = getDataDir("test-projects", "fixed-version-hashes-1")

core/test/unit/src/vcs/vcs.ts

@@ -423,7 +423,7 @@ describe("getModuleVersionString", () => {
     const garden = await makeTestGarden(projectRoot)
     const module = await garden.resolveModule("module-a")
 
-    const fixedVersionString = "v-f6743d2423"
+    const fixedVersionString = "v-3b072717eb"
     expect(module.version.versionString).to.eql(fixedVersionString)
 
     delete process.env.TEST_ENV_VAR