feat: also check DOCKER_AUTH_CONFIG for registry auth config as an alternative to config.json#6238
Conversation
…ternative to config.json
|
Can you run |
eddumelendez
left a comment
eddumelendez
left a comment
There was a problem hiding this comment.
LGTM! just left a small suggestion. Thanks @roseo1 !
|
QQ: this is very Gitlab specific, right? I couldn't find anything official in Docker docs |
|
Hi @eddumelendez, sorry for the delay and thank you for the tidy up, found another way round this so forgot to check. Yes, it is very gitlab specific, not sure if that changes anything with regards to the content of the PR? |
|
Thanks for answering! Nothing change. I've just updated the PR with a minor change and since it will break the behavior we are holding it for a bit but indeed planning to merge it. |
|
|
||
| @Test | ||
| public void lookupAuthConfigWithoutCredentials() throws URISyntaxException { | ||
| public void lookupAuthConfigWithoutCredentials() throws URISyntaxException, IOException { |
There was a problem hiding this comment.
Why was IOException added to all the throws clauses of the tests?
There was a problem hiding this comment.
| if (configFile.exists()) { | ||
| log.debug("RegistryAuthLocator reading from configFile: {}", configFile); | ||
| return OBJECT_MAPPER.readTree(configFile); | ||
| } else if (configEnv != null) { |
There was a problem hiding this comment.
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.
good catch! yes, you're right.
There was a problem hiding this comment.
@roseo1 can you take care of this, please? If not, LMK so I can do it before merge it.
|
Thanks for your contribution, @roseo1 ! This has been merged in |
4 participants
Resolves #3782.
Currently testcontainers-java checks docker config json for registry auth credentials. Like testcontainers-dotnet (PR testcontainers#540), would be good to also support the use of the
DOCKER_AUTH_CONFIGenvironment variable.As added to the docs, would respect the current preference:
DOCKER_CONFIGor{HOME}/.docker/config.jsonDOCKER_AUTH_CONFIGPotential impact:
Failure when attempting to lookup auth config. Please ignore if you don't have images in an authenticated registry. Details: (dockerImageName: <...>, configFile: /root/.docker/config.json. Falling back to docker-java default behaviour. Exception message: /root/.docker/config.json (No such file or directory)Failure when attempting to lookup auth config. Please ignore if you don't have images in an authenticated registry. Details: (dockerImageName: <...>, configFile: /root/.docker/config.json, configEnv: DOCKER_AUTH_CONFIG). Falling back to docker-java default behaviour. Exception message: No config supplied. Checked in order: /root/.docker/config.json (file not found), DOCKER_AUTH_CONFIG (not set)