diff --git a/core/src/plugins/container/helpers.ts b/core/src/plugins/container/helpers.ts index ac73d46c3e..eb61f2336e 100644 --- a/core/src/plugins/container/helpers.ts +++ b/core/src/plugins/container/helpers.ts @@ -373,15 +373,25 @@ const helpers = { moduleHasDockerfile(config: ContainerModuleConfig, version: ModuleVersion): boolean { // If we explicitly set a Dockerfile, we take that to mean you want it to be built. // If the file turns out to be missing, this will come up in the build handler. - return !!config.spec.dockerfile || version.files.includes(getDockerfilePath(config.path, config.spec.dockerfile)) + + if (!!config.spec.dockerfile) { + return true + } + + // NOTE: The fact that we overloaded the `image` field of a container module means the Dockerfile must be checked into version control + // This means it's not possible to use `copyFrom` or `generateFiles` to get it from dependencies or generate it at runtime. + // That's because the `image` field has the following two meanings: + // 1. Build an image with this name, if a Dockerfile exists + // 2. Deploy this image from the registry, if no Dockerfile exists + // This means we need to know if the Dockerfile exists before we know wether or not the Dockerfile will be present at runtime. + return version.files.includes(getDockerfilePath(config.path, config.spec.dockerfile)) }, async actionHasDockerfile(action: Resolved): Promise { - // If we explicitly set a Dockerfile, we take that to mean you want it to be built. - // If the file turns out to be missing, this will come up in the build handler. const dockerfile = action.getSpec("dockerfile") - const dockerfileSourcePath = getDockerfilePath(action.basePath(), dockerfile) - return action.getFullVersion().files.includes(dockerfileSourcePath) + // NOTE: it's important to check for the files existence in the build path to allow dynamically copying the Dockerfile from other actions using `copyFrom`. + const dockerfileSourcePath = getDockerfilePath(action.getBuildPath(), dockerfile) + return await pathExists(dockerfileSourcePath) }, /** diff --git a/core/src/vcs/vcs.ts b/core/src/vcs/vcs.ts index 1b37b367fa..e1b8a53e95 100644 --- a/core/src/vcs/vcs.ts +++ b/core/src/vcs/vcs.ts @@ -57,6 +57,12 @@ export async function validateGitInstall() { export interface TreeVersion { contentHash: string + /** + * Important! Do not use the files to determine if a file will exist when performing an action. + * Other mechanisms, e.g. the build command itself and `copyFrom` might affect available files at runtime. + * + * See also https://github.com/garden-io/garden/issues/5201 + */ files: string[] } diff --git a/core/test/unit/src/plugins/container/build.ts b/core/test/unit/src/plugins/container/build.ts index 9f7c44d319..96cef25baf 100644 --- a/core/test/unit/src/plugins/container/build.ts +++ b/core/test/unit/src/plugins/container/build.ts @@ -18,6 +18,7 @@ import { ContainerProvider, gardenPlugin } from "../../../../../src/plugins/cont import { containerHelpers } from "../../../../../src/plugins/container/helpers" import { joinWithPosix } from "../../../../../src/util/fs" import { getDataDir, TestGarden, makeTestGarden } from "../../../../helpers" +import { createFile } from "fs-extra" context("build.ts", () => { const projectRoot = getDataDir("test-project-container") @@ -85,6 +86,7 @@ context("build.ts", () => { sinon.replace(action, "getOutputs", () => ({ localImageId: "some/image" })) const buildPath = action.getBuildPath() + await createFile(join(buildPath, "Dockerfile")) const cmdArgs = getCmdArgs(action, buildPath) sinon.replace(containerHelpers, "dockerCli", async ({ cwd, args, ctx: _ctx }) => { @@ -109,6 +111,7 @@ context("build.ts", () => { sinon.replace(action, "getOutputs", () => ({ localImageId: "some/image" })) const buildPath = action.getBuildPath() + await createFile(join(buildPath, "docker-dir/Dockerfile")) const cmdArgs = getCmdArgs(action, buildPath) sinon.replace(containerHelpers, "dockerCli", async ({ cwd, args, ctx: _ctx }) => { diff --git a/plugins/jib/src/index.ts b/plugins/jib/src/index.ts index b7f9e4e7c2..9486ff0e4c 100644 --- a/plugins/jib/src/index.ts +++ b/plugins/jib/src/index.ts @@ -206,11 +206,7 @@ export const gardenPlugin = () => let projectType = spec.projectType if (!projectType) { - projectType = detectProjectType({ - actionName: action.name, - actionBasePath: action.basePath(), - actionFiles: action.getFullVersion().files, - }) + projectType = await detectProjectType(action) statusLine.info(`Detected project type ${projectType}`) } diff --git a/plugins/jib/src/util.ts b/plugins/jib/src/util.ts index 53b9fc5a5e..21764f0838 100644 --- a/plugins/jib/src/util.ts +++ b/plugins/jib/src/util.ts @@ -15,9 +15,10 @@ import { ContainerModuleBuildSpec, ContainerModuleSpec, } from "@garden-io/core/build/src/plugins/container/moduleConfig" -import { BuildAction, BuildActionConfig } from "@garden-io/core/build/src/actions/build" +import { BuildAction, BuildActionConfig, ResolvedBuildAction } from "@garden-io/core/build/src/actions/build" import { ContainerBuildOutputs } from "@garden-io/core/build/src/plugins/container/config" import { Resolved } from "@garden-io/core/build/src/actions/types" +import { pathExists } from "fs-extra" interface JibBuildSpec { dockerBuild?: boolean @@ -58,40 +59,32 @@ const gradlePaths = [ const mavenPaths = ["pom.xml", ".mvn"] const mavendPaths = ["pom.xml", ".mvnd"] -export function detectProjectType({ - actionName, - actionBasePath, - actionFiles, -}: { - actionName: string - actionBasePath: string - actionFiles: string[] -}): JibPluginType { +export async function detectProjectType(action: ResolvedBuildAction): Promise { // TODO: support the Jib CLI for (const filename of gradlePaths) { - const path = resolve(actionBasePath, filename) - if (actionFiles.includes(path)) { + const path = resolve(action.getBuildPath(), filename) + if (await pathExists(path)) { return "gradle" } } for (const filename of mavenPaths) { - const path = resolve(actionBasePath, filename) - if (actionFiles.includes(path)) { + const path = resolve(action.getBuildPath(), filename) + if (await pathExists(path)) { return "maven" } } for (const filename of mavendPaths) { - const path = resolve(actionBasePath, filename) - if (actionFiles.includes(path)) { + const path = resolve(action.getBuildPath(), filename) + if (await pathExists(path)) { return "mavend" } } throw new ConfigurationError({ - message: `Could not detect a gradle or maven project to build ${actionName}`, + message: `Could not detect a gradle or maven project to build ${action.longDescription()}`, }) } diff --git a/plugins/jib/test/util.ts b/plugins/jib/test/util.ts index 21e62bd150..f73f770443 100644 --- a/plugins/jib/test/util.ts +++ b/plugins/jib/test/util.ts @@ -9,10 +9,12 @@ import { expectError, makeTestGarden, TestGarden } from "@garden-io/sdk/build/src/testing" import { expect } from "chai" import { detectProjectType, getBuildFlags, JibBuildAction } from "../src/util" -import { resolve } from "path" +import { join, resolve } from "path" import { ResolvedConfigGraph } from "@garden-io/core/build/src/graph/config-graph" import { Resolved } from "@garden-io/core/build/src/actions/types" import { gardenPlugin } from "../src/index" +import { rm } from "fs/promises" +import { createFile } from "fs-extra" describe("util", function () { // eslint-disable-next-line no-invalid-this @@ -23,6 +25,7 @@ describe("util", function () { let garden: TestGarden let graph: ResolvedConfigGraph let action: Resolved + let buildPath: string before(async () => { garden = await makeTestGarden(projectRoot, { @@ -33,41 +36,35 @@ describe("util", function () { beforeEach(async () => { graph = await garden.getResolvedConfigGraph({ log: garden.log, emit: false }) action = graph.getBuild("foo") + buildPath = action.getBuildPath() }) - describe("detectProjectType", () => { - it("returns gradle if action files include a gradle config", () => { - expect( - detectProjectType({ - actionName: "foo", - actionBasePath: "/foo", - actionFiles: ["/foo/build.gradle"], - }) - ).to.equal("gradle") + describe("detectProjectType", async () => { + afterEach(async () => { + try { + await rm(join(`${buildPath}/build.gradle`), { recursive: true }) + } catch (e) {} + try { + await rm(join(`${buildPath}/pom.xml`), { recursive: true }) + } catch (e) {} }) - it("returns maven if action files include a maven config", () => { - expect( - detectProjectType({ - actionName: "foo", - actionBasePath: "/foo", - actionFiles: ["/foo/pom.xml"], - }) - ).to.equal("maven") + it("returns gradle if action files include a gradle config", async () => { + await createFile(resolve(buildPath, "build.gradle")) + + expect(await detectProjectType(action)).to.equal("gradle") }) - it("throws if no maven or gradle config is in the action file list", () => { - void expectError( - () => - detectProjectType({ - actionName: "foo", - actionBasePath: "/foo", - actionFiles: [], - }), - { - contains: "Could not detect a gradle or maven project to build foo", - } - ) + it("returns maven if action files include a maven config", async () => { + await createFile(resolve(buildPath, "pom.xml")) + + expect(await detectProjectType(action)).to.equal("maven") + }) + + it("throws if no maven or gradle config is in the action file list", async () => { + await expectError(() => detectProjectType(action), { + contains: `Could not detect a gradle or maven project to build ${action.longDescription()}`, + }) }) })