Upgrade ARG default values used in Docker FROM instructions (hold for rewrite release) - #1216
Merged
Conversation
…ns (#1213)" This reverts commit d33eca3. The recipe reached for rewrite-docker API that no released version of rewrite carries yet. `Docker.Argument.getText()`, `getTextWithVariables()` and `hasEnvironmentVariables()` arrived in openrewrite/rewrite#8576, and `org.openrewrite.docker.trait.ImageName` in openrewrite/rewrite#8599, both landed 2026-08-21, one day after v8.90.3. The Moderne CLI loads the LST classes in its own classloader, so a recipe runs against the rewrite-docker the CLI bundles rather than the one this artifact resolves. CLI 4.6.3 bundles rewrite-docker 8.90.3, where `Docker.Argument` exposes only `getContents()` and `ImageName` does not exist. `visitFile` reads a global `ARG` through `getText()` and `visitFrom` opens with `hasEnvironmentVariables()`, so the first `FROM` of every Dockerfile would raise `NoSuchMethodError`, surfacing as error markup on every Dockerfile of every `UpgradeToJava*` run. Nothing is lost by waiting: v3.42.1 predates this commit, so the breakage has not shipped. The state restored here is the #1212 fix, which reads an image reference through `DockerFrom` alone and so links against 8.90.3. Reapplied in a follow-up PR, to merge once a rewrite release carries #8576, #8590 and #8599 and the CLI picks it up.
Reapplies #1213, reverted in #1215 because the rewrite-docker API it reads `ARG` defaults through had not been released yet. Hold until a rewrite release carries openrewrite/rewrite#8576 (the `Docker.Argument` accessors), #8590 (the image reference grammar) and #8599 (`ImageName`), and confirm the Moderne CLI bundles that release, as the CLI loads the LST classes in its own classloader and so decides which rewrite-docker a recipe actually links against.
This reverts commit 901210a, keeping the branch side. Merging main in pulled #1215 across, and #1215 is the revert of the very commit this branch exists to reapply. The merge base still carried the `ARG` work and main had removed it, so the merge resolved to main's removal and emptied the branch: `git diff main...HEAD` came back with nothing, leaving the pull request proposing no change at all. Reverting the merge rather than dropping it also settles the branch. The merge stays in history, so main's revert counts as already merged here and undone on purpose. A later `main` merge brings its new commits without resurrecting the removal, which resetting the branch would leave it open to on the next `Update branch`.
openrewrite/rewrite#8608 moves `getText()`, `getTextWithVariables()`, `getQuoteStyle()` and `hasEnvironmentVariables()` off `Docker.Argument` and into `org.openrewrite.docker.internal.ArgumentContents`, then drops them from the LST type. That matters because of how the Moderne CLI splits one rewrite-docker jar across two classloaders: `org.openrewrite.docker.tree` resolves to the rewrite-docker the CLI bundles, while recipes, traits and `internal` load child-first from the recipe artifact. Reading an argument through the LST type therefore linked against the CLI's copy, which is where #1215 came from. The helpers now sit on the recipe's side of that split and read only members that predate the CLIs in the field, so they travel with this artifact. `ImageName` already sits on that side, so it needed no change. Verified against the pull request rather than assumed: rewrite-docker built at 6f5fd253 and published locally, and every one of the fifteen `org.openrewrite.docker.tree` members this recipe links against confirmed present in the 8.90.3 jar CLI 4.6.3 bundles, matching on descriptor.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reapplies #1213, which #1215 reverts. Do not merge until the dependency below is released.
Sits on top of openrewrite/rewrite#8608, merged 2026-08-22, which changes the merge gate for the better: this no longer has to wait for CLI adoption.
Why the gate shrank
#1215 reverted this because the Moderne CLI splits one rewrite-docker jar across two classloaders —
org.openrewrite.docker.treeresolves to the rewrite-docker the CLI bundles, while recipes, traits andinternalload child-first from the recipe artifact. Reading an argument throughDocker.Argument.getText()therefore linked against the CLI's copy, and CLI 4.6.3 bundles 8.90.3, where that method does not exist.openrewrite/rewrite#8608 moves those four accessors into
org.openrewrite.docker.internal.ArgumentContentsand drops them from the LST type. They now sit on the recipe's side of the split, so they travel with this artifact instead of being looked up in the CLI's jar.ImageNamealready sat on that side and needed no change.Verified, not assumed
UpgradeDockerImageVersionTestpasses against it.org.openrewrite.docker.treemember this recipe links against — 15 of them, read out of the compiled class constant pool — confirmed present in therewrite-docker-8.90.3.jarinside CLI 4.6.3, matching on descriptor, not just name.Merge gate
A rewrite release carrying openrewrite/rewrite#8608. No CLI upgrade is required, which is the point of #8608.
Green CI here is not the signal.
mainresolves8.91.0-SNAPSHOT, which already carries #8608, so the build passes today. A release build resolveslatest.releaseinstead — v3.42.1 pinnedrewrite-bom8.90.2 — so merging before rewrite 8.91.0 ships would break the next release build onArgumentContentsnot existing. It fails at compile time rather than shipping a broken artifact, but it blocks the release either way.Known limitation on an older CLI
The LST is built by the CLI's parser, so on a CLI predating openrewrite/rewrite#8576 an
ARGvalue still arrives in the old shape — quotes kept in the literal text,$VARunsplit. Two cases go unhandled there:ARG JAVA_VERSION="11"reads as"11"with the quotes, whichVERSIONED_TAGdoes not match, so it is skipped.ARG BASE=$OTHERreads as the literal text$OTHER, which is not a known image, so it is skipped.Both fall through to no change rather than a wrong one.
FROM eclipse-temurin:likewise still fails to parse until a CLI carries openrewrite/rewrite#8590. Full behaviour needs a CLI on the release above; correctness does not.