fix: allow generated files in build actions (#5230)

In the container and jib-container build actions, we checked for the
existance of files checked in to version control for certain checks.

Garden allows for files to be injected into the build action dynamically
at run time, e.g. by using `copyFrom` and `generateFiles`.

We want to check for actual presence of files in the action's build
directory instead of checking for the files that are included from
version control.

Fixes #5201

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<ContainerBuildAction>): Promise<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.
     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)
   },
 
   /**

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[]
 }
 

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 }) => {

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}`)
               }
 

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<JibPluginType> {
   // 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()}`,
   })
 }
 

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<JibBuildAction>
+  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()}`,
+      })
     })
   })