fix(dev): fix reload error when using templates

Before this fix, changing a Garden config during a `dev` command session
and running two subsequent commands would result in the first command
succeeding but the second failing with an error like:

"Build foo references template <some-template> which cannot be found.
Vailable templates: <None>"

This was because the config templates scanned by the first subcommand
after the config was changed weren't propagated to the parent Garden
instance, which was then cloned and passed to the second command (which
then had action configs, but no templates).

This was fixed by making the relevant instance variables on the Garden
class readonly (since reassignments in a child instance prevent newly
scanned configs from propagating to the parent via the shared data
structure) and emptying the maps before rescanning instead.

Also added a unit test case that checks this whole flow.

core/src/commands/base.ts

@@ -287,7 +287,6 @@ export abstract class Command<
     return withSessionContext({ sessionId, parentSessionId }, () =>
       wrapActiveSpan(this.getFullName(), async () => {
         const commandStartTime = new Date()
-        const server = this.server
 
         let garden = parentGarden
 

core/src/garden.ts

@@ -39,6 +39,7 @@ import {
   duplicatesByKey,
   getCloudDistributionName,
   getCloudLogSectionName,
+  clearObject,
 } from "./util/util"
 import {
   ConfigurationError,
@@ -229,9 +230,9 @@ interface GardenInstanceState {
 export class Garden {
   public log: Log
   private loadedPlugins?: GardenPluginSpec[]
-  protected actionConfigs: ActionConfigMap
-  protected moduleConfigs: ModuleConfigMap
-  protected workflowConfigs: WorkflowConfigMap
+  protected readonly actionConfigs: ActionConfigMap
+  protected readonly moduleConfigs: ModuleConfigMap
+  protected readonly workflowConfigs: WorkflowConfigMap
   protected configPaths: Set<string>
   private resolvedProviders: { [key: string]: Provider }
   protected readonly state: GardenInstanceState
@@ -247,7 +248,7 @@ export class Garden {
   public readonly treeCache: TreeCache
   public events: EventBus
   private tools?: { [key: string]: PluginTool }
-  public configTemplates: { [name: string]: ConfigTemplateConfig }
+  public readonly configTemplates: { [name: string]: ConfigTemplateConfig }
   private actionTypeBases: ActionTypeMap<ActionTypeDefinition<any>[]>
   private emittedWarnings: Set<string>
   public cloudApi: CloudApi | null
@@ -484,6 +485,7 @@ export class Garden {
   needsReload(v?: true) {
     if (v) {
       this.state.needsReload = true
+      this.state.configsScanned = false
     }
     return this.state.needsReload
   }
@@ -1248,6 +1250,23 @@ export class Garden {
     return version
   }
 
+  /**
+   * We clear these objects without reassigning them. If this Garden instance was cloned from a parent Garden instance,
+   * this will also clear the configs of the parent instance (which is what we want e.g. when rescanning configs during
+   * a subcommand after a reload is necessary during a dev command session).
+   *
+   * We need to clear before rescanning to make sure old/outdated configs are cleared away, and to avoid duplicate
+   * key errors when adding the newly scanned ones (and those generated from newly scanned config templates).
+   */
+  private clearConfigs() {
+    for (const kind of Object.getOwnPropertyNames(this.actionConfigs)) {
+      clearObject(this.actionConfigs[kind])
+    }
+    clearObject(this.moduleConfigs)
+    clearObject(this.workflowConfigs)
+    clearObject(this.configTemplates)
+  }
+
   /**
    * Scans the specified directories for Garden config files and returns a list of paths.
    */
@@ -1283,6 +1302,7 @@ export class Garden {
       }
 
       this.log.silly(`Scanning for configs (force=${force})`)
+      this.clearConfigs()
 
       // Add external sources that are defined at the project level. External sources are either kept in
       // the .garden/sources dir (and cloned there if needed), or they're linked to a local path via the link command.
@@ -1403,7 +1423,10 @@ export class Garden {
       )
 
       this.state.configsScanned = true
-      this.configTemplates = { ...this.configTemplates, ...keyBy(configTemplates, "name") }
+
+      for (const template of configTemplates) {
+        this.configTemplates[template.name] = template
+      }
 
       this.events.emit("configsScanned", {})
     })

core/src/util/testing.ts

@@ -12,9 +12,9 @@ import { isEqual, keyBy, set, mapValues, round } from "lodash"
 import { Garden, GardenOpts, GardenParams, GetConfigGraphParams, resolveGardenParams } from "../garden"
 import { DeepPrimitiveMap, StringMap } from "../config/common"
 import { ModuleConfig } from "../config/module"
-import { WorkflowConfig } from "../config/workflow"
+import { WorkflowConfig, WorkflowConfigMap } from "../config/workflow"
 import { Log, LogEntry } from "../logger/log-entry"
-import { GardenModule } from "../types/module"
+import { GardenModule, ModuleConfigMap } from "../types/module"
 import { findByName, getNames } from "./util"
 import { GardenError, InternalError } from "../exceptions"
 import { EventBus, EventName, Events } from "../events/events"
@@ -29,7 +29,7 @@ import { ConfigGraph } from "../graph/config-graph"
 import { SolveParams } from "../graph/solver"
 import { GraphResults } from "../graph/results"
 import { expect } from "chai"
-import { ActionConfig, ActionKind, ActionStatus } from "../actions/types"
+import { ActionConfig, ActionConfigMap, ActionKind, ActionStatus } from "../actions/types"
 import { WrappedActionRouterHandlers } from "../router/base"
 import {
   BuiltinArgs,
@@ -44,6 +44,7 @@ import { mkdirp, remove } from "fs-extra"
 import { GlobalConfigStore } from "../config-store/global"
 import { isPromise } from "./objects"
 import chalk from "chalk"
+import { ConfigTemplateConfig } from "../config/config-template"
 
 export class TestError extends GardenError {
   type = "_test"
@@ -164,9 +165,14 @@ export type TestGardenOpts = Partial<GardenOpts> & {
 
 export class TestGarden extends Garden {
   override events: TestEventBus
-  public override vcs!: VcsHandler // Not readonly, to allow overriding with a mocked handler in tests
-  public override secrets!: StringMap // Not readonly, to allow setting secrets in tests
-  public override variables!: DeepPrimitiveMap // Not readonly, to allow setting variables in tests
+  // Overriding the type declarations of a few instance variables to allow reassignment in test code.
+  public override actionConfigs!: ActionConfigMap
+  public override moduleConfigs!: ModuleConfigMap
+  public override workflowConfigs!: WorkflowConfigMap
+  public override configTemplates!: { [name: string]: ConfigTemplateConfig }
+  public override vcs!: VcsHandler
+  public override secrets!: StringMap
+  public override variables!: DeepPrimitiveMap
   private repoRoot!: string
   public cacheKey!: string
 

core/src/util/util.ts

@@ -512,6 +512,17 @@ export function getEnvVarName(identifier: string) {
   return identifier.replace(/-/g, "_").toUpperCase()
 }
 
+/**
+ * Removes all keys from `obj` (except those inherited from the object's prototype).
+ *
+ * Essentially a vanilla object analog of `map.clear()` for ES5 Maps.
+ */
+export function clearObject<T extends object>(obj: T) {
+  for (const key of Object.getOwnPropertyNames(obj)) {
+    delete obj[key]
+  }
+}
+
 /**
  * Picks the specified keys from the given object, and throws an error if one or more keys are not found.
  */

core/test/data/test-projects/config-templates/templates.garden.yml

@@ -7,7 +7,7 @@ configs:
     name: ${parent.name}-${inputs.name}
     include: []
     spec:
-      command: ["${inputs.value}"]
+      command: [echo, "echo-prefix", "${inputs.name}"]
   - kind: Deploy
     type: test
     name: ${parent.name}-${inputs.name}

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

@@ -10,8 +10,14 @@ import { expect } from "chai"
 import { Command, CommandGroup } from "../../../../src/commands/base"
 import { StringsParameter } from "../../../../src/cli/params"
 import stripAnsi from "strip-ansi"
+import { replaceInFile } from "replace-in-file"
 import { dedent } from "../../../../src/util/string"
-import { trimLineEnds } from "../../../helpers"
+import { getDataDir, makeTestGarden, trimLineEnds, withDefaultGlobalOpts } from "../../../helpers"
+import { DevCommand } from "../../../../src/commands/dev"
+import { ValidateCommand } from "../../../../src/commands/validate"
+import { uuidv4 } from "../../../../src/util/random"
+import { join } from "path"
+import { BuildCommand } from "../../../../src/commands/build"
 
 describe("Command", () => {
   describe("renderHelp", () => {
@@ -212,6 +218,80 @@ describe("Command", () => {
       ])
     })
   })
+
+  describe("run", () => {
+    // This applies e.g. when running a command on the dev command line.
+    context("when called with a parentSessionId and a reload is needed after a config change", () => {
+      const projectRoot = getDataDir("test-projects", "config-templates")
+
+      it("passes on changed configs and templates to the parent and subsequent child instances", async () => {
+        const garden = await makeTestGarden(projectRoot)
+        // `makeTestGarden` does some trickery to copy the project root into a temp directory and work from there
+        // (which is nice, since it avoids the need for cleanup).
+        // Therefore, we need to make our find & replace substitutions inside the temp dir here.
+        const tmpRoot = garden.projectRoot
+        const log = garden.log
+
+        const devCmd = new DevCommand()
+        const devCmdSessionId = uuidv4()
+
+        const validateCmd = new ValidateCommand()
+        const buildCmd = new BuildCommand()
+
+        // We run this command to trigger the initial scan for configs
+        await validateCmd.run({
+          log,
+          args: {},
+          opts: withDefaultGlobalOpts({}),
+          garden,
+          sessionId: uuidv4(),
+          parentSessionId: devCmdSessionId,
+          parentCommand: devCmd,
+        })
+
+        await replaceInFile({
+          files: join(tmpRoot, "templates.garden.yml"),
+          from: new RegExp("echo-prefix"),
+          to: "reloaded-prefix",
+        })
+        await replaceInFile({
+          files: join(tmpRoot, "actions.garden.yml"),
+          from: new RegExp("name: test"),
+          to: "name: reloaded-name",
+        })
+
+        // The modification to `templates.garden.yml` above would normally trigger a `configChanged` event which would
+        // result in this call, but we're doing it manually here to simplify the test setup.
+        garden.needsReload(true)
+        await validateCmd.run({
+          log,
+          args: {},
+          opts: withDefaultGlobalOpts({}),
+          garden,
+          sessionId: uuidv4(),
+          parentSessionId: devCmdSessionId,
+          parentCommand: devCmd,
+        })
+
+        const { result } = await buildCmd.run({
+          log,
+          args: { names: undefined },
+          opts: withDefaultGlobalOpts({ "watch": false, "force": false, "with-dependants": false }),
+          garden,
+          sessionId: uuidv4(),
+          parentSessionId: devCmdSessionId,
+          parentCommand: devCmd,
+        })
+
+        // This means that when the config was rescanned during the second `validate` command run:
+        // * The action, module and workflow configs and config templates were also set on the parent Garden instance
+        //   when the first validate command run rescanned & added configs.
+        //  * The build command run that came immediately after received updated config templates, and the appropriately
+        //    re-generated action configs from the updated template.
+        expect(result.build["foo-reloaded-name"].buildLog).to.eql("reloaded-prefix reloaded-name")
+      })
+    })
+  })
 })
 
 describe("CommandGroup", () => {