Twelve Fixes to a Pipeline That Was Already Passing¶
None of these were reported by anything. The pipeline was green before each fix and green after it. What changed is what the green meant: a coverage graph that was measuring infrastructure failures as test regressions, a success banner claiming images were published when nothing had been pushed, and a tag that would have deployed the four-character string null if one stage had ever been skipped.
The fixes are numbered inline in the Java Jenkinsfile as FIX #1 through FIX #13, with #11 absent because it was folded into another change during review. Six more, numbered R4#n, live in the GitHub Actions workflow. I kept the numbering in the files rather than in a changelog because the reason for each one only makes sense next to the line it protects.
They sort into four kinds of lie.
Lie One: The Interpolation That Renders "null"¶
Groovy interpolates ${VAR} inside a """...""" block at parse time. The shell inside that block expands $VAR at runtime. They look identical and they happen at completely different moments.
The CD handoff stage writes an image tag into the deployment repository. It opens with a guard:
The backslash matters more than anything else on the line. \${IMAGE_TAG} passes through Groovy untouched and reaches the shell as ${IMAGE_TAG}, which the shell evaluates when the stage runs.
Write it unescaped as ${IMAGE_TAG} and Groovy resolves it while parsing the pipeline, long before the stage executes. If the Versioning stage has not run, env.IMAGE_TAG is null, and Groovy renders null into a string as the literal four characters null. The guard becomes:
which is false, because "null" is a perfectly good non-empty string. The guard passes, the manifest is written with IMAGE_TAG=null, the commit lands, and the deployment tool is asked to pull an image tagged null.
The same escape rule applies to \${GIT_USER} and \${GIT_TOKEN}, injected by withCredentials into the shell environment. Those variables do not exist in Groovy scope at all, so writing them unescaped yields an empty string and a clone with no credentials.
The general shape
Any templating language nested inside another one has two evaluation times and one syntax. Groovy-in-shell, Helm-in-YAML, Terraform-in-JSON: the failure is never a syntax error, it is a value resolving at the wrong moment and producing something plausible.
Lie Two: Error Handling That Swallows the Wrong Errors¶
The CD stage should not commit when the tag is unchanged, because a re-run of the same commit produces an identical manifest and an empty commit is noise. The obvious way to express that:
This is wrong, and it is wrong in a way that only shows up on the day something else breaks. || echo catches every non-zero exit from git commit, not just the "nothing staged" one. Detached HEAD, a merge conflict, a corrupt .git/config, a failed pre-commit hook: all of them print "Nothing to commit" and let the stage continue to the push, which then fails with a non-fast-forward error that has nothing to do with the actual cause.
The replacement asks the precise question:
git diff --cached --quiet \
&& echo "ℹ️ Nothing to commit — image tag unchanged" \
|| git commit -m "ci: update java-monolith image tag to \${IMAGE_TAG} [skip ci]"
git diff --cached --quiet exits 0 when nothing is staged and 1 when something is. It answers "is there anything to commit" instead of "did commit fail", and it leaves real commit failures free to fail loudly.
|| true and || echo are the two most common ways to convert a loud failure into a silent one. Both are legitimate when the condition you are suppressing is the only one that can occur. Neither is legitimate as a way to make a red stage green.
Lie Three: Reports That Measure the Wrong Thing¶
Three fixes are about publishers producing data that looks like a code problem and is actually an infrastructure problem.
Coverage that silently vanishes. The SonarQube stage passes a JaCoCo report path. The original version composed it from APP_DIR:
With APP_DIR = '.' that expands to .../workspace/./target/site/jacoco/jacoco.xml. Every POSIX tool treats /./ as a no-op, so this looks harmless. SonarQube Scanner 4.x resolves it through Java's new File(path), which does not normalise the ./ segment, looks for a literal path that does not exist, finds no report, and proceeds without coverage data. No error. The analysis completes, the quality gate evaluates, and coverage reads zero because none was ingested.
The fix is to stop composing the path from a variable that is usually .:
Coverage trends poisoned by pre-test failures. recordCoverage in the post { always } block runs on every build, including builds that died in the Trivy scan three stages before any test ran. With no report to read, it writes a zero-coverage data point, and the trend graph shows a cliff. Someone reads that graph as a test regression, and it was a scanner timeout.
if (fileExists("${APP_DIR}/target/surefire-reports")) {
junit testResults: "${APP_DIR}/target/surefire-reports/*.xml", allowEmptyResults: true
recordCoverage(tools: [[parser: 'JACOCO', pattern: "${APP_DIR}/target/site/jacoco/jacoco.xml"]],
sourceCodeRetention: 'EVERY_BUILD')
} else {
echo '⏭️ Skipping junit + recordCoverage — no surefire-reports directory found.'
}
The guard distinguishes "tests ran and covered nothing" from "tests never ran". Only the first belongs on a trend graph.
A success banner that overstates. The publish stages are gated to main. The success message was not, so a green feature-branch build printed a banner listing three registries and an image tag as though everything had shipped:
def published = (env.GIT_BRANCH != null && env.GIT_BRANCH ==~ /^(origin\/)?main$/)
? 'PUBLISHED to all registries ✅'
: 'NOT PUBLISHED — non-main branch (build + scan only)'
A build summary is read far more often than the stage list above it. If it can be read as a claim about what shipped, it has to be conditional on what actually shipped.
Lie Four: Time Bombs¶
These four were all correct on the day they were written and become wrong later without anyone touching them.
The abbreviated SHA that grows. The image tag embeds a short commit SHA:
--short with no length gives Git the freedom to pick the shortest unambiguous prefix for the current repository. That is 7 characters today and 8 the day the object count crosses a threshold. Tag format changes silently, mid-project, and anything parsing tags by position breaks. Pin it:
The base image that never updates. docker build without --pull uses whatever base image is in the local daemon cache, which on a long-lived self-hosted agent can be months stale. In a pipeline whose entire purpose includes a Trivy scan, building on a cached base means scanning an image nobody would ever deploy, and passing. --pull forces a digest check and downloads only if it changed.
The directory name that collides. The CD stage clones into a working directory and cleans it first with rm -rf. Called cd-repo, that is one mkdir away from deleting real application source if anyone ever adds a directory by that name. Renaming to _cd_repo_tmp makes the collision implausible. The cost is nothing; the failure mode it removes is destroying uncommitted work in a workspace.
The branch name that gets renamed. git push origin main fails with src refspec main does not match any the day the deployment repository's default branch changes. git push origin HEAD pushes whatever is checked out and does not care what it is called.
Two That Are Purely About Blast Radius¶
docker logout needs an argument. The publish stages log into three registries in sequence. Bare docker logout on some Docker Engine versions does not just drop the Docker Hub session, it clears ~/.docker/config.json entirely, taking GHCR and Nexus credentials with it. Every logout in these pipelines names its registry:
Agent labels, not agent any. agent any will dispatch to a Windows node the moment one joins the controller, and every sh step, every trivy call and every docker build fails in a way that looks like the pipeline is broken rather than misplaced.
What the Numbering Is Actually For¶
The comments are long. A reviewer could reasonably call them excessive, and I would not have written them if the fixes had been for bugs that announce themselves.
But every fix above shares a property: the code was working when I changed it. There is no failing test to encode the reasoning, no incident ticket to link, and no diff that explains itself. Delete the comment and the next person to read git rev-parse --short=7 HEAD sees a suspiciously specific number and no reason not to simplify it. Delete the comment on git diff --cached --quiet and || echo looks like the cleaner idiom, because it is shorter and it reads better.
A comment that explains what a line does is noise, because the line already says that. A comment that explains what happens when someone removes it is the only durable place to store a decision that no test can hold.
That is the entire argument for keeping the numbering in the file instead of in a commit message nobody will run git blame to find.
Source¶
- Java Jenkinsfile,
FIX #1throughFIX #13 - Java GitHub Actions workflow,
FIX R4#2throughR4#11 - Python Jenkinsfile and Node Jenkinsfile, which carry the same fixes forward by reference
Series: CI Pipeline Engineering Across Three Applications (Part 5 of 8)