From 89ef61fb56930af7d35be05c5ff114d5995672e7 Mon Sep 17 00:00:00 2001 From: Tad Cordle Date: Tue, 14 Apr 2020 11:53:30 -0400 Subject: [PATCH 1/5] Respect DOCKER_CONFIG environment variable when finding credentials --- .../common/DefaultCredentialRetrievers.java | 27 +++++++++++---- .../DefaultCredentialRetrieversTest.java | 33 ++++++++++++++++++- 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/jib-plugins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/DefaultCredentialRetrievers.java b/jib-plugins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/DefaultCredentialRetrievers.java index f52e7e7e88..c49a1336a2 100644 --- a/jib-plugins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/DefaultCredentialRetrievers.java +++ b/jib-plugins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/DefaultCredentialRetrievers.java @@ -41,7 +41,8 @@ *
  • {@link CredentialRetrieverFactory#dockerCredentialHelper} for a known credential helper, if * set *
  • {@link CredentialRetrieverFactory#known} for known inferred credential, if set - *
  • {@link CredentialRetrieverFactory#dockerConfig} for {@code + *
  • {@link CredentialRetrieverFactory#dockerConfig} for {@code $DOCKER_CONFIG/config.json}, + * {@code $DOCKER_CONFIG/.dockerconfigjson}, {@code $DOCKER_CONFIG/.dockercfg}, * System.get("user.home")/.docker/config.json}, {@code * System.get("user.home")/.docker/.dockerconfigjson}, {@code * System.get("user.home")/.docker/.dockercfg}, {@code $HOME/.docker/config.json}, {@code @@ -57,11 +58,10 @@ public class DefaultCredentialRetrievers { * See https://docs.docker.com/engine/reference/commandline/login/#privileged-user-requirement. */ - private static final Path DOCKER_CONFIG_FILE = Paths.get(".docker", "config.json"); + private static final Path DOCKER_CONFIG_FILE = Paths.get("config.json"); // For Kubernetes: https://github.com/GoogleContainerTools/jib/issues/2260 - private static final Path KUBERNETES_DOCKER_CONFIG_FILE = - Paths.get(".docker", ".dockerconfigjson"); - private static final Path LEGACY_DOCKER_CONFIG_FILE = Paths.get(".docker", ".dockercfg"); + private static final Path KUBERNETES_DOCKER_CONFIG_FILE = Paths.get(".dockerconfigjson"); + private static final Path LEGACY_DOCKER_CONFIG_FILE = Paths.get(".dockercfg"); /** * Creates a new {@link DefaultCredentialRetrievers} with a given {@link @@ -166,10 +166,23 @@ public List asList() throws FileNotFoundException { credentialRetrievers.add(inferredCredentialRetriever); } + String dockerConfigEnv = environment.get("DOCKER_CONFIG"); + if (dockerConfigEnv != null) { + Path dockerConfigsPath = Paths.get(dockerConfigEnv); + credentialRetrievers.add( + credentialRetrieverFactory.dockerConfig(dockerConfigsPath.resolve(DOCKER_CONFIG_FILE))); + credentialRetrievers.add( + credentialRetrieverFactory.dockerConfig( + dockerConfigsPath.resolve(KUBERNETES_DOCKER_CONFIG_FILE))); + credentialRetrievers.add( + credentialRetrieverFactory.legacyDockerConfig( + dockerConfigsPath.resolve(LEGACY_DOCKER_CONFIG_FILE))); + } + String homeProperty = systemProperties.getProperty("user.home"); String homeEnvVar = environment.get("HOME"); if (homeProperty != null) { - Path home = Paths.get(homeProperty); + Path home = Paths.get(homeProperty).resolve(".docker"); credentialRetrievers.add( credentialRetrieverFactory.dockerConfig(home.resolve(DOCKER_CONFIG_FILE))); credentialRetrievers.add( @@ -178,7 +191,7 @@ public List asList() throws FileNotFoundException { credentialRetrieverFactory.legacyDockerConfig(home.resolve(LEGACY_DOCKER_CONFIG_FILE))); } if (homeEnvVar != null && !homeEnvVar.equals(homeProperty)) { - Path home = Paths.get(homeEnvVar); + Path home = Paths.get(homeEnvVar).resolve(".docker"); credentialRetrievers.add( credentialRetrieverFactory.dockerConfig(home.resolve(DOCKER_CONFIG_FILE))); credentialRetrievers.add( diff --git a/jib-plugins-common/src/test/java/com/google/cloud/tools/jib/plugins/common/DefaultCredentialRetrieversTest.java b/jib-plugins-common/src/test/java/com/google/cloud/tools/jib/plugins/common/DefaultCredentialRetrieversTest.java index 7c82ffaca5..1397cf0cce 100644 --- a/jib-plugins-common/src/test/java/com/google/cloud/tools/jib/plugins/common/DefaultCredentialRetrieversTest.java +++ b/jib-plugins-common/src/test/java/com/google/cloud/tools/jib/plugins/common/DefaultCredentialRetrieversTest.java @@ -51,6 +51,9 @@ public class DefaultCredentialRetrieversTest { @Mock private CredentialRetriever mockKnownCredentialRetriever; @Mock private CredentialRetriever mockInferredCredentialRetriever; @Mock private CredentialRetriever mockWellKnownCredentialHelpersCredentialRetriever; + @Mock private CredentialRetriever mockDockerConfigEnvDockerConfigCredentialRetriever; + @Mock private CredentialRetriever mockDockerConfigEnvKubernetesDockerConfigCredentialRetriever; + @Mock private CredentialRetriever mockDockerConfigEnvLegacyDockerConfigCredentialRetriever; @Mock private CredentialRetriever mockSystemHomeDockerConfigCredentialRetriever; @Mock private CredentialRetriever mockSystemHomeKubernetesDockerConfigCredentialRetriever; @Mock private CredentialRetriever mockSystemHomeLegacyDockerConfigCredentialRetriever; @@ -69,7 +72,12 @@ public class DefaultCredentialRetrieversTest { public void setUp() { properties = new Properties(); properties.setProperty("user.home", Paths.get("/system/home").toString()); - environment = ImmutableMap.of("HOME", Paths.get("/env/home").toString()); + environment = + ImmutableMap.of( + "HOME", + Paths.get("/env/home").toString(), + "DOCKER_CONFIG", + Paths.get("/docker_config").toString()); Mockito.when(mockCredentialRetrieverFactory.dockerCredentialHelper(Mockito.anyString())) .thenReturn(mockDockerCredentialHelperCredentialRetriever); @@ -80,6 +88,17 @@ public void setUp() { .thenReturn(mockInferredCredentialRetriever); Mockito.when(mockCredentialRetrieverFactory.wellKnownCredentialHelpers()) .thenReturn(mockWellKnownCredentialHelpersCredentialRetriever); + Mockito.when( + mockCredentialRetrieverFactory.dockerConfig(Paths.get("/docker_config/config.json"))) + .thenReturn(mockDockerConfigEnvDockerConfigCredentialRetriever); + Mockito.when( + mockCredentialRetrieverFactory.dockerConfig( + Paths.get("/docker_config/.dockerconfigjson"))) + .thenReturn(mockDockerConfigEnvKubernetesDockerConfigCredentialRetriever); + Mockito.when( + mockCredentialRetrieverFactory.legacyDockerConfig( + Paths.get("/docker_config/.dockercfg"))) + .thenReturn(mockDockerConfigEnvLegacyDockerConfigCredentialRetriever); Mockito.when( mockCredentialRetrieverFactory.dockerConfig( Paths.get("/system/home/.docker/config.json"))) @@ -114,6 +133,9 @@ public void testAsList() throws FileNotFoundException { .asList(); Assert.assertEquals( Arrays.asList( + mockDockerConfigEnvDockerConfigCredentialRetriever, + mockDockerConfigEnvKubernetesDockerConfigCredentialRetriever, + mockDockerConfigEnvLegacyDockerConfigCredentialRetriever, mockSystemHomeDockerConfigCredentialRetriever, mockSystemHomeKubernetesDockerConfigCredentialRetriever, mockSystemHomeLegacyDockerConfigCredentialRetriever, @@ -138,6 +160,9 @@ public void testAsList_all() throws FileNotFoundException { mockKnownCredentialRetriever, mockDockerCredentialHelperCredentialRetriever, mockInferredCredentialRetriever, + mockDockerConfigEnvDockerConfigCredentialRetriever, + mockDockerConfigEnvKubernetesDockerConfigCredentialRetriever, + mockDockerConfigEnvLegacyDockerConfigCredentialRetriever, mockSystemHomeDockerConfigCredentialRetriever, mockSystemHomeKubernetesDockerConfigCredentialRetriever, mockSystemHomeLegacyDockerConfigCredentialRetriever, @@ -166,6 +191,9 @@ public void testAsList_credentialHelperPath() throws IOException { Assert.assertEquals( Arrays.asList( mockDockerCredentialHelperCredentialRetriever, + mockDockerConfigEnvDockerConfigCredentialRetriever, + mockDockerConfigEnvKubernetesDockerConfigCredentialRetriever, + mockDockerConfigEnvLegacyDockerConfigCredentialRetriever, mockSystemHomeDockerConfigCredentialRetriever, mockSystemHomeKubernetesDockerConfigCredentialRetriever, mockSystemHomeLegacyDockerConfigCredentialRetriever, @@ -210,6 +238,9 @@ public void testDockerConfigRetrievers_noDuplicateRetrievers() throws FileNotFou .asList(); Assert.assertEquals( Arrays.asList( + mockDockerConfigEnvDockerConfigCredentialRetriever, + mockDockerConfigEnvKubernetesDockerConfigCredentialRetriever, + mockDockerConfigEnvLegacyDockerConfigCredentialRetriever, mockEnvHomeDockerConfigCredentialRetriever, mockEnvHomeKubernetesDockerConfigCredentialRetriever, mockEnvHomeLegacyDockerConfigCredentialRetriever, From 990b22ade8aa257ca76a0232232b3e27eb45f3db Mon Sep 17 00:00:00 2001 From: Tad Cordle Date: Tue, 14 Apr 2020 12:01:21 -0400 Subject: [PATCH 2/5] Changelog --- jib-gradle-plugin/CHANGELOG.md | 1 + jib-maven-plugin/CHANGELOG.md | 1 + 2 files changed, 2 insertions(+) diff --git a/jib-gradle-plugin/CHANGELOG.md b/jib-gradle-plugin/CHANGELOG.md index f259127aba..4ab5d3121c 100644 --- a/jib-gradle-plugin/CHANGELOG.md +++ b/jib-gradle-plugin/CHANGELOG.md @@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file. - Glob pattern support for `jib.extraDirectories.permissions`. ([#1200](https://github.com/GoogleContainerTools/jib/issues/1200)) - Support for image references with both a tag and a digest. ([#1481](https://github.com/GoogleContainerTools/jib/issues/1481)) +- The `DOCKER_CONFIG` environment variable is now checked for retrieving credentials from the docker config. ([#1618](https://github.com/GoogleContainerTools/jib/issues/1618)) ### Changed diff --git a/jib-maven-plugin/CHANGELOG.md b/jib-maven-plugin/CHANGELOG.md index 924acf7cb1..21f10cc539 100644 --- a/jib-maven-plugin/CHANGELOG.md +++ b/jib-maven-plugin/CHANGELOG.md @@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file. - Glob pattern support for ``. ([#1200](https://github.com/GoogleContainerTools/jib/issues/1200)) - Support for image references with both a tag and a digest. ([#1481](https://github.com/GoogleContainerTools/jib/issues/1481)) +- The `DOCKER_CONFIG` environment variable is now checked for retrieving credentials from the docker config. ([#1618](https://github.com/GoogleContainerTools/jib/issues/1618)) ### Changed From cd2b5819efd4eaaff7ffca502d9a94fa78da6d5f Mon Sep 17 00:00:00 2001 From: Tad Cordle Date: Tue, 14 Apr 2020 12:11:19 -0400 Subject: [PATCH 3/5] Code compression --- .../common/DefaultCredentialRetrievers.java | 43 ++++++++----------- 1 file changed, 17 insertions(+), 26 deletions(-) diff --git a/jib-plugins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/DefaultCredentialRetrievers.java b/jib-plugins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/DefaultCredentialRetrievers.java index c49a1336a2..79f0b44100 100644 --- a/jib-plugins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/DefaultCredentialRetrievers.java +++ b/jib-plugins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/DefaultCredentialRetrievers.java @@ -62,6 +62,7 @@ public class DefaultCredentialRetrievers { // For Kubernetes: https://github.com/GoogleContainerTools/jib/issues/2260 private static final Path KUBERNETES_DOCKER_CONFIG_FILE = Paths.get(".dockerconfigjson"); private static final Path LEGACY_DOCKER_CONFIG_FILE = Paths.get(".dockercfg"); + private static final Path DOCKER_DIRECTORY = Paths.get(".docker"); /** * Creates a new {@link DefaultCredentialRetrievers} with a given {@link @@ -71,8 +72,7 @@ public class DefaultCredentialRetrievers { * CredentialRetriever}s * @return a new {@link DefaultCredentialRetrievers} */ - public static DefaultCredentialRetrievers init( - CredentialRetrieverFactory credentialRetrieverFactory) { + static DefaultCredentialRetrievers init(CredentialRetrieverFactory credentialRetrieverFactory) { return new DefaultCredentialRetrievers( credentialRetrieverFactory, System.getProperties(), System.getenv()); } @@ -168,40 +168,31 @@ public List asList() throws FileNotFoundException { String dockerConfigEnv = environment.get("DOCKER_CONFIG"); if (dockerConfigEnv != null) { - Path dockerConfigsPath = Paths.get(dockerConfigEnv); - credentialRetrievers.add( - credentialRetrieverFactory.dockerConfig(dockerConfigsPath.resolve(DOCKER_CONFIG_FILE))); - credentialRetrievers.add( - credentialRetrieverFactory.dockerConfig( - dockerConfigsPath.resolve(KUBERNETES_DOCKER_CONFIG_FILE))); - credentialRetrievers.add( - credentialRetrieverFactory.legacyDockerConfig( - dockerConfigsPath.resolve(LEGACY_DOCKER_CONFIG_FILE))); + addDockerFiles(credentialRetrievers, Paths.get(dockerConfigEnv)); } String homeProperty = systemProperties.getProperty("user.home"); - String homeEnvVar = environment.get("HOME"); if (homeProperty != null) { - Path home = Paths.get(homeProperty).resolve(".docker"); - credentialRetrievers.add( - credentialRetrieverFactory.dockerConfig(home.resolve(DOCKER_CONFIG_FILE))); - credentialRetrievers.add( - credentialRetrieverFactory.dockerConfig(home.resolve(KUBERNETES_DOCKER_CONFIG_FILE))); - credentialRetrievers.add( - credentialRetrieverFactory.legacyDockerConfig(home.resolve(LEGACY_DOCKER_CONFIG_FILE))); + addDockerFiles(credentialRetrievers, Paths.get(homeProperty).resolve(DOCKER_DIRECTORY)); } + + String homeEnvVar = environment.get("HOME"); if (homeEnvVar != null && !homeEnvVar.equals(homeProperty)) { - Path home = Paths.get(homeEnvVar).resolve(".docker"); - credentialRetrievers.add( - credentialRetrieverFactory.dockerConfig(home.resolve(DOCKER_CONFIG_FILE))); - credentialRetrievers.add( - credentialRetrieverFactory.dockerConfig(home.resolve(KUBERNETES_DOCKER_CONFIG_FILE))); - credentialRetrievers.add( - credentialRetrieverFactory.legacyDockerConfig(home.resolve(LEGACY_DOCKER_CONFIG_FILE))); + addDockerFiles(credentialRetrievers, Paths.get(homeEnvVar).resolve(DOCKER_DIRECTORY)); } credentialRetrievers.add(credentialRetrieverFactory.wellKnownCredentialHelpers()); credentialRetrievers.add(credentialRetrieverFactory.googleApplicationDefaultCredentials()); return credentialRetrievers; } + + private void addDockerFiles(List credentialRetrievers, Path configDir) { + credentialRetrievers.add( + credentialRetrieverFactory.dockerConfig(configDir.resolve(DOCKER_CONFIG_FILE))); + credentialRetrievers.add( + credentialRetrieverFactory.dockerConfig(configDir.resolve(KUBERNETES_DOCKER_CONFIG_FILE))); + credentialRetrievers.add( + credentialRetrieverFactory.legacyDockerConfig( + configDir.resolve(LEGACY_DOCKER_CONFIG_FILE))); + } } From 7855ef3e0ac3cd958f6ee4a3d619ff28d23a4630 Mon Sep 17 00:00:00 2001 From: Tad Cordle Date: Tue, 14 Apr 2020 13:24:36 -0400 Subject: [PATCH 4/5] Feedback --- jib-gradle-plugin/CHANGELOG.md | 2 +- jib-maven-plugin/CHANGELOG.md | 2 +- .../common/DefaultCredentialRetrievers.java | 14 ++++++++++++-- .../DefaultCredentialRetrieversTest.java | 18 ++++++++++++++++++ 4 files changed, 32 insertions(+), 4 deletions(-) diff --git a/jib-gradle-plugin/CHANGELOG.md b/jib-gradle-plugin/CHANGELOG.md index 4ab5d3121c..253ef74839 100644 --- a/jib-gradle-plugin/CHANGELOG.md +++ b/jib-gradle-plugin/CHANGELOG.md @@ -7,7 +7,7 @@ All notable changes to this project will be documented in this file. - Glob pattern support for `jib.extraDirectories.permissions`. ([#1200](https://github.com/GoogleContainerTools/jib/issues/1200)) - Support for image references with both a tag and a digest. ([#1481](https://github.com/GoogleContainerTools/jib/issues/1481)) -- The `DOCKER_CONFIG` environment variable is now checked for retrieving credentials from the docker config. ([#1618](https://github.com/GoogleContainerTools/jib/issues/1618)) +- The `DOCKER_CONFIG` environment variable specifying the directory containing docker configs is now checked during credential retrieval. ([#1618](https://github.com/GoogleContainerTools/jib/issues/1618)) ### Changed diff --git a/jib-maven-plugin/CHANGELOG.md b/jib-maven-plugin/CHANGELOG.md index 21f10cc539..6e9c39c574 100644 --- a/jib-maven-plugin/CHANGELOG.md +++ b/jib-maven-plugin/CHANGELOG.md @@ -7,7 +7,7 @@ All notable changes to this project will be documented in this file. - Glob pattern support for ``. ([#1200](https://github.com/GoogleContainerTools/jib/issues/1200)) - Support for image references with both a tag and a digest. ([#1481](https://github.com/GoogleContainerTools/jib/issues/1481)) -- The `DOCKER_CONFIG` environment variable is now checked for retrieving credentials from the docker config. ([#1618](https://github.com/GoogleContainerTools/jib/issues/1618)) +- The `DOCKER_CONFIG` environment variable specifying the directory containing docker configs is now checked during credential retrieval. ([#1618](https://github.com/GoogleContainerTools/jib/issues/1618)) ### Changed diff --git a/jib-plugins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/DefaultCredentialRetrievers.java b/jib-plugins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/DefaultCredentialRetrievers.java index 79f0b44100..260c51ca24 100644 --- a/jib-plugins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/DefaultCredentialRetrievers.java +++ b/jib-plugins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/DefaultCredentialRetrievers.java @@ -166,19 +166,29 @@ public List asList() throws FileNotFoundException { credentialRetrievers.add(inferredCredentialRetriever); } + List checkedDockerDirs = new ArrayList<>(); String dockerConfigEnv = environment.get("DOCKER_CONFIG"); if (dockerConfigEnv != null) { + Path dockerConfigEnvPath = Paths.get(dockerConfigEnv); addDockerFiles(credentialRetrievers, Paths.get(dockerConfigEnv)); + checkedDockerDirs.add(dockerConfigEnvPath); } String homeProperty = systemProperties.getProperty("user.home"); if (homeProperty != null) { - addDockerFiles(credentialRetrievers, Paths.get(homeProperty).resolve(DOCKER_DIRECTORY)); + Path homePropertyPath = Paths.get(homeProperty).resolve(DOCKER_DIRECTORY); + if (!checkedDockerDirs.contains(homePropertyPath)) { + addDockerFiles(credentialRetrievers, homePropertyPath); + checkedDockerDirs.add(homePropertyPath); + } } String homeEnvVar = environment.get("HOME"); if (homeEnvVar != null && !homeEnvVar.equals(homeProperty)) { - addDockerFiles(credentialRetrievers, Paths.get(homeEnvVar).resolve(DOCKER_DIRECTORY)); + Path homeEnvDockerPath = Paths.get(homeEnvVar).resolve(DOCKER_DIRECTORY); + if (!checkedDockerDirs.contains(homeEnvDockerPath)) { + addDockerFiles(credentialRetrievers, homeEnvDockerPath); + } } credentialRetrievers.add(credentialRetrieverFactory.wellKnownCredentialHelpers()); diff --git a/jib-plugins-common/src/test/java/com/google/cloud/tools/jib/plugins/common/DefaultCredentialRetrieversTest.java b/jib-plugins-common/src/test/java/com/google/cloud/tools/jib/plugins/common/DefaultCredentialRetrieversTest.java index 1397cf0cce..811e0d840a 100644 --- a/jib-plugins-common/src/test/java/com/google/cloud/tools/jib/plugins/common/DefaultCredentialRetrieversTest.java +++ b/jib-plugins-common/src/test/java/com/google/cloud/tools/jib/plugins/common/DefaultCredentialRetrieversTest.java @@ -247,5 +247,23 @@ public void testDockerConfigRetrievers_noDuplicateRetrievers() throws FileNotFou mockWellKnownCredentialHelpersCredentialRetriever, mockApplicationDefaultCredentialRetriever), credentialRetrievers); + + environment = + ImmutableMap.of( + "HOME", + Paths.get("/env/home").toString(), + "DOCKER_CONFIG", + Paths.get("/env/home/.docker").toString()); + credentialRetrievers = + new DefaultCredentialRetrievers(mockCredentialRetrieverFactory, properties, environment) + .asList(); + Assert.assertEquals( + Arrays.asList( + mockEnvHomeDockerConfigCredentialRetriever, + mockEnvHomeKubernetesDockerConfigCredentialRetriever, + mockEnvHomeLegacyDockerConfigCredentialRetriever, + mockWellKnownCredentialHelpersCredentialRetriever, + mockApplicationDefaultCredentialRetriever), + credentialRetrievers); } } From bf33b16816333d9a45d1c1bddb2d228fe5e4923d Mon Sep 17 00:00:00 2001 From: Tad Cordle Date: Tue, 14 Apr 2020 15:59:22 -0400 Subject: [PATCH 5/5] Fix --- .../tools/jib/plugins/common/DefaultCredentialRetrievers.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jib-plugins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/DefaultCredentialRetrievers.java b/jib-plugins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/DefaultCredentialRetrievers.java index 260c51ca24..2c36f1e885 100644 --- a/jib-plugins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/DefaultCredentialRetrievers.java +++ b/jib-plugins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/DefaultCredentialRetrievers.java @@ -184,7 +184,7 @@ public List asList() throws FileNotFoundException { } String homeEnvVar = environment.get("HOME"); - if (homeEnvVar != null && !homeEnvVar.equals(homeProperty)) { + if (homeEnvVar != null) { Path homeEnvDockerPath = Paths.get(homeEnvVar).resolve(DOCKER_DIRECTORY); if (!checkedDockerDirs.contains(homeEnvDockerPath)) { addDockerFiles(credentialRetrievers, homeEnvDockerPath);