fix(auth): Non existing version command on credential helper is ignor…

…ed (#1190) Fixes #1159.
fabric8io · Apr 6, 2019 · dc82c82 · dc82c82
1 parent b414b9b
commit dc82c82
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 16 deletions.
diff --git a/doc/changelog.md b/doc/changelog.md
@@ -1,6 +1,6 @@
 # ChangeLog
 
-* 0.28.1
+* **0.28-SNAPSHOT**
   - Reintroduce minimal API-VERSION parameter in order to support docker versions below apiVersion 1.25
   - docs: Correct default image naming
   - Proxy settings are being ignored ([#1148](https://github.com/fabric8io/docker-maven-plugin/issues/1148))
@@ -13,6 +13,7 @@
   - Added 'autoRemove' option for running containers (#1179)
   - Added support for AWS EC2 instance roles when pushing to AWS ECR (#1186)
   - Add support for auto-pulling multiple base image for multi stage builds (#1057)
+  - Fix usage of credential helper that do not support 'version' command (#1159)
 
 * **0.28.0** (2018-12-13)
   - Update to JMockit 1.43

diff --git a/src/main/java/io/fabric8/maven/docker/util/AuthConfigFactory.java b/src/main/java/io/fabric8/maven/docker/util/AuthConfigFactory.java
@@ -230,7 +230,7 @@ private AuthConfig createStandardAuthConfig(boolean isPush, Map authConfigMap, S
             log.debug("AuthConfig: credentials from ~/.m2/setting.xml");
             return ret;
         }
-        
+
         // check EC2 instance role if registry is ECR
         if (EcrExtendedAuth.isAwsRegistry(registry)) {
             try {
@@ -253,8 +253,8 @@ private AuthConfig createStandardAuthConfig(boolean isPush, Map authConfigMap, S
     }
 
     // ===================================================================================================
-    
-    
+
+
     // if the local credentials don't contain user and password, use EC2 instance
     // role credentials
     private AuthConfig getAuthConfigFromEC2InstanceRole() throws IOException {
@@ -310,7 +310,7 @@ private AuthConfig getAuthConfigFromEC2InstanceRole() throws IOException {
             }
         }
     }
-        
+
     private AuthConfig getAuthConfigFromSystemProperties(LookupMode lookupMode) throws MojoExecutionException {
         Properties props = System.getProperties();
         String userKey = lookupMode.asSysProperty(AUTH_USERNAME);
@@ -423,9 +423,10 @@ private AuthConfig extractAuthConfigFromAuths(String registryToLookup, JsonObjec
 
     private AuthConfig extractAuthConfigFromCredentialsHelper(String registryToLookup, String credConfig) throws MojoExecutionException {
         CredentialHelperClient credentialHelper = new CredentialHelperClient(log, credConfig);
-        log.debug("AuthConfig: credentials from credential helper/store %s version %s",
+        String version = credentialHelper.getVersion();
+        log.debug("AuthConfig: credentials from credential helper/store %s%s",
                   credentialHelper.getName(),
-                  credentialHelper.getVersion());
+                  version != null ? " version " + version : "");
         return credentialHelper.getAuthConfig(registryToLookup);
     }
 
@@ -523,7 +524,7 @@ private AuthConfig validateMandatoryOpenShiftLogin(AuthConfig openShiftAuthConfi
                           useOpenAuthModeProp, kubeConfigEnv != null ? kubeConfigEnv : "~/.kube/config"));
 
     }
-   
+
 
     private Server checkForServer(Server server, String id, String registry, String user) {
 

diff --git a/src/main/java/io/fabric8/maven/docker/util/CredentialHelperClient.java b/src/main/java/io/fabric8/maven/docker/util/CredentialHelperClient.java
@@ -28,11 +28,11 @@ public String getName() {
         return credentialHelperName;
     }
 
-    public String getVersion() throws MojoExecutionException {
+    public String getVersion() {
         try {
             return new VersionCommand().getVersion();
         } catch (IOException e) {
-            throw new MojoExecutionException("Error getting the version of the configured credential helper",e);
+            return null;
         }
     }
 
@@ -79,9 +79,6 @@ protected void processLine(String line) {
 
         public String getVersion() throws IOException {
             execute();
-            if (version == null) {
-                log.verbose("The credentials helper \"%s\" didn't return a version string",CredentialHelperClient.this.credentialHelperName);
-            }
             return version;
         }
     }

diff --git a/src/test/java/io/fabric8/maven/docker/util/AuthConfigFactoryTest.java b/src/test/java/io/fabric8/maven/docker/util/AuthConfigFactoryTest.java
@@ -169,7 +169,7 @@ public void exec(File homeDir) throws IOException, MojoExecutionException {
                 expectedException.expect(MojoExecutionException.class);
                 expectedException.expectCause(Matchers.<Throwable>allOf(
                         instanceOf(IOException.class),
-                        hasProperty("message",startsWith("Failed to start 'docker-credential-credHelper1-does-not-exist version'"))
+                        hasProperty("message",startsWith("Failed to start 'docker-credential-credHelper1-does-not-exist get'"))
                 ));
                 factory.createAuthConfig(isPush,false,null,settings,"roland","registry1");
             }
@@ -185,7 +185,7 @@ public void exec(File homeDir) throws IOException, MojoExecutionException {
                 expectedException.expect(MojoExecutionException.class);
                 expectedException.expectCause(Matchers.<Throwable>allOf(
                         instanceOf(IOException.class),
-                        hasProperty("message",startsWith("Failed to start 'docker-credential-credsStore-does-not-exist version'"))
+                        hasProperty("message",startsWith("Failed to start 'docker-credential-credsStore-does-not-exist get'"))
                 ));
                 factory.createAuthConfig(isPush,false,null,settings,"roland",null);
             }
@@ -201,7 +201,7 @@ public void exec(File homeDir) throws IOException, MojoExecutionException {
                 expectedException.expect(MojoExecutionException.class);
                 expectedException.expectCause(Matchers.<Throwable>allOf(
                         instanceOf(IOException.class),
-                        hasProperty("message",startsWith("Failed to start 'docker-credential-credsStore-does-not-exist version'"))
+                        hasProperty("message",startsWith("Failed to start 'docker-credential-credsStore-does-not-exist get'"))
                 ));
                 factory.createAuthConfig(isPush,false,null,settings,"roland","registry2");
             }