Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: allow generated files in build actions #5230

Merged
merged 1 commit into from
Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions core/src/plugins/container/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
},

/**
Expand Down
6 changes: 6 additions & 0 deletions core/src/vcs/vcs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[]
}

Expand Down
3 changes: 3 additions & 0 deletions core/test/unit/src/plugins/container/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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 }) => {
Expand All @@ -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 }) => {
Expand Down
6 changes: 1 addition & 5 deletions plugins/jib/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`)
}

Expand Down
27 changes: 10 additions & 17 deletions plugins/jib/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()}`,
})
}

Expand Down
57 changes: 27 additions & 30 deletions plugins/jib/test/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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, {
Expand All @@ -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()}`,
})
})
})

Expand Down