From 86cef308ee1d6963f4b0f9d50820ebda9a9f38d8 Mon Sep 17 00:00:00 2001 From: Zixuan Liu Date: Sat, 18 May 2024 02:15:42 +0800 Subject: [PATCH 1/7] fix: support skipping tags --- src/main/java/io/fabric8/maven/docker/TagMojo.java | 7 +++++++ .../docker/config/BuildImageConfiguration.java | 12 ++++++++++++ .../docker/config/handler/property/ConfigKey.java | 1 + .../handler/property/PropertyConfigHandler.java | 1 + .../maven/docker/service/BuildXService.java | 10 ++++++---- .../docker/config/BuildImageConfigurationTest.java | 9 +++++++++ .../property/PropertyConfigHandlerTest.java | 14 ++++++++++++++ .../config/handler/property/ValueProviderTest.java | 10 ++++++++++ 8 files changed, 60 insertions(+), 4 deletions(-) diff --git a/src/main/java/io/fabric8/maven/docker/TagMojo.java b/src/main/java/io/fabric8/maven/docker/TagMojo.java index 992d6f870..d72b4f6ad 100644 --- a/src/main/java/io/fabric8/maven/docker/TagMojo.java +++ b/src/main/java/io/fabric8/maven/docker/TagMojo.java @@ -35,6 +35,13 @@ public void executeInternal(ServiceHub hub) throws DockerAccessException, MojoEx List imageConfigs = getResolvedImages(); for (ImageConfiguration imageConfig : imageConfigs) { BuildImageConfiguration buildConfig = imageConfig.getBuildConfiguration(); + if (buildConfig.skipTag()) { + continue; + } + if (buildConfig.isBuildX()) { + // Tag happens the building stage. + continue; + } hub.getBuildService().tagImage(imageConfig.getName(), tagName, repo, buildConfig.cleanupMode()); } diff --git a/src/main/java/io/fabric8/maven/docker/config/BuildImageConfiguration.java b/src/main/java/io/fabric8/maven/docker/config/BuildImageConfiguration.java index 7c378851a..fc046ab93 100644 --- a/src/main/java/io/fabric8/maven/docker/config/BuildImageConfiguration.java +++ b/src/main/java/io/fabric8/maven/docker/config/BuildImageConfiguration.java @@ -190,6 +190,9 @@ public class BuildImageConfiguration implements Serializable { @Parameter private Boolean skipPush; + @Parameter + private Boolean skipTag; + @Parameter private ArchiveCompression compression = ArchiveCompression.none; @@ -411,6 +414,10 @@ public boolean skipPush() { return skipPush != null ? skipPush : false; } + public boolean skipTag() { + return skipTag != null ? skipTag : false; + } + public boolean useDefaultExcludes() { return useDefaultExcludes != null ? useDefaultExcludes : true; @@ -733,6 +740,11 @@ public Builder skipPush(Boolean skipPush) { return this; } + public Builder skipTag(Boolean skipTag) { + config.skipTag = skipTag; + return this; + } + public Builder buildOptions(Map buildOptions) { config.buildOptions = buildOptions; return this; diff --git a/src/main/java/io/fabric8/maven/docker/config/handler/property/ConfigKey.java b/src/main/java/io/fabric8/maven/docker/config/handler/property/ConfigKey.java index 552ab22ea..cc734c292 100644 --- a/src/main/java/io/fabric8/maven/docker/config/handler/property/ConfigKey.java +++ b/src/main/java/io/fabric8/maven/docker/config/handler/property/ConfigKey.java @@ -126,6 +126,7 @@ public enum ConfigKey { SHMSIZE, SKIP_BUILD("skip.build"), SKIP_PUSH, + SKIP_TAG("skip.tag"), SKIP_RUN("skip.run"), STOP_NAME_PATTERN, COPY_NAME_PATTERN, diff --git a/src/main/java/io/fabric8/maven/docker/config/handler/property/PropertyConfigHandler.java b/src/main/java/io/fabric8/maven/docker/config/handler/property/PropertyConfigHandler.java index 366e8f5e6..82a0d7cd1 100644 --- a/src/main/java/io/fabric8/maven/docker/config/handler/property/PropertyConfigHandler.java +++ b/src/main/java/io/fabric8/maven/docker/config/handler/property/PropertyConfigHandler.java @@ -174,6 +174,7 @@ private BuildImageConfiguration extractBuildConfiguration(ImageConfiguration fro .workdir(valueProvider.getString(WORKDIR, config.getWorkdir())) .skip(valueProvider.getBoolean(SKIP_BUILD, config.getSkip())) .skipPush(valueProvider.getBoolean(SKIP_PUSH, config.getSkipPush())) + .skipTag(valueProvider.getBoolean(SKIP_TAG, config.skipTag())) .imagePullPolicy(valueProvider.getString(IMAGE_PULL_POLICY_BUILD, config.getImagePullPolicy())) .contextDir(valueProvider.getString(CONTEXT_DIR, config.getContextDirRaw())) .dockerArchive(valueProvider.getString(DOCKER_ARCHIVE, config.getDockerArchiveRaw())) diff --git a/src/main/java/io/fabric8/maven/docker/service/BuildXService.java b/src/main/java/io/fabric8/maven/docker/service/BuildXService.java index ba43ac0c5..d262f0f19 100644 --- a/src/main/java/io/fabric8/maven/docker/service/BuildXService.java +++ b/src/main/java/io/fabric8/maven/docker/service/BuildXService.java @@ -142,10 +142,12 @@ protected void buildX(List buildX, String builderName, BuildDirs buildDi append(cmdLine, "build", "--progress=plain", "--builder", builderName, "--platform", String.join(",", platforms), "--tag", new ImageName(imageConfig.getName()).getFullName(configuredRegistry)); - buildConfiguration.getTags().forEach(t -> { - cmdLine.add("--tag"); - cmdLine.add(new ImageName(imageConfig.getName(), t).getFullName(configuredRegistry)); - }); + if (!buildConfiguration.skipTag()) { + buildConfiguration.getTags().forEach(t -> { + cmdLine.add("--tag"); + cmdLine.add(new ImageName(imageConfig.getName(), t).getFullName(configuredRegistry)); + }); + } Map args = buildConfiguration.getArgs(); if (args != null) { diff --git a/src/test/java/io/fabric8/maven/docker/config/BuildImageConfigurationTest.java b/src/test/java/io/fabric8/maven/docker/config/BuildImageConfigurationTest.java index 982cdcefb..d30d005c3 100644 --- a/src/test/java/io/fabric8/maven/docker/config/BuildImageConfigurationTest.java +++ b/src/test/java/io/fabric8/maven/docker/config/BuildImageConfigurationTest.java @@ -213,4 +213,13 @@ void multipleAssembliesUniqueNames() { assemblies(Arrays.asList(assemblyConfigurationOne, assemblyConfigurationTwo)).build(); Assertions.assertThrows(IllegalArgumentException.class, () -> config.initAndValidate(logger)); } + + @Test + void skipTag() { + BuildImageConfiguration config = + new BuildImageConfiguration.Builder().skipTag(true) + .build(); + config.initAndValidate(logger); + Assertions.assertTrue(config.skipTag()); + } } diff --git a/src/test/java/io/fabric8/maven/docker/config/handler/property/PropertyConfigHandlerTest.java b/src/test/java/io/fabric8/maven/docker/config/handler/property/PropertyConfigHandlerTest.java index 5f0686982..80049492c 100644 --- a/src/test/java/io/fabric8/maven/docker/config/handler/property/PropertyConfigHandlerTest.java +++ b/src/test/java/io/fabric8/maven/docker/config/handler/property/PropertyConfigHandlerTest.java @@ -91,6 +91,20 @@ void testSkipPush() { Assertions.assertFalse(resolveExternalImageConfig(new String[] { k(ConfigKey.NAME), "image", k(ConfigKey.FROM), "busybox" }).getBuildConfiguration().skipPush()); } + @Test + void testSkipTag() { + Assertions.assertFalse( + resolveExternalImageConfig(getSkipTestData(ConfigKey.SKIP_TAG, false)).getBuildConfiguration() + .skipTag()); + Assertions.assertTrue( + resolveExternalImageConfig(getSkipTestData(ConfigKey.SKIP_TAG, true)).getBuildConfiguration() + .skipTag()); + + Assertions.assertFalse(resolveExternalImageConfig( + new String[]{k(ConfigKey.NAME), "image", k(ConfigKey.FROM), "busybox"}).getBuildConfiguration() + .skipTag()); + } + @Test void testSkipRun() { Assertions.assertFalse(resolveExternalImageConfig(getSkipTestData(ConfigKey.SKIP_RUN, false)).getRunConfiguration().skip()); diff --git a/src/test/java/io/fabric8/maven/docker/config/handler/property/ValueProviderTest.java b/src/test/java/io/fabric8/maven/docker/config/handler/property/ValueProviderTest.java index de189fe4d..dfacab6a0 100644 --- a/src/test/java/io/fabric8/maven/docker/config/handler/property/ValueProviderTest.java +++ b/src/test/java/io/fabric8/maven/docker/config/handler/property/ValueProviderTest.java @@ -190,4 +190,14 @@ void testGetMap() { Assertions.assertEquals("notignored1", m.get("ckey")); } + @Test + void testSkipTag() { + configure(PropertyMode.Override); + Assertions.assertNull(provider.getBoolean(ConfigKey.SKIP_TAG, null)); + + props.put("docker.skip.tag", "true"); + Assertions.assertTrue(provider.getBoolean(ConfigKey.SKIP_TAG, null)); + props.put("docker.skip.tag", "false"); + Assertions.assertFalse(provider.getBoolean(ConfigKey.SKIP_TAG, null)); + } } \ No newline at end of file From 359a6275de3bc3651b4bc158d4703ded015c9b92 Mon Sep 17 00:00:00 2001 From: Zixuan Liu Date: Sun, 19 May 2024 23:59:45 +0800 Subject: [PATCH 2/7] Add more tests --- .../io/fabric8/maven/docker/BuildMojo.java | 4 +- .../java/io/fabric8/maven/docker/TagMojo.java | 2 +- .../fabric8/maven/docker/BuildMojoTest.java | 101 ++++++++++++++++-- .../io/fabric8/maven/docker/MojoTestBase.java | 26 +++-- .../docker/service/BuildXServiceTest.java | 36 +++++++ 5 files changed, 151 insertions(+), 18 deletions(-) diff --git a/src/main/java/io/fabric8/maven/docker/BuildMojo.java b/src/main/java/io/fabric8/maven/docker/BuildMojo.java index f703e4a0d..a9ca73db3 100644 --- a/src/main/java/io/fabric8/maven/docker/BuildMojo.java +++ b/src/main/java/io/fabric8/maven/docker/BuildMojo.java @@ -105,7 +105,9 @@ private void proceedWithDockerBuild(ServiceHub hub, BuildService.BuildContext bu } else { buildService.buildImage(imageConfig, pullManager, buildContext, buildArchiveFile); if (!skipTag) { - buildService.tagImage(imageConfig); + if (!imageConfig.getBuildConfiguration().skipTag()) { + buildService.tagImage(imageConfig); + } } } } diff --git a/src/main/java/io/fabric8/maven/docker/TagMojo.java b/src/main/java/io/fabric8/maven/docker/TagMojo.java index d72b4f6ad..d15143c9f 100644 --- a/src/main/java/io/fabric8/maven/docker/TagMojo.java +++ b/src/main/java/io/fabric8/maven/docker/TagMojo.java @@ -39,7 +39,7 @@ public void executeInternal(ServiceHub hub) throws DockerAccessException, MojoEx continue; } if (buildConfig.isBuildX()) { - // Tag happens the building stage. + // Tag happens at the building stage. continue; } diff --git a/src/test/java/io/fabric8/maven/docker/BuildMojoTest.java b/src/test/java/io/fabric8/maven/docker/BuildMojoTest.java index 4d3bcb611..f535a4f53 100644 --- a/src/test/java/io/fabric8/maven/docker/BuildMojoTest.java +++ b/src/test/java/io/fabric8/maven/docker/BuildMojoTest.java @@ -11,6 +11,8 @@ import io.fabric8.maven.docker.service.BuildService; import io.fabric8.maven.docker.service.BuildXService; import io.fabric8.maven.docker.service.ImagePullManager; +import io.fabric8.maven.docker.util.ImageName; + import org.apache.maven.plugin.MojoExecutionException; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -31,6 +33,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; @ExtendWith(MockitoExtension.class) class BuildMojoTest extends MojoTestBase { @@ -241,6 +244,75 @@ void buildUsingBuildxWithMultipleAuth() throws IOException, MojoExecutionExcepti thenAuthContainsRegistry("custom-registry.org"); } + void buildUsingBuildxWithTag(boolean skipTag) throws IOException, MojoExecutionException { + givenBuildXService(); + + List tags = new ArrayList<>(); + tags.add("tag-" + System.currentTimeMillis()); + tags.add("tag-" + System.currentTimeMillis()); + + givenMavenProject(buildMojo); + ImageConfiguration imageConfiguration = singleImageConfiguration(builder -> { + builder.buildx(getBuildXPlatforms(TWO_BUILDX_PLATFORMS).build()); + builder.tags(tags); + builder.skipTag(skipTag); + }); + givenResolvedImages(buildMojo, Collections.singletonList(imageConfiguration)); + givenPackaging("jar"); + + whenMojoExecutes(); + + List fullTags = skipTag ? Collections.emptyList() : tags.stream() + .map(tag -> new ImageName(imageConfiguration.getName(), tag).getFullName()) + .collect(Collectors.toList()); + thenBuildxRun(null, null, true, null, fullTags); + } + + @Test + void buildUsingBuildxWithTag() throws IOException, MojoExecutionException { + buildUsingBuildxWithTag(false); + } + + @Test + void buildUsingBuildxWithSkipTag() throws IOException, MojoExecutionException { + buildUsingBuildxWithTag(true); + } + + void buildWithTag(boolean skipTag) throws IOException, MojoExecutionException { + givenMavenProject(buildMojo); + + List tags = new ArrayList<>(); + tags.add("tag-" + System.currentTimeMillis()); + tags.add("tag-" + System.currentTimeMillis()); + + ImageConfiguration imageConfiguration = singleImageConfiguration(builder -> { + builder.skipTag(skipTag); + builder.tags(tags); + }); + givenResolvedImages(buildMojo, Collections.singletonList(imageConfiguration)); + givenPackaging("jar"); + givenSkipPom(true); + + whenMojoExecutes(); + + thenBuildRun(); + + if (!skipTag) { + Mockito.verify(buildService, Mockito.times(1)) + .tagImage(Mockito.any(ImageConfiguration.class)); + } + } + + @Test + void buildWithTag() throws IOException, MojoExecutionException { + buildWithTag(false); + } + + @Test + void buildWithSkipTag() throws IOException, MojoExecutionException { + buildWithTag(true); + } + private void givenBuildXService() { BuildXService buildXService = new BuildXService(dockerAccess, dockerAssemblyManager, log, exec); @@ -288,24 +360,39 @@ private void verifyBuild(int wantedNumberOfInvocations) throws DockerAccessExcep .buildImage(Mockito.any(ImageConfiguration.class), Mockito.any(ImagePullManager.class), Mockito.any(BuildService.BuildContext.class), Mockito.any()); } + private void thenBuildxRun(String relativeConfigFile, String contextDir, boolean nativePlatformIncluded, + String attestation) throws MojoExecutionException { + thenBuildxRun(relativeConfigFile, contextDir, nativePlatformIncluded, attestation, Collections.emptyList()); + } + private void thenBuildxRun(String relativeConfigFile, String contextDir, - boolean nativePlatformIncluded, String attestation) throws MojoExecutionException { + boolean nativePlatformIncluded, String attestation, List tags) + throws MojoExecutionException { Path buildPath = projectBaseDirectory.toPath().resolve("target/docker/example/latest"); String config = getOsDependentBuild(buildPath, "docker"); - String configFile = relativeConfigFile != null ? getOsDependentBuild(projectBaseDirectory.toPath(), relativeConfigFile) : null; + String configFile = + relativeConfigFile != null ? getOsDependentBuild(projectBaseDirectory.toPath(), relativeConfigFile) : + null; List cmds = - BuildXService.append(new ArrayList<>(), "docker", "--config", config, "buildx", - "create", "--driver", "docker-container", "--name", "maven" , "--node", "maven0"); + BuildXService.append(new ArrayList<>(), "docker", "--config", config, "buildx", + "create", "--driver", "docker-container", "--name", "maven", "--node", "maven0"); if (configFile != null) { BuildXService.append(cmds, "--config", configFile.replace('/', File.separatorChar)); } Mockito.verify(exec).process(cmds); if (nativePlatformIncluded) { - List buildXLine = BuildXService.append(new ArrayList<>(), "docker", "--config", config, "buildx", + List buildXLine = BuildXService.append(new ArrayList<>(), "docker", "--config", config, "buildx", "build", "--progress=plain", "--builder", "maven", - "--platform", NATIVE_PLATFORM, "--tag", "example:latest", "--build-arg", "foo=bar"); + "--platform", NATIVE_PLATFORM, "--tag", "example:latest"); + + tags.forEach(tag -> { + buildXLine.add("--tag"); + buildXLine.add(tag); + }); + buildXLine.add("--build-arg"); + buildXLine.add("foo=bar"); if (attestation != null) { buildXLine.add(attestation); @@ -315,7 +402,7 @@ private void thenBuildxRun(String relativeConfigFile, String contextDir, buildXLine.add(getOsDependentBuild(buildPath, "build")); } else { Path contextPath = tmpDir.resolve("docker-build"); - BuildXService.append(buildXLine, "--file=" + contextPath.resolve("Dockerfile"), contextPath.toString() ); + BuildXService.append(buildXLine, "--file=" + contextPath.resolve("Dockerfile"), contextPath.toString()); } buildXLine.add("--load"); diff --git a/src/test/java/io/fabric8/maven/docker/MojoTestBase.java b/src/test/java/io/fabric8/maven/docker/MojoTestBase.java index 3855b55ee..19887c79c 100644 --- a/src/test/java/io/fabric8/maven/docker/MojoTestBase.java +++ b/src/test/java/io/fabric8/maven/docker/MojoTestBase.java @@ -17,6 +17,7 @@ import io.fabric8.maven.docker.util.AnsiLogger; import io.fabric8.maven.docker.util.AuthConfigFactory; import io.fabric8.maven.docker.util.GavLabel; +import java.util.function.Consumer; import org.apache.maven.execution.MavenSession; import org.apache.maven.model.Build; import org.apache.maven.monitor.logging.DefaultLog; @@ -113,18 +114,25 @@ protected ImageConfiguration singleImageWithBuild() { } ImageConfiguration singleImageConfiguration(BuildXConfiguration buildx, String contextDir) { - BuildImageConfiguration buildImageConfiguration = new BuildImageConfiguration.Builder() - .from("scratch") - .buildx(buildx) - .args(Collections.singletonMap("foo", "bar")) - .contextDir(contextDir) - .build(); + return singleImageConfiguration(builder -> { + builder.buildx(buildx); + builder.contextDir(contextDir); + }); + } + + ImageConfiguration singleImageConfiguration( + Consumer buildImageConfigurationConsumer) { + BuildImageConfiguration.Builder builder = new BuildImageConfiguration.Builder() + .from("scratch") + .args(Collections.singletonMap("foo", "bar")); + buildImageConfigurationConsumer.accept(builder); + BuildImageConfiguration buildImageConfiguration = builder.build(); buildImageConfiguration.initAndValidate(log); return new Builder() - .name("example:latest") - .buildConfig(buildImageConfiguration) - .build(); + .name("example:latest") + .buildConfig(buildImageConfiguration) + .build(); } ImageConfiguration singleImageConfigurationWithBuildWithSquash(BuildXConfiguration buildx, String contextDir) { diff --git a/src/test/java/io/fabric8/maven/docker/service/BuildXServiceTest.java b/src/test/java/io/fabric8/maven/docker/service/BuildXServiceTest.java index d3ab43ef0..129bb6787 100644 --- a/src/test/java/io/fabric8/maven/docker/service/BuildXServiceTest.java +++ b/src/test/java/io/fabric8/maven/docker/service/BuildXServiceTest.java @@ -1,8 +1,10 @@ package io.fabric8.maven.docker.service; +import io.fabric8.maven.docker.util.ImageName; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.io.File; @@ -261,6 +263,40 @@ void useBuilder_whenDockerBuildXIncompatibleWithConfigOverride_thenCopyBuildXBin } } + void testBuildWithTag(boolean skipTag) throws Exception { + List tags = new ArrayList<>(); + tags.add("tag-" + System.currentTimeMillis()); + tags.add("tag-" + System.currentTimeMillis()); + + //Given + buildConfigUsingBuildx(temporaryFolder, (buildX, buildImage) -> { + buildImage.skipTag(skipTag); + buildImage.tags(tags); + }); + + // When + buildx.build(projectPaths, imageConfig, configuredRegistry, authConfigList, buildArchive); + + String[] fullTags = tags.stream() + .map(tag -> new ImageName(imageConfig.getName(), tag).getFullName(configuredRegistry)) + .toArray(String[]::new); + if (skipTag) { + verifyBuildXArgumentNotPresentInExec(fullTags); + } else { + verifyBuildXArgumentPresentInExec(fullTags); + } + } + + @Test + void testBuildWithSkipTag() throws Exception { + testBuildWithTag(true); + } + + @Test + void testBuildWithTag() throws Exception { + testBuildWithTag(false); + } + private void givenAnImageConfiguration(String... platforms) { final BuildXConfiguration buildxConfig = new BuildXConfiguration.Builder() .platforms(Arrays.asList(platforms)) From ea00d1ad08276a519aa2a53e472e135d128abf66 Mon Sep 17 00:00:00 2001 From: Zixuan Liu Date: Mon, 20 May 2024 12:03:23 +0800 Subject: [PATCH 3/7] Add TagMojo test --- .../io/fabric8/maven/docker/TagMojoTest.java | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 src/test/java/io/fabric8/maven/docker/TagMojoTest.java diff --git a/src/test/java/io/fabric8/maven/docker/TagMojoTest.java b/src/test/java/io/fabric8/maven/docker/TagMojoTest.java new file mode 100644 index 000000000..a5a1711fb --- /dev/null +++ b/src/test/java/io/fabric8/maven/docker/TagMojoTest.java @@ -0,0 +1,82 @@ +package io.fabric8.maven.docker; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import io.fabric8.maven.docker.access.AuthConfig; +import io.fabric8.maven.docker.access.AuthConfigList; +import io.fabric8.maven.docker.access.DockerAccessException; +import io.fabric8.maven.docker.assembly.DockerAssemblyManager; +import io.fabric8.maven.docker.config.AttestationConfiguration; +import io.fabric8.maven.docker.config.BuildImageConfiguration; +import io.fabric8.maven.docker.config.BuildXConfiguration; +import io.fabric8.maven.docker.config.ImageConfiguration; +import io.fabric8.maven.docker.service.BuildService; +import io.fabric8.maven.docker.service.BuildXService; +import io.fabric8.maven.docker.service.ImagePullManager; +import io.fabric8.maven.docker.util.ImageName; +import java.io.File; +import java.io.IOException; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import org.apache.maven.plugin.MojoExecutionException; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.ArgumentCaptor; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class TagMojoTest extends MojoTestBase { + + private static final String NON_NATIVE_PLATFORM = "linux/amd64"; + private static final String NATIVE_PLATFORM = "linux/arm64"; + + @InjectMocks + private TagMojo tagMojo; + + void tag(boolean skipTag, boolean buildx) throws IOException, MojoExecutionException { + givenMavenProject(tagMojo); + + ImageConfiguration imageConfiguration = singleImageConfiguration(builder -> { + builder.skipTag(skipTag); + if (buildx) { + ArrayList platforms = new ArrayList<>(); + platforms.add(NON_NATIVE_PLATFORM); + platforms.add(NATIVE_PLATFORM); + BuildXConfiguration buildXConfiguration = new BuildXConfiguration.Builder().platforms(platforms).build(); + builder.buildx(buildXConfiguration); + } + }); + givenResolvedImages(tagMojo, Collections.singletonList(imageConfiguration)); + + tagMojo.executeInternal(serviceHub); + + verify(buildService, times(skipTag || buildx ? 0 : 1)).tagImage(any(), any(), any(), any()); + } + + @Test + void skipTag() throws IOException, MojoExecutionException { + tag(true, false); + } + + @Test + void tag() throws IOException, MojoExecutionException { + tag(false, false); + } + + @Test + void tagWhenUsingBuildx() throws IOException, MojoExecutionException { + tag(false, true); + } +} From 0a635be29fa7d0df81d53e9316c663a6b90bef7b Mon Sep 17 00:00:00 2001 From: Zixuan Liu Date: Mon, 20 May 2024 18:52:19 +0800 Subject: [PATCH 4/7] Improve if --- src/main/java/io/fabric8/maven/docker/BuildMojo.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/main/java/io/fabric8/maven/docker/BuildMojo.java b/src/main/java/io/fabric8/maven/docker/BuildMojo.java index a9ca73db3..f2e5f562c 100644 --- a/src/main/java/io/fabric8/maven/docker/BuildMojo.java +++ b/src/main/java/io/fabric8/maven/docker/BuildMojo.java @@ -104,10 +104,8 @@ private void proceedWithDockerBuild(ServiceHub hub, BuildService.BuildContext bu hub.getBuildXService().build(createProjectPaths(), imageConfig, null, createCompleteAuthConfigList(false, imageConfig, getRegistryConfig(pullRegistry), createMojoParameters()), buildArchiveFile); } else { buildService.buildImage(imageConfig, pullManager, buildContext, buildArchiveFile); - if (!skipTag) { - if (!imageConfig.getBuildConfiguration().skipTag()) { - buildService.tagImage(imageConfig); - } + if (!skipTag && !imageConfig.getBuildConfiguration().skipTag()) { + buildService.tagImage(imageConfig); } } } From 83e021580c80e56e7453e153cb9494308f0fbd9b Mon Sep 17 00:00:00 2001 From: Zixuan Liu Date: Mon, 20 May 2024 22:57:52 +0800 Subject: [PATCH 5/7] Fix code style --- .../java/io/fabric8/maven/docker/TagMojo.java | 5 +---- .../config/BuildImageConfiguration.java | 2 +- .../io/fabric8/maven/docker/TagMojoTest.java | 22 ------------------- 3 files changed, 2 insertions(+), 27 deletions(-) diff --git a/src/main/java/io/fabric8/maven/docker/TagMojo.java b/src/main/java/io/fabric8/maven/docker/TagMojo.java index d15143c9f..520a43cfd 100644 --- a/src/main/java/io/fabric8/maven/docker/TagMojo.java +++ b/src/main/java/io/fabric8/maven/docker/TagMojo.java @@ -35,10 +35,7 @@ public void executeInternal(ServiceHub hub) throws DockerAccessException, MojoEx List imageConfigs = getResolvedImages(); for (ImageConfiguration imageConfig : imageConfigs) { BuildImageConfiguration buildConfig = imageConfig.getBuildConfiguration(); - if (buildConfig.skipTag()) { - continue; - } - if (buildConfig.isBuildX()) { + if (buildConfig.skipTag() || buildConfig.isBuildX()) { // Tag happens at the building stage. continue; } diff --git a/src/main/java/io/fabric8/maven/docker/config/BuildImageConfiguration.java b/src/main/java/io/fabric8/maven/docker/config/BuildImageConfiguration.java index fc046ab93..9ecb07200 100644 --- a/src/main/java/io/fabric8/maven/docker/config/BuildImageConfiguration.java +++ b/src/main/java/io/fabric8/maven/docker/config/BuildImageConfiguration.java @@ -415,7 +415,7 @@ public boolean skipPush() { } public boolean skipTag() { - return skipTag != null ? skipTag : false; + return skipTag != null && skipTag; } public boolean useDefaultExcludes() { diff --git a/src/test/java/io/fabric8/maven/docker/TagMojoTest.java b/src/test/java/io/fabric8/maven/docker/TagMojoTest.java index a5a1711fb..f59518b77 100644 --- a/src/test/java/io/fabric8/maven/docker/TagMojoTest.java +++ b/src/test/java/io/fabric8/maven/docker/TagMojoTest.java @@ -3,37 +3,15 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import io.fabric8.maven.docker.access.AuthConfig; -import io.fabric8.maven.docker.access.AuthConfigList; -import io.fabric8.maven.docker.access.DockerAccessException; -import io.fabric8.maven.docker.assembly.DockerAssemblyManager; -import io.fabric8.maven.docker.config.AttestationConfiguration; -import io.fabric8.maven.docker.config.BuildImageConfiguration; import io.fabric8.maven.docker.config.BuildXConfiguration; import io.fabric8.maven.docker.config.ImageConfiguration; -import io.fabric8.maven.docker.service.BuildService; -import io.fabric8.maven.docker.service.BuildXService; -import io.fabric8.maven.docker.service.ImagePullManager; -import io.fabric8.maven.docker.util.ImageName; -import java.io.File; import java.io.IOException; -import java.nio.file.Path; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.stream.Collectors; import org.apache.maven.plugin.MojoExecutionException; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import org.junit.jupiter.api.io.TempDir; -import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; -import org.mockito.Mock; -import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; @ExtendWith(MockitoExtension.class) From cad1a56e7582cb68d9678d8aa0fee2e2c41b1912 Mon Sep 17 00:00:00 2001 From: Zixuan Liu Date: Thu, 23 May 2024 00:20:32 +0800 Subject: [PATCH 6/7] Improve test --- .../io/fabric8/maven/docker/TagMojoTest.java | 66 +++++++++++-------- .../docker/service/BuildXServiceTest.java | 15 ++--- 2 files changed, 44 insertions(+), 37 deletions(-) diff --git a/src/test/java/io/fabric8/maven/docker/TagMojoTest.java b/src/test/java/io/fabric8/maven/docker/TagMojoTest.java index f59518b77..d8426b74f 100644 --- a/src/test/java/io/fabric8/maven/docker/TagMojoTest.java +++ b/src/test/java/io/fabric8/maven/docker/TagMojoTest.java @@ -9,8 +9,11 @@ import java.util.ArrayList; import java.util.Collections; import org.apache.maven.plugin.MojoExecutionException; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.InjectMocks; import org.mockito.junit.jupiter.MockitoExtension; @@ -23,38 +26,47 @@ class TagMojoTest extends MojoTestBase { @InjectMocks private TagMojo tagMojo; - void tag(boolean skipTag, boolean buildx) throws IOException, MojoExecutionException { - givenMavenProject(tagMojo); + @Nested + class NormalBuild { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + @DisplayName("If skipTag is true, call tag; otherwise, do not call") + void tag(boolean skipTag) throws IOException, MojoExecutionException { + givenMavenProject(tagMojo); - ImageConfiguration imageConfiguration = singleImageConfiguration(builder -> { - builder.skipTag(skipTag); - if (buildx) { - ArrayList platforms = new ArrayList<>(); - platforms.add(NON_NATIVE_PLATFORM); - platforms.add(NATIVE_PLATFORM); - BuildXConfiguration buildXConfiguration = new BuildXConfiguration.Builder().platforms(platforms).build(); - builder.buildx(buildXConfiguration); - } - }); - givenResolvedImages(tagMojo, Collections.singletonList(imageConfiguration)); + ImageConfiguration imageConfiguration = singleImageConfiguration(builder -> { + builder.skipTag(skipTag); + }); + givenResolvedImages(tagMojo, Collections.singletonList(imageConfiguration)); - tagMojo.executeInternal(serviceHub); + tagMojo.executeInternal(serviceHub); - verify(buildService, times(skipTag || buildx ? 0 : 1)).tagImage(any(), any(), any(), any()); + verify(buildService, times(skipTag ? 0 : 1)).tagImage(any(), any(), any(), any()); + } } - @Test - void skipTag() throws IOException, MojoExecutionException { - tag(true, false); - } + @Nested + class BuildX { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + @DisplayName("Always skip tag") + void tag(boolean skipTag) throws IOException, MojoExecutionException { + givenMavenProject(tagMojo); - @Test - void tag() throws IOException, MojoExecutionException { - tag(false, false); - } + ImageConfiguration imageConfiguration = singleImageConfiguration(builder -> { + builder.skipTag(skipTag); + ArrayList platforms = new ArrayList<>(); + platforms.add(NON_NATIVE_PLATFORM); + platforms.add(NATIVE_PLATFORM); + BuildXConfiguration buildXConfiguration = + new BuildXConfiguration.Builder().platforms(platforms).build(); + builder.buildx(buildXConfiguration); + }); + givenResolvedImages(tagMojo, Collections.singletonList(imageConfiguration)); + + tagMojo.executeInternal(serviceHub); - @Test - void tagWhenUsingBuildx() throws IOException, MojoExecutionException { - tag(false, true); + verify(buildService, times(0)).tagImage(any(), any(), any(), any()); + } } } diff --git a/src/test/java/io/fabric8/maven/docker/service/BuildXServiceTest.java b/src/test/java/io/fabric8/maven/docker/service/BuildXServiceTest.java index 129bb6787..5e35792da 100644 --- a/src/test/java/io/fabric8/maven/docker/service/BuildXServiceTest.java +++ b/src/test/java/io/fabric8/maven/docker/service/BuildXServiceTest.java @@ -21,6 +21,8 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; @@ -263,6 +265,9 @@ void useBuilder_whenDockerBuildXIncompatibleWithConfigOverride_thenCopyBuildXBin } } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) void testBuildWithTag(boolean skipTag) throws Exception { List tags = new ArrayList<>(); tags.add("tag-" + System.currentTimeMillis()); @@ -287,16 +292,6 @@ void testBuildWithTag(boolean skipTag) throws Exception { } } - @Test - void testBuildWithSkipTag() throws Exception { - testBuildWithTag(true); - } - - @Test - void testBuildWithTag() throws Exception { - testBuildWithTag(false); - } - private void givenAnImageConfiguration(String... platforms) { final BuildXConfiguration buildxConfig = new BuildXConfiguration.Builder() .platforms(Arrays.asList(platforms)) From 41d98af217afc19e20958597ef698acee512b13d Mon Sep 17 00:00:00 2001 From: Zixuan Liu Date: Thu, 23 May 2024 00:23:32 +0800 Subject: [PATCH 7/7] Improve test --- .../fabric8/maven/docker/BuildMojoTest.java | 30 +++++-------------- 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/src/test/java/io/fabric8/maven/docker/BuildMojoTest.java b/src/test/java/io/fabric8/maven/docker/BuildMojoTest.java index f535a4f53..46dc7969a 100644 --- a/src/test/java/io/fabric8/maven/docker/BuildMojoTest.java +++ b/src/test/java/io/fabric8/maven/docker/BuildMojoTest.java @@ -18,6 +18,8 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; @@ -244,7 +246,9 @@ void buildUsingBuildxWithMultipleAuth() throws IOException, MojoExecutionExcepti thenAuthContainsRegistry("custom-registry.org"); } - void buildUsingBuildxWithTag(boolean skipTag) throws IOException, MojoExecutionException { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void buildWithTagByBuildx(boolean skipTag) throws IOException, MojoExecutionException { givenBuildXService(); List tags = new ArrayList<>(); @@ -268,17 +272,9 @@ void buildUsingBuildxWithTag(boolean skipTag) throws IOException, MojoExecutionE thenBuildxRun(null, null, true, null, fullTags); } - @Test - void buildUsingBuildxWithTag() throws IOException, MojoExecutionException { - buildUsingBuildxWithTag(false); - } - - @Test - void buildUsingBuildxWithSkipTag() throws IOException, MojoExecutionException { - buildUsingBuildxWithTag(true); - } - - void buildWithTag(boolean skipTag) throws IOException, MojoExecutionException { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void buildWithTagByNormalBuild(boolean skipTag) throws IOException, MojoExecutionException { givenMavenProject(buildMojo); List tags = new ArrayList<>(); @@ -303,16 +299,6 @@ void buildWithTag(boolean skipTag) throws IOException, MojoExecutionException { } } - @Test - void buildWithTag() throws IOException, MojoExecutionException { - buildWithTag(false); - } - - @Test - void buildWithSkipTag() throws IOException, MojoExecutionException { - buildWithTag(true); - } - private void givenBuildXService() { BuildXService buildXService = new BuildXService(dockerAccess, dockerAssemblyManager, log, exec);