Major Currently DockerRegistryToken makes no attempt to log you out when KeyMaterial.close is called. This makes it inappropriate for environments in which access to the registry credentials must be tightly controlled.
The problem is that ~/.dockercfg must be used to store the login globally for the user (typically, one slave agent), so if there are multiple executors on the slave, one log out while another is still using the login.
If https://github.com/docker/docker/issues/10318 or similar is implemented, that would be ideal, so that the authentication between executors does not clash.
Otherwise, it might be possible to use reference-counting. TBD if docker login/logout would preserve other fields, or if a separate file would be needed. There are potential locking issues there.
- relates to
-
JENKINS-38018 withDockerRegistry fails to authenticate with DockerHub
-
- Resolved
-
[JENKINS-28702] Clean up registry credentials
Joshua Hoblitt
added a comment - I stumbled across this issue when a test job able to push without setting up registry credentials – which is rather horrifying. I understand there are concurrency concerns but could we at least have an option to force cleanup when exiting the `withRegistry` block? I only allow one docker build on a slave at a time so this would cover my present use case.
Joshua Hoblitt
added a comment - I stumbled across this issue when a test job able to push without setting up registry credentials – which is rather horrifying. I understand there are concurrency concerns but could we at least have an option to force cleanup when exiting the `withRegistry` block? I only allow one docker build on a slave at a time so this would cover my present use case.
Docker 1.8 do support DOCKER_CONFIG that plugin should use so it can have distinct config dir per build, and fully delete it after build completion.
Main issue I can see is KeyMaterialFactory usage. API is designed to 1.create 2.contextualize 3.materialize, which would be OK.
But usage actually do run 1+2 together (1), which prevent combining KeyMaterialFactory using KeyMaterialFactory.plus() with a shared context. Changing this would require some incompatible API changes afaik.
(1) https://github.com/jenkinsci/docker-commons-plugin/blob/master/src/main/java/org/jenkinsci/plugins/docker/commons/credentials/DockerServerEndpoint.java#L142