fix(config): catch module self-references instead of crashing

garden-service/src/config/config-context.ts

@@ -93,14 +93,25 @@ export abstract class ConfigContext {
 
       if (typeof value === "function") {
         // call the function to resolve the value, then continue
-        value = await value()
+        if (opts.stack.includes(stackEntry)) {
+          throw new ConfigurationError(
+            `Circular reference detected when resolving key ${stackEntry} (from ${opts.stack.join(" -> ")})`,
+            {
+              nodePath,
+              fullPath,
+              opts,
+            },
+          )
+        }
+
+        opts.stack.push(stackEntry)
+        value = await value({ key: remainder, nodePath: nestedNodePath, opts })
       }
 
       // handle nested contexts
       if (value instanceof ConfigContext) {
-        const nestedKey = remainder
         opts.stack.push(stackEntry)
-        value = await value.resolve({ key: nestedKey, nodePath: nestedNodePath, opts })
+        value = await value.resolve({ key: remainder, nodePath: nestedNodePath, opts })
         break
       }
 
@@ -273,9 +284,14 @@ export class ModuleConfigContext extends ProjectConfigContext {
     this.environment = new EnvironmentContext(_this, environment.name)
 
     this.modules = new Map(moduleConfigs.map((config) =>
-      <[string, () => Promise<ModuleContext>]>[config.name, async () => {
+      <[string, () => Promise<ModuleContext>]>[config.name, async (opts: ContextResolveOpts) => {
         // NOTE: This is a temporary hacky solution until we implement module resolution as a TaskGraph task
-        const resolvedConfig = await garden.resolveModuleConfig(config.name)
+        const stackKey = "modules." + config.name
+        const resolvedConfig = await garden.resolveModuleConfig(config.name, {
+          configContext: _this,
+          ...opts,
+          stack: [...opts.stack || [], stackKey],
+        })
         const version = await garden.resolveVersion(resolvedConfig.name, resolvedConfig.build.dependencies)
         const buildPath = await garden.buildDir.buildPath(config.name)
 

garden-service/src/garden.ts

@@ -65,7 +65,7 @@ import { BaseTask } from "./tasks/base"
 import { LocalConfigStore } from "./config-store"
 import { getLinkedSources, ExternalSourceType } from "./util/ext-source-util"
 import { BuildDependencyConfig, ModuleConfig } from "./config/module"
-import { ProjectConfigContext, ModuleConfigContext } from "./config/config-context"
+import { ProjectConfigContext, ModuleConfigContext, ContextResolveOpts } from "./config/config-context"
 import { ActionHelper } from "./actions"
 import { createPluginContext } from "./plugin-context"
 import { ModuleAndRuntimeActions, Plugins, RegisterPluginParam } from "./types/plugin/plugin"
@@ -104,6 +104,10 @@ export interface GardenOpts {
   plugins?: Plugins,
 }
 
+interface ModuleConfigResolveOpts extends ContextResolveOpts {
+  configContext?: ModuleConfigContext
+}
+
 const scanLock = new AsyncLock()
 
 export class Garden {
@@ -501,15 +505,15 @@ export class Garden {
    * plugin handlers).
    * Scans for modules in the project root and remote/linked sources if it hasn't already been done.
    */
-  async resolveModuleConfigs(keys?: string[], configContext?: ModuleConfigContext): Promise<ModuleConfig[]> {
+  async resolveModuleConfigs(keys?: string[], opts: ModuleConfigResolveOpts = {}): Promise<ModuleConfig[]> {
     const configs = await this.getRawModuleConfigs(keys)
 
-    if (!configContext) {
-      configContext = new ModuleConfigContext(this, this.environment, Object.values(this.moduleConfigs))
+    if (!opts.configContext) {
+      opts.configContext = new ModuleConfigContext(this, this.environment, Object.values(this.moduleConfigs))
     }
 
     return Bluebird.map(configs, async (config) => {
-      config = await resolveTemplateStrings(cloneDeep(config), configContext!)
+      config = await resolveTemplateStrings(cloneDeep(config), opts.configContext!, opts)
 
       const configureHandler = await this.actions.getModuleActionHandler({
         actionType: "configure",
@@ -529,8 +533,8 @@ export class Garden {
   /**
    * Returns the module with the specified name. Throws error if it doesn't exist.
    */
-  async resolveModuleConfig(name: string, configContext?: ModuleConfigContext): Promise<ModuleConfig> {
-    return (await this.resolveModuleConfigs([name], configContext))[0]
+  async resolveModuleConfig(name: string, opts: ModuleConfigResolveOpts = {}): Promise<ModuleConfig> {
+    return (await this.resolveModuleConfigs([name], opts))[0]
   }
 
   /**

garden-service/src/template-string.ts

@@ -64,10 +64,12 @@ export async function resolveTemplateString(string: string, context: ConfigConte
 /**
  * Recursively parses and resolves all templated strings in the given object.
  */
-export async function resolveTemplateStrings<T extends object>(obj: T, context: ConfigContext): Promise<T> {
+export async function resolveTemplateStrings<T extends object>(
+  obj: T, context: ConfigContext, opts: ContextResolveOpts = {},
+): Promise<T> {
   return asyncDeepMap(
     obj,
-    (v) => typeof v === "string" ? resolveTemplateString(v, context) : v,
+    (v) => typeof v === "string" ? resolveTemplateString(v, context, opts) : v,
     // need to iterate sequentially to catch potential circular dependencies
     { concurrency: 1 },
   )

garden-service/test/unit/data/test-projects/module-self-ref/garden.yml

@@ -0,0 +1,6 @@
+project:
+  name: duplicate-module
+  environments:
+    - name: local
+      providers:
+        - name: test-plugin

garden-service/test/unit/data/test-projects/module-self-ref/module-a/garden.yml

@@ -0,0 +1,5 @@
+kind: Module
+name: module-a
+type: test
+build:
+  args: ["${modules.module-a.version}"]

garden-service/test/unit/src/garden.ts

@@ -203,6 +203,19 @@ describe("Garden", () => {
     })
   })
 
+  describe("resolveModuleConfigs", () => {
+    it("should throw if a module references itself in a template string", async () => {
+      const projectRoot = resolve(dataDir, "test-projects", "module-self-ref")
+      const garden = await makeTestGarden(projectRoot)
+      await expectError(
+        () => garden.resolveModuleConfigs(),
+        (err) => expect(err.message).to.equal(
+          "Circular reference detected when resolving key modules.module-a (from modules.module-a)",
+        ),
+      )
+    })
+  })
+
   describe("resolveVersion", () => {
     beforeEach(() => td.reset())
 

garden-service/test/unit/src/vcs/base.ts

@@ -11,6 +11,7 @@ import { expect } from "chai"
 import { cloneDeep } from "lodash"
 import { Garden } from "../../../../src/garden"
 import { ModuleConfigContext } from "../../../../src/config/config-context"
+import { ModuleConfig } from "../../../../src/config/module"
 
 class TestVcsHandler extends VcsHandler {
   name = "test"
@@ -85,10 +86,10 @@ describe("VcsHandler", () => {
   })
 
   describe("getVersionString", () => {
-    let moduleABefore
-    let moduleAAfter
-    let moduleBBefore
-    let moduleBAfter
+    let moduleABefore: ModuleConfig
+    let moduleAAfter: ModuleConfig
+    let moduleBBefore: ModuleConfig
+    let moduleBAfter: ModuleConfig
 
     before(async () => {
       const templateGarden = await makeTestGarden(getDataDir("test-project-variable-versioning"))
@@ -98,11 +99,11 @@ describe("VcsHandler", () => {
 
       const moduleAAfterEnv = cloneDeep(templateGarden.environment)
       moduleAAfterEnv.variables["echo-string"] = "something else"
-      const changedModuleConfigContext = new ModuleConfigContext(
+      const configContext = new ModuleConfigContext(
         templateGarden, moduleAAfterEnv, await templateGarden.getRawModuleConfigs())
 
-      moduleAAfter = await templateGarden.resolveModuleConfig("module-a", changedModuleConfigContext)
-      moduleBAfter = await templateGarden.resolveModuleConfig("module-b", changedModuleConfigContext)
+      moduleAAfter = await templateGarden.resolveModuleConfig("module-a", { configContext })
+      moduleBAfter = await templateGarden.resolveModuleConfig("module-b", { configContext })
     })
 
     it("should return a different version for a module when a variable used by it changes", async () => {