-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: also check DOCKER_AUTH_CONFIG for registry auth config as an alternative to config.json #6238
Merged
eddumelendez
merged 9 commits into
testcontainers:main
from
roseo1:docker_registry_auth
Feb 18, 2023
Merged
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
a91320b
feat: also check DOCKER_AUTH_CONFIG for registry auth config as an al…
de9fec6
Merge branch 'main' into docker_registry_auth
roseo1 9540fcc
Merge branch 'main' into docker_registry_auth
roseo1 b128785
fix formatting
d2e8927
Polish
eddumelendez 94b0f30
Merge branch 'main' into docker_registry_auth
roseo1 9d7a70f
Merge branch 'main' into docker_registry_auth
eddumelendez 1d6949b
Fix order: read env var first then config file
eddumelendez becfe9a
Fix docs
eddumelendez File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,12 +2,16 @@ | |
|
||
import com.github.dockerjava.api.model.AuthConfig; | ||
import com.google.common.io.Resources; | ||
import org.apache.commons.io.FileUtils; | ||
import org.apache.commons.lang3.SystemUtils; | ||
import org.jetbrains.annotations.NotNull; | ||
import org.junit.Test; | ||
|
||
import java.io.File; | ||
import java.io.IOException; | ||
import java.net.URI; | ||
import java.net.URISyntaxException; | ||
import java.nio.charset.StandardCharsets; | ||
import java.util.HashMap; | ||
import java.util.Map; | ||
|
||
|
@@ -16,7 +20,7 @@ | |
public class RegistryAuthLocatorTest { | ||
|
||
@Test | ||
public void lookupAuthConfigWithoutCredentials() throws URISyntaxException { | ||
public void lookupAuthConfigWithoutCredentials() throws URISyntaxException, IOException { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why was There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
final RegistryAuthLocator authLocator = createTestAuthLocator("config-empty.json"); | ||
|
||
final AuthConfig authConfig = authLocator.lookupAuthConfig( | ||
|
@@ -32,7 +36,7 @@ public void lookupAuthConfigWithoutCredentials() throws URISyntaxException { | |
} | ||
|
||
@Test | ||
public void lookupAuthConfigWithBasicAuthCredentials() throws URISyntaxException { | ||
public void lookupAuthConfigWithBasicAuthCredentials() throws URISyntaxException, IOException { | ||
final RegistryAuthLocator authLocator = createTestAuthLocator("config-basic-auth.json"); | ||
|
||
final AuthConfig authConfig = authLocator.lookupAuthConfig( | ||
|
@@ -48,7 +52,7 @@ public void lookupAuthConfigWithBasicAuthCredentials() throws URISyntaxException | |
} | ||
|
||
@Test | ||
public void lookupAuthConfigWithJsonKeyCredentials() throws URISyntaxException { | ||
public void lookupAuthConfigWithJsonKeyCredentials() throws URISyntaxException, IOException { | ||
final RegistryAuthLocator authLocator = createTestAuthLocator("config-with-json-key.json"); | ||
|
||
final AuthConfig authConfig = authLocator.lookupAuthConfig( | ||
|
@@ -64,7 +68,7 @@ public void lookupAuthConfigWithJsonKeyCredentials() throws URISyntaxException { | |
} | ||
|
||
@Test | ||
public void lookupAuthConfigUsingStore() throws URISyntaxException { | ||
public void lookupAuthConfigUsingStore() throws URISyntaxException, IOException { | ||
final RegistryAuthLocator authLocator = createTestAuthLocator("config-with-store.json"); | ||
|
||
final AuthConfig authConfig = authLocator.lookupAuthConfig( | ||
|
@@ -84,7 +88,7 @@ public void lookupAuthConfigUsingStore() throws URISyntaxException { | |
} | ||
|
||
@Test | ||
public void lookupAuthConfigUsingHelper() throws URISyntaxException { | ||
public void lookupAuthConfigUsingHelper() throws URISyntaxException, IOException { | ||
final RegistryAuthLocator authLocator = createTestAuthLocator("config-with-helper.json"); | ||
|
||
final AuthConfig authConfig = authLocator.lookupAuthConfig( | ||
|
@@ -104,7 +108,7 @@ public void lookupAuthConfigUsingHelper() throws URISyntaxException { | |
} | ||
|
||
@Test | ||
public void lookupAuthConfigUsingHelperWithToken() throws URISyntaxException { | ||
public void lookupAuthConfigUsingHelperWithToken() throws URISyntaxException, IOException { | ||
final RegistryAuthLocator authLocator = createTestAuthLocator("config-with-helper-using-token.json"); | ||
|
||
final AuthConfig authConfig = authLocator.lookupAuthConfig( | ||
|
@@ -118,7 +122,7 @@ public void lookupAuthConfigUsingHelperWithToken() throws URISyntaxException { | |
} | ||
|
||
@Test | ||
public void lookupUsingHelperEmptyAuth() throws URISyntaxException { | ||
public void lookupUsingHelperEmptyAuth() throws URISyntaxException, IOException { | ||
final RegistryAuthLocator authLocator = createTestAuthLocator("config-empty-auth-with-helper.json"); | ||
|
||
final AuthConfig authConfig = authLocator.lookupAuthConfig( | ||
|
@@ -138,7 +142,7 @@ public void lookupUsingHelperEmptyAuth() throws URISyntaxException { | |
} | ||
|
||
@Test | ||
public void lookupNonEmptyAuthWithHelper() throws URISyntaxException { | ||
public void lookupNonEmptyAuthWithHelper() throws URISyntaxException, IOException { | ||
final RegistryAuthLocator authLocator = createTestAuthLocator("config-existing-auth-with-helper.json"); | ||
|
||
final AuthConfig authConfig = authLocator.lookupAuthConfig( | ||
|
@@ -158,7 +162,7 @@ public void lookupNonEmptyAuthWithHelper() throws URISyntaxException { | |
} | ||
|
||
@Test | ||
public void lookupAuthConfigWithCredentialsNotFound() throws URISyntaxException { | ||
public void lookupAuthConfigWithCredentialsNotFound() throws URISyntaxException, IOException { | ||
Map<String, String> notFoundMessagesReference = new HashMap<>(); | ||
final RegistryAuthLocator authLocator = createTestAuthLocator( | ||
"config-with-store.json", | ||
|
@@ -184,7 +188,7 @@ public void lookupAuthConfigWithCredentialsNotFound() throws URISyntaxException | |
} | ||
|
||
@Test | ||
public void lookupAuthConfigWithCredStoreEmpty() throws URISyntaxException { | ||
public void lookupAuthConfigWithCredStoreEmpty() throws URISyntaxException, IOException { | ||
final RegistryAuthLocator authLocator = createTestAuthLocator("config-with-store-empty.json"); | ||
|
||
DockerImageName dockerImageName = DockerImageName.parse("registry2.example.com/org/repo"); | ||
|
@@ -193,18 +197,106 @@ public void lookupAuthConfigWithCredStoreEmpty() throws URISyntaxException { | |
assertThat(authConfig.getAuth()).as("CredStore field will be ignored, because value is blank").isNull(); | ||
} | ||
|
||
@Test | ||
public void lookupAuthConfigFromEnvVarWithCredStoreEmpty() throws URISyntaxException, IOException { | ||
final RegistryAuthLocator authLocator = createTestAuthLocator(null, "config-with-store-empty.json"); | ||
|
||
DockerImageName dockerImageName = DockerImageName.parse("registry2.example.com/org/repo"); | ||
final AuthConfig authConfig = authLocator.lookupAuthConfig(dockerImageName, new AuthConfig()); | ||
|
||
assertThat(authConfig.getAuth()).as("CredStore field will be ignored, because value is blank").isNull(); | ||
} | ||
|
||
@Test | ||
public void lookupAuthConfigWithoutConfigFile() throws URISyntaxException, IOException { | ||
final RegistryAuthLocator authLocator = createTestAuthLocator(null); | ||
|
||
final AuthConfig authConfig = authLocator.lookupAuthConfig( | ||
DockerImageName.parse("unauthenticated.registry.org/org/repo"), | ||
new AuthConfig() | ||
); | ||
|
||
assertThat(authConfig.getRegistryAddress()) | ||
.as("Default docker registry URL is set on auth config") | ||
.isEqualTo("https://index.docker.io/v1/"); | ||
assertThat(authConfig.getUsername()).as("No username is set").isNull(); | ||
assertThat(authConfig.getPassword()).as("No password is set").isNull(); | ||
} | ||
|
||
@Test | ||
public void lookupAuthConfigRespectsCheckOrderPreference() throws URISyntaxException, IOException { | ||
final RegistryAuthLocator authLocator = createTestAuthLocator("config-basic-auth.json", "config-empty.json"); | ||
|
||
final AuthConfig authConfig = authLocator.lookupAuthConfig( | ||
DockerImageName.parse("registry.example.com/org/repo"), | ||
new AuthConfig() | ||
); | ||
|
||
assertThat(authConfig.getRegistryAddress()) | ||
.as("Default docker registry URL is set on auth config") | ||
.isEqualTo("https://registry.example.com"); | ||
assertThat(authConfig.getUsername()).as("Username is set").isEqualTo("user"); | ||
assertThat(authConfig.getPassword()).as("Password is set").isEqualTo("pass"); | ||
} | ||
|
||
@Test | ||
public void lookupAuthConfigFromEnvironmentVariable() throws URISyntaxException, IOException { | ||
final RegistryAuthLocator authLocator = createTestAuthLocator(null, "config-basic-auth.json"); | ||
|
||
final AuthConfig authConfig = authLocator.lookupAuthConfig( | ||
DockerImageName.parse("registry.example.com/org/repo"), | ||
new AuthConfig() | ||
); | ||
|
||
assertThat(authConfig.getRegistryAddress()) | ||
.as("Default docker registry URL is set on auth config") | ||
.isEqualTo("https://registry.example.com"); | ||
assertThat(authConfig.getUsername()).as("Username is set").isEqualTo("user"); | ||
assertThat(authConfig.getPassword()).as("Password is set").isEqualTo("pass"); | ||
} | ||
|
||
@NotNull | ||
private RegistryAuthLocator createTestAuthLocator(String configName, String envConfigName) | ||
throws URISyntaxException, IOException { | ||
return createTestAuthLocator(configName, envConfigName, new HashMap<>()); | ||
} | ||
|
||
@NotNull | ||
private RegistryAuthLocator createTestAuthLocator(String configName) throws URISyntaxException { | ||
return createTestAuthLocator(configName, new HashMap<>()); | ||
private RegistryAuthLocator createTestAuthLocator(String configName) throws URISyntaxException, IOException { | ||
return createTestAuthLocator(configName, null, 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()); | ||
throws URISyntaxException, IOException { | ||
return createTestAuthLocator(configName, null, notFoundMessagesReference); | ||
} | ||
|
||
String commandPathPrefix = configFile.getParentFile().getAbsolutePath() + "/"; | ||
@NotNull | ||
private RegistryAuthLocator createTestAuthLocator( | ||
String configName, | ||
String envConfigName, | ||
Map<String, String> notFoundMessagesReference | ||
) throws URISyntaxException, IOException { | ||
File configFile = null; | ||
String commandPathPrefix = ""; | ||
String commandExtension = ""; | ||
String configEnv = null; | ||
|
||
if (configName != null) { | ||
configFile = new File(Resources.getResource("auth-config/" + configName).toURI()); | ||
|
||
commandPathPrefix = configFile.getParentFile().getAbsolutePath() + "/"; | ||
} else { | ||
configFile = new File(new URI("file:///not-exists.json")); | ||
} | ||
|
||
if (envConfigName != null) { | ||
final File envConfigFile = new File(Resources.getResource("auth-config/" + envConfigName).toURI()); | ||
configEnv = FileUtils.readFileToString(envConfigFile, StandardCharsets.UTF_8); | ||
|
||
commandPathPrefix = envConfigFile.getParentFile().getAbsolutePath() + "/"; | ||
} | ||
|
||
if (SystemUtils.IS_OS_WINDOWS) { | ||
commandPathPrefix += "win/"; | ||
|
@@ -214,6 +306,12 @@ private RegistryAuthLocator createTestAuthLocator(String configName, Map<String, | |
commandExtension = ".bat"; | ||
} | ||
|
||
return new RegistryAuthLocator(configFile, commandPathPrefix, commandExtension, notFoundMessagesReference); | ||
return new RegistryAuthLocator( | ||
configFile, | ||
configEnv, | ||
commandPathPrefix, | ||
commandExtension, | ||
notFoundMessagesReference | ||
); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the env var be evaluated first? Usually, you can use env vars to override config files but this would not be the case here (if you have both the conf file and the env var). Maybe I'm missing something here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch! yes, you're right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@roseo1 can you take care of this, please? If not, LMK so I can do it before merge it.