diff --git a/Dockerfile b/Dockerfile index 38fdf47..a81b61f 100755 --- a/Dockerfile +++ b/Dockerfile @@ -23,7 +23,15 @@ COPY backend/ ./ # # writeBuildInfo runs against the compiled output, so it has to follow tsc, and # it warns rather than fails when it finds no .git. -RUN npm run build && node dist/writeBuildInfo.js +# Passed in rather than discovered. Whoever builds knows the commit; the build +# does not go looking for it, which is what broke every deploy when it did +# (#235) and what building in CI did not fix (#237). Empty by default, so a +# build that does not pass one behaves exactly as before — Portainer cannot +# supply it and still deploys, which is the property that must not regress. +# +# docker build --build-arg GIT_COMMIT="$(git rev-parse --short HEAD)" . +ARG GIT_COMMIT= +RUN GIT_COMMIT="$GIT_COMMIT" npm run build && GIT_COMMIT="$GIT_COMMIT" node dist/writeBuildInfo.js FROM node:20-bookworm-slim WORKDIR /app diff --git a/backend/src/writeBuildInfo.ts b/backend/src/writeBuildInfo.ts index ea15e88..fc17d7f 100644 --- a/backend/src/writeBuildInfo.ts +++ b/backend/src/writeBuildInfo.ts @@ -37,9 +37,32 @@ function findGitDir(explicit?: string): string | null { return candidates.find((candidate) => existsSync(candidate)) ?? null; } -export function buildStamp(gitDir: string | null): BuildInfo { +/** + * A commit passed in by whoever is building, or null. + * + * This is the half that actually works in the environments that matter. The + * Dockerfile deliberately does not copy `.git` — doing so broke every Portainer + * deploy (#235) — and #237 established that building in CI changes nothing, + * because the copy is what was missing rather than the history. So the builder + * has to hand the commit over rather than the build going to look for it (#248). + * + * Empty is treated as absent. A `--build-arg GIT_COMMIT=` with nothing after it + * is what an unset shell variable expands to, and stamping the image with an + * empty string would be worse than saying "unknown" — it reads as a commit that + * happens to be blank rather than as one nobody supplied. + */ +function passedCommit(value: string | undefined): string | null { + return value !== undefined && value.trim() !== '' ? value.trim() : null; +} + +export function buildStamp(gitDir: string | null, passed?: string): BuildInfo { + const supplied = passedCommit(passed); + return { - commit: gitDir ? resolveCommit(gitSourceAt(gitDir)) : UNKNOWN_COMMIT, + // The passed value wins. It is the only one available where this matters, + // and reading .git remains the fallback so a local build still stamps + // itself without anyone having to remember the argument. + commit: supplied ?? (gitDir ? resolveCommit(gitSourceAt(gitDir)) : UNKNOWN_COMMIT), // Whole seconds: this is read by a person comparing it to when they // pressed a button, not by anything that needs precision. builtAt: new Date().toISOString().replace(/\.\d{3}Z$/, 'Z') @@ -50,15 +73,16 @@ export function buildStamp(gitDir: string | null): BuildInfo { // deployment — the same reasoning as backfillImageReencode.ts (#231). if (require.main === module) { const gitDir = findGitDir(process.argv[2]); - const stamp = buildStamp(gitDir); + const stamp = buildStamp(gitDir, process.env.GIT_COMMIT); if (stamp.commit === UNKNOWN_COMMIT) { // Loud, because a deploy that cannot say what it is defeats the point of // the stamp — but a warning, not a failure. const where = gitDir ? ` at ${gitDir}` : ''; console.warn( - `[build-info] no readable .git found${where} — ` + - `the admin will report the commit as "${UNKNOWN_COMMIT}"` + `[build-info] no GIT_COMMIT passed and no readable .git found${where} — ` + + `the admin will report the commit as "${UNKNOWN_COMMIT}". ` + + `Pass --build-arg GIT_COMMIT="$(git rev-parse --short HEAD)" to stamp it.` ); } diff --git a/backend/tests/unit/buildInfo.test.ts b/backend/tests/unit/buildInfo.test.ts index fd453fa..ee98122 100644 --- a/backend/tests/unit/buildInfo.test.ts +++ b/backend/tests/unit/buildInfo.test.ts @@ -1,4 +1,7 @@ import { resolveCommit, GitSource, UNKNOWN_COMMIT } from '../../src/buildInfo'; +// Safe to import: writeBuildInfo guards its side effects behind +// require.main === module, so pulling in buildStamp cannot rewrite a stamp. +import { buildStamp } from '../../src/writeBuildInfo'; // A source with nothing in it, so each test states only the files it cares // about. Every field is deliberately explicit — a missing `.git` is a normal @@ -107,3 +110,37 @@ describe('resolveCommit', () => { expect(resolveCommit(source({ head: ' \n' }))).toBe(UNKNOWN_COMMIT); }); }); + +describe('buildStamp with a commit passed in', () => { + // The half that works where it matters. The Dockerfile does not copy .git — + // doing so broke every Portainer deploy (#235) — and building in CI did not + // change that (#237), so the builder has to hand the commit over (#248). + it('prefers a passed commit over reading .git', () => { + expect(buildStamp('/nonexistent/.git', '3085970').commit).toBe('3085970'); + }); + + it('uses a passed commit when there is no .git at all', () => { + expect(buildStamp(null, '3085970').commit).toBe('3085970'); + }); + + it('trims a passed commit', () => { + expect(buildStamp(null, ' 3085970 ').commit).toBe('3085970'); + }); + + // `--build-arg GIT_COMMIT=` with an unset shell variable expands to this. + // Stamping an empty string would read as a commit that happens to be blank + // rather than as one nobody supplied. + it.each(['', ' '])('treats %p as no commit at all', (passed) => { + expect(buildStamp(null, passed).commit).toBe('unknown'); + }); + + it('still says unknown when nothing is passed and there is no .git', () => { + expect(buildStamp(null).commit).toBe('unknown'); + }); + + // The property #235 bought and that must not regress: a build with neither + // still produces a stamp rather than failing. + it('always produces a builtAt', () => { + expect(buildStamp(null).builtAt).toMatch(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z$/); + }); +});