Details

    • Similar Issues:

      Description

      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.

        Attachments

          Issue Links

            Activity

            Hide
            ndeloof Nicolas De Loof added a comment -

            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

            Show
            ndeloof Nicolas De Loof added a comment - 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
            Hide
            jglick Jesse Glick added a comment -

            Cf. JENKINS-38018 regarding use of docker logout.

            Show
            jglick Jesse Glick added a comment - Cf. JENKINS-38018 regarding use of docker logout .
            Hide
            jhoblitt 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.

            Show
            jhoblitt 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.
            Hide
            jglick Jesse Glick added a comment -

            Fixed along with JENKINS-38018.

            Show
            jglick Jesse Glick added a comment - Fixed along with  JENKINS-38018 .

              People

              • Assignee:
                jglick Jesse Glick
                Reporter:
                jglick Jesse Glick
              • Votes:
                4 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: