ci(java): require explicit java_version for variant builds, drop repo-var fallback

[sfreudenthaler]

Fixes #35986

What

Makes .sdkmanrc the single knob for the dotCMS Java version and makes the pipeline self-heal the JDK base image instead of failing when it's missing. Follow-up to #35914 (Java 25 default) and #35915 (variant auto-build retired).

Design pivot (agreed with @wezell, #feat-java25 Jun 5): the earlier version of this PR failed fast when a variant's Java version was empty/malformed/equal-to-default. That's a stepping stone, not the destination. Instead: check whether dotcms/java-base:${version} exists and build + push it on demand if not — reused both in the variant release and in the normal dotCMS image build, so ticking or overriding .sdkmanrc "just works".

Self-healing base image

The dotCMS image is FROM dotcms/java-base:${SDKMAN_JAVA_VERSION} (dotCMS/src/main/docker/original/Dockerfile). That base was previously only ever built by the manual cicd_manual_build-java-base.yml workflow, so bumping .sdkmanrc to a version whose base hadn't been pre-built would break every build.

• New composite action core-cicd/ensure-java-base — anonymous existence check (docker buildx imagetools inspect); when the tag is missing it builds the base:
  • Docker Hub creds present → build multi-arch and push (publishes it for everyone).
  • No creds (fork PR) → build locally (--load, host arch) so the current build still resolves FROM — the build never fails just because the base was absent.
  • Restores the default Docker builder afterward so maven-job's subsequent plain docker build / docker save is unaffected.
• Wired into maven-job between get-sdkman-version and the dotCMS image build, reusing the already-resolved SDKMAN_JAVA_VERSION so the base tag and the FROM build-arg stay in lockstep. This is the single chokepoint every Docker build flows through → covers PR, trunk, release, and variant builds.
• Optional DOCKER_USERNAME / DOCKER_TOKEN secrets threaded through cicd_comp_build-phase.yml and passed by the publishing callers (release, variant, trunk, merge-queue, nightly, lts, manual-deploy) so a missing base gets published. PR builds don't pass them → graceful local-only build. The dotCMS image build itself still pulls the public base anonymously, so these are not required for a normal build.
• cicd_manual_build-java-base.yml refactored to reuse the action (force: true), removing the duplicated buildx / build-push logic — one source of truth for how the base is built.

flowchart TD
    SDK[".sdkmanrc / -Dsdkman.java.version"] --> RES["maven-job: resolve<br/>SDKMAN_JAVA_VERSION"]
    RES --> ENS["ensure-java-base action"]
    ENS -->|"exists on Docker Hub"| NOOP["no-op"]
    ENS -->|"missing + creds"| PUSH["build multi-arch + PUSH<br/>dotcms/java-base:VERSION"]
    ENS -->|"missing, no creds"| LOAD["build amd64 + --load locally"]
    NOOP --> IMG["docker build dotCMS image<br/>FROM dotcms/java-base:VERSION"]
    PUSH --> IMG
    LOAD --> IMG

Loading

Variant workflow (cicd_7-release-java-variant.yml) — manual-only guard kept

workflow_dispatch-only, so its guard never affects automated runs. java_version stays required and an equals-default request still errors (it would be a redundant duplicate of the primary release image — the empty-dispatch protection that prevents overwriting primary Docker tags). A missing base is no longer a dead-end: it's self-healed by the shared maven-job step. Also added an explicit empty-default_java check so a wrong branch name or missing/malformed .sdkmanrc fails loudly instead of the equality check silently passing.

.sdkmanrc as single source of truth in the pom (retained from earlier review feedback)

Unchanged from the prior iteration and survives the rework intact:

• jdk.min.version chains to the effective ${dotcms.core.compiler.release}.
• A build-helper:regex-property execution derives sdkmanrc.java.major from the .sdkmanrc java entry.
• An enforcer rule fails every build (local + CI, at validate) when dotcms.core.compiler.release.default doesn't match the .sdkmanrc major, naming both files in the remediation message.
• .mvn/maven-build-cache-config.xml gains runAlways for the property-read / regex-property / enforcer executions so a cache hit can't skip the drift guard.

Net effect — a future default-Java cutover (e.g. java-29) is: update .sdkmanrc + one number in parent/pom.xml, dispatch/merge as usual, and the base image is produced automatically. Forget the pom number and every build fails at validate with remediation instructions.

Why

After #35915 the variant workflow is manual-only, but an empty dispatch still silently fell back to repo vars (now pointing at the Java 25 default) — a redundant duplicate of the primary image; deleting those vars without a guard would let an empty dispatch overwrite the primary release Docker tags. More broadly, the only way to introduce a new Java version's base image was a manual pre-build step. This PR removes both foot-guns: the workflow requires a deliberate variant version, and the base image builds itself on demand wherever it's needed.

Verification

• actionlint clean on all changed workflows; every changed YAML parses.
• ensure-java-base decision logic dry-run across all paths: exists→no-op, missing+creds→push multi-arch, missing+no-creds→load amd64, force+push=false→validation-only, push=true+no-creds→error.
• pom files untouched, so prior ./mvnw validate results (default / variant-override / drift-simulation) still hold.
• Remaining (CI): confirm a normal build logs "base exists → skip"; dispatch cicd_manual_build-java-base.yml for a throwaway version; dispatch the variant workflow against a Java version whose base is absent and confirm it self-heals then builds.

⚠️ Post-merge step

Delete the now-unused repository variables (settings, not code):

RELEASE_JAVA_VARIANT_VERSION   (currently 25.0.2-ms)
RELEASE_JAVA_VARIANT_SUFFIX    (currently java-25)

🤖 Generated with Claude Code

[claude]

Claude finished @sfreudenthaler's task in 3m 50s —— View job

---

PR Review

• Read changed files
• Review workflow changes
• Review pom.xml changes
• Review new ensure-java-base action
• Review maven-job changes
• Post findings

---

Overall this is well-constructed. The approach is sound: single source of truth for the Java version, fail-fast guard on the variant workflow, self-healing base image. A few things worth a second look:

ensure-java-base/action.yml

built output is set at plan time, before the build step executes.
steps.plan.outputs.built is set to ${need_build} — if the actual Build Java Base Image step fails, the action still emits built=true. No callers in the current codebase consume this output, so it's harmless now, but could mislead if a caller is added later. Consider emitting it as a step output from a post-build step instead, or documenting that it means "build was attempted."

Restore default Docker builder is gated on steps.buildx.outcome == 'success' but runs under always().
This is the right pattern, but note: the buildx step ID appears at line 161 in the action, yet it's skipped (if: steps.plan.outputs.need_build == 'true') on the common no-op path. On the no-op path, steps.buildx.outcome is 'skipped', so the restore step is also skipped — which is correct since no builder was set up. This is fine, just confirm the no-op path leaves the default builder untouched (it does, since setup-buildx-action was never called).

No tr -d '\r' on the awk read in maven-job's get-sdkman-version step.
cicd_7-release-java-variant.yml correctly strips \r from the GitHub API response (line 141). But the get-sdkman-version step in maven-job (line 334) uses awk -F= '/^java=/ {print $2}' on the local .sdkmanrc without stripping carriage returns:

SDKMAN_JAVA_VERSION=$(awk -F= '/^java=/ {print $2}' .sdkmanrc)

If .sdkmanrc is ever committed with CRLF line endings (e.g. on a Windows dev workstation), this would produce a tag like dotcms/java-base:25.0.2-ms\r and silently fail or create a bad image tag. Low risk on Linux CI runners, but inconsistent with the CRLF guard elsewhere. Fix this →

parent/pom.xml

The enforcer <message> for the null-property case may mislead on a .sdkmanrc parse failure.
Line 1196:

<message>Property sdkmanrc.java.major was not derived from .sdkmanrc - the derive-sdkmanrc-java-major execution did not run before the enforcer.</message>

This fires when sdkmanrc.java.major is null/empty. With runAlways set in the cache config, the most likely cause of an empty value isn't a missing execution — it's a malformed .sdkmanrc (no java= line, CRLF breaking the regex, etc.). The message directing devs to check execution ordering could be a dead end. Consider: "sdkmanrc.java.major is unset — check that .sdkmanrc contains a valid java=<version> entry." Fix this →

jdk.min.version now follows -Ddotcms.core.compiler.release overrides — verify behavior for the release-11 pinned modules.
jdk.min.version = ${dotcms.core.compiler.release} means passing -Ddotcms.core.compiler.release=11 lowers the minimum JDK floor to 11. The PR says this passes the full reactor validate. If a submodule overrides dotcms.core.compiler.release in its own POM (rather than via -D), that module's jdk.min.version will also track its local override — which is probably correct. Worth confirming with the release-11-pinned modules the PR mentions.

.mvn/maven-build-cache-config.xml

runAlways with artifactId="maven-enforcer-plugin" affects all enforcer executions across all modules, not just enforce-java-version.
Every enforce goal in every submodule will be re-run on cache hits. This is likely the correct intent (all enforcement should fire regardless of cache state), but it widens scope beyond the stated goal and may measurably slow down cached builds in a large reactor. Consider scoping by execution ID if the enforcer plugin supports it, or document the intentional scope.

cicd_7-release-java-variant.yml

artifact_suffix input is still required: false even though the effective suffix is always derived.
The input description says "if not set, derived from java-version major." This is intentional flexibility, but it means a caller that supplies a non-normalized suffix (e.g. java-25 vs java25) gets a different effective suffix than one using the auto-derived value. The normalization strip at line 166 (while [[ "${artifact_suffix}" =~ ^[-_] ]]) only strips leading separators, not trailing ones or mixed ones. This is fine for the documented convention but could silently produce unexpected artifact names if a user passes something like _java-25_ms. Very low risk for a manual-only workflow.

No permissions: contents: read on the extract-version job.
The job calls gh api repos/…/contents/.sdkmanrc using ${{ github.token }}. The default token has contents: read unless the workflow-level permissions block restricts it, but the variant workflow has no top-level permissions: block. This is fine — the default is read-only — but worth adding explicitly given that the other jobs do declare permissions.

No issues found

• The artifact_suffix derivation once in extract-version and passing it through all phases correctly fixes the java21 vs java-21 mismatch flagged by Codex.
• The sdkman.java.version override in maven-job correctly wires variant JDK into the Docker image.
• The manual build-java-base workflow correctly delegates to ensure-java-base with force=true.
• Cache-hit runAlways entries correctly cover all three executions that must fire regardless of cache state.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ebcb99b3dc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[sfreudenthaler]

ready for review

[wezell]

Does this, or can we make this, update the mvn parent/pox.xml variables ? These in particular:

        <jdk.min.version>25</jdk.min.version>
        <maven.compiler.argument.source>${maven.compiler.source}</maven.compiler.argument.source>
        <maven.compiler.argument.target>${maven.compiler.target}</maven.compiler.argument.target>
        <maven.compiler.argument.testSource>${maven.compiler.testSource}</maven.compiler.argument.testSource>
        <maven.compiler.argument.testTarget>${maven.compiler.testTarget}</maven.compiler.argument.testTarget>

Or is this not a good idea? It would be very nice to have .sdkman variable be the single source of truth.

[sfreudenthaler]

Does this, or can we make this, update the mvn parent/pox.xml variables ? These in particular:

        <jdk.min.version>25</jdk.min.version>
        <maven.compiler.argument.source>${maven.compiler.source}</maven.compiler.argument.source>
        <maven.compiler.argument.target>${maven.compiler.target}</maven.compiler.argument.target>
        <maven.compiler.argument.testSource>${maven.compiler.testSource}</maven.compiler.argument.testSource>
        <maven.compiler.argument.testTarget>${maven.compiler.testTarget}</maven.compiler.argument.testTarget>

Or is this not a good idea? It would be very nice to have .sdkman variable be the single source of truth.

let me look tonight. agree and it's in the spirit of what i'm trying to push towards in this PR

[sfreudenthaler]

@wezell ready for rereivew. got it down to 2 changes. that's the best claude thinks we can do while keeping good ide support.

90fb73cb5e

[sfreudenthaler]

🚧 Converting to draft — design pivot agreed with @wezell

Parking this pending a redesign. Capturing the agreed direction from the #feat-java25 thread (Jun 5) so the context isn't lost.

What this PR does today (the narrow hardening)

• java_version becomes a required dispatch input; all vars.RELEASE_JAVA_VARIANT_* fallbacks removed
• fail-fast guard in extract-version (empty / malformed / equals-default → error)
• .sdkmanrc as single source of truth in parent/pom.xml via a regex-property + enforcer drift guard
• variant Docker image fix in maven-job (-Dsdkman.java.version now passed through)

This is all valid and green — but it's fail-fast-if-the-version-is-wrong, which is a stepping stone, not the destination.

The direction instead (@wezell)

Make .sdkmanrc the single knob and have the pipeline self-heal the base image rather than erroring:

An action that checks whether the Java base exists — e.g. dotcms/java-base:25.0.2-ms on Docker Hub. If it does, return OK. If it does not, build and push that image for that version before building dotCMS. Reuse that action here (variant release) and also earlier when building the main dotCMS image, so we can tick or override the .sdkmanrc variable and everything just works.

Net design:

1. New reusable action: ensure java-base:${version} exists → build+push if missing
2. Wire it into both the normal dotCMS image build and the variant-release workflow
3. .sdkmanrc (or an explicit override) is the only thing you change to switch Java versions — no manual base-image prep, no fail-fast dead-ends

Status / urgency

Java 25 prod rollout completed Jun 8, so the variant workflow is no longer time-pressured. Reopening for review once reworked around the base-image-existence action. The pom/enforcer single-source-of-truth piece here likely survives the rework intact.

[wezell]

Love this. Once merged, let's exercise the job by ticking our .sdkmanrc java to 25.0.2-ms