Improved RegistryAuthLocator and added tests for Windows (#868)

aulea · rnorth · commit 53e5ece48b7d · 2018-09-20T21:03:29.000+01:00
#756 tested on Windows. Fixed RegistryAuthLocatorTest on Windows and also allowed better fallbacks from running credential provider (to allow lookup alternative AuthConfigs), when: 1) there is no hostName, then there is no point to ask credentials 2) when credential helper response with "credentials not found in native keychain" to try other resources Main reason for failing for me on Windows machine was #710 changes. When i used Netty or OkHttp together with npipe, then it worked fine. Yesterday evening i found out the reason and today morning i found also fix in master for that :-) - #865, breaking docker response by line breaks.
diff --git a/core/src/main/java/org/testcontainers/utility/RegistryAuthLocator.java b/core/src/main/java/org/testcontainers/utility/RegistryAuthLocator.java
@@ -5,16 +5,19 @@
 import com.github.dockerjava.api.model.AuthConfig;
 import com.google.common.annotations.VisibleForTesting;
 import org.apache.commons.lang.StringUtils;
-import org.apache.commons.lang.SystemUtils;
 import org.slf4j.Logger;
+import org.zeroturnaround.exec.InvalidResultException;
 import org.zeroturnaround.exec.ProcessExecutor;
 
 import java.io.ByteArrayInputStream;
 import java.io.File;
+import java.io.IOException;
 import java.util.Base64;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 
 import static org.apache.commons.lang.StringUtils.isBlank;
 import static org.slf4j.LoggerFactory.getLogger;
@@ -28,14 +31,27 @@ public class RegistryAuthLocator {
     private static final Logger log = getLogger(RegistryAuthLocator.class);
     private static final String DEFAULT_REGISTRY_NAME = "index.docker.io";
     private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
+
     private static RegistryAuthLocator instance;
+
     private final String commandPathPrefix;
+    private final String commandExtension;
     private final File configFile;
 
+    /**
+     * key - credential helper's name
+     * value - helper's response for "credentials not found" use case
+     */
+    private final Map<String, String> CREDENTIALS_HELPERS_NOT_FOUND_MESSAGE_CACHE;
+
     @VisibleForTesting
-    RegistryAuthLocator(File configFile, String commandPathPrefix) {
+    RegistryAuthLocator(File configFile, String commandPathPrefix, String commandExtension,
+                        Map<String, String> notFoundMessageHolderReference) {
         this.configFile = configFile;
         this.commandPathPrefix = commandPathPrefix;
+        this.commandExtension = commandExtension;
+
+        this.CREDENTIALS_HELPERS_NOT_FOUND_MESSAGE_CACHE = notFoundMessageHolderReference;
     }
 
     /**
@@ -45,6 +61,9 @@ protected RegistryAuthLocator() {
             System.getProperty("user.home") + "/.docker");
         this.configFile = new File(dockerConfigLocation + "/config.json");
         this.commandPathPrefix = "";
+        this.commandExtension = "";
+
+        this.CREDENTIALS_HELPERS_NOT_FOUND_MESSAGE_CACHE = new HashMap<>();
     }
 
     public synchronized static RegistryAuthLocator instance() {
@@ -79,12 +98,6 @@ static void setInstance(RegistryAuthLocator overrideInstance) {
      */
     public AuthConfig lookupAuthConfig(DockerImageName dockerImageName, AuthConfig defaultAuthConfig) {
 
-        if (SystemUtils.IS_OS_WINDOWS) {
-            log.debug("RegistryAuthLocator is not supported on Windows. Please help test or improve it and update " +
-                "https://github.com/testcontainers/testcontainers-java/issues/756");
-            return defaultAuthConfig;
-        }
-
         log.debug("Looking up auth config for image: {}", dockerImageName);
 
         log.debug("RegistryAuthLocator has configFile: {} ({}) and commandPathPrefix: {}",
@@ -119,7 +132,7 @@ public AuthConfig lookupAuthConfig(DockerImageName dockerImageName, AuthConfig d
             log.debug("no matching Auth Configs - falling back to defaultAuthConfig [{}]", toSafeString(defaultAuthConfig));
             // otherwise, defaultAuthConfig should already contain any credentials available
         } catch (Exception e) {
-            log.debug("Failure when attempting to lookup auth config (dockerImageName: {}, configFile: {}. Falling back to docker-java default behaviour. Exception message: {}",
+            log.warn("Failure when attempting to lookup auth config (dockerImageName: {}, configFile: {}. Falling back to docker-java default behaviour. Exception message: {}",
                 dockerImageName,
                 configFile,
                 e.getMessage());
@@ -189,38 +202,126 @@ private Map.Entry<String, JsonNode> findAuthNode(final JsonNode config, final St
         return null;
     }
 
-    private AuthConfig runCredentialProvider(String hostName, String credHelper) throws Exception {
-        final String credentialHelperName = commandPathPrefix + "docker-credential-" + credHelper;
-        String data;
+    private AuthConfig runCredentialProvider(String hostName, String helperOrStoreName) throws Exception {
+
+        if (isBlank(hostName)) {
+            log.debug("There is no point to locate AuthConfig for blank hostName. Return NULL to allow fallback");
+            return null;
+        }
+
+        final String credentialProgramName = getCredentialProgramName(helperOrStoreName);
+        final String data;
 
-        log.debug("Executing docker credential helper: {} to locate auth config for: {}",
-            credentialHelperName, hostName);
+        log.debug("Executing docker credential provider: {} to locate auth config for: {}",
+            credentialProgramName, hostName);
 
         try {
-            data = new ProcessExecutor()
-                .command(credentialHelperName, "get")
-                .redirectInput(new ByteArrayInputStream(hostName.getBytes()))
-                .readOutput(true)
-                .exitValueNormal()
-                .timeout(30, TimeUnit.SECONDS)
-                .execute()
-                .outputUTF8()
-                .trim();
+            data = runCredentialProgram(hostName, credentialProgramName);
+        } catch (InvalidResultException e) {
+
+            final String responseErrorMsg = extractCredentialProviderErrorMessage(e);
+
+            if (!isBlank(responseErrorMsg)) {
+                String credentialsNotFoundMsg = getGenericCredentialsNotFoundMsg(credentialProgramName);
+                if (credentialsNotFoundMsg != null && credentialsNotFoundMsg.equals(responseErrorMsg)) {
+                    log.info("Credentials not found for host ({}) when using credential helper/store ({})",
+                        hostName,
+                        credentialProgramName);
+
+                    return null;
+                }
+
+                log.debug("Failure running docker credential helper/store ({}) with output '{}'",
+                    credentialProgramName, responseErrorMsg);
+
+            } else {
+                log.debug("Failure running docker credential helper/store ({})", credentialProgramName);
+            }
+
+            throw e;
         } catch (Exception e) {
-            log.debug("Failure running docker credential helper ({})", credentialHelperName);
+            log.debug("Failure running docker credential helper/store ({})", credentialProgramName);
             throw e;
         }
 
         final JsonNode helperResponse = OBJECT_MAPPER.readTree(data);
-        log.debug("Credential helper provided auth config for: {}", hostName);
+        log.debug("Credential helper/store provided auth config for: {}", hostName);
 
         return new AuthConfig()
             .withRegistryAddress(helperResponse.at("/ServerURL").asText())
             .withUsername(helperResponse.at("/Username").asText())
             .withPassword(helperResponse.at("/Secret").asText());
     }
 
+    private String getCredentialProgramName(String credHelper) {
+        return commandPathPrefix + "docker-credential-" + credHelper + commandExtension;
+    }
+
     private String effectiveRegistryName(DockerImageName dockerImageName) {
         return StringUtils.defaultIfEmpty(dockerImageName.getRegistry(), DEFAULT_REGISTRY_NAME);
     }
+
+    private String getGenericCredentialsNotFoundMsg(String credentialHelperName) {
+        if (!CREDENTIALS_HELPERS_NOT_FOUND_MESSAGE_CACHE.containsKey(credentialHelperName)) {
+            String credentialsNotFoundMsg = discoverCredentialsHelperNotFoundMessage(credentialHelperName);
+            if (!isBlank(credentialsNotFoundMsg)) {
+                CREDENTIALS_HELPERS_NOT_FOUND_MESSAGE_CACHE.put(credentialHelperName, credentialsNotFoundMsg);
+            }
+        }
+
+        return CREDENTIALS_HELPERS_NOT_FOUND_MESSAGE_CACHE.get(credentialHelperName);
+    }
+
+    private String discoverCredentialsHelperNotFoundMessage(String credentialHelperName) {
+        // will do fake call to given credential helper to find out with which message
+        // it response when there are no credentials for given hostName
+
+        // hostName should be valid, but most probably not existing
+        // IF its not enough, then should probably run 'list' command first to be sure...
+        final String notExistentFakeHostName = "https://not.a.real.registry/url";
+
+        String credentialsNotFoundMsg = null;
+        try {
+            runCredentialProgram(notExistentFakeHostName, credentialHelperName);
+
+            // should not reach here
+            log.warn("Failure running docker credential helper ({}) with fake call, expected 'credentials not found' response",
+                credentialHelperName);
+        } catch(Exception e) {
+            if (e instanceof InvalidResultException) {
+                credentialsNotFoundMsg = extractCredentialProviderErrorMessage((InvalidResultException)e);
+            }
+
+            if (isBlank(credentialsNotFoundMsg)) {
+                log.warn("Failure running docker credential helper ({}) with fake call, expected 'credentials not found' response. Exception message: {}",
+                    credentialHelperName,
+                    e.getMessage());
+            } else {
+                log.debug("Got credentials not found error message from docker credential helper - {}", credentialsNotFoundMsg);
+            }
+        }
+
+        return credentialsNotFoundMsg;
+    }
+
+    private String extractCredentialProviderErrorMessage(InvalidResultException invalidResultEx) {
+        if (invalidResultEx.getResult() != null && invalidResultEx.getResult().hasOutput()) {
+            return invalidResultEx.getResult().outputString().trim();
+        }
+        return null;
+    }
+
+    private String runCredentialProgram(String hostName, String credentialHelperName)
+        throws InvalidResultException, InterruptedException, TimeoutException, IOException {
+
+        return new ProcessExecutor()
+                        .command(credentialHelperName, "get")
+                        .redirectInput(new ByteArrayInputStream(hostName.getBytes()))
+                        .readOutput(true)
+                        .exitValueNormal()
+                        .timeout(30, TimeUnit.SECONDS)
+                        .execute()
+                        .outputUTF8()
+                        .trim();
+    }
 }
diff --git a/core/src/test/java/org/testcontainers/utility/RegistryAuthLocatorTest.java b/core/src/test/java/org/testcontainers/utility/RegistryAuthLocatorTest.java
@@ -4,23 +4,17 @@
 import com.google.common.io.Resources;
 import org.apache.commons.lang.SystemUtils;
 import org.jetbrains.annotations.NotNull;
-import org.junit.Assume;
-import org.junit.BeforeClass;
 import org.junit.Test;
 
 import java.io.File;
 import java.net.URISyntaxException;
+import java.util.HashMap;
+import java.util.Map;
 
 import static org.rnorth.visibleassertions.VisibleAssertions.assertEquals;
 import static org.rnorth.visibleassertions.VisibleAssertions.assertNull;
 
 public class RegistryAuthLocatorTest {
-
-    @BeforeClass
-    public static void nonWindowsTest() throws Exception {
-        Assume.assumeFalse(SystemUtils.IS_OS_WINDOWS);
-    }
-
     @Test
     public void lookupAuthConfigWithoutCredentials() throws URISyntaxException {
         final RegistryAuthLocator authLocator = createTestAuthLocator("config-empty.json");
@@ -87,10 +81,46 @@ public void lookupNonEmptyAuthWithHelper() throws URISyntaxException {
         assertEquals("Correct password is obtained from a credential helper", "secret", authConfig.getPassword());
     }
 
+    @Test
+    public void lookupAuthConfigWithCredentialsNotFound() throws URISyntaxException {
+        Map<String, String> notFoundMessagesReference = new HashMap<>();
+        final RegistryAuthLocator authLocator = createTestAuthLocator("config-with-store.json", notFoundMessagesReference);
+
+        DockerImageName dockerImageName = new DockerImageName("registry2.example.com/org/repo");
+        final AuthConfig authConfig = authLocator.lookupAuthConfig(dockerImageName, new AuthConfig());
+
+        assertNull("No username should have been obtained from a credential store", authConfig.getUsername());
+        assertNull("No secret should have been obtained from a credential store", authConfig.getPassword());
+        assertEquals("Should have one 'credentials not found' message discovered", 1, notFoundMessagesReference.size());
+
+        String discoveredMessage = notFoundMessagesReference.values().iterator().next();
+
+        assertEquals(
+            "Not correct message discovered",
+            "Fake credentials not found on credentials store 'https://not.a.real.registry/url'",
+            discoveredMessage);
+    }
+
     @NotNull
     private RegistryAuthLocator createTestAuthLocator(String configName) throws URISyntaxException {
-        final File configFile = new File(Resources.getResource("auth-config/" + configName).toURI());
-        return new RegistryAuthLocator(configFile, configFile.getParentFile().getAbsolutePath() + "/");
+        return createTestAuthLocator(configName, new HashMap<>());
     }
 
+    @NotNull
+    private RegistryAuthLocator createTestAuthLocator(String configName, Map<String, String> notFoundMessagesReference) throws URISyntaxException {
+        final File configFile = new File(Resources.getResource("auth-config/" + configName).toURI());
+
+        String commandPathPrefix = configFile.getParentFile().getAbsolutePath() + "/";
+        String commandExtension = "";
+
+        if (SystemUtils.IS_OS_WINDOWS) {
+            commandPathPrefix += "win/";
+
+            // need to provide executable extension otherwise won't run it
+            // with real docker wincredential exe there is no problem
+            commandExtension = ".bat";
+        }
+
+        return new RegistryAuthLocator(configFile, commandPathPrefix, commandExtension, notFoundMessagesReference);
+    }
 }
diff --git a/core/src/test/resources/auth-config/docker-credential-fake b/core/src/test/resources/auth-config/docker-credential-fake
@@ -4,7 +4,16 @@ if [[ $1 != "get" ]]; then
     exit 1
 fi
 
-read > /dev/null
+read inputLine
+
+if [[ $inputLine == "registry2.example.com" ]]; then
+    echo Fake credentials not found on credentials store \'$inputLine\' 1>&2
+    exit 1
+fi
+if [[ $inputLine == "https://not.a.real.registry/url" ]]; then
+    echo Fake credentials not found on credentials store \'$inputLine\' 1>&2
+    exit 1
+fi
 
 echo '{' \
      '  "ServerURL": "url",' \
diff --git a/core/src/test/resources/auth-config/win/docker-credential-fake.bat b/core/src/test/resources/auth-config/win/docker-credential-fake.bat
@@ -0,0 +1,21 @@
+@echo off
+if not "%1" == "get" (
+    exit 1
+)
+
+set /p inputLine=""
+
+if "%inputLine%" == "registry2.example.com" (
+     echo Fake credentials not found on credentials store '%inputLine%' 1>&2
+     exit 1
+)
+if "%inputLine%" == "https://not.a.real.registry/url" (
+     echo Fake credentials not found on credentials store '%inputLine%' 1>&2
+     exit 1
+)
+
+echo {
+echo   "ServerURL": "url",
+echo   "Username": "username",
+echo   "Secret": "secret"
+echo }
