From 5eede51c28f42715ba96baf215cf529aabf35902 Mon Sep 17 00:00:00 2001 From: Chanseok Oh Date: Wed, 23 Sep 2020 18:13:37 -0400 Subject: [PATCH 1/6] Fix platform configuration in Gradle plugin --- .../jib/api/buildplan/ContainerBuildPlan.java | 3 +- .../configuration/ContainerConfiguration.java | 3 +- .../tools/jib/gradle/BaseImageParameters.java | 18 ++++++++---- .../tools/jib/gradle/JibExtensionTest.java | 29 +++++++++++++++++++ 4 files changed, 44 insertions(+), 9 deletions(-) diff --git a/jib-build-plan/src/main/java/com/google/cloud/tools/jib/api/buildplan/ContainerBuildPlan.java b/jib-build-plan/src/main/java/com/google/cloud/tools/jib/api/buildplan/ContainerBuildPlan.java index 6545f00b99..e11e29430e 100644 --- a/jib-build-plan/src/main/java/com/google/cloud/tools/jib/api/buildplan/ContainerBuildPlan.java +++ b/jib-build-plan/src/main/java/com/google/cloud/tools/jib/api/buildplan/ContainerBuildPlan.java @@ -39,8 +39,7 @@ public static class Builder { private Instant creationTime = Instant.EPOCH; private ImageFormat format = ImageFormat.Docker; - // note that a LinkedHashSet instead of HashSet has been used so as to preserve the platform - // order + // LinkedHashSet to preserve the order private Set platforms = new LinkedHashSet<>(Collections.singleton(new Platform("amd64", "linux"))); diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/configuration/ContainerConfiguration.java b/jib-core/src/main/java/com/google/cloud/tools/jib/configuration/ContainerConfiguration.java index 869ae077dc..6656917386 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/configuration/ContainerConfiguration.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/configuration/ContainerConfiguration.java @@ -47,8 +47,7 @@ public static class Builder { */ private static final Instant DEFAULT_CREATION_TIME = Instant.EPOCH; - // note that a LinkedHashSet instead of HashSet has been used so as to preserve the platform - // order + // LinkedHashSet to preserve the order private Set platforms = new LinkedHashSet<>(Collections.singleton(new Platform("amd64", "linux"))); private Instant creationTime = DEFAULT_CREATION_TIME; diff --git a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/BaseImageParameters.java b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/BaseImageParameters.java index 3b182b06f6..c4d0ede426 100644 --- a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/BaseImageParameters.java +++ b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/BaseImageParameters.java @@ -35,22 +35,30 @@ public class BaseImageParameters { private final PlatformParametersSpec platformParametersSpec; private final ListProperty platforms; + private final ObjectFactory objectFactory; + @Inject public BaseImageParameters(ObjectFactory objectFactory) { + this.objectFactory = objectFactory; auth = objectFactory.newInstance(AuthParameters.class, "from.auth"); platforms = objectFactory.listProperty(PlatformParameters.class).empty(); platformParametersSpec = objectFactory.newInstance(PlatformParametersSpec.class, objectFactory, platforms); - - PlatformParameters platform = new PlatformParameters(); - platform.setOs("linux"); - platform.setArchitecture("amd64"); - platforms.add(platform); } @Nested @Optional public ListProperty getPlatforms() { + if (platforms.get().isEmpty()) { + PlatformParameters amd64Linux = new PlatformParameters(); + amd64Linux.setArchitecture("amd64"); + amd64Linux.setOs("linux"); + + ListProperty platforms = + objectFactory.listProperty(PlatformParameters.class); + platforms.add(amd64Linux); + return platforms; + } return platforms; } diff --git a/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/JibExtensionTest.java b/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/JibExtensionTest.java index a29c34fa59..3a5212740c 100644 --- a/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/JibExtensionTest.java +++ b/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/JibExtensionTest.java @@ -25,6 +25,7 @@ import java.nio.file.Paths; import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.Properties; import org.gradle.api.Project; import org.gradle.testfixtures.ProjectBuilder; @@ -56,17 +57,45 @@ public void testFrom() { Assert.assertNull(testJibExtension.getFrom().getImage()); Assert.assertNull(testJibExtension.getFrom().getCredHelper()); + List defaultPlatforms = testJibExtension.getFrom().getPlatforms().get(); + Assert.assertEquals(1, defaultPlatforms.size()); + Assert.assertEquals("amd64", defaultPlatforms.get(0).getArchitecture()); + Assert.assertEquals("linux", defaultPlatforms.get(0).getOs()); + + // Ensures calling getPlatforms() again doesn't increase the resulting list. + List defaultPlatformsAgain = + testJibExtension.getFrom().getPlatforms().get(); + Assert.assertEquals(1, defaultPlatformsAgain.size()); + testJibExtension.from( from -> { from.setImage("some image"); from.setCredHelper("some cred helper"); from.auth(auth -> auth.setUsername("some username")); from.auth(auth -> auth.setPassword("some password")); + from.platforms( + platformSpec -> { + platformSpec.platform( + platform -> { + platform.setArchitecture("arm"); + platform.setOs("windows"); + }); + }); }); Assert.assertEquals("some image", testJibExtension.getFrom().getImage()); Assert.assertEquals("some cred helper", testJibExtension.getFrom().getCredHelper()); Assert.assertEquals("some username", testJibExtension.getFrom().getAuth().getUsername()); Assert.assertEquals("some password", testJibExtension.getFrom().getAuth().getPassword()); + + List platforms = testJibExtension.getFrom().getPlatforms().get(); + Assert.assertEquals(1, platforms.size()); + Assert.assertEquals("arm", platforms.get(0).getArchitecture()); + Assert.assertEquals("windows", platforms.get(0).getOs()); + } + + @Test + public void testFromPlatforms() { + List list = testJibExtension.getFrom().getPlatforms().get(); } @Test From d97224ce6ff00d9e2b3b969671609dc34dac3e98 Mon Sep 17 00:00:00 2001 From: Chanseok Oh Date: Wed, 23 Sep 2020 18:16:44 -0400 Subject: [PATCH 2/6] Remove dead code --- .../com/google/cloud/tools/jib/gradle/JibExtensionTest.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/JibExtensionTest.java b/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/JibExtensionTest.java index 3a5212740c..726edb1b00 100644 --- a/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/JibExtensionTest.java +++ b/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/JibExtensionTest.java @@ -93,11 +93,6 @@ public void testFrom() { Assert.assertEquals("windows", platforms.get(0).getOs()); } - @Test - public void testFromPlatforms() { - List list = testJibExtension.getFrom().getPlatforms().get(); - } - @Test public void testTo() { Assert.assertNull(testJibExtension.getTo().getImage()); From 1257b11ec821aac1ba963eb824fec5475dfad8af Mon Sep 17 00:00:00 2001 From: Chanseok Oh Date: Wed, 23 Sep 2020 18:21:37 -0400 Subject: [PATCH 3/6] CHANGELOG --- jib-gradle-plugin/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/jib-gradle-plugin/CHANGELOG.md b/jib-gradle-plugin/CHANGELOG.md index f8b80e1337..301b717aff 100644 --- a/jib-gradle-plugin/CHANGELOG.md +++ b/jib-gradle-plugin/CHANGELOG.md @@ -14,6 +14,7 @@ All notable changes to this project will be documented in this file. ### Fixed - Fixed `NullPointerException` during input validation (in Java 9+) when configuring Jib parameters using certain immutable collections (such as `List.of()`). ([#2702](https://github.com/GoogleContainerTools/jib/issues/2702)) +- Fixed an issue that configuring `jib.from.platforms` was always additive to the default `amd64/linux` platform. ([#2783](https://github.com/GoogleContainerTools/jib/issues/2783)) ## 2.5.0 From a80b6daeb85f297aba9a5650a9490fe88727e8d4 Mon Sep 17 00:00:00 2001 From: Chanseok Oh Date: Wed, 23 Sep 2020 18:30:52 -0400 Subject: [PATCH 4/6] code style --- .../tools/jib/gradle/BaseImageParameters.java | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/BaseImageParameters.java b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/BaseImageParameters.java index c4d0ede426..7d212c7dfa 100644 --- a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/BaseImageParameters.java +++ b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/BaseImageParameters.java @@ -49,17 +49,18 @@ public BaseImageParameters(ObjectFactory objectFactory) { @Nested @Optional public ListProperty getPlatforms() { - if (platforms.get().isEmpty()) { - PlatformParameters amd64Linux = new PlatformParameters(); - amd64Linux.setArchitecture("amd64"); - amd64Linux.setOs("linux"); - - ListProperty platforms = - objectFactory.listProperty(PlatformParameters.class); - platforms.add(amd64Linux); + if (!platforms.get().isEmpty()) { return platforms; } - return platforms; + + PlatformParameters amd64Linux = new PlatformParameters(); + amd64Linux.setArchitecture("amd64"); + amd64Linux.setOs("linux"); + + ListProperty defaultPlatforms = + objectFactory.listProperty(PlatformParameters.class); + defaultPlatforms.add(amd64Linux); + return defaultPlatforms; } public void platforms(Action action) { From d64c1ecb24ac47b3063e2e598334e4bc4ec959a2 Mon Sep 17 00:00:00 2001 From: Chanseok Oh Date: Thu, 24 Sep 2020 10:27:29 -0400 Subject: [PATCH 5/6] Reset default platform when given "platforms" action --- .../tools/jib/gradle/BaseImageParameters.java | 21 +++++++------------ .../tools/jib/gradle/JibExtensionTest.java | 5 ----- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/BaseImageParameters.java b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/BaseImageParameters.java index 7d212c7dfa..ecfa269f0d 100644 --- a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/BaseImageParameters.java +++ b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/BaseImageParameters.java @@ -41,29 +41,24 @@ public class BaseImageParameters { public BaseImageParameters(ObjectFactory objectFactory) { this.objectFactory = objectFactory; auth = objectFactory.newInstance(AuthParameters.class, "from.auth"); - platforms = objectFactory.listProperty(PlatformParameters.class).empty(); + platforms = objectFactory.listProperty(PlatformParameters.class); platformParametersSpec = objectFactory.newInstance(PlatformParametersSpec.class, objectFactory, platforms); - } - - @Nested - @Optional - public ListProperty getPlatforms() { - if (!platforms.get().isEmpty()) { - return platforms; - } PlatformParameters amd64Linux = new PlatformParameters(); amd64Linux.setArchitecture("amd64"); amd64Linux.setOs("linux"); + platforms.add(amd64Linux); + } - ListProperty defaultPlatforms = - objectFactory.listProperty(PlatformParameters.class); - defaultPlatforms.add(amd64Linux); - return defaultPlatforms; + @Nested + @Optional + public ListProperty getPlatforms() { + return platforms; } public void platforms(Action action) { + platforms.empty(); action.execute(platformParametersSpec); } diff --git a/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/JibExtensionTest.java b/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/JibExtensionTest.java index 726edb1b00..ab6269127d 100644 --- a/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/JibExtensionTest.java +++ b/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/JibExtensionTest.java @@ -62,11 +62,6 @@ public void testFrom() { Assert.assertEquals("amd64", defaultPlatforms.get(0).getArchitecture()); Assert.assertEquals("linux", defaultPlatforms.get(0).getOs()); - // Ensures calling getPlatforms() again doesn't increase the resulting list. - List defaultPlatformsAgain = - testJibExtension.getFrom().getPlatforms().get(); - Assert.assertEquals(1, defaultPlatformsAgain.size()); - testJibExtension.from( from -> { from.setImage("some image"); From 939c4ef72eda3489883f7b2116de10847e777276 Mon Sep 17 00:00:00 2001 From: Chanseok Oh Date: Thu, 24 Sep 2020 10:35:06 -0400 Subject: [PATCH 6/6] Remove unused variable --- .../com/google/cloud/tools/jib/gradle/BaseImageParameters.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/BaseImageParameters.java b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/BaseImageParameters.java index ecfa269f0d..1637687ce7 100644 --- a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/BaseImageParameters.java +++ b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/BaseImageParameters.java @@ -35,11 +35,8 @@ public class BaseImageParameters { private final PlatformParametersSpec platformParametersSpec; private final ListProperty platforms; - private final ObjectFactory objectFactory; - @Inject public BaseImageParameters(ObjectFactory objectFactory) { - this.objectFactory = objectFactory; auth = objectFactory.newInstance(AuthParameters.class, "from.auth"); platforms = objectFactory.listProperty(PlatformParameters.class); platformParametersSpec =