From 4ba3f2d8e959d83461faa92770d2d84364b5222c Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 20 Aug 2026 15:56:38 +0200 Subject: [PATCH 01/11] Upgrade `ARG` default values used in Docker `FROM` instructions Follow up to #1212, which left any `FROM` built from a variable untouched. When the variable is a global `ARG` with a literal default, that default can be upgraded instead, so `ARG java_version=17` used as `FROM eclipse-temurin:${java_version}` becomes `ARG java_version=25`. --- .../migrate/UpgradeDockerImageVersion.java | 226 ++++++++++++++++-- .../resources/META-INF/rewrite/recipes.csv | 2 +- .../UpgradeDockerImageVersionTest.java | 127 ++++++++++ 3 files changed, 332 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java b/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java index 4a1d24c7a4..329967fab7 100644 --- a/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java +++ b/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java @@ -17,18 +17,28 @@ import lombok.EqualsAndHashCode; import lombok.Value; +import org.jspecify.annotations.Nullable; import org.openrewrite.ExecutionContext; import org.openrewrite.Option; import org.openrewrite.Recipe; import org.openrewrite.TreeVisitor; +import org.openrewrite.docker.DockerIsoVisitor; import org.openrewrite.docker.trait.DockerFrom; +import org.openrewrite.docker.tree.Docker; +import org.openrewrite.internal.ListUtils; +import java.util.HashMap; import java.util.HashSet; +import java.util.List; +import java.util.Map; import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; import static java.util.Arrays.asList; +import static java.util.Collections.emptyMap; +import static java.util.Collections.singletonList; +import static java.util.Objects.requireNonNull; @EqualsAndHashCode(callSuper = false) @Value @@ -50,48 +60,220 @@ public class UpgradeDockerImageVersion extends Recipe { private static final int OLDEST_VERSION = 8; private static final Pattern VERSIONED_TAG = Pattern.compile("(\\d{1,3})(\\D.*)?"); + private static final String ARG_DEFAULTS = "argDefaults"; + private static final String ARG_UPGRADES = "argUpgrades"; + String displayName = "Upgrade Docker image Java version"; String description = "Upgrade Docker image tags to use the specified Java version. " + "Updates common Java Docker images including eclipse-temurin, amazoncorretto, azul/zulu-openjdk, " + "and others. Also migrates deprecated images (openjdk, adoptopenjdk) to eclipse-temurin, " + - "preserving any tag suffix such as `-jre-alpine`. Image references built from build arguments or " + - "environment variables are left untouched, as their value can not be determined statically. A digest " + - "pin is dropped when the tag is upgraded, as the stale digest would otherwise keep resolving to the " + - "old image."; + "preserving any tag suffix such as `-jre-alpine`. When a `FROM` is built from a build argument, the " + + "default value of the corresponding global `ARG` is upgraded instead, such that `ARG java_version=17` " + + "used as `FROM eclipse-temurin:${java_version}` becomes `ARG java_version=25`. Image references built " + + "from arguments without a default value are left untouched, as their value can not be determined " + + "statically. A digest pin is dropped when the tag is upgraded, as the stale digest would otherwise " + + "keep resolving to the old image."; @Override public TreeVisitor getVisitor() { if (version == null) { return TreeVisitor.noop(); } - return new DockerFrom.Matcher().asVisitor((image, ctx) -> { - String imageName = image.getImageName().orElse(""); - String tag = image.getTag().orElse(""); - if (containsVariable(imageName) || containsVariable(tag)) { - return image.getTree(); + return new DockerIsoVisitor() { + + @Override + public Docker.File visitFile(Docker.File file, ExecutionContext ctx) { + Map defaults = new HashMap<>(); + for (Docker.Arg arg : file.getGlobalArgs()) { + Docker.Literal literalDefault = literalDefault(arg); + if (literalDefault != null) { + defaults.put(arg.getName().getText(), literalDefault.getText()); + } + } + Map upgrades = new HashMap<>(); + getCursor().putMessage(ARG_DEFAULTS, defaults); + getCursor().putMessage(ARG_UPGRADES, upgrades); + + Docker.File f = super.visitFile(file, ctx); + if (upgrades.isEmpty()) { + return f; + } + return f.withGlobalArgs(ListUtils.map(f.getGlobalArgs(), arg -> { + String upgraded = upgrades.get(arg.getName().getText()); + Docker.Literal literalDefault = literalDefault(arg); + if (upgraded == null || literalDefault == null) { + return arg; + } + return arg.withValue(requireNonNull(arg.getValue()) + .withContents(singletonList(literalDefault.withText(upgraded)))); + })); } - Matcher matcher = VERSIONED_TAG.matcher(tag); - if (!matcher.matches()) { - return image.getTree(); + @Override + public Docker.From visitFrom(Docker.From from, ExecutionContext ctx) { + DockerFrom image = new DockerFrom(getCursor()); + String imageName = image.getImageName().orElse(""); + String tag = image.getTag().orElse(""); + if (!containsVariable(imageName) && !containsVariable(tag)) { + return upgradeLiteralFrom(image, imageName, tag); + } + return upgradeThroughArgs(from, + getCursor().getNearestMessage(ARG_DEFAULTS, emptyMap()), + getCursor().getNearestMessage(ARG_UPGRADES, new HashMap<>())); } - int currentVersion = Integer.parseInt(matcher.group(1)); - if (currentVersion < OLDEST_VERSION || version <= currentVersion) { + + private Docker.From upgradeLiteralFrom(DockerFrom image, String imageName, String tag) { + String newTag = upgradedTag(tag); + if (newTag == null) { + return image.getTree(); + } + if (DEPRECATED_IMAGES.contains(imageName)) { + return image.withImageReference(NEW_IMAGE + ":" + newTag); + } + if (CURRENT_IMAGES.contains(imageName)) { + return image.withTag(newTag).withDigest(null); + } return image.getTree(); } - String newTag = version + (matcher.group(2) == null ? "" : matcher.group(2)); - if (DEPRECATED_IMAGES.contains(imageName)) { - return image.withImageReference(NEW_IMAGE + ":" + newTag); - } - if (CURRENT_IMAGES.contains(imageName)) { - return image.withTag(newTag).withDigest(null); + /** + * Upgrade a {@code FROM} whose image name or tag is built from a build argument, by rewriting the + * default value of the global {@code ARG} that supplies it. Only arguments that carry a literal + * default are resolvable; anything else is left untouched. + */ + private Docker.From upgradeThroughArgs(Docker.From from, Map defaults, Map upgrades) { + String imageVariable = soleVariable(from.getImageName()); + String imageName = imageVariable == null ? + literalText(from.getImageName()) : + defaults.get(imageVariable); + if (imageName == null) { + return from; + } + + String tagVariable; + String tag; + if (from.getTag() == null) { + // A single argument holding the whole `name:tag` reference, as in `FROM ${BASE_IMAGE}` + String[] reference = splitReference(imageName); + if (imageVariable == null || reference == null) { + return from; + } + imageName = reference[0]; + tag = reference[1]; + tagVariable = imageVariable; + } else { + tagVariable = leadingVariable(from.getTag()); + tag = tagVariable == null ? literalText(from.getTag()) : defaults.get(tagVariable); + } + if (tag == null) { + return from; + } + + String newImageName = upgradedImageName(imageName); + String newTag = upgradedTag(tag); + if (newImageName == null || newTag == null) { + return from; + } + + if (tagVariable != null && tagVariable.equals(imageVariable)) { + upgrades.put(imageVariable, newImageName + ":" + newTag); + return from; + } + if (tagVariable == null) { + from = new DockerFrom(getCursor()).withTag(newTag); + } else { + upgrades.put(tagVariable, newTag); + } + if (!newImageName.equals(imageName)) { + if (imageVariable == null) { + from = withImageName(from, newImageName); + } else { + upgrades.put(imageVariable, newImageName); + } + } + return from.withDigest(null); } - return image.getTree(); - }); + }; + } + + private @Nullable String upgradedImageName(String imageName) { + if (DEPRECATED_IMAGES.contains(imageName)) { + return NEW_IMAGE; + } + return CURRENT_IMAGES.contains(imageName) ? imageName : null; + } + + private @Nullable String upgradedTag(String tag) { + Matcher matcher = VERSIONED_TAG.matcher(tag); + if (!matcher.matches()) { + return null; + } + int currentVersion = Integer.parseInt(matcher.group(1)); + if (currentVersion < OLDEST_VERSION || version <= currentVersion) { + return null; + } + return version + (matcher.group(2) == null ? "" : matcher.group(2)); } private static boolean containsVariable(String imageReferencePart) { return imageReferencePart.indexOf('$') != -1; } + + private static Docker.@Nullable Literal literalDefault(Docker.Arg arg) { + Docker.Argument value = arg.getValue(); + return value == null ? null : sole(value.getContents(), Docker.Literal.class); + } + + private static @Nullable String literalText(Docker.Argument argument) { + Docker.Literal literal = sole(argument.getContents(), Docker.Literal.class); + return literal == null ? null : literal.getText(); + } + + private static @Nullable String soleVariable(Docker.Argument argument) { + Docker.EnvironmentVariable variable = sole(argument.getContents(), Docker.EnvironmentVariable.class); + return variable == null ? null : variable.getName(); + } + + /** + * The name of the variable a tag starts with, as in the {@code JAVA_VERSION} of {@code ${JAVA_VERSION}-jre}, + * or null when the tag does not start with a variable or holds a further variable we can not resolve. + */ + private static @Nullable String leadingVariable(Docker.Argument argument) { + List contents = argument.getContents(); + if (contents.isEmpty() || !(contents.get(0) instanceof Docker.EnvironmentVariable)) { + return null; + } + for (int i = 1; i < contents.size(); i++) { + if (!(contents.get(i) instanceof Docker.Literal)) { + return null; + } + } + return ((Docker.EnvironmentVariable) contents.get(0)).getName(); + } + + private static @Nullable T sole(List contents, Class type) { + if (contents.size() == 1 && type.isInstance(contents.get(0))) { + return type.cast(contents.get(0)); + } + return null; + } + + /** + * Splits an image reference into its name and tag, dropping any digest; null when there is no tag. + */ + private static String @Nullable [] splitReference(String reference) { + int at = reference.indexOf('@'); + String withoutDigest = at == -1 ? reference : reference.substring(0, at); + int colon = withoutDigest.indexOf(':', withoutDigest.lastIndexOf('/') + 1); + if (colon == -1) { + return null; + } + return new String[]{withoutDigest.substring(0, colon), withoutDigest.substring(colon + 1)}; + } + + private static Docker.From withImageName(Docker.From from, String imageName) { + Docker.Argument argument = from.getImageName(); + Docker.Literal literal = requireNonNull(sole(argument.getContents(), Docker.Literal.class)); + return from.withImageName(argument.withContents(singletonList(literal.withText(imageName)))); + } } diff --git a/src/main/resources/META-INF/rewrite/recipes.csv b/src/main/resources/META-INF/rewrite/recipes.csv index dde61456e9..8dc21248de 100644 --- a/src/main/resources/META-INF/rewrite/recipes.csv +++ b/src/main/resources/META-INF/rewrite/recipes.csv @@ -91,7 +91,7 @@ maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.U maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.UpgradeBuildToJava24ForKotlin1x,Upgrade build to Java 24 for Kotlin 1.x,"Kotlin versions before 2.3 only support up to Java 24, and Kotlin 1.x cannot be safely upgraded automatically because crossing the K2 compiler default introduced in Kotlin 2.0 is a source-breaking change. Such modules are therefore capped at Java 24 and annotated with an explanation. Modules already on Kotlin 2.0-2.2 are instead bumped to Kotlin 2.3 by `UpgradeKotlinForJava25` so they can reach Java 25. Applies only to modules that actually compile Kotlin (i.e. contain `.kt` source files), so transitive `kotlin-stdlib` dependencies do not trigger the cap.",9,,,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,, maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.UpgradeBuildToJava25,Upgrade build to Java 25 (non-Kotlin),"Upgrades build files to Java 25 for modules without Kotlin source files. This covers pure Java projects, including those that only pick up `kotlin-stdlib` transitively through another dependency.",9,,,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,, maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.UpgradeBuildToJava25ForKotlin,Upgrade build to Java 25 for Kotlin 2.3+,Upgrades build files to Java 25 for Kotlin modules already on Kotlin 2.3 or later.,9,,,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,, -maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.UpgradeDockerImageVersion,Upgrade Docker image Java version,"Upgrade Docker image tags to use the specified Java version. Updates common Java Docker images including eclipse-temurin, amazoncorretto, azul/zulu-openjdk, and others. Also migrates deprecated images (openjdk, adoptopenjdk) to eclipse-temurin, preserving any tag suffix such as `-jre-alpine`. Image references built from build arguments or environment variables are left untouched, as their value can not be determined statically. A digest pin is dropped when the tag is upgraded, as the stale digest would otherwise keep resolving to the old image.",1,,,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,"[{""name"":""version"",""type"":""Integer"",""displayName"":""Java version"",""description"":""The Java version to upgrade to."",""example"":""11"",""required"":true}]", +maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.UpgradeDockerImageVersion,Upgrade Docker image Java version,"Upgrade Docker image tags to use the specified Java version. Updates common Java Docker images including eclipse-temurin, amazoncorretto, azul/zulu-openjdk, and others. Also migrates deprecated images (openjdk, adoptopenjdk) to eclipse-temurin, preserving any tag suffix such as `-jre-alpine`. When a `FROM` is built from a build argument, the default value of the corresponding global `ARG` is upgraded instead, such that `ARG java_version=17` used as `FROM eclipse-temurin:${java_version}` becomes `ARG java_version=25`. Image references built from arguments without a default value are left untouched, as their value can not be determined statically. A digest pin is dropped when the tag is upgraded, as the stale digest would otherwise keep resolving to the old image.",1,,,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,"[{""name"":""version"",""type"":""Integer"",""displayName"":""Java version"",""description"":""The Java version to upgrade to."",""example"":""11"",""required"":true}]", maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.UpgradeJavaVersion,Upgrade Java version,"Upgrade build plugin configuration to use the specified Java version. This recipe changes `java.toolchain.languageVersion` in `build.gradle(.kts)` of gradle projects, or maven-compiler-plugin target version and related settings. Will not downgrade if the version is newer than the specified version.",8,,,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,"[{""name"":""version"",""type"":""Integer"",""displayName"":""Java version"",""description"":""The Java version to upgrade to."",""example"":""11"",""required"":true}]", maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.UpgradeKotlinForJava25,Upgrade Kotlin to 2.3 for Java 25 compatibility,"Only Kotlin 2.3 and later can target Java 25 bytecode, so modules on an older Kotlin are otherwise capped at Java 24. This recipe upgrades modules that compile Kotlin (i.e. contain `.kt` source files) and are already on Kotlin 2.0, 2.1, or 2.2 up to the latest Kotlin 2.3, so they can subsequently be migrated to Java 25. Modules on Kotlin 1.x are left untouched, as crossing the K2 compiler default introduced in Kotlin 2.0 is a source-breaking change that should not be applied automatically. As a safety net the module is also floored at Java 24: if the Kotlin upgrade cannot be applied (for instance because the version is managed externally by a parent or BOM), the module still lands on Java 24 rather than being left behind, and is raised the rest of the way to Java 25 only once it actually reaches Kotlin 2.3.",12,,,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,,"[{""name"":""org.openrewrite.maven.table.MavenMetadataFailures"",""displayName"":""Maven metadata failures"",""instanceName"":""Maven metadata failures"",""description"":""Attempts to resolve maven metadata that failed."",""columns"":[{""name"":""group"",""type"":""String"",""displayName"":""Group id"",""description"":""The groupId of the artifact for which the metadata download failed.""},{""name"":""artifactId"",""type"":""String"",""displayName"":""Artifact id"",""description"":""The artifactId of the artifact for which the metadata download failed.""},{""name"":""version"",""type"":""String"",""displayName"":""Version"",""description"":""The version of the artifact for which the metadata download failed.""},{""name"":""mavenRepositoryUri"",""type"":""String"",""displayName"":""Maven repository"",""description"":""The URL of the Maven repository that the metadata download failed on.""},{""name"":""snapshots"",""type"":""String"",""displayName"":""Snapshots"",""description"":""Does the repository support snapshots.""},{""name"":""releases"",""type"":""String"",""displayName"":""Releases"",""description"":""Does the repository support releases.""},{""name"":""failure"",""type"":""String"",""displayName"":""Failure"",""description"":""The reason the metadata download failed.""}]}]" maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.UpgradeKotlinJvmTargetVersion,Upgrade Kotlin `jvmTarget` to match the Java version,Align the Kotlin `jvmTarget` with the project's Java version so the Kotlin compiler emits bytecode at the same level as `javac`. Covers `kotlin-maven-plugin` `` configuration and the Gradle `kotlinOptions { jvmTarget = ... }` / `compilerOptions { jvmTarget = ... }` blocks (Groovy and Kotlin DSL). Will not downgrade if the existing Kotlin target is higher than the requested version.,1,,,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,"[{""name"":""version"",""type"":""Integer"",""displayName"":""Java version"",""description"":""The Java version to align Kotlin's `jvmTarget` with."",""example"":""21"",""required"":true}]", diff --git a/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java b/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java index 12e37af70e..9c6e2dcf05 100644 --- a/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java @@ -85,6 +85,133 @@ void doNotChangeVariableImageReferences(String from) { ); } + @CsvSource({ + // The argument holds the bare version + "java_version=17, eclipse-temurin:${java_version}, java_version=25, eclipse-temurin:${java_version}", + "java_version=17, eclipse-temurin:$java_version, java_version=25, eclipse-temurin:$java_version", + "JAVA_VERSION=11, eclipse-temurin:${JAVA_VERSION}-jre, JAVA_VERSION=25, eclipse-temurin:${JAVA_VERSION}-jre", + "JAVA_VERSION=8, amazoncorretto:${JAVA_VERSION}-alpine, JAVA_VERSION=25, amazoncorretto:${JAVA_VERSION}-alpine", + // The argument holds the version and a suffix + "IMAGE_TAG=11-jre-alpine, eclipse-temurin:${IMAGE_TAG}, IMAGE_TAG=25-jre-alpine, eclipse-temurin:${IMAGE_TAG}", + // The argument holds the whole image reference + "BASE_IMAGE=eclipse-temurin:11-jre, ${BASE_IMAGE}, BASE_IMAGE=eclipse-temurin:25-jre, ${BASE_IMAGE}", + "BASE_IMAGE=openjdk:11-jre, ${BASE_IMAGE}, BASE_IMAGE=eclipse-temurin:25-jre, ${BASE_IMAGE}", + "BASE_IMAGE=eclipse-temurin:11-jre@sha256:1234567890abcdef, ${BASE_IMAGE}, BASE_IMAGE=eclipse-temurin:25-jre, ${BASE_IMAGE}", + // The argument holds the image name only + "BASE_IMAGE=eclipse-temurin, ${BASE_IMAGE}:11-jre, BASE_IMAGE=eclipse-temurin, ${BASE_IMAGE}:25-jre", + "BASE_IMAGE=openjdk, ${BASE_IMAGE}:11-jre, BASE_IMAGE=eclipse-temurin, ${BASE_IMAGE}:25-jre", + }) + @ParameterizedTest + void upgradeArgumentDefaultValue(String beforeArg, String beforeFrom, String afterArg, String afterFrom) { + rewriteRun( + spec -> spec.recipe(new UpgradeDockerImageVersion(25)), + docker( + """ + ARG %s + FROM %s + """.formatted(beforeArg, beforeFrom), + """ + ARG %s + FROM %s + """.formatted(afterArg, afterFrom) + ) + ); + } + + @Test + void upgradeDeprecatedImageNameAlongsideArgumentDefaultValue() { + rewriteRun( + spec -> spec.recipe(new UpgradeDockerImageVersion(25)), + docker( + """ + ARG JAVA_VERSION=11 + FROM openjdk:${JAVA_VERSION}-jre + """, + """ + ARG JAVA_VERSION=25 + FROM eclipse-temurin:${JAVA_VERSION}-jre + """ + ) + ); + } + + @Test + void upgradeArgumentDefaultValueSharedByStages() { + rewriteRun( + spec -> spec.recipe(new UpgradeDockerImageVersion(25)), + docker( + """ + ARG JAVA_VERSION=11 + FROM eclipse-temurin:${JAVA_VERSION}-jdk AS builder + FROM eclipse-temurin:${JAVA_VERSION}-jre + """, + """ + ARG JAVA_VERSION=25 + FROM eclipse-temurin:${JAVA_VERSION}-jdk AS builder + FROM eclipse-temurin:${JAVA_VERSION}-jre + """ + ) + ); + } + + @Test + void dropDigestPinWhenUpgradingArgumentDefaultValue() { + rewriteRun( + spec -> spec.recipe(new UpgradeDockerImageVersion(25)), + docker( + """ + ARG JAVA_VERSION=11 + FROM eclipse-temurin:${JAVA_VERSION}-jre@sha256:1234567890abcdef + """, + """ + ARG JAVA_VERSION=25 + FROM eclipse-temurin:${JAVA_VERSION}-jre + """ + ) + ); + } + + @CsvSource({ + // Arguments that are not used in a FROM are left alone + "JAVA_VERSION=11, eclipse-temurin:25-jre", + // Arguments for unrelated images are left alone + "NODE_VERSION=20, node:${NODE_VERSION}-alpine", + // Arguments already at or beyond the target version are left alone + "JAVA_VERSION=25, eclipse-temurin:${JAVA_VERSION}-jre", + "JAVA_VERSION=26, eclipse-temurin:${JAVA_VERSION}-jre", + // Arguments not holding a leading version are left alone + "JAVA_VERSION=latest, eclipse-temurin:${JAVA_VERSION}", + "SUFFIX=-jre, eclipse-temurin:11${SUFFIX}", + // Arguments that only hold part of the image name are left alone + "REGISTRY=docker.io, ${REGISTRY}/eclipse-temurin:11-jre", + }) + @ParameterizedTest + void doNotChangeUnrelatedArgumentDefaultValues(String arg, String from) { + rewriteRun( + spec -> spec.recipe(new UpgradeDockerImageVersion(25)), + docker( + """ + ARG %s + FROM %s + """.formatted(arg, from) + ) + ); + } + + @Test + void doNotChangeArgumentDeclaredAfterFirstFrom() { + rewriteRun( + spec -> spec.recipe(new UpgradeDockerImageVersion(25)), + docker( + """ + FROM eclipse-temurin:25-jre + ARG JAVA_VERSION=11 + RUN echo "${JAVA_VERSION}" + """ + ) + ); + } + @CsvSource({ // Unrelated images are left alone "FROM ubuntu:22.04", From f17397e58a97960e90eefae441aa6c38e6796cbc Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 20 Aug 2026 16:39:44 +0200 Subject: [PATCH 02/11] Do not bump `ARG` defaults shared with images we leave alone Three follow ups from review: - A `FROM` whose image we do not upgrade now vetoes the arguments feeding it, so `ARG VERSION=11` used by both `eclipse-temurin:${VERSION}` and `node:${VERSION}` is left alone rather than turning the latter into `node:25`. - Drop the digest pin when an argument holding a whole `name:tag` reference is upgraded, as the stale digest would keep resolving to the old image. - Upgrade quoted default values, keeping their quotes. The parser hands an `ARG` value to us as a single literal with the quotes still in its text and no quote style, so `ARG JAVA_VERSION="11"` never matched a version before. --- .../migrate/UpgradeDockerImageVersion.java | 60 ++++++++++++++++--- .../UpgradeDockerImageVersionTest.java | 53 ++++++++++++++++ 2 files changed, 106 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java b/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java index 329967fab7..46b5061a2a 100644 --- a/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java +++ b/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java @@ -62,6 +62,7 @@ public class UpgradeDockerImageVersion extends Recipe { private static final String ARG_DEFAULTS = "argDefaults"; private static final String ARG_UPGRADES = "argUpgrades"; + private static final String ARG_BLOCKED = "argBlocked"; String displayName = "Upgrade Docker image Java version"; String description = "Upgrade Docker image tags to use the specified Java version. " + @@ -87,14 +88,17 @@ public Docker.File visitFile(Docker.File file, ExecutionContext ctx) { for (Docker.Arg arg : file.getGlobalArgs()) { Docker.Literal literalDefault = literalDefault(arg); if (literalDefault != null) { - defaults.put(arg.getName().getText(), literalDefault.getText()); + defaults.put(arg.getName().getText(), QuotedText.of(literalDefault.getText()).getText()); } } Map upgrades = new HashMap<>(); + Set blocked = new HashSet<>(); getCursor().putMessage(ARG_DEFAULTS, defaults); getCursor().putMessage(ARG_UPGRADES, upgrades); + getCursor().putMessage(ARG_BLOCKED, blocked); Docker.File f = super.visitFile(file, ctx); + upgrades.keySet().removeAll(blocked); if (upgrades.isEmpty()) { return f; } @@ -104,8 +108,9 @@ public Docker.File visitFile(Docker.File file, ExecutionContext ctx) { if (upgraded == null || literalDefault == null) { return arg; } + String requoted = QuotedText.of(literalDefault.getText()).requote(upgraded); return arg.withValue(requireNonNull(arg.getValue()) - .withContents(singletonList(literalDefault.withText(upgraded)))); + .withContents(singletonList(literalDefault.withText(requoted)))); })); } @@ -119,7 +124,8 @@ public Docker.From visitFrom(Docker.From from, ExecutionContext ctx) { } return upgradeThroughArgs(from, getCursor().getNearestMessage(ARG_DEFAULTS, emptyMap()), - getCursor().getNearestMessage(ARG_UPGRADES, new HashMap<>())); + getCursor().getNearestMessage(ARG_UPGRADES, new HashMap<>()), + getCursor().getNearestMessage(ARG_BLOCKED, new HashSet<>())); } private Docker.From upgradeLiteralFrom(DockerFrom image, String imageName, String tag) { @@ -139,14 +145,16 @@ private Docker.From upgradeLiteralFrom(DockerFrom image, String imageName, Strin /** * Upgrade a {@code FROM} whose image name or tag is built from a build argument, by rewriting the * default value of the global {@code ARG} that supplies it. Only arguments that carry a literal - * default are resolvable; anything else is left untouched. + * default are resolvable; anything else is left untouched. Arguments that also feed an image we + * do not upgrade are blocked, as a shared argument can not be bumped for one image alone. */ - private Docker.From upgradeThroughArgs(Docker.From from, Map defaults, Map upgrades) { + private Docker.From upgradeThroughArgs(Docker.From from, Map defaults, Map upgrades, Set blocked) { String imageVariable = soleVariable(from.getImageName()); String imageName = imageVariable == null ? literalText(from.getImageName()) : defaults.get(imageVariable); if (imageName == null) { + block(blocked, imageVariable, from.getTag() == null ? null : leadingVariable(from.getTag())); return from; } @@ -170,14 +178,18 @@ private Docker.From upgradeThroughArgs(Docker.From from, Map def } String newImageName = upgradedImageName(imageName); + if (newImageName == null) { + block(blocked, imageVariable, tagVariable); + return from; + } String newTag = upgradedTag(tag); - if (newImageName == null || newTag == null) { + if (newTag == null) { return from; } if (tagVariable != null && tagVariable.equals(imageVariable)) { upgrades.put(imageVariable, newImageName + ":" + newTag); - return from; + return from.withDigest(null); } if (tagVariable == null) { from = new DockerFrom(getCursor()).withTag(newTag); @@ -215,6 +227,15 @@ private Docker.From upgradeThroughArgs(Docker.From from, Map def return version + (matcher.group(2) == null ? "" : matcher.group(2)); } + private static void block(Set blocked, @Nullable String imageVariable, @Nullable String tagVariable) { + if (imageVariable != null) { + blocked.add(imageVariable); + } + if (tagVariable != null) { + blocked.add(tagVariable); + } + } + private static boolean containsVariable(String imageReferencePart) { return imageReferencePart.indexOf('$') != -1; } @@ -276,4 +297,29 @@ private static Docker.From withImageName(Docker.From from, String imageName) { Docker.Literal literal = requireNonNull(sole(argument.getContents(), Docker.Literal.class)); return from.withImageName(argument.withContents(singletonList(literal.withText(imageName)))); } + + /** + * The source text of an {@code ARG} default value, split into the quotes surrounding it and the text within, as + * the parser keeps any quotes as part of the literal. Splitting the two apart lets a value be matched unquoted, + * and written back with the very same quoting. + */ + @Value + private static class QuotedText { + String quote; + String text; + + static QuotedText of(String source) { + if (source.length() > 1) { + char first = source.charAt(0); + if ((first == '"' || first == '\'') && source.charAt(source.length() - 1) == first) { + return new QuotedText(String.valueOf(first), source.substring(1, source.length() - 1)); + } + } + return new QuotedText("", source); + } + + String requote(String replacement) { + return quote + replacement + quote; + } + } } diff --git a/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java b/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java index 9c6e2dcf05..d7041fc712 100644 --- a/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java @@ -100,6 +100,10 @@ void doNotChangeVariableImageReferences(String from) { // The argument holds the image name only "BASE_IMAGE=eclipse-temurin, ${BASE_IMAGE}:11-jre, BASE_IMAGE=eclipse-temurin, ${BASE_IMAGE}:25-jre", "BASE_IMAGE=openjdk, ${BASE_IMAGE}:11-jre, BASE_IMAGE=eclipse-temurin, ${BASE_IMAGE}:25-jre", + // A quoted default value keeps its quotes + "JAVA_VERSION=\"11\", eclipse-temurin:${JAVA_VERSION}, JAVA_VERSION=\"25\", eclipse-temurin:${JAVA_VERSION}", + "IMAGE_TAG=\"11-jre\", eclipse-temurin:${IMAGE_TAG}, IMAGE_TAG=\"25-jre\", eclipse-temurin:${IMAGE_TAG}", + "BASE_IMAGE=\"openjdk:11-jre\", ${BASE_IMAGE}, BASE_IMAGE=\"eclipse-temurin:25-jre\", ${BASE_IMAGE}", }) @ParameterizedTest void upgradeArgumentDefaultValue(String beforeArg, String beforeFrom, String afterArg, String afterFrom) { @@ -118,6 +122,23 @@ void upgradeArgumentDefaultValue(String beforeArg, String beforeFrom, String aft ); } + @Test + void upgradeSingleQuotedArgumentDefaultValue() { + rewriteRun( + spec -> spec.recipe(new UpgradeDockerImageVersion(25)), + docker( + """ + ARG IMAGE_TAG='11-jre' + FROM eclipse-temurin:${IMAGE_TAG} + """, + """ + ARG IMAGE_TAG='25-jre' + FROM eclipse-temurin:${IMAGE_TAG} + """ + ) + ); + } + @Test void upgradeDeprecatedImageNameAlongsideArgumentDefaultValue() { rewriteRun( @@ -171,6 +192,37 @@ void dropDigestPinWhenUpgradingArgumentDefaultValue() { ); } + @Test + void dropDigestPinWhenUpgradingArgumentHoldingWholeReference() { + rewriteRun( + spec -> spec.recipe(new UpgradeDockerImageVersion(25)), + docker( + """ + ARG BASE_IMAGE=eclipse-temurin:11-jre + FROM ${BASE_IMAGE}@sha256:1234567890abcdef + """, + """ + ARG BASE_IMAGE=eclipse-temurin:25-jre + FROM ${BASE_IMAGE} + """ + ) + ); + } + + @Test + void doNotUpgradeArgumentSharedWithAnImageWeDoNotUpgrade() { + rewriteRun( + spec -> spec.recipe(new UpgradeDockerImageVersion(25)), + docker( + """ + ARG VERSION=11 + FROM eclipse-temurin:${VERSION} AS build + FROM node:${VERSION} + """ + ) + ); + } + @CsvSource({ // Arguments that are not used in a FROM are left alone "JAVA_VERSION=11, eclipse-temurin:25-jre", @@ -181,6 +233,7 @@ void dropDigestPinWhenUpgradingArgumentDefaultValue() { "JAVA_VERSION=26, eclipse-temurin:${JAVA_VERSION}-jre", // Arguments not holding a leading version are left alone "JAVA_VERSION=latest, eclipse-temurin:${JAVA_VERSION}", + "JAVA_VERSION=\"latest\", eclipse-temurin:${JAVA_VERSION}", "SUFFIX=-jre, eclipse-temurin:11${SUFFIX}", // Arguments that only hold part of the image name are left alone "REGISTRY=docker.io, ${REGISTRY}/eclipse-temurin:11-jre", From 0ba1189da9c8142665865ee568656528ebad2754 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 20 Aug 2026 16:51:20 +0200 Subject: [PATCH 03/11] Trim comments and javadoc that restate the code --- .../migrate/UpgradeDockerImageVersion.java | 23 +++++-------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java b/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java index 46b5061a2a..145d7f0e38 100644 --- a/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java +++ b/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java @@ -142,12 +142,6 @@ private Docker.From upgradeLiteralFrom(DockerFrom image, String imageName, Strin return image.getTree(); } - /** - * Upgrade a {@code FROM} whose image name or tag is built from a build argument, by rewriting the - * default value of the global {@code ARG} that supplies it. Only arguments that carry a literal - * default are resolvable; anything else is left untouched. Arguments that also feed an image we - * do not upgrade are blocked, as a shared argument can not be bumped for one image alone. - */ private Docker.From upgradeThroughArgs(Docker.From from, Map defaults, Map upgrades, Set blocked) { String imageVariable = soleVariable(from.getImageName()); String imageName = imageVariable == null ? @@ -161,7 +155,7 @@ private Docker.From upgradeThroughArgs(Docker.From from, Map def String tagVariable; String tag; if (from.getTag() == null) { - // A single argument holding the whole `name:tag` reference, as in `FROM ${BASE_IMAGE}` + // A single argument holding the whole reference, as in `FROM ${BASE_IMAGE}` String[] reference = splitReference(imageName); if (imageVariable == null || reference == null) { return from; @@ -227,6 +221,9 @@ private Docker.From upgradeThroughArgs(Docker.From from, Map def return version + (matcher.group(2) == null ? "" : matcher.group(2)); } + /** + * Withhold arguments feeding an image we do not upgrade, as a shared argument can not be bumped for one image alone. + */ private static void block(Set blocked, @Nullable String imageVariable, @Nullable String tagVariable) { if (imageVariable != null) { blocked.add(imageVariable); @@ -255,10 +252,6 @@ private static boolean containsVariable(String imageReferencePart) { return variable == null ? null : variable.getName(); } - /** - * The name of the variable a tag starts with, as in the {@code JAVA_VERSION} of {@code ${JAVA_VERSION}-jre}, - * or null when the tag does not start with a variable or holds a further variable we can not resolve. - */ private static @Nullable String leadingVariable(Docker.Argument argument) { List contents = argument.getContents(); if (contents.isEmpty() || !(contents.get(0) instanceof Docker.EnvironmentVariable)) { @@ -279,9 +272,6 @@ private static boolean containsVariable(String imageReferencePart) { return null; } - /** - * Splits an image reference into its name and tag, dropping any digest; null when there is no tag. - */ private static String @Nullable [] splitReference(String reference) { int at = reference.indexOf('@'); String withoutDigest = at == -1 ? reference : reference.substring(0, at); @@ -299,9 +289,8 @@ private static Docker.From withImageName(Docker.From from, String imageName) { } /** - * The source text of an {@code ARG} default value, split into the quotes surrounding it and the text within, as - * the parser keeps any quotes as part of the literal. Splitting the two apart lets a value be matched unquoted, - * and written back with the very same quoting. + * The parser keeps any quotes around an {@code ARG} default value as part of the literal text, so they have to be + * taken off before matching a version, and put back on when writing the upgraded value. */ @Value private static class QuotedText { From a1882f578ee142e90ed03f66035b99444d2a6601 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 20 Aug 2026 17:10:52 +0200 Subject: [PATCH 04/11] Plan `ARG` upgrades before rewriting any `FROM` Rewriting a `FROM` as it was visited, and only withholding the matching `ARG` bump after the traversal, left half applied edits behind: a dropped digest pin or a rename to `eclipse-temurin` next to an argument still holding the old version. The whole file is now planned up front, and replayed until the set of withheld arguments stops growing, as withholding one argument can rule out the images that depend on it. Only the surviving plan is applied. Every give up path now withholds the arguments that `FROM` reads. --- .../migrate/UpgradeDockerImageVersion.java | 196 ++++++++++-------- .../UpgradeDockerImageVersionTest.java | 71 +++++++ 2 files changed, 184 insertions(+), 83 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java b/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java index 145d7f0e38..ef9f69b6aa 100644 --- a/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java +++ b/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java @@ -32,6 +32,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.UUID; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -60,9 +61,7 @@ public class UpgradeDockerImageVersion extends Recipe { private static final int OLDEST_VERSION = 8; private static final Pattern VERSIONED_TAG = Pattern.compile("(\\d{1,3})(\\D.*)?"); - private static final String ARG_DEFAULTS = "argDefaults"; - private static final String ARG_UPGRADES = "argUpgrades"; - private static final String ARG_BLOCKED = "argBlocked"; + private static final String FROM_REPLACEMENTS = "fromReplacements"; String displayName = "Upgrade Docker image Java version"; String description = "Upgrade Docker image tags to use the specified Java version. " + @@ -91,14 +90,12 @@ public Docker.File visitFile(Docker.File file, ExecutionContext ctx) { defaults.put(arg.getName().getText(), QuotedText.of(literalDefault.getText()).getText()); } } - Map upgrades = new HashMap<>(); - Set blocked = new HashSet<>(); - getCursor().putMessage(ARG_DEFAULTS, defaults); - getCursor().putMessage(ARG_UPGRADES, upgrades); - getCursor().putMessage(ARG_BLOCKED, blocked); + ArgPlan plan = planUpgrades(file, defaults); + getCursor().putMessage(FROM_REPLACEMENTS, plan.getFromReplacements()); Docker.File f = super.visitFile(file, ctx); - upgrades.keySet().removeAll(blocked); + + Map upgrades = plan.getArgUpgrades(); if (upgrades.isEmpty()) { return f; } @@ -116,90 +113,106 @@ public Docker.File visitFile(Docker.File file, ExecutionContext ctx) { @Override public Docker.From visitFrom(Docker.From from, ExecutionContext ctx) { - DockerFrom image = new DockerFrom(getCursor()); - String imageName = image.getImageName().orElse(""); - String tag = image.getTag().orElse(""); - if (!containsVariable(imageName) && !containsVariable(tag)) { - return upgradeLiteralFrom(image, imageName, tag); + if (containsVariable(from.getImageName()) || containsVariable(from.getTag())) { + Map replacements = getCursor().getNearestMessage(FROM_REPLACEMENTS, emptyMap()); + return replacements.getOrDefault(from.getId(), from); } - return upgradeThroughArgs(from, - getCursor().getNearestMessage(ARG_DEFAULTS, emptyMap()), - getCursor().getNearestMessage(ARG_UPGRADES, new HashMap<>()), - getCursor().getNearestMessage(ARG_BLOCKED, new HashSet<>())); - } - private Docker.From upgradeLiteralFrom(DockerFrom image, String imageName, String tag) { - String newTag = upgradedTag(tag); + DockerFrom image = new DockerFrom(getCursor()); + String newTag = upgradedTag(image.getTag().orElse("")); if (newTag == null) { - return image.getTree(); + return from; } + String imageName = image.getImageName().orElse(""); if (DEPRECATED_IMAGES.contains(imageName)) { return image.withImageReference(NEW_IMAGE + ":" + newTag); } if (CURRENT_IMAGES.contains(imageName)) { return image.withTag(newTag).withDigest(null); } - return image.getTree(); + return from; } + }; + } - private Docker.From upgradeThroughArgs(Docker.From from, Map defaults, Map upgrades, Set blocked) { - String imageVariable = soleVariable(from.getImageName()); - String imageName = imageVariable == null ? - literalText(from.getImageName()) : - defaults.get(imageVariable); - if (imageName == null) { - block(blocked, imageVariable, from.getTag() == null ? null : leadingVariable(from.getTag())); - return from; - } - - String tagVariable; - String tag; - if (from.getTag() == null) { - // A single argument holding the whole reference, as in `FROM ${BASE_IMAGE}` - String[] reference = splitReference(imageName); - if (imageVariable == null || reference == null) { - return from; + /** + * Withholding an argument can rule out the images that depend on it, which in turn can withhold further arguments, + * so the whole file is planned over and over until the set of withheld arguments stops growing. Only then is it + * known which `FROM` instructions may be rewritten, as an image may not be moved to a tag that is never written. + */ + private ArgPlan planUpgrades(Docker.File file, Map defaults) { + Map upgrades = new HashMap<>(); + Map replacements = new HashMap<>(); + Set blocked = new HashSet<>(); + int blockedCount; + do { + blockedCount = blocked.size(); + upgrades.clear(); + replacements.clear(); + for (Docker.Stage stage : file.getStages()) { + Docker.From from = stage.getFrom(); + if (containsVariable(from.getImageName()) || containsVariable(from.getTag())) { + Docker.From planned = planFrom(from, defaults, upgrades, blocked); + if (planned != from) { + replacements.put(from.getId(), planned); } - imageName = reference[0]; - tag = reference[1]; - tagVariable = imageVariable; - } else { - tagVariable = leadingVariable(from.getTag()); - tag = tagVariable == null ? literalText(from.getTag()) : defaults.get(tagVariable); - } - if (tag == null) { - return from; } + } + } while (blockedCount < blocked.size()); + return new ArgPlan(upgrades, replacements); + } - String newImageName = upgradedImageName(imageName); - if (newImageName == null) { - block(blocked, imageVariable, tagVariable); - return from; - } - String newTag = upgradedTag(tag); - if (newTag == null) { - return from; - } + private Docker.From planFrom(Docker.From from, Map defaults, Map upgrades, Set blocked) { + String imageVariable = soleVariable(from.getImageName()); + String tagVariable = from.getTag() == null ? null : leadingVariable(from.getTag()); + String imageName = imageVariable == null ? + literalText(from.getImageName()) : + defaultValue(defaults, blocked, imageVariable); + if (imageName == null) { + return block(from, blocked, imageVariable, tagVariable); + } - if (tagVariable != null && tagVariable.equals(imageVariable)) { - upgrades.put(imageVariable, newImageName + ":" + newTag); - return from.withDigest(null); - } - if (tagVariable == null) { - from = new DockerFrom(getCursor()).withTag(newTag); - } else { - upgrades.put(tagVariable, newTag); - } - if (!newImageName.equals(imageName)) { - if (imageVariable == null) { - from = withImageName(from, newImageName); - } else { - upgrades.put(imageVariable, newImageName); - } - } - return from.withDigest(null); + String tag; + boolean wholeReference = from.getTag() == null; + if (wholeReference) { + // A single argument holding the whole reference, as in `FROM ${BASE_IMAGE}` + String[] reference = imageVariable == null ? null : splitReference(imageName); + if (reference == null) { + return block(from, blocked, imageVariable, null); } - }; + imageName = reference[0]; + tag = reference[1]; + tagVariable = imageVariable; + } else { + tag = tagVariable == null ? literalText(from.getTag()) : defaultValue(defaults, blocked, tagVariable); + } + if (tag == null) { + return block(from, blocked, imageVariable, tagVariable); + } + + String newImageName = upgradedImageName(imageName); + String newTag = upgradedTag(tag); + if (newImageName == null || newTag == null) { + return block(from, blocked, imageVariable, tagVariable); + } + + if (wholeReference) { + upgrades.put(requireNonNull(imageVariable), newImageName + ":" + newTag); + return from.withDigest(null); + } + if (tagVariable == null) { + from = from.withTag(withText(requireNonNull(from.getTag()), newTag)); + } else { + upgrades.put(tagVariable, newTag); + } + if (!newImageName.equals(imageName)) { + if (imageVariable == null) { + from = from.withImageName(withText(from.getImageName(), newImageName)); + } else { + upgrades.put(imageVariable, newImageName); + } + } + return from.withDigest(null); } private @Nullable String upgradedImageName(String imageName) { @@ -222,19 +235,31 @@ private Docker.From upgradeThroughArgs(Docker.From from, Map def } /** - * Withhold arguments feeding an image we do not upgrade, as a shared argument can not be bumped for one image alone. + * Withhold arguments feeding an image we leave alone, as a shared argument can not be bumped for one image alone. */ - private static void block(Set blocked, @Nullable String imageVariable, @Nullable String tagVariable) { + private static Docker.From block(Docker.From from, Set blocked, @Nullable String imageVariable, @Nullable String tagVariable) { if (imageVariable != null) { blocked.add(imageVariable); } if (tagVariable != null) { blocked.add(tagVariable); } + return from; } - private static boolean containsVariable(String imageReferencePart) { - return imageReferencePart.indexOf('$') != -1; + private static @Nullable String defaultValue(Map defaults, Set blocked, String variable) { + return blocked.contains(variable) ? null : defaults.get(variable); + } + + private static boolean containsVariable(Docker.@Nullable Argument argument) { + if (argument != null) { + for (Docker.ArgumentContent content : argument.getContents()) { + if (content instanceof Docker.EnvironmentVariable) { + return true; + } + } + } + return false; } private static Docker.@Nullable Literal literalDefault(Docker.Arg arg) { @@ -282,10 +307,15 @@ private static boolean containsVariable(String imageReferencePart) { return new String[]{withoutDigest.substring(0, colon), withoutDigest.substring(colon + 1)}; } - private static Docker.From withImageName(Docker.From from, String imageName) { - Docker.Argument argument = from.getImageName(); + private static Docker.Argument withText(Docker.Argument argument, String text) { Docker.Literal literal = requireNonNull(sole(argument.getContents(), Docker.Literal.class)); - return from.withImageName(argument.withContents(singletonList(literal.withText(imageName)))); + return argument.withContents(singletonList(literal.withText(text))); + } + + @Value + private static class ArgPlan { + Map argUpgrades; + Map fromReplacements; } /** diff --git a/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java b/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java index d7041fc712..48405347d2 100644 --- a/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java @@ -223,6 +223,77 @@ void doNotUpgradeArgumentSharedWithAnImageWeDoNotUpgrade() { ); } + @Test + void keepDigestPinWhenTheSharedArgumentIsNotUpgraded() { + rewriteRun( + spec -> spec.recipe(new UpgradeDockerImageVersion(25)), + docker( + """ + ARG VERSION=11 + FROM eclipse-temurin:${VERSION}@sha256:1234567890abcdef AS build + FROM node:${VERSION} + """ + ) + ); + } + + @Test + void keepDeprecatedImageWhenTheSharedArgumentIsNotUpgraded() { + rewriteRun( + spec -> spec.recipe(new UpgradeDockerImageVersion(25)), + docker( + """ + ARG VERSION=11 + FROM openjdk:${VERSION} AS build + FROM node:${VERSION} + """ + ) + ); + } + + @Test + void keepImageArgumentWhenTheSharedVersionArgumentIsNotUpgraded() { + rewriteRun( + spec -> spec.recipe(new UpgradeDockerImageVersion(25)), + docker( + """ + ARG IMAGE=openjdk + ARG VERSION=11 + FROM ${IMAGE}:${VERSION} AS build + FROM node:${VERSION} + """ + ) + ); + } + + @Test + void doNotUpgradeImageArgumentSharedWithAnUntaggedImage() { + rewriteRun( + spec -> spec.recipe(new UpgradeDockerImageVersion(25)), + docker( + """ + ARG BASE=openjdk + FROM ${BASE}:11-jre AS build + FROM ${BASE} + """ + ) + ); + } + + @Test + void doNotUpgradeImageArgumentSharedWithAnImageStuckOnItsTag() { + rewriteRun( + spec -> spec.recipe(new UpgradeDockerImageVersion(25)), + docker( + """ + ARG BASE=openjdk + FROM ${BASE}:11-jre AS build + FROM ${BASE}:latest + """ + ) + ); + } + @CsvSource({ // Arguments that are not used in a FROM are left alone "JAVA_VERSION=11, eclipse-temurin:25-jre", From df053d212611d16a41f522e5ef46c5d266987852 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 21 Aug 2026 18:50:44 +0200 Subject: [PATCH 05/11] Read `ARG` default values through the new `Argument` accessors openrewrite/rewrite#8576 routes `ARG` values through the same path as every other argument, so quotes are modelled in `Literal.quoteStyle` and `$VAR` references become `EnvironmentVariable`. The local `QuotedText` that took the quotes off the literal text by hand is no longer needed, and reading a value is now `Argument.getText()`, which gives up on a reference the same way. --- .../migrate/UpgradeDockerImageVersion.java | 61 +++---------------- .../UpgradeDockerImageVersionTest.java | 14 +++++ 2 files changed, 21 insertions(+), 54 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java b/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java index ef9f69b6aa..c582f3c03b 100644 --- a/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java +++ b/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java @@ -85,9 +85,9 @@ public TreeVisitor getVisitor() { public Docker.File visitFile(Docker.File file, ExecutionContext ctx) { Map defaults = new HashMap<>(); for (Docker.Arg arg : file.getGlobalArgs()) { - Docker.Literal literalDefault = literalDefault(arg); - if (literalDefault != null) { - defaults.put(arg.getName().getText(), QuotedText.of(literalDefault.getText()).getText()); + String value = arg.getValue() == null ? null : arg.getValue().getText(); + if (value != null) { + defaults.put(arg.getName().getText(), value); } } @@ -101,13 +101,7 @@ public Docker.File visitFile(Docker.File file, ExecutionContext ctx) { } return f.withGlobalArgs(ListUtils.map(f.getGlobalArgs(), arg -> { String upgraded = upgrades.get(arg.getName().getText()); - Docker.Literal literalDefault = literalDefault(arg); - if (upgraded == null || literalDefault == null) { - return arg; - } - String requoted = QuotedText.of(literalDefault.getText()).requote(upgraded); - return arg.withValue(requireNonNull(arg.getValue()) - .withContents(singletonList(literalDefault.withText(requoted)))); + return upgraded == null ? arg : arg.withValue(withText(requireNonNull(arg.getValue()), upgraded)); })); } @@ -166,7 +160,7 @@ private Docker.From planFrom(Docker.From from, Map defaults, Map String imageVariable = soleVariable(from.getImageName()); String tagVariable = from.getTag() == null ? null : leadingVariable(from.getTag()); String imageName = imageVariable == null ? - literalText(from.getImageName()) : + from.getImageName().getText() : defaultValue(defaults, blocked, imageVariable); if (imageName == null) { return block(from, blocked, imageVariable, tagVariable); @@ -184,7 +178,7 @@ private Docker.From planFrom(Docker.From from, Map defaults, Map tag = reference[1]; tagVariable = imageVariable; } else { - tag = tagVariable == null ? literalText(from.getTag()) : defaultValue(defaults, blocked, tagVariable); + tag = tagVariable == null ? from.getTag().getText() : defaultValue(defaults, blocked, tagVariable); } if (tag == null) { return block(from, blocked, imageVariable, tagVariable); @@ -252,24 +246,7 @@ private static Docker.From block(Docker.From from, Set blocked, @Nullabl } private static boolean containsVariable(Docker.@Nullable Argument argument) { - if (argument != null) { - for (Docker.ArgumentContent content : argument.getContents()) { - if (content instanceof Docker.EnvironmentVariable) { - return true; - } - } - } - return false; - } - - private static Docker.@Nullable Literal literalDefault(Docker.Arg arg) { - Docker.Argument value = arg.getValue(); - return value == null ? null : sole(value.getContents(), Docker.Literal.class); - } - - private static @Nullable String literalText(Docker.Argument argument) { - Docker.Literal literal = sole(argument.getContents(), Docker.Literal.class); - return literal == null ? null : literal.getText(); + return argument != null && argument.hasEnvironmentVariables(); } private static @Nullable String soleVariable(Docker.Argument argument) { @@ -317,28 +294,4 @@ private static class ArgPlan { Map argUpgrades; Map fromReplacements; } - - /** - * The parser keeps any quotes around an {@code ARG} default value as part of the literal text, so they have to be - * taken off before matching a version, and put back on when writing the upgraded value. - */ - @Value - private static class QuotedText { - String quote; - String text; - - static QuotedText of(String source) { - if (source.length() > 1) { - char first = source.charAt(0); - if ((first == '"' || first == '\'') && source.charAt(source.length() - 1) == first) { - return new QuotedText(String.valueOf(first), source.substring(1, source.length() - 1)); - } - } - return new QuotedText("", source); - } - - String requote(String replacement) { - return quote + replacement + quote; - } - } } diff --git a/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java b/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java index 48405347d2..47b08da851 100644 --- a/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java @@ -294,6 +294,20 @@ void doNotUpgradeImageArgumentSharedWithAnImageStuckOnItsTag() { ); } + @Test + void doNotUpgradeArgumentDefaultingToAnotherVariable() { + rewriteRun( + spec -> spec.recipe(new UpgradeDockerImageVersion(25)), + docker( + """ + ARG DEFAULT_VERSION + ARG JAVA_VERSION=${DEFAULT_VERSION} + FROM eclipse-temurin:${JAVA_VERSION} + """ + ) + ); + } + @CsvSource({ // Arguments that are not used in a FROM are left alone "JAVA_VERSION=11, eclipse-temurin:25-jre", From aefbe627b3e90abac2e66e5ca75b03b6e6cac9b7 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 21 Aug 2026 22:27:25 +0200 Subject: [PATCH 06/11] Leave a `FROM` with a dangling tag separator alone openrewrite/rewrite#8590 splits an image reference in the grammar, so `FROM eclipse-temurin:` now parses rather than failing. Cover the shape that newly reaches the recipe. --- .../openrewrite/java/migrate/UpgradeDockerImageVersionTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java b/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java index 47b08da851..fa671ea122 100644 --- a/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java @@ -356,6 +356,7 @@ void doNotChangeArgumentDeclaredAfterFirstFrom() { "FROM node:20-alpine", // Tags without a leading Java version are left alone "FROM eclipse-temurin:latest", + "FROM eclipse-temurin:", // Already at or beyond the target version "FROM eclipse-temurin:25-jre", "FROM eclipse-temurin:26-jre", From afba6c0b5471b134b7478e3ce13eea92d2599ae7 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 21 Aug 2026 23:37:51 +0200 Subject: [PATCH 07/11] Do not bump `ARG` defaults shared with images we leave alone An argument holding only an image name carries no version, so renaming a deprecated image holds for every `FROM` that reads it, whatever tag each one carries. Withhold a version alone: ARG BASE=openjdk ARG BASE=eclipse-temurin FROM ${BASE}:11-jre -> FROM ${BASE}:25-jre FROM ${BASE}:latest FROM ${BASE}:latest A `FROM` whose image we can not resolve, or is not one we upgrade, still withholds both, as we know too little to rename either. Hoist the recipe into `defaults`, leaving the one test that varies the target version to override it. --- .../migrate/UpgradeDockerImageVersion.java | 11 ++++-- .../UpgradeDockerImageVersionTest.java | 39 +++++++++---------- 2 files changed, 26 insertions(+), 24 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java b/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java index c582f3c03b..6d580b90f0 100644 --- a/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java +++ b/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java @@ -172,7 +172,7 @@ private Docker.From planFrom(Docker.From from, Map defaults, Map // A single argument holding the whole reference, as in `FROM ${BASE_IMAGE}` String[] reference = imageVariable == null ? null : splitReference(imageName); if (reference == null) { - return block(from, blocked, imageVariable, null); + return from; } imageName = reference[0]; tag = reference[1]; @@ -185,10 +185,13 @@ private Docker.From planFrom(Docker.From from, Map defaults, Map } String newImageName = upgradedImageName(imageName); - String newTag = upgradedTag(tag); - if (newImageName == null || newTag == null) { + if (newImageName == null) { return block(from, blocked, imageVariable, tagVariable); } + String newTag = upgradedTag(tag); + if (newTag == null) { + return block(from, blocked, null, tagVariable); + } if (wholeReference) { upgrades.put(requireNonNull(imageVariable), newImageName + ":" + newTag); @@ -230,6 +233,8 @@ private Docker.From planFrom(Docker.From from, Map defaults, Map /** * Withhold arguments feeding an image we leave alone, as a shared argument can not be bumped for one image alone. + * Only a version is withheld this way; renaming a deprecated image holds for every `FROM` that reads the argument, + * whatever tag it carries. */ private static Docker.From block(Docker.From from, Set blocked, @Nullable String imageVariable, @Nullable String tagVariable) { if (imageVariable != null) { diff --git a/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java b/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java index fa671ea122..b3381a6967 100644 --- a/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java @@ -18,12 +18,18 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; +import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; import static org.openrewrite.docker.Assertions.docker; class UpgradeDockerImageVersionTest implements RewriteTest { + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new UpgradeDockerImageVersion(25)); + } + @CsvSource({ // Deprecated images migrate to eclipse-temurin "openjdk, 8, eclipse-temurin, 17, 17", @@ -73,7 +79,6 @@ void upgradeDockerImage(String fromImage, String fromTag, String toImage, String @ParameterizedTest void doNotChangeVariableImageReferences(String from) { rewriteRun( - spec -> spec.recipe(new UpgradeDockerImageVersion(25)), docker( """ ARG IMAGE_NAME @@ -108,7 +113,6 @@ void doNotChangeVariableImageReferences(String from) { @ParameterizedTest void upgradeArgumentDefaultValue(String beforeArg, String beforeFrom, String afterArg, String afterFrom) { rewriteRun( - spec -> spec.recipe(new UpgradeDockerImageVersion(25)), docker( """ ARG %s @@ -125,7 +129,6 @@ void upgradeArgumentDefaultValue(String beforeArg, String beforeFrom, String aft @Test void upgradeSingleQuotedArgumentDefaultValue() { rewriteRun( - spec -> spec.recipe(new UpgradeDockerImageVersion(25)), docker( """ ARG IMAGE_TAG='11-jre' @@ -142,7 +145,6 @@ void upgradeSingleQuotedArgumentDefaultValue() { @Test void upgradeDeprecatedImageNameAlongsideArgumentDefaultValue() { rewriteRun( - spec -> spec.recipe(new UpgradeDockerImageVersion(25)), docker( """ ARG JAVA_VERSION=11 @@ -159,7 +161,6 @@ void upgradeDeprecatedImageNameAlongsideArgumentDefaultValue() { @Test void upgradeArgumentDefaultValueSharedByStages() { rewriteRun( - spec -> spec.recipe(new UpgradeDockerImageVersion(25)), docker( """ ARG JAVA_VERSION=11 @@ -178,7 +179,6 @@ void upgradeArgumentDefaultValueSharedByStages() { @Test void dropDigestPinWhenUpgradingArgumentDefaultValue() { rewriteRun( - spec -> spec.recipe(new UpgradeDockerImageVersion(25)), docker( """ ARG JAVA_VERSION=11 @@ -195,7 +195,6 @@ void dropDigestPinWhenUpgradingArgumentDefaultValue() { @Test void dropDigestPinWhenUpgradingArgumentHoldingWholeReference() { rewriteRun( - spec -> spec.recipe(new UpgradeDockerImageVersion(25)), docker( """ ARG BASE_IMAGE=eclipse-temurin:11-jre @@ -212,7 +211,6 @@ void dropDigestPinWhenUpgradingArgumentHoldingWholeReference() { @Test void doNotUpgradeArgumentSharedWithAnImageWeDoNotUpgrade() { rewriteRun( - spec -> spec.recipe(new UpgradeDockerImageVersion(25)), docker( """ ARG VERSION=11 @@ -226,7 +224,6 @@ void doNotUpgradeArgumentSharedWithAnImageWeDoNotUpgrade() { @Test void keepDigestPinWhenTheSharedArgumentIsNotUpgraded() { rewriteRun( - spec -> spec.recipe(new UpgradeDockerImageVersion(25)), docker( """ ARG VERSION=11 @@ -240,7 +237,6 @@ void keepDigestPinWhenTheSharedArgumentIsNotUpgraded() { @Test void keepDeprecatedImageWhenTheSharedArgumentIsNotUpgraded() { rewriteRun( - spec -> spec.recipe(new UpgradeDockerImageVersion(25)), docker( """ ARG VERSION=11 @@ -254,7 +250,6 @@ void keepDeprecatedImageWhenTheSharedArgumentIsNotUpgraded() { @Test void keepImageArgumentWhenTheSharedVersionArgumentIsNotUpgraded() { rewriteRun( - spec -> spec.recipe(new UpgradeDockerImageVersion(25)), docker( """ ARG IMAGE=openjdk @@ -267,28 +262,36 @@ void keepImageArgumentWhenTheSharedVersionArgumentIsNotUpgraded() { } @Test - void doNotUpgradeImageArgumentSharedWithAnUntaggedImage() { + void upgradeImageArgumentSharedWithAnUntaggedImage() { rewriteRun( - spec -> spec.recipe(new UpgradeDockerImageVersion(25)), docker( """ ARG BASE=openjdk FROM ${BASE}:11-jre AS build FROM ${BASE} + """, + """ + ARG BASE=eclipse-temurin + FROM ${BASE}:25-jre AS build + FROM ${BASE} """ ) ); } @Test - void doNotUpgradeImageArgumentSharedWithAnImageStuckOnItsTag() { + void upgradeImageArgumentSharedWithAnImageStuckOnItsTag() { rewriteRun( - spec -> spec.recipe(new UpgradeDockerImageVersion(25)), docker( """ ARG BASE=openjdk FROM ${BASE}:11-jre AS build FROM ${BASE}:latest + """, + """ + ARG BASE=eclipse-temurin + FROM ${BASE}:25-jre AS build + FROM ${BASE}:latest """ ) ); @@ -297,7 +300,6 @@ void doNotUpgradeImageArgumentSharedWithAnImageStuckOnItsTag() { @Test void doNotUpgradeArgumentDefaultingToAnotherVariable() { rewriteRun( - spec -> spec.recipe(new UpgradeDockerImageVersion(25)), docker( """ ARG DEFAULT_VERSION @@ -326,7 +328,6 @@ void doNotUpgradeArgumentDefaultingToAnotherVariable() { @ParameterizedTest void doNotChangeUnrelatedArgumentDefaultValues(String arg, String from) { rewriteRun( - spec -> spec.recipe(new UpgradeDockerImageVersion(25)), docker( """ ARG %s @@ -339,7 +340,6 @@ void doNotChangeUnrelatedArgumentDefaultValues(String arg, String from) { @Test void doNotChangeArgumentDeclaredAfterFirstFrom() { rewriteRun( - spec -> spec.recipe(new UpgradeDockerImageVersion(25)), docker( """ FROM eclipse-temurin:25-jre @@ -364,7 +364,6 @@ void doNotChangeArgumentDeclaredAfterFirstFrom() { @ParameterizedTest void doNotChangeUnrelatedImages(String from) { rewriteRun( - spec -> spec.recipe(new UpgradeDockerImageVersion(25)), docker(from) ); } @@ -376,7 +375,6 @@ void doNotChangeUnrelatedImages(String from) { @ParameterizedTest void dropStaleDigestPin(String before, String after) { rewriteRun( - spec -> spec.recipe(new UpgradeDockerImageVersion(25)), docker(before, after) ); } @@ -384,7 +382,6 @@ void dropStaleDigestPin(String before, String after) { @Test void changeLiteralImageAlongsideVariableImage() { rewriteRun( - spec -> spec.recipe(new UpgradeDockerImageVersion(25)), docker( """ ARG IMAGE_TAG From 40a9509c773588976087b239ffb352761f860468 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 21 Aug 2026 23:48:06 +0200 Subject: [PATCH 08/11] Bump an `ARG` default shared with an image we leave alone An image we do not recognise may well be an internal one built on the same Java version, and if it shared a version argument before it should move along with it: ARG VERSION=11 ARG VERSION=25 FROM eclipse-temurin:${VERSION} -> FROM eclipse-temurin:${VERSION} FROM acme/base:${VERSION} FROM acme/base:${VERSION} Each `FROM` now stands on its own, so the repeated planning that let one withheld argument withhold the next is gone, and a visitor reads the file in one pass. Group the tests that assert no change into a nested class. --- .../migrate/UpgradeDockerImageVersion.java | 59 ++--- .../UpgradeDockerImageVersionTest.java | 213 ++++++++++-------- 2 files changed, 137 insertions(+), 135 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java b/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java index 6d580b90f0..ae3da2504d 100644 --- a/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java +++ b/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java @@ -130,40 +130,33 @@ public Docker.From visitFrom(Docker.From from, ExecutionContext ctx) { } /** - * Withholding an argument can rule out the images that depend on it, which in turn can withhold further arguments, - * so the whole file is planned over and over until the set of withheld arguments stops growing. Only then is it - * known which `FROM` instructions may be rewritten, as an image may not be moved to a tag that is never written. + * An argument is shared by every `FROM` that reads it, so which ones may be rewritten is only known once the whole + * file has been read. */ private ArgPlan planUpgrades(Docker.File file, Map defaults) { Map upgrades = new HashMap<>(); Map replacements = new HashMap<>(); - Set blocked = new HashSet<>(); - int blockedCount; - do { - blockedCount = blocked.size(); - upgrades.clear(); - replacements.clear(); - for (Docker.Stage stage : file.getStages()) { - Docker.From from = stage.getFrom(); + new DockerIsoVisitor() { + @Override + public Docker.From visitFrom(Docker.From from, Integer p) { if (containsVariable(from.getImageName()) || containsVariable(from.getTag())) { - Docker.From planned = planFrom(from, defaults, upgrades, blocked); + Docker.From planned = planFrom(from, defaults, upgrades); if (planned != from) { replacements.put(from.getId(), planned); } } + return from; } - } while (blockedCount < blocked.size()); + }.visit(file, 0); return new ArgPlan(upgrades, replacements); } - private Docker.From planFrom(Docker.From from, Map defaults, Map upgrades, Set blocked) { + private Docker.From planFrom(Docker.From from, Map defaults, Map upgrades) { String imageVariable = soleVariable(from.getImageName()); String tagVariable = from.getTag() == null ? null : leadingVariable(from.getTag()); - String imageName = imageVariable == null ? - from.getImageName().getText() : - defaultValue(defaults, blocked, imageVariable); + String imageName = imageVariable == null ? from.getImageName().getText() : defaults.get(imageVariable); if (imageName == null) { - return block(from, blocked, imageVariable, tagVariable); + return from; } String tag; @@ -178,19 +171,16 @@ private Docker.From planFrom(Docker.From from, Map defaults, Map tag = reference[1]; tagVariable = imageVariable; } else { - tag = tagVariable == null ? from.getTag().getText() : defaultValue(defaults, blocked, tagVariable); + tag = tagVariable == null ? from.getTag().getText() : defaults.get(tagVariable); } if (tag == null) { - return block(from, blocked, imageVariable, tagVariable); + return from; } String newImageName = upgradedImageName(imageName); - if (newImageName == null) { - return block(from, blocked, imageVariable, tagVariable); - } String newTag = upgradedTag(tag); - if (newTag == null) { - return block(from, blocked, null, tagVariable); + if (newImageName == null || newTag == null) { + return from; } if (wholeReference) { @@ -231,25 +221,6 @@ private Docker.From planFrom(Docker.From from, Map defaults, Map return version + (matcher.group(2) == null ? "" : matcher.group(2)); } - /** - * Withhold arguments feeding an image we leave alone, as a shared argument can not be bumped for one image alone. - * Only a version is withheld this way; renaming a deprecated image holds for every `FROM` that reads the argument, - * whatever tag it carries. - */ - private static Docker.From block(Docker.From from, Set blocked, @Nullable String imageVariable, @Nullable String tagVariable) { - if (imageVariable != null) { - blocked.add(imageVariable); - } - if (tagVariable != null) { - blocked.add(tagVariable); - } - return from; - } - - private static @Nullable String defaultValue(Map defaults, Set blocked, String variable) { - return blocked.contains(variable) ? null : defaults.get(variable); - } - private static boolean containsVariable(Docker.@Nullable Argument argument) { return argument != null && argument.hasEnvironmentVariables(); } diff --git a/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java b/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java index b3381a6967..ae04666104 100644 --- a/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java @@ -15,6 +15,7 @@ */ package org.openrewrite.java.migrate; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; @@ -69,26 +70,6 @@ void upgradeDockerImage(String fromImage, String fromTag, String toImage, String ); } - @CsvSource({ - "FROM ${IMAGE_NAME}:${IMAGE_TAG}", - "FROM $IMAGE_NAME:$IMAGE_TAG", - "FROM ${IMAGE_NAME}:11", - "FROM eclipse-temurin:${IMAGE_TAG}", - "FROM ${REGISTRY}/eclipse-temurin:11-jre", - }) - @ParameterizedTest - void doNotChangeVariableImageReferences(String from) { - rewriteRun( - docker( - """ - ARG IMAGE_NAME - ARG IMAGE_TAG - ARG REGISTRY - %s - """.formatted(from) - ) - ); - } @CsvSource({ // The argument holds the bare version @@ -209,46 +190,61 @@ void dropDigestPinWhenUpgradingArgumentHoldingWholeReference() { } @Test - void doNotUpgradeArgumentSharedWithAnImageWeDoNotUpgrade() { + void upgradeArgumentSharedWithAnImageWeDoNotUpgrade() { rewriteRun( docker( """ ARG VERSION=11 FROM eclipse-temurin:${VERSION} AS build FROM node:${VERSION} + """, + """ + ARG VERSION=25 + FROM eclipse-temurin:${VERSION} AS build + FROM node:${VERSION} """ ) ); } @Test - void keepDigestPinWhenTheSharedArgumentIsNotUpgraded() { + void dropDigestPinWhenUpgradingASharedArgument() { rewriteRun( docker( """ ARG VERSION=11 FROM eclipse-temurin:${VERSION}@sha256:1234567890abcdef AS build FROM node:${VERSION} + """, + """ + ARG VERSION=25 + FROM eclipse-temurin:${VERSION} AS build + FROM node:${VERSION} """ ) ); } @Test - void keepDeprecatedImageWhenTheSharedArgumentIsNotUpgraded() { + void upgradeDeprecatedImageAlongsideASharedArgument() { rewriteRun( docker( """ ARG VERSION=11 FROM openjdk:${VERSION} AS build FROM node:${VERSION} + """, + """ + ARG VERSION=25 + FROM eclipse-temurin:${VERSION} AS build + FROM node:${VERSION} """ ) ); } @Test - void keepImageArgumentWhenTheSharedVersionArgumentIsNotUpgraded() { + void upgradeImageAndVersionArgumentsSharedWithAnotherImage() { rewriteRun( docker( """ @@ -256,6 +252,12 @@ void keepImageArgumentWhenTheSharedVersionArgumentIsNotUpgraded() { ARG VERSION=11 FROM ${IMAGE}:${VERSION} AS build FROM node:${VERSION} + """, + """ + ARG IMAGE=eclipse-temurin + ARG VERSION=25 + FROM ${IMAGE}:${VERSION} AS build + FROM node:${VERSION} """ ) ); @@ -297,76 +299,9 @@ void upgradeImageArgumentSharedWithAnImageStuckOnItsTag() { ); } - @Test - void doNotUpgradeArgumentDefaultingToAnotherVariable() { - rewriteRun( - docker( - """ - ARG DEFAULT_VERSION - ARG JAVA_VERSION=${DEFAULT_VERSION} - FROM eclipse-temurin:${JAVA_VERSION} - """ - ) - ); - } - @CsvSource({ - // Arguments that are not used in a FROM are left alone - "JAVA_VERSION=11, eclipse-temurin:25-jre", - // Arguments for unrelated images are left alone - "NODE_VERSION=20, node:${NODE_VERSION}-alpine", - // Arguments already at or beyond the target version are left alone - "JAVA_VERSION=25, eclipse-temurin:${JAVA_VERSION}-jre", - "JAVA_VERSION=26, eclipse-temurin:${JAVA_VERSION}-jre", - // Arguments not holding a leading version are left alone - "JAVA_VERSION=latest, eclipse-temurin:${JAVA_VERSION}", - "JAVA_VERSION=\"latest\", eclipse-temurin:${JAVA_VERSION}", - "SUFFIX=-jre, eclipse-temurin:11${SUFFIX}", - // Arguments that only hold part of the image name are left alone - "REGISTRY=docker.io, ${REGISTRY}/eclipse-temurin:11-jre", - }) - @ParameterizedTest - void doNotChangeUnrelatedArgumentDefaultValues(String arg, String from) { - rewriteRun( - docker( - """ - ARG %s - FROM %s - """.formatted(arg, from) - ) - ); - } - @Test - void doNotChangeArgumentDeclaredAfterFirstFrom() { - rewriteRun( - docker( - """ - FROM eclipse-temurin:25-jre - ARG JAVA_VERSION=11 - RUN echo "${JAVA_VERSION}" - """ - ) - ); - } - @CsvSource({ - // Unrelated images are left alone - "FROM ubuntu:22.04", - "FROM node:20-alpine", - // Tags without a leading Java version are left alone - "FROM eclipse-temurin:latest", - "FROM eclipse-temurin:", - // Already at or beyond the target version - "FROM eclipse-temurin:25-jre", - "FROM eclipse-temurin:26-jre", - }) - @ParameterizedTest - void doNotChangeUnrelatedImages(String from) { - rewriteRun( - docker(from) - ); - } @CsvSource({ "FROM openjdk:11-jre@sha256:1234567890abcdef, FROM eclipse-temurin:25-jre", @@ -396,4 +331,100 @@ void changeLiteralImageAlongsideVariableImage() { ) ); } + + @Nested + class NoChange { + + @CsvSource({ + "FROM ${IMAGE_NAME}:${IMAGE_TAG}", + "FROM $IMAGE_NAME:$IMAGE_TAG", + "FROM ${IMAGE_NAME}:11", + "FROM eclipse-temurin:${IMAGE_TAG}", + "FROM ${REGISTRY}/eclipse-temurin:11-jre", + }) + @ParameterizedTest + void variableImageReferences(String from) { + rewriteRun( + docker( + """ + ARG IMAGE_NAME + ARG IMAGE_TAG + ARG REGISTRY + %s + """.formatted(from) + ) + ); + } + + @Test + void anArgumentDefaultingToAnotherVariable() { + rewriteRun( + docker( + """ + ARG DEFAULT_VERSION + ARG JAVA_VERSION=${DEFAULT_VERSION} + FROM eclipse-temurin:${JAVA_VERSION} + """ + ) + ); + } + + @CsvSource({ + // Arguments that are not used in a FROM are left alone + "JAVA_VERSION=11, eclipse-temurin:25-jre", + // Arguments for unrelated images are left alone + "NODE_VERSION=20, node:${NODE_VERSION}-alpine", + // Arguments already at or beyond the target version are left alone + "JAVA_VERSION=25, eclipse-temurin:${JAVA_VERSION}-jre", + "JAVA_VERSION=26, eclipse-temurin:${JAVA_VERSION}-jre", + // Arguments not holding a leading version are left alone + "JAVA_VERSION=latest, eclipse-temurin:${JAVA_VERSION}", + "JAVA_VERSION=\"latest\", eclipse-temurin:${JAVA_VERSION}", + "SUFFIX=-jre, eclipse-temurin:11${SUFFIX}", + // Arguments that only hold part of the image name are left alone + "REGISTRY=docker.io, ${REGISTRY}/eclipse-temurin:11-jre", + }) + @ParameterizedTest + void unrelatedArgumentDefaultValues(String arg, String from) { + rewriteRun( + docker( + """ + ARG %s + FROM %s + """.formatted(arg, from) + ) + ); + } + + @Test + void anArgumentDeclaredAfterTheFirstFrom() { + rewriteRun( + docker( + """ + FROM eclipse-temurin:25-jre + ARG JAVA_VERSION=11 + RUN echo "${JAVA_VERSION}" + """ + ) + ); + } + + @CsvSource({ + // Unrelated images are left alone + "FROM ubuntu:22.04", + "FROM node:20-alpine", + // Tags without a leading Java version are left alone + "FROM eclipse-temurin:latest", + "FROM eclipse-temurin:", + // Already at or beyond the target version + "FROM eclipse-temurin:25-jre", + "FROM eclipse-temurin:26-jre", + }) + @ParameterizedTest + void unrelatedImages(String from) { + rewriteRun( + docker(from) + ); + } + } } From 8fc0a7ad62945f4e739a8c4c2159ff044d8e9de2 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sat, 22 Aug 2026 00:04:44 +0200 Subject: [PATCH 09/11] Upgrade a Java image pulled through a registry Mirroring stock images through an internal registry is ordinary, so read the repository past a registry rather than treating the whole reference as a name we do not know: ARG REGISTRY ARG REGISTRY FROM ${REGISTRY}/eclipse-temurin:11-jre -> FROM ${REGISTRY}/eclipse-temurin:25-jre FROM docker.io/openjdk:11-jre FROM docker.io/eclipse-temurin:25-jre Which leading segment is a registry follows Docker's own rule: one holding a `.` or a `:`, or `localhost`. Anything else belongs to the repository, so `azul/zulu-openjdk` still reads as a name and `mycompany/eclipse-temurin` is left alone. Also rewrite only the `ARG` declaration a default was read from, as a name may be declared more than once and the others carry no value to upgrade. --- .../migrate/UpgradeDockerImageVersion.java | 73 +++++++++++++++---- .../UpgradeDockerImageVersionTest.java | 73 +++++++++++++++++-- 2 files changed, 123 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java b/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java index ae3da2504d..47027f4386 100644 --- a/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java +++ b/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java @@ -84,10 +84,12 @@ public TreeVisitor getVisitor() { @Override public Docker.File visitFile(Docker.File file, ExecutionContext ctx) { Map defaults = new HashMap<>(); + Map declarations = new HashMap<>(); for (Docker.Arg arg : file.getGlobalArgs()) { String value = arg.getValue() == null ? null : arg.getValue().getText(); if (value != null) { defaults.put(arg.getName().getText(), value); + declarations.put(arg.getName().getText(), arg.getId()); } } @@ -100,8 +102,12 @@ public Docker.File visitFile(Docker.File file, ExecutionContext ctx) { return f; } return f.withGlobalArgs(ListUtils.map(f.getGlobalArgs(), arg -> { - String upgraded = upgrades.get(arg.getName().getText()); - return upgraded == null ? arg : arg.withValue(withText(requireNonNull(arg.getValue()), upgraded)); + String name = arg.getName().getText(); + String upgraded = upgrades.get(name); + if (upgraded == null || !arg.getId().equals(declarations.get(name))) { + return arg; + } + return arg.withValue(withText(requireNonNull(arg.getValue()), upgraded)); })); } @@ -118,13 +124,14 @@ public Docker.From visitFrom(Docker.From from, ExecutionContext ctx) { return from; } String imageName = image.getImageName().orElse(""); - if (DEPRECATED_IMAGES.contains(imageName)) { - return image.withImageReference(NEW_IMAGE + ":" + newTag); + String newImageName = upgradedImageName(imageName); + if (newImageName == null) { + return from; } - if (CURRENT_IMAGES.contains(imageName)) { - return image.withTag(newTag).withDigest(null); + if (!newImageName.equals(imageName)) { + return image.withImageReference(newImageName + ":" + newTag); } - return from; + return image.withTag(newTag).withDigest(null); } }; } @@ -154,7 +161,10 @@ public Docker.From visitFrom(Docker.From from, Integer p) { private Docker.From planFrom(Docker.From from, Map defaults, Map upgrades) { String imageVariable = soleVariable(from.getImageName()); String tagVariable = from.getTag() == null ? null : leadingVariable(from.getTag()); - String imageName = imageVariable == null ? from.getImageName().getText() : defaults.get(imageVariable); + String variableRegistry = imageVariable == null ? repositoryAfterVariableRegistry(from.getImageName()) : null; + String imageName = imageVariable == null ? + (variableRegistry == null ? from.getImageName().getText() : variableRegistry) : + defaults.get(imageVariable); if (imageName == null) { return from; } @@ -169,7 +179,6 @@ private Docker.From planFrom(Docker.From from, Map defaults, Map } imageName = reference[0]; tag = reference[1]; - tagVariable = imageVariable; } else { tag = tagVariable == null ? from.getTag().getText() : defaults.get(tagVariable); } @@ -194,7 +203,8 @@ private Docker.From planFrom(Docker.From from, Map defaults, Map } if (!newImageName.equals(imageName)) { if (imageVariable == null) { - from = from.withImageName(withText(from.getImageName(), newImageName)); + from = from.withImageName(withText(from.getImageName(), + variableRegistry == null ? newImageName : "/" + newImageName)); } else { upgrades.put(imageVariable, newImageName); } @@ -203,10 +213,43 @@ private Docker.From planFrom(Docker.From from, Map defaults, Map } private @Nullable String upgradedImageName(String imageName) { - if (DEPRECATED_IMAGES.contains(imageName)) { - return NEW_IMAGE; + int repository = repositoryIndex(imageName); + String registry = imageName.substring(0, repository); + String name = imageName.substring(repository); + if (DEPRECATED_IMAGES.contains(name)) { + return registry + NEW_IMAGE; + } + return CURRENT_IMAGES.contains(name) ? imageName : null; + } + + /** + * Where the repository starts within an image name. Docker reads a leading path segment holding a `.` or a `:`, + * or `localhost`, as the registry to pull from; anything else belongs to the repository, as in `azul/zulu-openjdk`. + */ + private static int repositoryIndex(String imageName) { + int slash = imageName.indexOf('/'); + if (slash == -1) { + return 0; + } + String host = imageName.substring(0, slash); + return "localhost".equals(host) || host.indexOf('.') != -1 || host.indexOf(':') != -1 ? slash + 1 : 0; + } + + /** + * The repository of an image name whose registry is a variable, as in the `eclipse-temurin` of + * `${REGISTRY}/eclipse-temurin`, or null when the name is not of that shape. + */ + private static @Nullable String repositoryAfterVariableRegistry(Docker.Argument imageName) { + List contents = imageName.getContents(); + if (contents.size() < 2 || !(contents.get(0) instanceof Docker.EnvironmentVariable)) { + return null; + } + Docker.ArgumentContent last = contents.get(contents.size() - 1); + if (!(last instanceof Docker.Literal)) { + return null; } - return CURRENT_IMAGES.contains(imageName) ? imageName : null; + String text = ((Docker.Literal) last).getText(); + return text.startsWith("/") ? text.substring(1) : null; } private @Nullable String upgradedTag(String tag) { @@ -261,8 +304,8 @@ private static boolean containsVariable(Docker.@Nullable Argument argument) { } private static Docker.Argument withText(Docker.Argument argument, String text) { - Docker.Literal literal = requireNonNull(sole(argument.getContents(), Docker.Literal.class)); - return argument.withContents(singletonList(literal.withText(text))); + return argument.withContents(ListUtils.mapLast(argument.getContents(), + content -> content instanceof Docker.Literal ? ((Docker.Literal) content).withText(text) : content)); } @Value diff --git a/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java b/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java index ae04666104..036a1665b8 100644 --- a/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java @@ -70,7 +70,6 @@ void upgradeDockerImage(String fromImage, String fromTag, String toImage, String ); } - @CsvSource({ // The argument holds the bare version "java_version=17, eclipse-temurin:${java_version}, java_version=25, eclipse-temurin:${java_version}", @@ -299,10 +298,6 @@ void upgradeImageArgumentSharedWithAnImageStuckOnItsTag() { ); } - - - - @CsvSource({ "FROM openjdk:11-jre@sha256:1234567890abcdef, FROM eclipse-temurin:25-jre", "FROM eclipse-temurin:11-jre@sha256:1234567890abcdef, FROM eclipse-temurin:25-jre", @@ -332,6 +327,69 @@ void changeLiteralImageAlongsideVariableImage() { ); } + @Test + void upgradeOnlyTheArgumentDeclarationHoldingTheDefault() { + rewriteRun( + docker( + """ + ARG JAVA_VERSION + ARG JAVA_VERSION=11 + FROM eclipse-temurin:${JAVA_VERSION} + """, + """ + ARG JAVA_VERSION + ARG JAVA_VERSION=25 + FROM eclipse-temurin:${JAVA_VERSION} + """ + ) + ); + } + + @CsvSource({ + // A registry the FROM spells out + "docker.io/eclipse-temurin:11-jre, docker.io/eclipse-temurin:25-jre", + "docker.io/openjdk:11-jre, docker.io/eclipse-temurin:25-jre", + "myregistry:5000/openjdk:11-jre, myregistry:5000/eclipse-temurin:25-jre", + "localhost/eclipse-temurin:11-jre, localhost/eclipse-temurin:25-jre", + // A registry the FROM builds from an argument + "${REGISTRY}/eclipse-temurin:11-jre, ${REGISTRY}/eclipse-temurin:25-jre", + "${REGISTRY}/openjdk:11-jre, ${REGISTRY}/eclipse-temurin:25-jre", + "${REGISTRY}/azul/zulu-openjdk:11-jdk, ${REGISTRY}/azul/zulu-openjdk:25-jdk", + }) + @ParameterizedTest + void upgradeAnImageBehindARegistry(String before, String after) { + rewriteRun( + docker( + """ + ARG REGISTRY + FROM %s + """.formatted(before), + """ + ARG REGISTRY + FROM %s + """.formatted(after) + ) + ); + } + + @Test + void upgradeArgumentDefaultValueBehindAVariableRegistry() { + rewriteRun( + docker( + """ + ARG REGISTRY + ARG JAVA_VERSION=11 + FROM ${REGISTRY}/eclipse-temurin:${JAVA_VERSION}-jre + """, + """ + ARG REGISTRY + ARG JAVA_VERSION=25 + FROM ${REGISTRY}/eclipse-temurin:${JAVA_VERSION}-jre + """ + ) + ); + } + @Nested class NoChange { @@ -340,7 +398,6 @@ class NoChange { "FROM $IMAGE_NAME:$IMAGE_TAG", "FROM ${IMAGE_NAME}:11", "FROM eclipse-temurin:${IMAGE_TAG}", - "FROM ${REGISTRY}/eclipse-temurin:11-jre", }) @ParameterizedTest void variableImageReferences(String from) { @@ -381,8 +438,8 @@ void anArgumentDefaultingToAnotherVariable() { "JAVA_VERSION=latest, eclipse-temurin:${JAVA_VERSION}", "JAVA_VERSION=\"latest\", eclipse-temurin:${JAVA_VERSION}", "SUFFIX=-jre, eclipse-temurin:11${SUFFIX}", - // Arguments that only hold part of the image name are left alone - "REGISTRY=docker.io, ${REGISTRY}/eclipse-temurin:11-jre", + // A leading segment that is not a registry host belongs to the repository name + "JAVA_VERSION=11, mycompany/eclipse-temurin:${JAVA_VERSION}-jre", }) @ParameterizedTest void unrelatedArgumentDefaultValues(String arg, String from) { From 8f0ba52d9f20a73027cf9e7b1348782d8daf148b Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sat, 22 Aug 2026 01:29:13 +0200 Subject: [PATCH 10/11] Read a registry through rewrite-docker's `ImageName` The rule for which leading segment of an image name is a registry now lives upstream, in `org.openrewrite.docker.trait.ImageName`, so read the registry and repository from it rather than keeping a second copy of the rule here. That also covers a segment upstream reads as a registry and the copy here did not: a repository name may not be uppercase, so `MyRegistry/openjdk` names a registry, not a repository. Reading the name through `getTextWithVariables()` lets one parse serve a spelt-out registry and a `${REGISTRY}` alike, which retires the second helper that recognised the latter by the shape of its contents. --- .../migrate/UpgradeDockerImageVersion.java | 65 +++++++------------ .../UpgradeDockerImageVersionTest.java | 2 + 2 files changed, 27 insertions(+), 40 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java b/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java index 47027f4386..8adf1a0c7c 100644 --- a/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java +++ b/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java @@ -24,6 +24,7 @@ import org.openrewrite.TreeVisitor; import org.openrewrite.docker.DockerIsoVisitor; import org.openrewrite.docker.trait.DockerFrom; +import org.openrewrite.docker.trait.ImageName; import org.openrewrite.docker.tree.Docker; import org.openrewrite.internal.ListUtils; @@ -161,9 +162,8 @@ public Docker.From visitFrom(Docker.From from, Integer p) { private Docker.From planFrom(Docker.From from, Map defaults, Map upgrades) { String imageVariable = soleVariable(from.getImageName()); String tagVariable = from.getTag() == null ? null : leadingVariable(from.getTag()); - String variableRegistry = imageVariable == null ? repositoryAfterVariableRegistry(from.getImageName()) : null; String imageName = imageVariable == null ? - (variableRegistry == null ? from.getImageName().getText() : variableRegistry) : + from.getImageName().getTextWithVariables() : defaults.get(imageVariable); if (imageName == null) { return from; @@ -203,8 +203,7 @@ private Docker.From planFrom(Docker.From from, Map defaults, Map } if (!newImageName.equals(imageName)) { if (imageVariable == null) { - from = from.withImageName(withText(from.getImageName(), - variableRegistry == null ? newImageName : "/" + newImageName)); + from = from.withImageName(withRepository(from.getImageName(), imageName, newImageName)); } else { upgrades.put(imageVariable, newImageName); } @@ -213,43 +212,12 @@ private Docker.From planFrom(Docker.From from, Map defaults, Map } private @Nullable String upgradedImageName(String imageName) { - int repository = repositoryIndex(imageName); - String registry = imageName.substring(0, repository); - String name = imageName.substring(repository); - if (DEPRECATED_IMAGES.contains(name)) { - return registry + NEW_IMAGE; + ImageName parsed = ImageName.parse(imageName); + if (DEPRECATED_IMAGES.contains(parsed.getPath())) { + String registry = parsed.getRegistry(); + return registry == null ? NEW_IMAGE : registry + '/' + NEW_IMAGE; } - return CURRENT_IMAGES.contains(name) ? imageName : null; - } - - /** - * Where the repository starts within an image name. Docker reads a leading path segment holding a `.` or a `:`, - * or `localhost`, as the registry to pull from; anything else belongs to the repository, as in `azul/zulu-openjdk`. - */ - private static int repositoryIndex(String imageName) { - int slash = imageName.indexOf('/'); - if (slash == -1) { - return 0; - } - String host = imageName.substring(0, slash); - return "localhost".equals(host) || host.indexOf('.') != -1 || host.indexOf(':') != -1 ? slash + 1 : 0; - } - - /** - * The repository of an image name whose registry is a variable, as in the `eclipse-temurin` of - * `${REGISTRY}/eclipse-temurin`, or null when the name is not of that shape. - */ - private static @Nullable String repositoryAfterVariableRegistry(Docker.Argument imageName) { - List contents = imageName.getContents(); - if (contents.size() < 2 || !(contents.get(0) instanceof Docker.EnvironmentVariable)) { - return null; - } - Docker.ArgumentContent last = contents.get(contents.size() - 1); - if (!(last instanceof Docker.Literal)) { - return null; - } - String text = ((Docker.Literal) last).getText(); - return text.startsWith("/") ? text.substring(1) : null; + return CURRENT_IMAGES.contains(parsed.getPath()) ? imageName : null; } private @Nullable String upgradedTag(String tag) { @@ -303,6 +271,23 @@ private static boolean containsVariable(Docker.@Nullable Argument argument) { return new String[]{withoutDigest.substring(0, colon), withoutDigest.substring(colon + 1)}; } + /// The registry an image is pulled from is left as written, which may be a variable, so only the trailing + /// repository is rewritten. + private static Docker.Argument withRepository(Docker.Argument imageName, String from, String to) { + String oldPath = ImageName.parse(from).getPath(); + String newPath = ImageName.parse(to).getPath(); + return imageName.withContents(ListUtils.mapLast(imageName.getContents(), content -> { + if (!(content instanceof Docker.Literal)) { + return content; + } + Docker.Literal literal = (Docker.Literal) content; + String text = literal.getText(); + return text.endsWith(oldPath) ? + literal.withText(text.substring(0, text.length() - oldPath.length()) + newPath) : + literal; + })); + } + private static Docker.Argument withText(Docker.Argument argument, String text) { return argument.withContents(ListUtils.mapLast(argument.getContents(), content -> content instanceof Docker.Literal ? ((Docker.Literal) content).withText(text) : content)); diff --git a/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java b/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java index 036a1665b8..2cf1e5efb6 100644 --- a/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java @@ -351,6 +351,8 @@ void upgradeOnlyTheArgumentDeclarationHoldingTheDefault() { "docker.io/openjdk:11-jre, docker.io/eclipse-temurin:25-jre", "myregistry:5000/openjdk:11-jre, myregistry:5000/eclipse-temurin:25-jre", "localhost/eclipse-temurin:11-jre, localhost/eclipse-temurin:25-jre", + // A repository name may not be uppercase, so an uppercase segment can only be a registry + "MyRegistry/openjdk:11-jre, MyRegistry/eclipse-temurin:25-jre", // A registry the FROM builds from an argument "${REGISTRY}/eclipse-temurin:11-jre, ${REGISTRY}/eclipse-temurin:25-jre", "${REGISTRY}/openjdk:11-jre, ${REGISTRY}/eclipse-temurin:25-jre", From 506a4f1934df80d75b26a360b2b3abab34251e82 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sat, 22 Aug 2026 01:46:30 +0200 Subject: [PATCH 11/11] Upgrade a `FROM` as it is visited Planning every `FROM` before rewriting any was there to withhold an argument another `FROM` ruled out. Nothing is withheld now, so each one can be upgraded where it is met, which retires the second traversal, the plan holder and the map of replacements keyed by id. Read the declaration an argument's default came from by its value rather than by its id, so the two maps become one. --- .../migrate/UpgradeDockerImageVersion.java | 70 +++++-------------- .../UpgradeDockerImageVersionTest.java | 5 +- 2 files changed, 21 insertions(+), 54 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java b/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java index 8adf1a0c7c..32c489aa3e 100644 --- a/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java +++ b/src/main/java/org/openrewrite/java/migrate/UpgradeDockerImageVersion.java @@ -33,13 +33,11 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.UUID; import java.util.regex.Matcher; import java.util.regex.Pattern; import static java.util.Arrays.asList; import static java.util.Collections.emptyMap; -import static java.util.Collections.singletonList; import static java.util.Objects.requireNonNull; @EqualsAndHashCode(callSuper = false) @@ -62,7 +60,8 @@ public class UpgradeDockerImageVersion extends Recipe { private static final int OLDEST_VERSION = 8; private static final Pattern VERSIONED_TAG = Pattern.compile("(\\d{1,3})(\\D.*)?"); - private static final String FROM_REPLACEMENTS = "fromReplacements"; + private static final String ARG_DEFAULTS = "argDefaults"; + private static final String ARG_UPGRADES = "argUpgrades"; String displayName = "Upgrade Docker image Java version"; String description = "Upgrade Docker image tags to use the specified Java version. " + @@ -85,38 +84,37 @@ public TreeVisitor getVisitor() { @Override public Docker.File visitFile(Docker.File file, ExecutionContext ctx) { Map defaults = new HashMap<>(); - Map declarations = new HashMap<>(); for (Docker.Arg arg : file.getGlobalArgs()) { String value = arg.getValue() == null ? null : arg.getValue().getText(); if (value != null) { defaults.put(arg.getName().getText(), value); - declarations.put(arg.getName().getText(), arg.getId()); } } - ArgPlan plan = planUpgrades(file, defaults); - getCursor().putMessage(FROM_REPLACEMENTS, plan.getFromReplacements()); + Map upgrades = new HashMap<>(); + getCursor().putMessage(ARG_DEFAULTS, defaults); + getCursor().putMessage(ARG_UPGRADES, upgrades); Docker.File f = super.visitFile(file, ctx); - - Map upgrades = plan.getArgUpgrades(); if (upgrades.isEmpty()) { return f; } return f.withGlobalArgs(ListUtils.map(f.getGlobalArgs(), arg -> { String name = arg.getName().getText(); String upgraded = upgrades.get(name); - if (upgraded == null || !arg.getId().equals(declarations.get(name))) { + // A name may be declared more than once; only the declaration the default was read from moves + if (upgraded == null || arg.getValue() == null || !defaults.get(name).equals(arg.getValue().getText())) { return arg; } - return arg.withValue(withText(requireNonNull(arg.getValue()), upgraded)); + return arg.withValue(withText(arg.getValue(), upgraded)); })); } @Override public Docker.From visitFrom(Docker.From from, ExecutionContext ctx) { if (containsVariable(from.getImageName()) || containsVariable(from.getTag())) { - Map replacements = getCursor().getNearestMessage(FROM_REPLACEMENTS, emptyMap()); - return replacements.getOrDefault(from.getId(), from); + return upgradeThroughArgs(from, + getCursor().getNearestMessage(ARG_DEFAULTS, emptyMap()), + getCursor().getNearestMessage(ARG_UPGRADES, new HashMap<>())); } DockerFrom image = new DockerFrom(getCursor()); @@ -137,29 +135,7 @@ public Docker.From visitFrom(Docker.From from, ExecutionContext ctx) { }; } - /** - * An argument is shared by every `FROM` that reads it, so which ones may be rewritten is only known once the whole - * file has been read. - */ - private ArgPlan planUpgrades(Docker.File file, Map defaults) { - Map upgrades = new HashMap<>(); - Map replacements = new HashMap<>(); - new DockerIsoVisitor() { - @Override - public Docker.From visitFrom(Docker.From from, Integer p) { - if (containsVariable(from.getImageName()) || containsVariable(from.getTag())) { - Docker.From planned = planFrom(from, defaults, upgrades); - if (planned != from) { - replacements.put(from.getId(), planned); - } - } - return from; - } - }.visit(file, 0); - return new ArgPlan(upgrades, replacements); - } - - private Docker.From planFrom(Docker.From from, Map defaults, Map upgrades) { + private Docker.From upgradeThroughArgs(Docker.From from, Map defaults, Map upgrades) { String imageVariable = soleVariable(from.getImageName()); String tagVariable = from.getTag() == null ? null : leadingVariable(from.getTag()); String imageName = imageVariable == null ? @@ -213,11 +189,12 @@ private Docker.From planFrom(Docker.From from, Map defaults, Map private @Nullable String upgradedImageName(String imageName) { ImageName parsed = ImageName.parse(imageName); - if (DEPRECATED_IMAGES.contains(parsed.getPath())) { + String path = parsed.getPath(); + if (DEPRECATED_IMAGES.contains(path)) { String registry = parsed.getRegistry(); return registry == null ? NEW_IMAGE : registry + '/' + NEW_IMAGE; } - return CURRENT_IMAGES.contains(parsed.getPath()) ? imageName : null; + return CURRENT_IMAGES.contains(path) ? imageName : null; } private @Nullable String upgradedTag(String tag) { @@ -237,8 +214,9 @@ private static boolean containsVariable(Docker.@Nullable Argument argument) { } private static @Nullable String soleVariable(Docker.Argument argument) { - Docker.EnvironmentVariable variable = sole(argument.getContents(), Docker.EnvironmentVariable.class); - return variable == null ? null : variable.getName(); + List contents = argument.getContents(); + return contents.size() == 1 && contents.get(0) instanceof Docker.EnvironmentVariable ? + ((Docker.EnvironmentVariable) contents.get(0)).getName() : null; } private static @Nullable String leadingVariable(Docker.Argument argument) { @@ -254,13 +232,6 @@ private static boolean containsVariable(Docker.@Nullable Argument argument) { return ((Docker.EnvironmentVariable) contents.get(0)).getName(); } - private static @Nullable T sole(List contents, Class type) { - if (contents.size() == 1 && type.isInstance(contents.get(0))) { - return type.cast(contents.get(0)); - } - return null; - } - private static String @Nullable [] splitReference(String reference) { int at = reference.indexOf('@'); String withoutDigest = at == -1 ? reference : reference.substring(0, at); @@ -293,9 +264,4 @@ private static Docker.Argument withText(Docker.Argument argument, String text) { content -> content instanceof Docker.Literal ? ((Docker.Literal) content).withText(text) : content)); } - @Value - private static class ArgPlan { - Map argUpgrades; - Map fromReplacements; - } } diff --git a/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java b/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java index 2cf1e5efb6..8908b10b34 100644 --- a/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/UpgradeDockerImageVersionTest.java @@ -19,6 +19,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; @@ -395,7 +396,7 @@ void upgradeArgumentDefaultValueBehindAVariableRegistry() { @Nested class NoChange { - @CsvSource({ + @ValueSource(strings = { "FROM ${IMAGE_NAME}:${IMAGE_TAG}", "FROM $IMAGE_NAME:$IMAGE_TAG", "FROM ${IMAGE_NAME}:11", @@ -468,7 +469,7 @@ void anArgumentDeclaredAfterTheFirstFrom() { ); } - @CsvSource({ + @ValueSource(strings = { // Unrelated images are left alone "FROM ubuntu:22.04", "FROM node:20-alpine",