From 1efbab58f1440af5f93132587f63b25e40413fec Mon Sep 17 00:00:00 2001 From: Thorarinn Sigurdsson Date: Sat, 4 Nov 2023 20:13:04 +0100 Subject: [PATCH] fix(core): resolve templates in `source.path` (#5345) Before this fix, we weren't resolving template strings on the optional `source.path` action config field. This caused errors when `git` was called on a path that still had unresolved template strings. This was fixed by preprocessing action configs (which resolves template strings on them) before passing the paths to the VCS helper that computes minimal roots. --- core/src/graph/actions.ts | 154 +++++++++++++----- .../action-source-path/project.garden.yml | 2 + .../some-dir/with-source.garden.yml | 2 +- 3 files changed, 119 insertions(+), 39 deletions(-) diff --git a/core/src/graph/actions.ts b/core/src/graph/actions.ts index bcc98e00b0..90eb7df1f6 100644 --- a/core/src/graph/actions.ts +++ b/core/src/graph/actions.ts @@ -18,6 +18,7 @@ import { actionKinds, ActionMode, ActionModeMap, + ActionModes, ActionWrapperParams, Executed, Resolved, @@ -124,51 +125,45 @@ export const actionConfigsToGraph = profileAsync(async function actionConfigsToG } } + const router = await garden.getActionRouter() + + // We need to preprocess the action configs to make sure any template strings have been resolved on the `source.path` + // field (if any). + // + // We then finish the process of converting the configs to actions once we've computed the minimal roots. + // + // Doing this in two steps makes the code a bit less readable, but it's worth it for the performance boost. + const preprocessResults: { [key: string]: PreprocessActionResult } = {} + const computedActionModes: { [key: string]: { mode: ActionMode; explicitMode: boolean } } = {} + await Promise.all( + Object.entries(configsByKey).map(async ([key, config]) => { + const { mode, explicitMode } = getActionMode(config, actionModes, log) + computedActionModes[key] = { mode, explicitMode } + preprocessResults[key] = await preprocessActionConfig({ + garden, + config, + router, + log, + mode, + }) + }) + ) + // Optimize file scanning by avoiding unnecessarily broad scans when project is not in repo root. - const allPaths = Object.values(configsByKey).map((c) => getSourcePath(c)) + const preprocessedConfigs = Object.values(preprocessResults).map((r) => r.config) + const allPaths = preprocessedConfigs.map((c) => getSourcePath(c)) const minimalRoots = await garden.vcs.getMinimalRoots(log, allPaths) - const router = await garden.getActionRouter() - // TODO: Maybe we could optimize resolving tree versions, avoid parallel scanning of the same directory etc. const graph = new MutableConfigGraph({ actions: [], moduleGraph, groups: groupConfigs }) await Promise.all( - Object.entries(configsByKey).map(async ([key, config]) => { - // Apply action modes - let mode: ActionMode = "default" - let explicitMode = false // set if a key is explicitly set (as opposed to a wildcard match) - - for (const pattern of actionModes.sync || []) { - if (key === pattern) { - explicitMode = true - mode = "sync" - log.silly(`Action ${key} set to ${mode} mode, matched on exact key`) - break - } else if (minimatch(key, pattern)) { - mode = "sync" - log.silly(`Action ${key} set to ${mode} mode, matched with pattern '${pattern}'`) - break - } - } - - // Local mode takes precedence over sync - // TODO: deduplicate - for (const pattern of actionModes.local || []) { - if (key === pattern) { - explicitMode = true - mode = "local" - log.silly(`Action ${key} set to ${mode} mode, matched on exact key`) - break - } else if (minimatch(key, pattern)) { - mode = "local" - log.silly(`Action ${key} set to ${mode} mode, matched with pattern '${pattern}'`) - break - } - } + Object.entries(preprocessResults).map(async ([key, res]) => { + const { config, supportedModes, templateContext } = res + const { mode, explicitMode } = computedActionModes[key] try { - const action = await actionFromConfig({ + const action = await processActionConfig({ garden, graph, config, @@ -177,6 +172,8 @@ export const actionConfigsToGraph = profileAsync(async function actionConfigsToG configsByKey, mode, linkedSources, + templateContext, + supportedModes, scanRoot: minimalRoots[getSourcePath(config)], }) @@ -210,6 +207,41 @@ export const actionConfigsToGraph = profileAsync(async function actionConfigsToG return graph }) +function getActionMode(config: ActionConfig, actionModes: ActionModeMap, log: Log) { + let mode: ActionMode = "default" + const key = actionReferenceToString(config) + let explicitMode = false // set if a key is explicitly set (as opposed to a wildcard match) + + for (const pattern of actionModes.sync || []) { + if (key === pattern) { + explicitMode = true + mode = "sync" + log.silly(`Action ${key} set to ${mode} mode, matched on exact key`) + break + } else if (minimatch(key, pattern)) { + mode = "sync" + log.silly(`Action ${key} set to ${mode} mode, matched with pattern '${pattern}'`) + break + } + } + + // Local mode takes precedence over sync + // TODO: deduplicate + for (const pattern of actionModes.local || []) { + if (key === pattern) { + explicitMode = true + mode = "local" + log.silly(`Action ${key} set to ${mode} mode, matched on exact key`) + break + } else if (minimatch(key, pattern)) { + mode = "local" + log.silly(`Action ${key} set to ${mode} mode, matched with pattern '${pattern}'`) + break + } + } + return { mode, explicitMode } +} + export const actionFromConfig = profileAsync(async function actionFromConfig({ garden, graph, @@ -240,6 +272,46 @@ export const actionFromConfig = profileAsync(async function actionFromConfig({ log, }) + return processActionConfig({ + garden, + graph, + config, + router, + log, + configsByKey, + mode, + linkedSources, + templateContext, + supportedModes, + scanRoot, + }) +}) + +async function processActionConfig({ + garden, + graph, + config, + router, + log, + configsByKey, + mode, + linkedSources, + templateContext, + supportedModes, + scanRoot, +}: { + garden: Garden + graph: ConfigGraph + config: ActionConfig + router: ActionRouter + log: Log + configsByKey: ActionConfigsByKey + mode: ActionMode + linkedSources: LinkedSourceMap + templateContext: ActionConfigContext + supportedModes: ActionModes + scanRoot?: string +}) { const actionTypes = await garden.getActionTypes() const definition = actionTypes[config.kind][config.type]?.spec const compatibleTypes = [config.type, ...getActionTypeBases(definition, actionTypes[config.kind]).map((t) => t.name)] @@ -355,7 +427,7 @@ export const actionFromConfig = profileAsync(async function actionFromConfig({ } else { return config satisfies never } -}) +} export function actionNameConflictError(configA: ActionConfig, configB: ActionConfig, rootPath: string) { return new ConfigurationError({ @@ -498,6 +570,12 @@ function getActionSchema(kind: ActionKind) { } } +interface PreprocessActionResult { + config: ActionConfig + supportedModes: ActionModes + templateContext: ActionConfigContext +} + export const preprocessActionConfig = profileAsync(async function preprocessActionConfig({ garden, config, @@ -510,7 +588,7 @@ export const preprocessActionConfig = profileAsync(async function preprocessActi router: ActionRouter mode: ActionMode log: Log -}) { +}): Promise { const description = describeActionConfig(config) const templateName = config.internal.templateName diff --git a/core/test/data/test-projects/action-source-path/project.garden.yml b/core/test/data/test-projects/action-source-path/project.garden.yml index 8afa2d120a..6bfb1986cf 100644 --- a/core/test/data/test-projects/action-source-path/project.garden.yml +++ b/core/test/data/test-projects/action-source-path/project.garden.yml @@ -1,6 +1,8 @@ apiVersion: garden.io/v1 kind: Project name: action-source-path +variables: + sourcePath: "../" environments: - name: local providers: diff --git a/core/test/data/test-projects/action-source-path/some-dir/with-source.garden.yml b/core/test/data/test-projects/action-source-path/some-dir/with-source.garden.yml index 28461e092d..e672dbcb9a 100644 --- a/core/test/data/test-projects/action-source-path/some-dir/with-source.garden.yml +++ b/core/test/data/test-projects/action-source-path/some-dir/with-source.garden.yml @@ -2,6 +2,6 @@ kind: Build type: test name: with-source source: - path: ../ + path: ${var.sourcePath} include: [some-dir/**/*] exclude: [some-dir/tps-report.txt]