fix(docker): build the image version from the workflow input, not git tags by mbevc1 · Pull Request #1140 · kosli-dev/cli

mbevc1 · 2026-08-29T13:54:29Z

Changes

Simplify passing tags to Docker builds and smoke tests
Amend the logic to also treat non v prefixed versions as dev for updating purposes - saving update check roundtrips for intermediate Docker images

Details

The image derived its version from git describe inside the build while the smoke test inferred it from the image tag; a release tag cut at the same commit as an in-flight main pipeline made the two disagree and turned the smoke test red.

The version is now resolved once in prepare — a v-prefixed tag as-is, anything else as dev+<tag> — and both the build arg and the smoke-test expectation read that one output. The build job no longer fetches tags. The match is loose (^v[0-9]) so -rc tags still count as releases.

In the Makefile, VERSION is honoured only as a command-line assignment so an ambient one cannot stamp a release; metadata clearing keys on BINARY_VERSION rather than GIT_TAG, so an explicit version no longer picks up a +<sha> suffix; and make docker passes the tag at HEAD.

EXPECTED_VERSION is now required by the smoke test and asserted by equality.

Images report what they did before — v2.39.0 for releases, dev+<sha> otherwise — except the racy case. Supersedes the fetch-tags approach from #1137.

Checklist

Docs are autogenerated from CLI help — make sure the help text reflects your changes
Helm chart (charts/k8s-reporter/) updated, if needed. Note: these changes live in a separate PR
Terraform provider and related changes (terraform-provider-kosli, terraform-aws-evidence-reporter, terraform-aws-kosli-reporter) updated, if needed

claude · 2026-08-29T13:55:06Z

Claude finished @mbevc1's task in 6m 7s —— View job

Review — PR #1140

Gather context (diff, related files, prior review threads)
Phase 1: static analysis
Phase 2: review the four changed files
Post review

Static analysis

check	result
`go build ./...`	pass
`go vet ./...`	pass
`go mod tidy` + `git diff go.mod go.sum`	clean

The final diff contains no Go changes — internal/version/update_check.go and its test were reverted in 70f0350, so the earlier review thread about the vacuous TestCheckForUpdate_ShaTaggedBuild is moot.

End-to-end trace

I followed both callers through to version --short rather than reasoning from the diff alone:

release.yml — on: push: tags: v* → dawidd6/action-get-tag → v2.39.0 → matches ^v[0-9] → VERSION=v2.39.0 → Dockerfile ARG → make build VERSION=v2.39.0 (origin = command line) → version=v2.39.0, metadata cleared → --short prints v2.39.0 = EXPECTED_VERSION. ✅

main.yml — pre-build emits head -c 8 of the sha → no ^v[0-9] → VERSION=dev+2cd8d803 → version=dev+2cd8d803, metadata cleared → --short prints dev+2cd8d803. ✅

Three things I specifically wanted to confirm, all of which hold:

Dropping fetch-tags: true is genuinely safe. GIT_TAG is recursively expanded and only referenced in the non-command-line branch, so with VERSION=… on the command line git describe never runs inside the image build. GIT_COMMIT/GIT_DIRTY still need .git, which .dockerignore deliberately keeps.
The reverted Go change really is unnecessary. checkForUpdateWithURL short-circuits on strings.HasPrefix(currentVersion, "dev") (update_check.go:68) before the HTTP call, and version.go:52 passes GetVersion(), which for branch images is now the literal dev+<tag>. The PR body's "saving update check roundtrips" goal is met by the workflow change alone.
Making EXPECTED_VERSION required fails safe. If the output ever arrived empty, the :? exits before the results file is seeded — and the attest step's if [ ! -s "$RESULTS_FILE" ] branch (docker.yml:375) turns that into a non-compliant attestation rather than a silent pass. Worth stating, since that guard is what makes the newly-mandatory variable safe.

Findings

1. scripts/docker-smoke-tests.sh — the shape assertion for dev builds is gone (nit). Detail and a suggested three-line fix are inline. Short version: the old code asserted ^dev\+[0-9a-f]{7,}$ independently; now both sides read needs.prepare.outputs.version, so the comparison is the build agreeing with itself. Low severity — the classifier only ever sees two shapes from the current callers.

What's good here

The metadata fix is the real content of this PR and it's correct. Moving the clear condition from GIT_TAG to BINARY_VERSION is what closes run 33247442774: make build VERSION= at a tagged HEAD used to report a bare dev with the sha silently dropped, and now reports dev+<sha>.
dev+<tag> rather than a bare tag for branch images is the better call than the intermediate 075d126a state — it keeps HasPrefix(…, "dev") meaningful and avoided a Go change entirely. Reverting the earlier update_check.go edit once that was true is the right instinct.
filter-out command line override is subtle and the comment earns its place, including the "origin yields multi-word strings" note. The environment override case it defends against needs the makefile to assign VERSION first — as you established, unreachable today, but cheap insurance against a later VERSION ?=.
The Dockerfile stage-level ARG re-declaration carries a comment explaining why, which is exactly the kind of thing that gets deleted by someone tidying up otherwise.
The version rule now lives in one place with a stated reason, and the build is a function of the build request rather than of the checkout's tags — which is what Published container images report their version as dev, and their tree state as dirty #1133 actually asked for.

No blocking issues. LGTM

Note: make and ad-hoc shell invocations were blocked by the sandbox in this run, so the Makefile resolution matrix was verified by reading the rules rather than executing them — it agrees with the matrix you posted.
· branch 20260829_docker_improve

mbevc1 · 2026-08-29T14:05:10Z

Findings 1 and 2 fixed in 67a86b8 and answered on their threads. Closing out the other two.

Finding 3 (^v[0-9] anchoring) — taken. Now ^v[0-9][A-Za-z0-9.+_-]*$ in prepare. Unreachable in practice as you say (docker tags reject ' before ldflags would see it), but the strict/loose split it creates with the smoke-test tripwire is useful on its own — details on the finding-1 thread.

Finding 4 (global ARG VERSION="") — no change, agreed it isn't load-bearing. Keeping it as the contract for docker build --build-arg, since that's the documented way to build a release image by hand. The comment sits on it rather than on the stage-level ARG, which is the half that actually does the work.

make ldflags, as requested:

invocation	`version`	`metadata`	reports
`make ldflags`	`v2.39.0`	(cleared)	`v2.39.0`
`make ldflags VERSION=`	(unset → `dev`)	`0b757e0f`	`dev+0b757e0f`
`make ldflags VERSION=v9.9.9`	`v9.9.9`	(cleared)	`v9.9.9`
`VERSION=1.2.3 make ldflags`	`v2.39.0`	(cleared)	`v2.39.0`

Row 2 is the case that was failing in run 33247442774: HEAD tagged v2.39.0, build correctly reports dev+<sha>. Row 4 is finding 2.

… tags The docker image derived its version from `git describe` inside the build, while the smoke test inferred the expected version from the image tag. Those two disagree whenever a release tag is cut at the same commit as an in-flight push-to-main pipeline: the build picks up the new tag and reports v<x.y.z>, the test still expects dev+<sha>, and the version smoke test fails. Resolve the version once in the `prepare` job and feed it to both sides — the build bakes it in as a VERSION build arg, the smoke test asserts the image reports it. This is what #1133 originally suggested; #1137 closed that issue with `fetch-tags: true` instead, which is what let a stray tag reach the build. The build job no longer fetches tags, so the image is a function of the build request rather than of whichever tags the checkout happens to have by the time it runs. The Makefile needs `origin` rather than `ifdef` to tell "VERSION not passed" (local build, fall back to the tag at HEAD) from "VERSION passed empty" (CI, not a release). Metadata clearing moves from GIT_TAG to BINARY_VERSION so an explicit version is self-consistent: `make build VERSION=v2.39.0` on an untagged tree previously produced v2.39.0+<sha>, matching neither goreleaser nor the smoke test. `make docker` passes the tag at HEAD so a local image build at a release tag still reports it; a bare `docker build .` reports dev+<sha>.

Address review feedback on the version build-arg change. The Makefile now honours VERSION only as a command-line assignment. `origin` reports `environment` for an inherited variable, so an ambient VERSION in the caller's shell beat the tag at HEAD — and with metadata cleared it stamped a fake clean release. The Dockerfile passes VERSION on the command line, so the one caller that needs the override still has it. The tag classifier in `prepare` is anchored at both ends: the value reaches single-quoted ldflags, so a tag carrying a quote or whitespace is no longer treated as a release. Build arg and smoke-test expectation come from one workflow output, so nothing was checking the mapping itself — a misclassification would make both halves agree on the wrong version and pass. The smoke test now re-derives the release case from the image tag: a tag matching the looser ^v[0-9] must carry its own version, so a tag the strict classifier rejects fails loudly instead of being published as dev+<sha>.

`origin` returns "environment override" when the makefile assigns VERSION and `-e` makes the environment win. `filter` matched the trailing `override` and treated that as a command-line assignment, re-opening the hole the comment above it claims to close. `filter-out` keys on the leftover word instead, so only a genuine command-line or override-directive assignment counts. The Makefile does not assign VERSION today, so this is not currently reachable — it stops a later `VERSION ?=` default from quietly restoring the bug. The smoke-test tripwire only checked one direction: a release tag with a wrong or empty version. The mirror case — a sha tag carrying a release version, which would publish a dev image reporting v<x.y.z> — passed. Derive the expectation from the image tag once and compare, covering both.

Drop the release classifier. The image now reports whatever tag it is published under — a release version from release.yml, a short sha from main.yml — so the build arg and the smoke-test expectation are the same expression and cannot drift. That removes the prepare output, both tag regexes and the smoke-test tripwire they needed. Non-release images now report the sha instead of dev+<sha>, so the update check keys on the v prefix a release always has rather than on the "dev" string that no longer appears. It skips for dev builds, dev+<sha> and shas like, and now does so before the HTTP call rather than after it. EXPECTED_VERSION is always set by CI, so the smoke test requires it and asserts equality; its dev+<sha> branch is gone.

Resolve the version once in prepare rather than passing the tag straight through: a v-prefixed tag is reported as-is, anything else becomes dev+<tag>. Both the build arg and the smoke-test expectation read that one output, so the baked version and the asserted one still cannot drift. Branch images therefore keep reporting dev+<sha> exactly as before, which makes the update check's existing "dev" prefix test correct again — so the v-prefix change to update_check.go and its test are reverted. The match is deliberately loose. This repo cuts -rc tags, which the stricter ^v[0-9]+\.[0-9]+\.[0-9]+$ used in install-script-tests.yml would misread as dev builds.

## Summary Adds the changelog entry for CLI **v2.39.2**, plus the Docker image version fix from v2.39.0 that was missing. Rebased on `main` after #376 merged. #376 had already documented v2.39.0 and v2.39.1, so this PR is now purely additive — no deletions. ## Conflict resolution Four conflicts in `changelog/index.mdx`, resolved as follows: - **v2.39.2** — new, kept as generated. Placed above the August 31 Platform entry, keeping the file newest-first. - **v2.39.0 and v2.39.1 prose** — kept the versions already on `main` from #376 and discarded this branch's rewordings, since they had been through review and use the correct command name (see below). - **The August 24–28 Platform entries** — kept `main`'s; this branch predated them. ## The Docker version fix This branch carried a Docker version bullet on both v2.39.0 and v2.39.1, and `main` had neither. Checking upstream, only one of them is a user-facing fix: - **v2.39.0** ([cli#1137](kosli-dev/cli#1137)) — the real fix. Published images identified themselves as `dev+<sha>` instead of their release tag, so `kosli version` in a container could not name the release. Two causes in the Docker build path, both silent. Any image published before v2.39.0 is affected. **Kept**, reworded around that symptom. - **v2.39.1** ([cli#1140](kosli-dev/cli#1140)) — not a user-facing fix. It resolves the version once in the workflow instead of via `git describe` in the build, superseding cli#1137's `fetch-tags` approach, and fixes a smoke-test flake when a release tag is cut at the same commit as an in-flight main pipeline. Its own description says images "report what they did before … except the racy case". **Dropped** as an internal build change. That also removes the problem of two consecutive releases each claiming to fix the image version with different causes. Both bullets appeared only in the releases' "What's Changed" lists rather than their curated sections, which is likely why the first pass missed them. ## One correction worth noting This branch described the v2.39.1 help-text change as affecting `kosli snapshot azure-apps`. That name comes from the upstream release note, but the command is `kosli snapshot azure` — the generated reference page is `client_reference/kosli_snapshot_azure.md` and there is no `azure-apps` page. `main`'s wording is correct and links the reference; that is what was kept. ## Checks - `mint broken-links` — clean apart from the pre-existing `/getting_started/service-accounts` break in `tutorials/working_with_controls.mdx`, which is on `main` and untouched here. - `<Update>` open/close tags balance at 117 each; no empty headings; entries remain in newest-first date order. --------- Co-authored-by: mintlify[bot] <109931778+mintlify[bot]@users.noreply.github.com> Co-authored-by: Dan Grøndahl <dan@kosli.com>

@mbevc1

kosli-cli 2.39.1 Created-by: HarmonybrewBot Commit-by: HarmonybrewBot Merged-by: HarmonybrewBot Description: Created by `brew bump` --- Created with `brew bump-formula-pr`.<details> <summary>release notes</summary> <pre>- Improved help text wording for `attest jira`, `attest sonar`, and `snapshot azure-apps` commands. - Fixed doc generation: carets (`^`) in example titles are now rendered as backticks in Mintlify output. ## What's Changed * fix(docs): minor Mintify docs generation fixes by @mbevc1 in kosli-dev/cli#1139 * chore(claude): prevent code comment proliferation by @mbevc1 in kosli-dev/cli#1141 * fix(docker): build the image version from the workflow input, not git tags by @mbevc1 in kosli-dev/cli#1140 **Full Changelog**: https://github.com/kosli-dev/cli/compare/v2.39.0...v2.39.1</pre> <p>View the full release notes at <a href="https://github.com/kosli-dev/cli/releases/tag/v2.39.1">https://github.com/kosli-dev/cli/releases/tag/v2.39.1</a>.</p> </details> <hr> See merge request: Harmonybrew/homebrew-core!18114

github-actions Bot added github-actions fix labels Aug 29, 2026

mbevc1 changed the title ~~fix(docker): build the image version from the workflow input, not git…~~ fix(docker): build the image version from the workflow input, not git tags Aug 29, 2026