Skip to content
Snippets Groups Projects

Add support for credentials store

Merged Kirill Shirinkin requested to merge (removed):master into master
All threads resolved!

What does this MR do?

With this MR runner will correctly fetch credentials store from credsStore, if it's configured.

Why was this MR needed?

In some environments, specifically in AWS, credentials are provided via credentials store, as documented here: https://docs.docker.com/engine/reference/commandline/login/#credentials-store. Currently this auth method doesn't work with gitlab runner. In case of AWS the only solution would be to re-trigger aws ecr get-login in a cron job every 12 hours to re-fetch credentials. See this Issue for more details: https://gitlab.com/gitlab-org/gitlab-ci-multi-runner/issues/1583

With new implementation, in addition to all previous ways to provide credentials, Gitlab Runner will also use credsStore, in a similar way Docker CLI does it.

Please notice, that this MR still doesn't cover all possible use cases, specifically it doesn't cover usage of "credsHelpers" key of Docker config - I can add it in next MR, if I get more time to work on this.

In any case, even without credsHelper support, this code change simplifies usage of Gitlab Runner combined with AWS ECR or GCE GCR, making authentication simple, transparent and automated. Also see:

What are the relevant issue numbers?

https://gitlab.com/gitlab-org/gitlab-ci-multi-runner/issues/1583

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • @Fodoj

    It is interesting. You are missing tests for this storage.

    @nick.thomas Could you jump-in and help with godep issue?

  • @tmaczukin how close are we to merging https://gitlab.com/gitlab-org/gitlab-ci-multi-runner/merge_requests/505 ? It might be easier to do that then use govendor to add the new dependency here...

  • @Fodoj I purposefully avoided introducing support for these extra auth config methods in https://gitlab.com/gitlab-org/gitlab-ci-multi-runner/merge_requests/301 because it pulled in so many extra packages. It doesn't feel like the docker code is built to allow sensible re-use of a subset of this code.

    Can we produce a full dependency graph? It's possible there's an alternative way to get these auth mechanisms working without pulling in quite so many extra packages.

  • @nick.thomas I can try, just need to get the build running here in CI.. So I guess the fix to godeps isssue is to wait for !505 (merged), and try it with govendor, right?

  • You're missing a changelog entry, and you have a failed pipeline.

    @zj Changelog entries are generated with script from merge requests titles at version release. There is no need to add them in the MR :)

    @nick.thomas @Fodoj I hope I will find time and merge !505 (merged) in next week.

  • @Fodoj !505 (merged) was just merged into master branch :wink:

  • added executordocker ~58728 labels

  • Kirill Shirinkin added 112 commits

    added 112 commits

    Compare with previous version

  • @tmaczukin doesn't look like dependencies got resolved properly even with govendor https://gitlab.com/Fodoj/gitlab-ci-multi-runner/builds/13606851 Any idea why this could happen?

  • @Fodoj Version of github.com/opencontainers/runtime-spec was incompatible with github.com/docker/docker/oci. oci uses specs.Namespace struct which was removed 7 months (!!) ago in this commit https://github.com/opencontainers/runtime-spec/commit/e918daac262554b30644cab48f8a53ea656aa908 (merged here: https://github.com/opencontainers/runtime-spec/commit/a39be468c9cfd7e873889687ce64376ee41e25d3).

    So I've downgraded runtime-spec to 313f40bdfcc04c6b0f7b8a8c3e91a7b7a3a0ef4e which is the last revision containing expected structs.

    After this I found another problem of missing SystemCertPool from github.com/docker/go-connections/tlsconfig so I've updated all three subpackages of github.com/docker/go-connections that we have in our vendor/vendor.json. The bindata job is passing now.

    Let's wait and look if tests will work after changes. Pipeline is here: https://gitlab.com/gitlab-org/gitlab-ci-multi-runner/pipelines/7431231. If it will work, then feel free to cherry-pick this commit into your repository: a9b463b7.

  • Kirill Shirinkin added 147 commits

    added 147 commits

    • b0744932...39e98a3d - 146 commits from branch gitlab-org:master
    • 51f2bcfc - Merge branch 'master' of gitlab.com:gitlab-org/gitlab-ci-multi-runner

    Compare with previous version

  • Kirill Shirinkin added 2 commits

    added 2 commits

    • a9b463b7 - Update dependencies
    • 981837ad - Merge branch 'credentials-store-support' of gitlab.com:gitlab-org/gitlab-ci-multi-runner

    Compare with previous version

  • added 1 commit

    Compare with previous version

  • added 1 commit

    • f5e45b18 - Add accidentally removed windows packages

    Compare with previous version

  • added 1 commit

    Compare with previous version

  • added 1 commit

    Compare with previous version

  • added 1 commit

    Compare with previous version

  • added 1 commit

    Compare with previous version

  • added 1 commit

    Compare with previous version

  • @tmaczukin tests and build finally passed. Sorry it took so long. Can we get this merged or there is something else I need to do?

  • Is there any update on when this can be merged? Will it make it into 9.4?

  • @tmaczukin @nick.thomas Can some update from the project team be provided here? @Fodoj completed this over a month ago and despite a chase from him in this thread there has been no feedback at all as to what will happen with this now. I'm sure I'm not the only one who is very keen to see this released and I'm sure @Fodoj is keen for the work he spent quite some time working on to be released. Thanks.

  • Yeah, at least getting a review would be a-mazing, guys. :)

  • added customer label

  • ZD: https://gitlab.zendesk.com/agent/tickets/80520

    Customer has asked if we can move this MR forward, thanks!

  • @Fodoj I'm sorry for so long delay!

    I've checked changes introduced by this MR and in general it looks good - the change itself is short and clean. But I'm terrified by the number of changes in vendor/ directory and the way that they were added: added by you, added by me, then again added, something removed by mistake, added again... all of this in few different commits so it's hard to check what exactly got changed.

    Could you do the following on your local working copy and check if it still works properly:

    1. Go to the project directory and checkout to the branch where your changes exist.

    2. Execute a following script (I assume that Runner's project exists under $GOPATH/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner - if no please update the script so it will match your environment):

      cd $GOPATH/src/github.com/docker/docker
      git pull
      git checkout v1.13.0
      
      cd $GOPATH/src/github.com/docker/go-connections
      git checkout master
      git pull
      
      cd $GOPATH/src/github.com/opencontainers/runtime-spec/specs-go
      git pull
      git checkout 313f40bdfcc04c6b0f7b8a8c3e91a7b7a3a0ef4e
      
      cd $GOPATH/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner
      git fetch origin
      git reset --hard origin/master
      git cherry-pick b0744932
      make update_govendor_dependencies
      govendor list -no-status | grep -e github.com/docker/docker -e github.com/docker/go-connections -e github.com/opencontainers/runtime-spec/specs-go | xargs govendor update
      git add .
      git commit --amend

    This script will:

    • update all dependencies, that I found as relevant to your change, to a proper versions;
    • reset your branch to current origin/master state and add your credentials store support on top of it;
    • update all dependencies in another commit (last four lines).

    Having this please compile it and test if it's working. After doing this on my computer I can say that at least unit tests are passing, but I have no Credentials Store configured to use with Docker and you will probably know better what to check.

    If this will work, then please force push the changes and we can merge this! :)

  • Tomasz Maczukin changed milestone to %9.5

    changed milestone to %9.5

  • Kirill Shirinkin added 132 commits

    added 132 commits

    Compare with previous version

  • Run your script and tested newly compiled runner.

    Test .gitlab-ci.yml tries to pull image from my private AWS ECR registry:

    image: 236110368157.dkr.ecr.eu-central-1.amazonaws.com/fodoj:latest
    
    test:
      script:
        - echo 'hola'
    

    Before

    Running with gitlab-ci-multi-runner 9.3.0 (3df822b)
      on  ()
    Using Docker executor with image 236110368157.dkr.ecr.eu-central-1.amazonaws.com/fodoj:latest ...
    Using docker image sha256:b05cc44af8d9b956f5c5058d864bcfbd299f26797c8de3aa7f3c56626166c2b2 for predefined container...
    Pulling docker image 236110368157.dkr.ecr.eu-central-1.amazonaws.com/fodoj:latest ...
    ERROR: Preparation failed: Error: No such image: 236110368157.dkr.ecr.eu-central-1.amazonaws.com/fodoj:latest
    Will be retried in 3s ...
    Using Docker executor with image 236110368157.dkr.ecr.eu-central-1.amazonaws.com/fodoj:latest ...
    Using docker image sha256:b05cc44af8d9b956f5c5058d864bcfbd299f26797c8de3aa7f3c56626166c2b2 for predefined container...
    Pulling docker image 236110368157.dkr.ecr.eu-central-1.amazonaws.com/fodoj:latest ...
    ERROR: Preparation failed: Error: No such image: 236110368157.dkr.ecr.eu-central-1.amazonaws.com/fodoj:latest
    Will be retried in 3s ...

    It can not pull the image.

    After

    Now trying to run with my changes:

    Using Docker executor with image 236110368157.dkr.ecr.eu-central-1.amazonaws.com/fodoj:latest ...
    Using docker image sha256:2226d46f75a7343d4fd5739c091a1268bd6c4751948668942073a46ea21a181f for predefined container...
    Pulling docker image 236110368157.dkr.ecr.eu-central-1.amazonaws.com/fodoj:latest ...
    Using docker image 236110368157.dkr.ecr.eu-central-1.amazonaws.com/fodoj:latest ID=sha256:97e7e1844a4da3fa487a417948c6fe23ae1796af4b971e57384d82e31f1d0e1f for build container...
    Running on runner--project-0-concurrent-0 via localhost.localdomain...

    It successfully pulled image, life is good.

    Hope we can merge it now, after tests in CI pass as well :)

  • Kirill Shirinkin resolved all discussions

    resolved all discussions

  • @Fodoj Looks good, let's wait for Pipeline to finish.

    I'm curious - what dependencies you still needed to change?

  • @tmaczukin I didn't change anything extra, just run your script and that's it. It seems like windows build failed due to missing dependency. I am not that familiar with windows, do you know how to resolve this issue?

  • @Fodoj I've updated the steps:

    cd $GOPATH/src/github.com/docker/docker
    git pull
    git checkout v1.13.0
    
    cd $GOPATH/src/github.com/docker/go-connections
    git checkout master
    git pull
    
    cd $GOPATH/src/github.com/opencontainers/runtime-spec/specs-go
    git pull
    git checkout 313f40bdfcc04c6b0f7b8a8c3e91a7b7a3a0ef4e
    
    cd $GOPATH/src/golang.org/x/sys
    git pull
    git checkout 042a8f53ce82bbe081222da955159491e32146a0
    
    cd $GOPATH/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner
    git fetch origin
    git reset --hard origin/master
    git cherry-pick b0744932
    make update_govendor_dependencies
    govendor list -no-status | grep -e github.com/docker/docker -e github.com/docker/go-connections -e github.com/opencontainers/runtime-spec/specs-go -e golang.org/x/sys | xargs govendor update
    git add .
    git commit --amend

    Let's wait for https://gitlab.com/gitlab-org/gitlab-ci-multi-runner/pipelines/10670734 to be finished and if all jobs will be green, then you can update your MR and repeat the tests :)

  • Kirill Shirinkin added 14 commits

    added 14 commits

    Compare with previous version

  • Pushed. Let's see if will pass. :)

  • @tmaczukin yes, it still works, pulls image from AWS ECR :)

  • OK. Tests are passing, change itself is short and simple. number of dependencies changed for this one update is scaring me, but let's merge this. We will release RC.1 soon so we can check how it works generally.

    @Fodoj Thanks for your work! :)

  • Tomasz Maczukin approved this merge request

    approved this merge request

  • Tomasz Maczukin mentioned in commit 2b9142c0

    mentioned in commit 2b9142c0

  • Does this mean that we can have a runner's host logged into, for example, DockerHub, which will allow pushing to their registry from a pipeline?

    Right now the lack of long-running tokens like deploy keys, etc., at Docker Hub is making for some pretty exotic solutions just to have a pipeline that can handle pushing an image to the public (assuming of course one doesn't want to either store their user/pass as a secret variable and create a CI user for each project at DockerHub ;-)).

  • mentioned in issue #3469 (closed)

  • Neil Jarrett mentioned in issue #4202

    mentioned in issue #4202

  • Andrei Burd mentioned in merge request !1386 (merged)

    mentioned in merge request !1386 (merged)

  • Please register or sign in to reply
    Loading