From 3417b8c0686ef5352697cd55a27ea274296b8b89 Mon Sep 17 00:00:00 2001 From: Thorarinn Sigurdsson Date: Tue, 23 Jan 2024 21:43:34 +0100 Subject: [PATCH] fix(core): fix "repo" scan mode for remote actions 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. --- core/src/constants.ts | 2 +- core/src/garden.ts | 2 +- core/src/graph/actions.ts | 83 +++++++++++-------- .../src/actions/action-configs-to-graph.ts | 40 +++++++++ core/test/unit/src/graph/actions.ts | 2 + docs/reference/project-config.md | 18 ++-- examples/remote-sources/garden.yml | 3 + 7 files changed, 106 insertions(+), 44 deletions(-) diff --git a/core/src/constants.ts b/core/src/constants.ts index 967b6245dc..59627479f5 100644 --- 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 index d68847e4c6..b02e09f611 100644 --- 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 index 9b84480462..5e2b48f453 100644 --- 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,7 +357,8 @@ async function processActionConfig({ log: Log configsByKey: ActionConfigsByKey mode: ActionMode - linkedSources: LinkedSourceMap + linkedSource: LinkedSource | null + remoteSourcePath: string | null templateContext: ActionConfigContext supportedModes: ActionModes scanRoot?: string @@ -360,30 +366,6 @@ async function processActionConfig({ 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 { 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 index 504123c633..8d802c4948 100644 --- 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 index e6a4de273c..723bfca816 100644 --- 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 index c9b0781953..e408ff474f 100644 --- 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 index cd77b282c1..fb2aca3e6a 100644 --- 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