diff --git a/core/src/garden.ts b/core/src/garden.ts index 362746d043..686427a1df 100644 --- a/core/src/garden.ts +++ b/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 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[]> private emittedWarnings: Set 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", {}) }) diff --git a/core/src/util/testing.ts b/core/src/util/testing.ts index c581de7a6a..7e3d7aa419 100644 --- a/core/src/util/testing.ts +++ b/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 & { 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 & { log?: Log }) { return super.processTasks({ ...params, log: params.log || this.log }) } diff --git a/core/src/util/util.ts b/core/src/util/util.ts index 0a69024fa5..a9e1ade995 100644 --- a/core/src/util/util.ts +++ b/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(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. */ diff --git a/core/test/data/test-projects/config-template-reloading/actions.garden.yml b/core/test/data/test-projects/config-template-reloading/actions.garden.yml new file mode 100644 index 0000000000..cc4f7e1a56 --- /dev/null +++ b/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} diff --git a/core/test/data/test-projects/config-template-reloading/project.garden.yml b/core/test/data/test-projects/config-template-reloading/project.garden.yml new file mode 100644 index 0000000000..5b75835ce6 --- /dev/null +++ b/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 diff --git a/core/test/data/test-projects/config-template-reloading/schema.json b/core/test/data/test-projects/config-template-reloading/schema.json new file mode 100644 index 0000000000..e337ba56f5 --- /dev/null +++ b/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" + } + } +} diff --git a/core/test/data/test-projects/config-template-reloading/templates.garden.yml b/core/test/data/test-projects/config-template-reloading/templates.garden.yml new file mode 100644 index 0000000000..b456a7b2d3 --- /dev/null +++ b/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}" diff --git a/core/test/data/test-projects/config-template-reloading/workflows.garden.yml b/core/test/data/test-projects/config-template-reloading/workflows.garden.yml new file mode 100644 index 0000000000..95e2719001 --- /dev/null +++ b/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} diff --git a/core/test/data/test-projects/config-templates/templates.garden.yml b/core/test/data/test-projects/config-templates/templates.garden.yml index 54c09948a1..b456a7b2d3 100644 --- a/core/test/data/test-projects/config-templates/templates.garden.yml +++ b/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} diff --git a/core/test/unit/src/commands/base.ts b/core/test/unit/src/commands/base.ts index 5ebf6107e8..85d5257050 100644 --- a/core/test/unit/src/commands/base.ts +++ b/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", () => { diff --git a/core/test/unit/src/garden.ts b/core/test/unit/src/garden.ts index a8fa2d4817..1b1651e0af 100644 --- a/core/test/unit/src/garden.ts +++ b/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