fix(core): fix "repo" scan mode for remote actions (#5660)

Before this fix, remote actions (i.e. actions with `source.repository.url` set, not sources declared at the project level) didn't work correctly when using the new `repo` git scanning mode. Also set the default git scanning mode back to `repo`, since this bug (which we've now fixed) was the reason we reverted to `subtree`. The error arose because the base path wasn't being set to the locally cloned path of the remote source's repo early enough in the resolution process. This led to the repo scanning logic using the action's config directory instead of the cloned source directory as the repo root (thus ignoring the sources). We've added a unit test that verifies that the base path is being set correctly when the action graph is initialized.
garden-io · Jan 24, 2024 · d38f1c5 · d38f1c5
1 parent e0a3671
commit d38f1c5
Show file tree

Hide file tree

Showing 7 changed files with 106 additions and 44 deletions.
diff --git a/core/src/constants.ts b/core/src/constants.ts
@@ -18,7 +18,7 @@ const extractedRoot = process.env.GARDEN_SEA_EXTRACTED_ROOT
 
 export const gitScanModes = ["repo", "subtree"] as const
 export type GitScanMode = (typeof gitScanModes)[number]
-export const defaultGitScanMode: GitScanMode = "subtree"
+export const defaultGitScanMode: GitScanMode = "repo"
 
 export const GARDEN_CORE_ROOT = !!extractedRoot
   ? resolve(extractedRoot, "src", "core")

diff --git a/core/src/garden.ts b/core/src/garden.ts
@@ -1573,7 +1573,7 @@ export class Garden {
   }
 
   /**
-   * Clones the project/module source if needed and returns the path (either from .garden/sources or from a local path)
+   * Clones the project/action/module source if needed and returns the path (either from .garden/sources or from a local path)
    */
   public async resolveExtSourcePath({
     name,

diff --git a/core/src/graph/actions.ts b/core/src/graph/actions.ts
@@ -152,6 +152,7 @@ export const actionConfigsToGraph = profileAsync(async function actionConfigsToG
           garden,
           config,
           router,
+          linkedSources,
           log,
           mode,
         })
@@ -208,7 +209,7 @@ export const actionConfigsToGraph = profileAsync(async function actionConfigsToG
 
   await Promise.all(
     Object.entries(preprocessResults).map(async ([key, res]) => {
-      const { config, supportedModes, templateContext } = res
+      const { config, linkedSource, remoteSourcePath, supportedModes, templateContext } = res
       const { mode, explicitMode } = computedActionModes[key]
 
       try {
@@ -219,7 +220,8 @@ export const actionConfigsToGraph = profileAsync(async function actionConfigsToG
           log,
           configsByKey,
           mode,
-          linkedSources,
+          linkedSource,
+          remoteSourcePath,
           templateContext,
           supportedModes,
           scanRoot: minimalRoots[getSourcePath(config)],
@@ -312,11 +314,12 @@ export const actionFromConfig = profileAsync(async function actionFromConfig({
   scanRoot?: string
 }) {
   // Call configure handler and validate
-  const { config, supportedModes, templateContext } = await preprocessActionConfig({
+  const { config, supportedModes, linkedSource, remoteSourcePath, templateContext } = await preprocessActionConfig({
     garden,
     config: inputConfig,
     router,
     mode,
+    linkedSources,
     log,
   })
 
@@ -327,7 +330,8 @@ export const actionFromConfig = profileAsync(async function actionFromConfig({
     log,
     configsByKey,
     mode,
-    linkedSources,
+    linkedSource,
+    remoteSourcePath,
     templateContext,
     supportedModes,
     scanRoot,
@@ -341,7 +345,8 @@ async function processActionConfig({
   log,
   configsByKey,
   mode,
-  linkedSources,
+  linkedSource,
+  remoteSourcePath,
   templateContext,
   supportedModes,
   scanRoot,
@@ -352,38 +357,15 @@ async function processActionConfig({
   log: Log
   configsByKey: ActionConfigsByKey
   mode: ActionMode
-  linkedSources: LinkedSourceMap
+  linkedSource: LinkedSource | null
+  remoteSourcePath: string | null
   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)]
-  const repositoryUrl = config.source?.repository?.url
-  const key = actionReferenceToString(config)
-
-  let linked: LinkedSource | null = null
-  let remoteSourcePath: string | null = null
-  if (repositoryUrl) {
-    if (config.internal.remoteClonePath) {
-      // Carry over clone path from converted module
-      remoteSourcePath = config.internal.remoteClonePath
-    } else {
-      remoteSourcePath = await garden.resolveExtSourcePath({
-        name: key,
-        sourceType: "action",
-        repositoryUrl,
-        linkedSources: Object.values(linkedSources),
-      })
-
-      config.internal.basePath = remoteSourcePath
-    }
-
-    if (linkedSources[key]) {
-      linked = linkedSources[key]
-    }
-  }
 
   const configPath = relative(garden.projectRoot, config.internal.configFilePath || config.internal.basePath)
 
@@ -453,7 +435,7 @@ async function processActionConfig({
     projectRoot: garden.projectRoot,
     treeVersion,
     variables,
-    linkedSource: linked,
+    linkedSource,
     remoteSourcePath,
     moduleName: config.internal.moduleName,
     moduleVersion: config.internal.moduleVersion,
@@ -618,20 +600,24 @@ function getActionSchema(kind: ActionKind) {
 interface PreprocessActionResult {
   config: ActionConfig
   supportedModes: ActionModes
+  remoteSourcePath: string | null
+  linkedSource: LinkedSource | null
   templateContext: ActionConfigContext
 }
 
 export const preprocessActionConfig = profileAsync(async function preprocessActionConfig({
   garden,
   config,
-  mode,
   router,
+  mode,
+  linkedSources,
   log,
 }: {
   garden: Garden
   config: ActionConfig
   router: ActionRouter
   mode: ActionMode
+  linkedSources: LinkedSourceMap
   log: Log
 }): Promise<PreprocessActionResult> {
   const description = describeActionConfig(config)
@@ -809,7 +795,38 @@ export const preprocessActionConfig = profileAsync(async function preprocessActi
     })
   }
 
-  return { config, supportedModes, templateContext: builtinFieldContext }
+  const repositoryUrl = config.source?.repository?.url
+  const key = actionReferenceToString(config)
+
+  let linkedSource: LinkedSource | null = null
+  let remoteSourcePath: string | null = null
+  if (repositoryUrl) {
+    if (config.internal.remoteClonePath) {
+      // Carry over clone path from converted module
+      remoteSourcePath = config.internal.remoteClonePath
+    } else {
+      remoteSourcePath = await garden.resolveExtSourcePath({
+        name: key,
+        sourceType: "action",
+        repositoryUrl,
+        linkedSources: Object.values(linkedSources),
+      })
+
+      config.internal.basePath = remoteSourcePath
+    }
+
+    if (linkedSources[key]) {
+      linkedSource = linkedSources[key]
+    }
+  }
+
+  return {
+    config,
+    supportedModes,
+    remoteSourcePath,
+    linkedSource,
+    templateContext: builtinFieldContext,
+  }
 })
 
 function dependenciesFromActionConfig(

diff --git a/core/test/unit/src/actions/action-configs-to-graph.ts b/core/test/unit/src/actions/action-configs-to-graph.ts
@@ -20,6 +20,7 @@ import {
   DEFAULT_RUN_TIMEOUT_SEC,
   DEFAULT_TEST_TIMEOUT_SEC,
 } from "../../../../src/constants.js"
+import { getRemoteSourceLocalPath } from "../../../../src/util/ext-source-util.js"
 
 describe("actionConfigsToGraph", () => {
   let tmpDir: TempDirectory
@@ -1029,6 +1030,45 @@ describe("actionConfigsToGraph", () => {
     expect(action.getConfig("timeout")).to.equal(123)
   })
 
+  describe("action with source.repository.url set", () => {
+    it("sets the base path to the local cloned path when a repositoryUrl is specified", async () => {
+      const repoUrl = "https://github.com/garden-io/garden-example-remote-module-jworker.git#main"
+      const graph = await actionConfigsToGraph({
+        garden,
+        log,
+        groupConfigs: [],
+        configs: [
+          {
+            kind: "Build",
+            type: "test",
+            name: "foo",
+            timeout: DEFAULT_BUILD_TIMEOUT_SEC,
+            internal: {
+              basePath: tmpDir.path,
+            },
+            spec: {},
+            source: {
+              repository: { url: repoUrl },
+            },
+          },
+        ],
+        moduleGraph: new ModuleGraph([], {}),
+        actionModes: {},
+        linkedSources: {},
+        environmentName: garden.environmentName,
+      })
+      const action = graph.getBuild("foo")
+
+      const clonePath = getRemoteSourceLocalPath({
+        name: action.key(),
+        url: repoUrl,
+        type: "action",
+        gardenDirPath: garden.gardenDirPath,
+      })
+      expect(action._config.internal.basePath.startsWith(clonePath)).to.be.true
+    })
+  })
+
   describe("file inclusion-exclusion", () => {
     const getBaseParams = ({ include, exclude }: { include?: string[]; exclude?: string[] }) => ({
       garden,

diff --git a/core/test/unit/src/graph/actions.ts b/core/test/unit/src/graph/actions.ts
@@ -48,6 +48,7 @@ describe("preprocessActionConfig", () => {
           garden,
           config,
           router,
+          linkedSources: {},
           mode: "default",
           log: garden.log,
         })
@@ -73,6 +74,7 @@ describe("preprocessActionConfig", () => {
           garden,
           config,
           router,
+          linkedSources: {},
           mode: "default",
           log: garden.log,
         })

diff --git a/docs/reference/project-config.md b/docs/reference/project-config.md
@@ -155,11 +155,11 @@ scan:
   exclude:
 
   git:
-    # Choose how to perform scans of git repositories. Defaults to `subtree`. The `subtree` runs individual git scans
-    # on each action/module path. The `repo` mode scans entire repositories and then filters down to files matching
-    # the paths, includes and excludes for each action/module. This can be considerably more efficient for large
-    # projects with many actions/modules.
-    mode: subtree
+    # Choose how to perform scans of git repositories. Defaults to `repo`. The `subtree` runs individual git scans on
+    # each action/module path. The `repo` mode scans entire repositories and then filters down to files matching the
+    # paths, includes and excludes for each action/module. This can be considerably more efficient for large projects
+    # with many actions/modules.
+    mode: repo
 
 # A list of output values that the project should export. These are exported by the `garden get outputs` command, as
 # well as when referencing a project as a sub-project within another project.
@@ -576,11 +576,11 @@ scan:
 
 [scan](#scan) > [git](#scangit) > mode
 
-Choose how to perform scans of git repositories. Defaults to `subtree`. The `subtree` runs individual git scans on each action/module path. The `repo` mode scans entire repositories and then filters down to files matching the paths, includes and excludes for each action/module. This can be considerably more efficient for large projects with many actions/modules.
+Choose how to perform scans of git repositories. Defaults to `repo`. The `subtree` runs individual git scans on each action/module path. The `repo` mode scans entire repositories and then filters down to files matching the paths, includes and excludes for each action/module. This can be considerably more efficient for large projects with many actions/modules.
 
-| Type     | Allowed Values    | Default     | Required |
-| -------- | ----------------- | ----------- | -------- |
-| `string` | "repo", "subtree" | `"subtree"` | Yes      |
+| Type     | Allowed Values    | Default  | Required |
+| -------- | ----------------- | -------- | -------- |
+| `string` | "repo", "subtree" | `"repo"` | Yes      |
 
 ### `outputs[]`
 

diff --git a/examples/remote-sources/garden.yml b/examples/remote-sources/garden.yml
@@ -1,6 +1,9 @@
 apiVersion: garden.io/v1
 kind: Project
 name: remote-sources
+scan:
+  git:
+    mode: repo
 sources:
   - name: web-services
     # use #your-branch to specify a branch, #v0.3.0 for a tag or a full length commit SHA1