fix(core): correctly apply source.path in VCS logic (#5305)

Before this fix, we weren't accounting for the `source.path` config
option for actions when determining the path passed to `getFiles`.

This meant that actions using `source.path` would essentially have their
versions computed using an empty file list.

core/src/actions/base.ts

@@ -422,7 +422,7 @@ export abstract class BaseAction<
       return true
     }
 
-    const path = this.basePath()
+    const path = this.sourcePath()
 
     for (const source of linkedSources) {
       if (path === source.path || pathIsInside(path, source.path)) {
@@ -443,14 +443,13 @@ export abstract class BaseAction<
     return internal?.groupName
   }
 
-  // TODO: rename to sourcePath
-  basePath(): string {
+  sourcePath(): string {
     const basePath = this.remoteSourcePath || this._config.internal.basePath
     const sourceRelPath = this._config.source?.path
 
     if (sourceRelPath) {
       // TODO: validate that this is a directory here?
-      return joinWithPosix(basePath, sourceRelPath)
+      return getSourceAbsPath(basePath, sourceRelPath)
     } else {
       return basePath
     }
@@ -670,7 +669,7 @@ export abstract class RuntimeAction<
    */
   getBuildPath() {
     const buildAction = this.getBuildAction()
-    return buildAction?.getBuildPath() || this.basePath()
+    return buildAction?.getBuildPath() || this.sourcePath()
   }
 }
 
@@ -852,6 +851,11 @@ export function actionRefMatches(a: ActionReference, b: ActionReference) {
   return a.kind === b.kind && a.name === b.name
 }
 
+export function getSourceAbsPath(basePath: string, sourceRelPath: string) {
+  // TODO: validate that this is a directory here?
+  return joinWithPosix(basePath, sourceRelPath)
+}
+
 export function describeActionConfig(config: ActionConfig) {
   const d = `${config.type} ${config.kind} ${config.name}`
   if (config.internal?.moduleName) {

core/src/actions/build.ts

@@ -188,7 +188,7 @@ export class BuildAction<
    */
   getBuildPath() {
     if (this._config.buildAtSource) {
-      return this.basePath()
+      return this.sourcePath()
     } else {
       return join(this.baseBuildDirectory, this.name)
     }

core/src/actions/helpers.ts

@@ -83,7 +83,7 @@ export async function warnOnLinkedActions(garden: Garden, log: Log, actions: Act
 
   const linkedActionsMsg = actions
     .filter((a) => a.isLinked(linkedSources))
-    .map((a) => `${a.longDescription()} linked to path ${chalk.white(a.basePath())}`)
+    .map((a) => `${a.longDescription()} linked to path ${chalk.white(a.sourcePath())}`)
     .map((msg) => "  " + msg) // indent list
 
   if (linkedActionsMsg.length > 0) {

core/src/build-staging/build-staging.ts

@@ -61,13 +61,13 @@ export class BuildStaging {
     }
 
     // Normalize to relative POSIX-style paths
-    const files = action.getFullVersion().files.map((f) => normalizeRelativePath(action.basePath(), f))
+    const files = action.getFullVersion().files.map((f) => normalizeRelativePath(action.sourcePath(), f))
 
     const buildPath = action.getBuildPath()
     await this.ensureDir(buildPath)
 
     await this.sync({
-      sourceRoot: resolve(this.projectRoot, action.basePath()),
+      sourceRoot: resolve(this.projectRoot, action.sourcePath()),
       targetRoot: buildPath,
       withDelete,
       log,

core/src/config/base.ts

@@ -48,7 +48,21 @@ export interface YamlDocumentWithSource extends Document {
 }
 
 export interface GardenResourceInternalFields {
+  /**
+   * The path/working directory where commands and operations relating to the config should be executed. This is
+   * most commonly the directory containing the config file.
+   *
+   * Note: WHen possible, use `action.getSourcePath()` instead, since it factors in remote source paths and source
+   * overrides (i.e. `BaseActionConfig.source.path`). This is a lower-level field that doesn't contain template strings,
+   * and can thus be used early in the resolution flow.
+   */
   basePath: string
+  /**
+   * The path to the resource's config file, if any.
+   *
+   * Configs that are read from a file should always have this set, but generated configs (e.g. from templates
+   * or `augmentGraph` handlers) don't necessarily have a path on disk.
+   */
   configFilePath?: string
   // -> set by templates
   inputs?: DeepPrimitiveMap

core/src/config/template-contexts/actions.ts

@@ -219,7 +219,7 @@ class ActionReferencesContext extends ConfigContext {
           version: action.versionString(),
           disabled: action.isDisabled(),
           buildPath: action.getBuildPath(),
-          sourcePath: action.basePath(),
+          sourcePath: action.sourcePath(),
           mode: action.mode(),
           variables: action.getVariables(),
         })
@@ -297,7 +297,7 @@ export class ActionSpecContext extends OutputConfigContext {
 
     const name = action.name
     const buildPath = action.getBuildPath()
-    const sourcePath = action.basePath()
+    const sourcePath = action.sourcePath()
     const parentName = internal?.parentName
     const templateName = internal?.templateName
 

core/src/graph/actions.ts

@@ -63,7 +63,7 @@ import { LinkedSource, LinkedSourceMap } from "../config-store/local"
 import { relative } from "path"
 import { profileAsync } from "../util/profiling"
 import { uuidv4 } from "../util/random"
-import { getConfigBasePath } from "../vcs/vcs"
+import { getSourcePath } from "../vcs/vcs"
 import { actionIsDisabled } from "../actions/base"
 
 export const actionConfigsToGraph = profileAsync(async function actionConfigsToGraph({
@@ -125,7 +125,7 @@ export const actionConfigsToGraph = profileAsync(async function actionConfigsToG
   }
 
   // Optimize file scanning by avoiding unnecessarily broad scans when project is not in repo root.
-  const allPaths = Object.values(configsByKey).map((c) => getConfigBasePath(c))
+  const allPaths = Object.values(configsByKey).map((c) => getSourcePath(c))
   const minimalRoots = await garden.vcs.getMinimalRoots(log, allPaths)
 
   const router = await garden.getActionRouter()
@@ -177,7 +177,7 @@ export const actionConfigsToGraph = profileAsync(async function actionConfigsToG
           configsByKey,
           mode,
           linkedSources,
-          scanRoot: minimalRoots[getConfigBasePath(config)],
+          scanRoot: minimalRoots[getSourcePath(config)],
         })
 
         if (!action.supportsMode(mode)) {

core/src/plugins/hadolint/hadolint.ts

@@ -179,15 +179,15 @@ provider.addHandler("augmentGraph", async ({ ctx, actions }) => {
     .filter(isHadolintTest)
     // Can't really reason about templated dockerfile spec field
     .filter((a) => !mayContainTemplateString(a.getConfig("spec").dockerfilePath))
-    .map((a) => resolve(a.basePath(), a.getConfig("spec").dockerfilePath))
+    .map((a) => resolve(a.sourcePath(), a.getConfig("spec").dockerfilePath))
 
   const pickCompatibleAction = (action: BaseAction): action is BuildAction | HadolintTest => {
     // Make sure we don't step on an existing custom hadolint module
     if (isHadolintTest(action)) {
       const dockerfilePath = action.getConfig("spec").dockerfilePath
       if (
         !mayContainTemplateString(dockerfilePath) &&
-        existingHadolintDockerfiles.includes(resolve(action.basePath(), dockerfilePath))
+        existingHadolintDockerfiles.includes(resolve(action.sourcePath(), dockerfilePath))
       ) {
         return false
       }
@@ -222,7 +222,7 @@ provider.addHandler("augmentGraph", async ({ ctx, actions }) => {
       description: `hadolint test for '${action.longDescription()}' (auto-generated)`,
       include,
       internal: {
-        basePath: action.basePath(),
+        basePath: action.sourcePath(),
       },
       timeout: action.getConfig().timeout,
       spec: {
@@ -286,20 +286,20 @@ hadolintTest.addHandler("configure", async ({ ctx, config }) => {
 
 hadolintTest.addHandler("run", async ({ ctx, log, action }) => {
   const spec = action.getSpec()
-  const dockerfilePath = join(action.basePath(), spec.dockerfilePath)
+  const dockerfilePath = join(action.sourcePath(), spec.dockerfilePath)
   const startedAt = new Date()
   let dockerfile: string
 
   try {
     dockerfile = (await readFile(dockerfilePath)).toString()
   } catch {
     throw new ConfigurationError({
-      message: `hadolint: Could not find Dockerfile at ${spec.dockerfilePath}. Action path: ${action.basePath()}`,
+      message: `hadolint: Could not find Dockerfile at ${spec.dockerfilePath}. Action path: ${action.sourcePath()}`,
     })
   }
 
   let configPath: string
-  const moduleConfigPath = join(action.basePath(), configFilename)
+  const moduleConfigPath = join(action.sourcePath(), configFilename)
   const projectConfigPath = join(ctx.projectRoot, configFilename)
 
   if (await pathExists(moduleConfigPath)) {

core/src/plugins/kubernetes/container/deployment.ts

@@ -609,7 +609,7 @@ export function configureVolumes(
       volumes.push({
         name: volumeName,
         hostPath: {
-          path: resolve(action.basePath(), volume.hostPath),
+          path: resolve(action.sourcePath(), volume.hostPath),
         },
       })
     } else if (volume.action) {

core/src/plugins/kubernetes/container/sync.ts

@@ -33,7 +33,7 @@ export const k8sContainerStartSync: DeployActionHandler<"startSync", ContainerDe
     log,
     action,
     actionDefaults: {},
-    basePath: action.basePath(),
+    basePath: action.sourcePath(),
     defaultNamespace,
     defaultTarget: target,
     deployedResources,
@@ -75,7 +75,7 @@ export const k8sContainerGetSyncStatus: DeployActionHandler<"getSyncStatus", Con
     log,
     action,
     actionDefaults: {},
-    basePath: action.basePath(),
+    basePath: action.sourcePath(),
     defaultNamespace,
     defaultTarget: target,
     deployedResources,

core/src/plugins/kubernetes/helm/sync.ts

@@ -41,7 +41,7 @@ export const helmStartSync: DeployActionHandler<"startSync", HelmDeployAction> =
     action,
     actionDefaults: spec.sync.defaults || {},
     defaultTarget: spec.defaultTarget,
-    basePath: action.basePath(),
+    basePath: action.sourcePath(),
     defaultNamespace: namespace,
     deployedResources,
     syncs: spec.sync.paths,
@@ -79,7 +79,7 @@ export const helmGetSyncStatus: DeployActionHandler<"getSyncStatus", HelmDeployA
     action,
     actionDefaults: spec.sync.defaults || {},
     defaultTarget: spec.defaultTarget,
-    basePath: action.basePath(),
+    basePath: action.sourcePath(),
     defaultNamespace: namespace,
     deployedResources,
     syncs: spec.sync.paths,

core/src/plugins/kubernetes/kubernetes-type/sync.ts

@@ -44,7 +44,7 @@ export const kubernetesStartSync: DeployActionHandler<"startSync", KubernetesDep
     action,
     actionDefaults: spec.sync.defaults || {},
     defaultTarget: spec.defaultTarget,
-    basePath: action.basePath(),
+    basePath: action.sourcePath(),
     defaultNamespace: namespace,
     deployedResources,
     syncs: spec.sync.paths,
@@ -84,7 +84,7 @@ export const kubernetesGetSyncStatus: DeployActionHandler<"getSyncStatus", Kuber
     action,
     actionDefaults: spec.sync.defaults || {},
     defaultTarget: spec.defaultTarget,
-    basePath: action.basePath(),
+    basePath: action.sourcePath(),
     defaultNamespace: namespace,
     deployedResources,
     syncs: spec.sync.paths,

core/src/plugins/kubernetes/local-mode.ts

@@ -540,7 +540,7 @@ function getLocalAppCommand({ spec: localModeSpec, action }: StartLocalModeParam
   }
   const commandName = command[0]
   const commandArgs = command.slice(1)
-  const cwd = isAbsolute(commandName) ? undefined : action.basePath()
+  const cwd = isAbsolute(commandName) ? undefined : action.sourcePath()
   return { command: commandName, args: commandArgs, cwd, description: "Local app" }
 }
 

core/src/plugins/kubernetes/sync.ts

@@ -264,7 +264,7 @@ export function convertContainerSyncSpec(
   const spec = action.getSpec()
   const kind: SyncableKind = spec.daemon ? "DaemonSet" : "Deployment"
   const target = { kind, name: action.name }
-  const sourcePath = action.basePath()
+  const sourcePath = action.sourcePath()
   const syncSpec = spec.sync
 
   if (!syncSpec || !target) {
@@ -832,7 +832,7 @@ function getSyncKeyPrefix(ctx: PluginContext, action: SupportedRuntimeAction) {
  *  It cannot contain any characters that can break the command execution (like / \ < > | :).
  */
 function getSyncKey({ ctx, action, spec }: PrepareSyncParams, target: SyncableResource): string {
-  const sourcePath = relative(action.basePath(), spec.sourcePath)
+  const sourcePath = relative(action.sourcePath(), spec.sourcePath)
   const containerPath = spec.containerPath
   return kebabCase(
     `${getSyncKeyPrefix(ctx, action)}${target.kind}--${target.metadata.name}--${sourcePath}--${containerPath}`

core/src/plugins/kubernetes/volumes/configmap.ts

@@ -161,7 +161,7 @@ function getKubernetesAction(action: Resolved<ConfigmapAction>) {
     type: "kubernetes",
     name: action.name,
     internal: {
-      basePath: action.basePath(),
+      basePath: action.sourcePath(),
     },
     include: [],
     timeout: KUBECTL_DEFAULT_TIMEOUT,

core/src/plugins/kubernetes/volumes/persistentvolumeclaim.ts

@@ -207,7 +207,7 @@ function getKubernetesAction(action: Resolved<PersistentVolumeClaimAction>) {
     type: "kubernetes",
     name: action.name,
     internal: {
-      basePath: action.basePath(),
+      basePath: action.sourcePath(),
     },
     include: [],
     timeout: KUBECTL_DEFAULT_TIMEOUT,

core/src/tasks/resolve-action.ts

@@ -147,7 +147,7 @@ export class ResolveActionTask<T extends Action> extends BaseActionTask<T, Resol
 
     const actionVariables = resolveTemplateStrings({
       value: await mergeVariables({
-        basePath: action.basePath(),
+        basePath: action.sourcePath(),
         variables: config.variables,
         varfiles: config.varfiles,
       }),
@@ -261,7 +261,7 @@ export class ResolveActionTask<T extends Action> extends BaseActionTask<T, Resol
       })
     }
 
-    const path = this.action.basePath()
+    const path = this.action.sourcePath()
     const internal = this.action.getInternal()
 
     spec = validateWithPath({

core/src/vcs/vcs.ts

@@ -27,7 +27,7 @@ import type { TaskConfig } from "../config/task"
 import type { TestConfig } from "../config/test"
 import type { GardenModule } from "../types/module"
 import { validateInstall } from "../util/validateInstall"
-import { isActionConfig } from "../actions/base"
+import { getSourceAbsPath, isActionConfig } from "../actions/base"
 import type { BaseActionConfig } from "../actions/types"
 import { Garden } from "../garden"
 import chalk from "chalk"
@@ -195,7 +195,7 @@ export abstract class VcsHandler {
     }
 
     const configPath = getConfigFilePath(config)
-    const path = getConfigBasePath(config)
+    const path = getSourcePath(config)
 
     let result: TreeVersion = { contentHash: NEW_RESOURCE_VERSION, files: [] }
 
@@ -264,7 +264,7 @@ export abstract class VcsHandler {
 
   async resolveTreeVersion(params: GetTreeVersionParams): Promise<TreeVersion> {
     // the version file is used internally to specify versions outside of source control
-    const path = getConfigBasePath(params.config)
+    const path = getSourcePath(params.config)
     const versionFilePath = join(path, GARDEN_TREEVERSION_FILENAME)
     const fileVersion = await readTreeVersionFile(versionFilePath)
     return fileVersion || (await this.getTreeVersion(params))
@@ -445,7 +445,7 @@ export function hashStrings(hashes: string[]) {
 }
 
 export function getResourceTreeCacheKey(config: ModuleConfig | BaseActionConfig) {
-  const cacheKey = ["source", getConfigBasePath(config)]
+  const cacheKey = ["source", getSourcePath(config)]
 
   if (config.include) {
     cacheKey.push("include", hashStrings(config.include.sort()))
@@ -461,8 +461,14 @@ export function getConfigFilePath(config: ModuleConfig | BaseActionConfig) {
   return isActionConfig(config) ? config.internal?.configFilePath : config.configPath
 }
 
-export function getConfigBasePath(config: ModuleConfig | BaseActionConfig) {
-  return isActionConfig(config) ? config.internal.basePath : config.path
+export function getSourcePath(config: ModuleConfig | BaseActionConfig) {
+  if (isActionConfig(config)) {
+    const basePath = config.internal.basePath
+    const sourceRelPath = config.source?.path
+    return sourceRelPath ? getSourceAbsPath(basePath, sourceRelPath) : basePath
+  } else {
+    return config.path
+  }
 }
 
 export function describeConfig(config: ModuleConfig | BaseActionConfig) {

core/test/data/test-projects/action-source-path/project.garden.yml

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

core/test/data/test-projects/action-source-path/some-dir/some-other-file.txt

@@ -0,0 +1 @@
+foo

core/test/data/test-projects/action-source-path/some-dir/tps-report.txt

@@ -0,0 +1 @@
+We're putting the new cover letter on all of these now, Peter. Didn't you get the memo?

core/test/data/test-projects/action-source-path/some-dir/with-source.garden.yml

@@ -0,0 +1,7 @@
+kind: Build
+type: test
+name: with-source
+source:
+  path: ../
+include: [some-dir/**/*]
+exclude: [some-dir/tps-report.txt]