bake: git auth support for remote definitions#2363

crazy-max · 2024-03-27T22:43:10Z

Adds support for Git auth when using remote bake definitions through BUILDX_BAKE_GIT_AUTH_TOKEN and BUILDX_BAKE_GIT_AUTH_HEADER envs var that will set GIT_AUTH_TOKEN and GIT_AUTH_HEADER secrets respectively, matching https://github.com/moby/buildkit/blob/ad5a64c68f20435c52a51db73ef9246d6a3feba6/client/llb/source.go#L270-L271

Also adds BUILDX_BAKE_GIT_SSH to be consistent so user can provide SSH sock paths.

crazy-max · 2024-03-28T09:48:10Z

@tonistiigi While adding tests I discovered a strange behavior with git source in BuildKit when authenticated. It seems most of the requests send the authorization header but not the one when checking the default branch in https://github.com/moby/buildkit/blob/ad5a64c68f20435c52a51db73ef9246d6a3feba6/source/git/source.go#L732 even if header is set in
https://github.com/moby/buildkit/blob/ad5a64c68f20435c52a51db73ef9246d6a3feba6/source/git/source.go#L350 or https://github.com/moby/buildkit/blob/ad5a64c68f20435c52a51db73ef9246d6a3feba6/source/git/source.go#L422.

See https://github.com/docker/buildx/actions/runs/8465151975/job/23191259148#step:7:635

    testutilserve.go:52: Request Method: GET
    testutilserve.go:53: Request URL: /test.git/info/refs?service=git-upload-pack
    testutilserve.go:54: Request Headers:
    testutilserve.go:57:   Cache-Control: no-cache
    testutilserve.go:57:   User-Agent: git/2.43.0
    testutilserve.go:57:   Accept: */*
    testutilserve.go:57:   Accept-Encoding: deflate, gzip, br
    testutilserve.go:57:   Pragma: no-cache
    testutilserve.go:57:   Git-Protocol: version=2
    testutilserve.go:60: access token used: "cptgkeatnyrttvqj1pmrlj8sv"
    testutilserve.go:62: basic auth: user="" pass=""
    bake.go:175: 
        	Error Trace:	/src/tests/bake.go:175
        	            				/src/vendor/github.com/moby/buildkit/util/testutil/integration/run.go:93
        	            				/src/vendor/github.com/moby/buildkit/util/testutil/integration/run.go:207
        	Error:      	Received unexpected error:
        	            	exit status 1
        	Test:       	TestIntegration/TestBakeRemoteAuth/worker=docker-container
        	Messages:   	ERROR: failed to solve: failed to load cache key: error fetching default branch for repository http://127.0.0.1:43081/test.git: git error: exit status 128
        	            	stderr:
        	            	fatal: could not read Username for 'http://127.0.0.1:43081/': terminal prompts disabled

So for now in this test I just check the very first request is authenticated but I think we need a fix on BuildKit side.

Works fine though when testing directly:

$ BUILDX_BAKE_GIT_AUTH_TOKEN=... docker buildx bake https://github.com/docker/test-docker-action.git image --print
#0 building with "default" instance using docker driver

#1 [internal] load git source https://github.com/docker/test-docker-action.git
#1 0.520 ref: refs/heads/master HEAD
#1 0.520 99d5da274d46b04a3936bbb9811880a56b9cc238       HEAD
#1 1.005 99d5da274d46b04a3936bbb9811880a56b9cc238       refs/heads/master
#1 CACHED
{
  "group": {
    "default": {
      "targets": [
        "image"
      ]
    }
  },
  "target": {
    "image": {
      "context": "https://github.com/docker/test-docker-action.git",
      "dockerfile": "./multistage.Dockerfile",
      "platforms": [
        "linux/amd64",
        "linux/arm64"
      ]
    }
  }
}

Edit

seems to be a limitation with ls-remote command that does not provide an option to directly set custom HTTP headers 😞. Maybe instead of using a git command to list refs we could do a simple HTTP request for smart service?

Edit2

after testing with:

$ curl https://github.com/docker/buildx/info/refs?service=git-upload-pack --output -
001e# service=git-upload-pack
000001558abef5908705e49f7ba88ef8c957e1127b597a2a HEADmulti_ack thin-pack side-band side-band-64k ofs-delta shallow deepen-since deepen-not deepen-relative no-progress include-tag multi_ack_detailed allow-tip-sha1-in-want allow-reachable-sha1-in-want no-done symref=HEAD:refs/heads/master filter object-format=sha1 agent=git/github-de75ed0166ec
007eef78ae7bcd872a51be7b271f7fe8696349cd6e4a refs/heads/dependabot/go_modules/github.com/docker/docker-26.0.0-rc3incompatible
003f8abef5908705e49f7ba88ef8c957e1127b597a2a refs/heads/master
003e86bdced7766639d56baa4c7c449a4f6468490f87 refs/heads/v0.10
003e4e547752afe829ccd0a5b5ccfe4901c7cde9f88b refs/heads/v0.11
003e89154c7d33034e912b6775f2016715f07ca4ae96 refs/heads/v0.12
003ed7069985e5cae70826eee1b595b80ac3cb517697 refs/heads/v0.13
...

It seems we could use symref=HEAD:refs/heads/master?

Test on gitlab:

$ curl https://gitlab.com/nvidia/container-images/cuda.git/info/refs?service=git-upload-pack --output -
001e# service=git-upload-pack
00000148f66651a424c86057620809f176f5ee252ac62f60 HEADmulti_ack thin-pack side-band side-band-64k ofs-delta shallow deepen-since deepen-not deepen-relative no-progress include-tag multi_ack_detailed allow-tip-sha1-in-want allow-reachable-sha1-in-want no-done symref=HEAD:refs/heads/master filter object-format=sha1 agent=git/2.43.2
003ff66651a424c86057620809f176f5ee252ac62f60 refs/heads/master
0049d0a86ed3be14c5f97ca398719228de32452ea6e1 refs/merge-requests/13/head
004a32374c14fee5f89e8592c3abb4c6546be77c71d4 refs/merge-requests/13/merge

Works with basic auth as well.

Also we should make sure this is a server supporting Git's smart service by checking Content-Type: application/x-git-upload-pack-advertisement header, othwise fallback to current beahvior with ls-remote command.

bake/remote.go

thaJeztah · 2024-03-28T14:55:02Z

Will these always be for bake only, or could these be for "build" as well? (curious if the BUILDX_BAKE_GIT_AUTH_HEADER env vars must contain _BAKE_)

crazy-max · 2024-03-28T15:58:19Z

Will these always be for bake only, or could these be for "build" as well? (curious if the BUILDX_BAKE_GIT_AUTH_HEADER env vars must contain _BAKE_)

Only bake, this is used to build remote bake definitions. For build we already use secrets to build from a remote Git context.

thaJeztah · 2024-03-28T16:44:40Z

Ah! So.. naive question; if that doesn't require special env-vars; why does bake need them?

thaJeztah · 2024-03-28T16:47:23Z

Re-reading; you mention remote definitions (so equivalent to "remote dockerfile"); I know docker build / docker buildx build also allows building from a remote Dockerfile (Dockerfile could be remote, but build-context could be local) but not sure it currently does from a "git" URL; if that's not supported, then ... perhaps it should as well?

crazy-max · 2024-03-28T16:49:03Z

Ah! So.. naive question; if that doesn't require special env-vars; why does bake need them?

Because a remote definition is not a build context per-say. We can't set secrets directly as this will be used also in the underlying builds from the definition. What @tonistiigi suggested in #2360 (comment) with a builtin target name could work but UX wise this is not great.

crazy-max · 2024-03-28T16:57:56Z

Re-reading; you mention remote definitions (so equivalent to "remote dockerfile"); I know docker build / docker buildx build also allows building from a remote Dockerfile (Dockerfile could be remote, but build-context could be local) but not sure it currently does from a "git" URL; if that's not supported, then ... perhaps it should as well?

Yes indeed I don't think we have that for the Dockerfile atm. For bake we have "builtin" ways to use either a local or remote context, as well as merging local and remote definitions: https://docs.docker.com/build/bake/remote-definition/

@dvdksn Talking about merging local and remote bake definitions, I don't think we have this documented atm #1838

Edit: nevermind https://docs.docker.com/build/bake/remote-definition/#remote-definition-with-the---file-flag

tonistiigi · 2024-04-02T17:22:04Z

bake/remote.go

+		}
+
+		var gitAuthSecrets []*controllerapi.Secret
+		if _, ok := os.LookupEnv("BUILDX_BAKE_GIT_AUTH_TOKEN"); ok {


Probably out of scope, but unclear to me how the controller takes Env keys as input. Doesn't the controller have its own env if it is running in another process?

Hum good point, it seems to work when connecting to local controller. I don't think there is another process except when using --invoke and setting envs through invoke config:

buildx/controller/pb/controller.proto

Line 199 in a7d59ae

repeated string Env = 3;

tonistiigi · 2024-04-02T17:26:42Z

bake/remote.go

-		if err == nil {
-			session = append(session, ssh)
+		var sshSpecs []*controllerapi.SSH
+		if v := os.Getenv("BUILDX_BAKE_GIT_SSH"); v != "" {


ID of this needs to be set with MountSSHSock() if not default. Setting it to other ID would not work otherwise. So probably it should just be the path.

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>

crazy-max added area/bake needs/docs-follow-up labels Mar 27, 2024

crazy-max added this to the v0.14.0 milestone Mar 27, 2024

crazy-max force-pushed the bake-remote-token branch 2 times, most recently from b04c3f0 to 35bbed2 Compare March 28, 2024 07:47

crazy-max requested a review from tonistiigi March 28, 2024 07:47

crazy-max marked this pull request as ready for review March 28, 2024 07:47

crazy-max force-pushed the bake-remote-token branch 6 times, most recently from 23b8fd2 to d7a8f4d Compare March 28, 2024 09:36

crazy-max force-pushed the bake-remote-token branch from d7a8f4d to 1009504 Compare March 28, 2024 09:50

crazy-max mentioned this pull request Mar 28, 2024

bake: handle git auth token when parsing remote definition docker/actions-toolkit#288

Merged

crazy-max commented Mar 28, 2024

View reviewed changes

bake/remote.go Outdated Show resolved Hide resolved

crazy-max mentioned this pull request Mar 28, 2024

client: expose git auth header and token keys moby/buildkit#4809

Merged

crazy-max force-pushed the bake-remote-token branch from 1009504 to a686f8d Compare March 28, 2024 16:46

crazy-max force-pushed the bake-remote-token branch from a686f8d to c7e9dab Compare March 28, 2024 16:52

crazy-max mentioned this pull request Mar 28, 2024

vendor: github.com/moby/buildkit 25bec7145b39 (v0.14.0-dev) #2366

Merged

crazy-max mentioned this pull request Apr 2, 2024

default to git context docker/bake-action#193

Open

crazy-max force-pushed the bake-remote-token branch from c7e9dab to 0da453d Compare April 2, 2024 17:18

tonistiigi reviewed Apr 2, 2024

View reviewed changes

crazy-max mentioned this pull request Apr 4, 2024

tests: build secret #2381

Merged

crazy-max force-pushed the bake-remote-token branch 2 times, most recently from 020ac59 to 8cb85e5 Compare April 4, 2024 12:02

bake: git auth support for remote definitions

4d39259

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>

crazy-max force-pushed the bake-remote-token branch from 8cb85e5 to 4d39259 Compare April 4, 2024 12:12

tonistiigi approved these changes Apr 4, 2024

View reviewed changes

crazy-max merged commit 59cc107 into docker:master Apr 4, 2024
63 checks passed

crazy-max deleted the bake-remote-token branch April 4, 2024 17:37

dvdksn mentioned this pull request Apr 8, 2024

bake: remote auth variables docker/docs#19749

Merged

2 tasks

crazy-max removed the needs/docs-follow-up label Jun 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bake: git auth support for remote definitions#2363

bake: git auth support for remote definitions #2363

crazy-max commented Mar 27, 2024 •
edited

Loading

crazy-max commented Mar 28, 2024 •
edited

Loading

thaJeztah commented Mar 28, 2024

crazy-max commented Mar 28, 2024 •
edited

Loading

thaJeztah commented Mar 28, 2024

thaJeztah commented Mar 28, 2024

crazy-max commented Mar 28, 2024

crazy-max commented Mar 28, 2024 •
edited

Loading

tonistiigi Apr 2, 2024

crazy-max Apr 4, 2024

tonistiigi Apr 2, 2024

bake: git auth support for remote definitions#2363

bake: git auth support for remote definitions #2363

Conversation

crazy-max commented Mar 27, 2024 • edited Loading

crazy-max commented Mar 28, 2024 • edited Loading

Edit

Edit2

thaJeztah commented Mar 28, 2024

crazy-max commented Mar 28, 2024 • edited Loading

thaJeztah commented Mar 28, 2024

thaJeztah commented Mar 28, 2024

crazy-max commented Mar 28, 2024

crazy-max commented Mar 28, 2024 • edited Loading

tonistiigi Apr 2, 2024

Choose a reason for hiding this comment

crazy-max Apr 4, 2024

Choose a reason for hiding this comment

tonistiigi Apr 2, 2024

Choose a reason for hiding this comment

crazy-max commented Mar 27, 2024 •
edited

Loading

crazy-max commented Mar 28, 2024 •
edited

Loading

crazy-max commented Mar 28, 2024 •
edited

Loading

crazy-max commented Mar 28, 2024 •
edited

Loading