fix(dev): fix reload error when using templates

core/src/garden.ts

@@ -11,8 +11,9 @@ const { ensureDir } = fsExtra
 import { platform, arch } from "os"
 import { relative, resolve } from "path"
 import cloneDeep from "fast-copy"
-import { flatten, sortBy, keyBy, mapValues, groupBy, set } from "lodash-es"
 import AsyncLock from "async-lock"
+import { flatten, groupBy, keyBy, mapValues, set, sortBy } from "lodash-es"
+import { username } from "username"
 
 import { TreeCache } from "./cache.js"
 import { getBuiltinPlugins } from "./plugins/plugins.js"
@@ -28,8 +29,15 @@ import {
   defaultNamespace,
   defaultEnvironment,
 } from "./config/project.js"
-import { getCloudDistributionName, getCloudLogSectionName } from "./util/cloud.js"
-import { findByName, pickKeys, getPackageVersion, getNames, findByNames, duplicatesByKey } from "./util/util.js"
+import {
+  findByName,
+  pickKeys,
+  getPackageVersion,
+  getNames,
+  findByNames,
+  duplicatesByKey,
+  clearObject,
+} from "./util/util.js"
 import type { GardenError } from "./exceptions.js"
 import {
   ConfigurationError,
@@ -96,7 +104,6 @@ import {
 import { dedent, deline, naturalList, wordWrap } from "./util/string.js"
 import { DependencyGraph } from "./graph/common.js"
 import { Profile, profileAsync } from "./util/profiling.js"
-import { username } from "username"
 import {
   throwOnMissingSecretKeys,
   resolveTemplateString,
@@ -160,6 +167,7 @@ import { detectModuleOverlap, makeOverlapErrors } from "./util/module-overlap.js
 import { GotHttpError } from "./util/http.js"
 import { styles } from "./logger/styles.js"
 import { renderDuration } from "./logger/util.js"
+import { getCloudDistributionName, getCloudLogSectionName } from "./util/cloud.js"
 
 const defaultLocalAddress = "localhost"
 
@@ -235,9 +243,9 @@ export class Garden {
   public log: Log
   private gardenInitLog?: 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
@@ -253,7 +261,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
@@ -491,6 +499,7 @@ export class Garden {
   needsReload(v?: true) {
     if (v) {
       this.state.needsReload = true
+      this.state.configsScanned = false
     }
     return this.state.needsReload
   }
@@ -1270,6 +1279,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).
+   */
+  protected 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.
    */
@@ -1305,6 +1331,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.
@@ -1425,7 +1452,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

@@ -14,9 +14,9 @@ import type { GardenOpts, GardenParams, GetConfigGraphParams } from "../garden.j
 import { Garden, resolveGardenParams } from "../garden.js"
 import type { DeepPrimitiveMap, StringMap } from "../config/common.js"
 import type { ModuleConfig } from "../config/module.js"
-import type { WorkflowConfig } from "../config/workflow.js"
+import type { WorkflowConfig, WorkflowConfigMap } from "../config/workflow.js"
 import { resolveMsg, type Log, type LogEntry } from "../logger/log-entry.js"
-import type { GardenModule } from "../types/module.js"
+import type { GardenModule, ModuleConfigMap } from "../types/module.js"
 import { findByName, getNames } from "./util.js"
 import { GardenError, InternalError } from "../exceptions.js"
 import type { EventName, Events } from "../events/events.js"
@@ -32,7 +32,7 @@ import type { ConfigGraph } from "../graph/config-graph.js"
 import type { SolveParams } from "../graph/solver.js"
 import type { GraphResults } from "../graph/results.js"
 import { expect } from "chai"
-import type { ActionConfig, ActionKind, ActionStatus } from "../actions/types.js"
+import type { ActionConfig, ActionConfigMap, ActionKind, ActionStatus } from "../actions/types.js"
 import type { WrappedActionRouterHandlers } from "../router/base.js"
 import type {
   BuiltinArgs,
@@ -48,6 +48,7 @@ const { mkdirp, remove } = fsExtra
 import { GlobalConfigStore } from "../config-store/global.js"
 import { isPromise } from "./objects.js"
 import { styles } from "../logger/styles.js"
+import type { ConfigTemplateConfig } from "../config/config-template.js"
 
 export class TestError extends GardenError {
   type = "_test"
@@ -164,15 +165,22 @@ export type TestGardenOpts = Partial<GardenOpts> & {
   noTempDir?: boolean
   onlySpecifiedPlugins?: boolean
   remoteContainerAuth?: boolean
+  clearConfigsOnScan?: boolean
 }
 
 export class TestGarden extends Garden {
   override events: TestEventBus
-  public declare vcs: VcsHandler // Not readonly, to allow overriding with a mocked handler in tests
-  public declare secrets: StringMap // Not readonly, to allow setting secrets in tests
-  public declare 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 declare actionConfigs: ActionConfigMap
+  public declare moduleConfigs: ModuleConfigMap
+  public declare workflowConfigs: WorkflowConfigMap
+  public declare configTemplates: { [name: string]: ConfigTemplateConfig }
+  public declare vcs: VcsHandler
+  public declare secrets: StringMap
+  public declare variables: DeepPrimitiveMap
   private repoRoot!: string
   public cacheKey!: string
+  public clearConfigsOnScan = false
 
   constructor(params: GardenParams) {
     super(params)
@@ -222,6 +230,14 @@ export class TestGarden extends Garden {
     return garden
   }
 
+  protected override clearConfigs() {
+    if (this.clearConfigsOnScan) {
+      super.clearConfigs()
+    } else {
+      // No-op: We need to disable this method, because it breaks test cases that manually set configs.
+    }
+  }
+
   override async processTasks(params: Omit<SolveParams, "log"> & { log?: Log }) {
     return super.processTasks({ ...params, log: params.log || this.log })
   }

core/src/util/util.ts

@@ -481,6 +481,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-template-reloading/actions.garden.yml

@@ -0,0 +1,9 @@
+kind: RenderTemplate
+template: combo
+name: foo
+inputs:
+  name: test
+  # Resolvable at parse time
+  envName: ${environment.name}
+  # Resolvable later
+  providerKey: ${providers.test-plugin.outputs.testKey}

core/test/data/test-projects/config-template-reloading/project.garden.yml

@@ -0,0 +1,7 @@
+apiVersion: garden.io/v1
+kind: Project
+name: config-templates
+environments:
+  - name: local
+providers:
+  - name: test-plugin

core/test/data/test-projects/config-template-reloading/schema.json

@@ -0,0 +1,14 @@
+{
+  "type": "object",
+  "properties": {
+    "name": {
+      "type": "string"
+    },
+    "envName": {
+      "type": "string"
+    },
+    "providerKey": {
+      "type": "string"
+    }
+  }
+}

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

@@ -0,0 +1,30 @@
+kind: ConfigTemplate
+name: combo
+inputsSchemaPath: schema.json
+configs:
+  - kind: Build
+    type: test
+    name: ${parent.name}-${inputs.name}
+    include: []
+    spec:
+      command: [echo, "echo-prefix", "${inputs.name}"]
+  - kind: Deploy
+    type: test
+    name: ${parent.name}-${inputs.name}
+    build: ${parent.name}-${inputs.name}
+  - kind: Test
+    type: test
+    name: ${parent.name}-${inputs.name}
+    dependencies: ["build.${parent.name}-${inputs.name}"]
+    spec:
+      command: [echo, "${inputs.envName}", "${inputs.providerKey}"]
+
+---
+
+kind: ConfigTemplate
+name: workflows
+configs:
+  - kind: Workflow
+    name: ${parent.name}-${inputs.name}
+    steps:
+      - script: echo "${inputs.envName}"

core/test/data/test-projects/config-template-reloading/workflows.garden.yml

@@ -0,0 +1,6 @@
+kind: RenderTemplate
+template: workflows
+name: foo
+inputs:
+  name: test
+  envName: ${environment.name}

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.js"
 import { StringsParameter } from "../../../../src/cli/params.js"
 import stripAnsi from "strip-ansi"
-import { dedent } from "../../../../src/util/string.js"
-import { trimLineEnds } from "../../../helpers.js"
+import replaceInFile from "replace-in-file"
+import { join } from "path"
+import dedent from "dedent"
+import { BuildCommand } from "../../../../src/commands/build.js"
+import { DevCommand } from "../../../../src/commands/dev.js"
+import { ValidateCommand } from "../../../../src/commands/validate.js"
+import { uuidv4 } from "../../../../src/util/random.js"
+import { trimLineEnds, getDataDir, makeTestGarden, withDefaultGlobalOpts } from "../../../helpers.js"
 
 describe("Command", () => {
   describe("renderHelp", () => {
@@ -212,6 +218,83 @@ 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-template-reloading")
+
+      it("passes on changed configs and templates to the parent and subsequent child instances", async () => {
+        const garden = await makeTestGarden(projectRoot)
+        // Here, we're testing the config clearing logic, so we turn this behavior on (which is off by default for
+        // TestGarden instances).
+        garden.clearConfigsOnScan = true
+        // `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.replaceInFile({
+          files: join(tmpRoot, "templates.garden.yml"),
+          from: new RegExp("echo-prefix"),
+          to: "reloaded-prefix",
+        })
+        await replaceInFile.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", () => {

core/test/unit/src/garden.ts

@@ -2903,7 +2903,7 @@ describe("Garden", () => {
       expect(test).to.exist
 
       expect(build.type).to.equal("test")
-      expect(build.spec.command).to.eql(["${inputs.value}"]) // <- resolved later
+      expect(build.spec.command).to.include("${inputs.name}") // <- resolved later
       expect(omit(build.internal, "yamlDoc")).to.eql(internal)
 
       expect(deploy["build"]).to.equal("${parent.name}-${inputs.name}") // <- resolved later