Skip to content

fix(imagebuild): derive build Job name from all build inputs - #58

Merged
ashishmax31 merged 1 commit into
mainfrom
fix/build-job-name-honors-build-inputs
Aug 2, 2026
Merged

fix(imagebuild): derive build Job name from all build inputs#58
ashishmax31 merged 1 commit into
mainfrom
fix/build-job-name-honors-build-inputs

Conversation

@ashishmax31

Copy link
Copy Markdown
Contributor

Problem

BuildJobName hashed only the source revision. Two ImageBuilds that differ solely in dockerfilePath or contextPath mapped to the same Job name in the same namespace.

#54 fixed the ImageBuild CR name (whole-spec hash), but not the Job name. Failure path when a user changes only dockerFilePath:

  1. StackResource creates ImageBuild chore: add helm chart packaging and github release to CI pipeline #2 with a new CR name.
  2. ImageBuild chore: add helm chart packaging and github release to CI pipeline #2 reconciles and computes the same Job name as fix: skip synchronous replication config when NumSynchronousReplicas is 0 #1.
  3. reconcileBuildJob does a plain Get by name — no ownership check — and finds fix: skip synchronous replication config when NumSynchronousReplicas is 0 #1's completed Job.
  4. It reports BuildComplete and sets Status.ImageUrl. No build ever runs. The tag is revision-derived, so the URL resolves to the image built from the old Dockerfile. Silent stale deploy.

Same class of bug for contextPath, repository and buildArgs — none were in the Job name.

Change

BuildJobName becomes a method on *ImageBuild that hashes the whole spec: source revision, Dockerfile path, context path, repository, build args. Mirrors what ImageBuildName already does. Cancelled is zeroed before hashing — cancelling deletes the Job, it does not change the image.

buildInputsHash generalized to any so both name helpers share it. Both controller call sites now use buildConfig.BuildJobName().

k8s name limits

The Job name is copied verbatim into the batch.kubernetes.io/job-name label on every pod the Job creates, and label values cap at 63. Enforced structurally: maxResourceNameInJob = 63 - 8 (hash) - 6 ("-build") - 1 = 48.

TestBuildJobName_HonorsK8sNameLimits asserts length <= 63 and DNS-1123 label validity across 11 cases: 200-char resource name, truncation landing on a dash, non-ASCII revision, empty resource name, empty revision, separators-only name.

Not in scope

Test plan

  • go build ./...
  • go vet ./api/... ./internal/...
  • go test ./api/builds/v1alpha1/... ./internal/... — all green
  • New regression test: same commit, different dockerfilePath (and contextPath, build args, repository) produce different Job names

Not run: make test-integration (~11 min, needs Docker).

BuildJobName hashed only the source revision, so two ImageBuilds that
differed solely in dockerfilePath or contextPath mapped to the same Job
name. PR #54 gave those builds distinct ImageBuild CR names, but the
second build's reconcile did a plain Get by Job name, found the first
build's completed Job, and reported BuildComplete without ever running a
build. The image tag is revision-derived, so the status ImageUrl then
pointed at the image built from the old Dockerfile.

Make BuildJobName a method on *ImageBuild that hashes the whole spec:
source revision, Dockerfile path, context path, repository and build
args. Cancelled is zeroed before hashing since cancelling deletes the Job
rather than changing the image.

The 63-character ceiling still holds structurally: the resource-name
segment is capped at 63 - 8 (hash) - 6 ("-build") - 1. Tests assert both
the length limit and DNS-1123 label validity across long, empty and
non-ASCII inputs, plus the regression case where only the Dockerfile path
changes.
@ashishmax31
ashishmax31 merged commit 354280b into main Aug 2, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant