From 808e09f12bfdccf0e3ff5d5616e3a9b44aee5b29 Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Sun, 12 Jul 2026 10:44:28 +0200 Subject: [PATCH] dockerfile: treat ADD source as git URL when --keep-git-dir is set Signed-off-by: Pierre Fenoll --- frontend/dockerfile/dfgitutil/git_ref.go | 9 +-- frontend/dockerfile/dfgitutil/git_ref_test.go | 53 ++++++++++++++++- .../dockerfile/dockerfile2llb/convert_copy.go | 17 +++--- .../dockerfile2llb/convert_copy_test.go | 58 +++++++++++++++++++ frontend/dockerfile/dockerfile2llb/epoch.go | 7 ++- frontend/dockerfile/dockerfile_addgit_test.go | 34 +++++++++++ frontend/dockerfile/docs/reference.md | 11 ++++ frontend/dockerui/context.go | 2 +- 8 files changed, 176 insertions(+), 15 deletions(-) create mode 100644 frontend/dockerfile/dockerfile2llb/convert_copy_test.go diff --git a/frontend/dockerfile/dfgitutil/git_ref.go b/frontend/dockerfile/dfgitutil/git_ref.go index 6d08293d0145..4a9e298c134f 100644 --- a/frontend/dockerfile/dfgitutil/git_ref.go +++ b/frontend/dockerfile/dfgitutil/git_ref.go @@ -68,7 +68,9 @@ type GitRef struct { } // ParseGitRef parses a git ref. -func ParseGitRef(ref string) (*GitRef, bool, error) { +// An HTTP(S) URL is a valid git ref when it has the ".git" suffix, +// or when `knownGit` is true (such as when `--keep-git-dir` is passed). +func ParseGitRef(ref string, knownGit bool) (*GitRef, bool, error) { res := &GitRef{} var ( @@ -104,9 +106,8 @@ func ParseGitRef(ref string) (*GitRef, bool, error) { } switch remote.Scheme { - // An HTTP(S) URL is considered to be a valid git ref only when it has the ".git[...]" suffix. case gitutil.HTTPProtocol, gitutil.HTTPSProtocol: - if !strings.HasSuffix(remote.Path, ".git") { + if !knownGit && !strings.HasSuffix(remote.Path, ".git") { return nil, false, errors.WithStack(cerrdefs.ErrInvalidArgument) } } @@ -244,7 +245,7 @@ func (gf *GitRef) loadQuery(query url.Values) error { // FragmentFormat returns a simplified git URL in fragment format. // If the URL cannot be parsed, the original string is returned with false. func FragmentFormat(remote string, withSubdir bool) (string, bool) { - gitRef, _, err := ParseGitRef(remote) + gitRef, _, err := ParseGitRef(remote, false) if err != nil || gitRef == nil { return remote, false } diff --git a/frontend/dockerfile/dfgitutil/git_ref_test.go b/frontend/dockerfile/dfgitutil/git_ref_test.go index 138e62ab6bfe..2eca4957fe08 100644 --- a/frontend/dockerfile/dfgitutil/git_ref_test.go +++ b/frontend/dockerfile/dfgitutil/git_ref_test.go @@ -10,6 +10,7 @@ import ( func TestParseGitRef(t *testing.T) { cases := []struct { ref string + knownGit bool expected *GitRef err string }{ @@ -268,10 +269,60 @@ func TestParseGitRef(t *testing.T) { ref: "https://github.com/moby/buildkit.git?invalid=123", err: "unexpected query \"invalid\"", }, + { + // HTTP(S) URLs do not need the ".git" suffix when the ref is known to be a git URL, + // e.g., for git forges that do not support the suffix, such as sourcehut. + ref: "https://git.sr.ht/~foo/bar", + knownGit: true, + expected: &GitRef{ + Remote: "https://git.sr.ht/~foo/bar", + ShortName: "bar", + }, + }, + { + ref: "https://git.sr.ht/~foo/bar#main", + knownGit: true, + expected: &GitRef{ + Remote: "https://git.sr.ht/~foo/bar", + ShortName: "bar", + Ref: "main", + }, + }, + { + ref: "https://github.com/moby/buildkit", + knownGit: true, + expected: &GitRef{ + Remote: "https://github.com/moby/buildkit", + ShortName: "buildkit", + }, + }, + { + ref: "https://github.com/moby/buildkit.git", + knownGit: true, + expected: &GitRef{ + Remote: "https://github.com/moby/buildkit.git", + ShortName: "buildkit", + }, + }, + { + ref: "http://example.com/foo?tag=v1.0.0", + knownGit: true, + expected: &GitRef{ + Remote: "http://example.com/foo", + ShortName: "foo", + Ref: "refs/tags/v1.0.0", + UnencryptedTCP: true, + }, + }, + { + ref: "./.git", + knownGit: true, + expected: nil, + }, } for i, tt := range cases { t.Run(fmt.Sprintf("case%d", i+1), func(t *testing.T) { - got, _, err := ParseGitRef(tt.ref) + got, _, err := ParseGitRef(tt.ref, tt.knownGit) if tt.expected == nil { require.Nil(t, got) require.Error(t, err) diff --git a/frontend/dockerfile/dockerfile2llb/convert_copy.go b/frontend/dockerfile/dockerfile2llb/convert_copy.go index 75d5913d347c..dacf2b26550a 100644 --- a/frontend/dockerfile/dockerfile2llb/convert_copy.go +++ b/frontend/dockerfile/dockerfile2llb/convert_copy.go @@ -84,6 +84,9 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { } } + // An explicit --keep-git-dir flag identifies the source as a git URL + knownGit := cfg.keepGitDir != nil + if cfg.checksum != "" { if !cfg.isAddCommand { return errors.New("checksum can't be specified for COPY") @@ -91,7 +94,7 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { if len(cfg.params.SourcePaths) != 1 { return errors.New("checksum can't be specified for multiple sources") } - if !isHTTPSource(cfg.params.SourcePaths[0]) && !isGitSource(cfg.params.SourcePaths[0]) { + if src := cfg.params.SourcePaths[0]; !isHTTPSource(src, knownGit) && !isGitSource(src, knownGit) { return errors.New("checksum requires HTTP(S) or Git sources") } } @@ -126,7 +129,7 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { for _, src := range cfg.params.SourcePaths { commitMessage.WriteString(" " + src) - gitRef, isGit, gitRefErr := dfgitutil.ParseGitRef(src) + gitRef, isGit, gitRefErr := dfgitutil.ParseGitRef(src, knownGit) if gitRefErr != nil && isGit { return gitRefErr } @@ -181,7 +184,7 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { } else { a = a.Copy(st, "/", dest, opts...) } - } else if isHTTPSource(src) { + } else if isHTTPSource(src, knownGit) { if !cfg.isAddCommand { return errors.New("source can't be a URL for COPY") } @@ -351,16 +354,16 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { return commitToHistory(&d.image, commitMessage.String(), true, &d.state, d.epoch) } -func isHTTPSource(src string) bool { +func isHTTPSource(src string, knownGit bool) bool { if !strings.HasPrefix(src, "http://") && !strings.HasPrefix(src, "https://") { return false } - return !isGitSource(src) + return !isGitSource(src, knownGit) } -func isGitSource(src string) bool { +func isGitSource(src string, knownGit bool) bool { // https://github.com/ORG/REPO.git is a git source, not an http source - if gitRef, isGit, _ := dfgitutil.ParseGitRef(src); gitRef != nil && isGit { + if gitRef, isGit, _ := dfgitutil.ParseGitRef(src, knownGit); gitRef != nil && isGit { return true } return false diff --git a/frontend/dockerfile/dockerfile2llb/convert_copy_test.go b/frontend/dockerfile/dockerfile2llb/convert_copy_test.go new file mode 100644 index 000000000000..71f5ddd09ee0 --- /dev/null +++ b/frontend/dockerfile/dockerfile2llb/convert_copy_test.go @@ -0,0 +1,58 @@ +package dockerfile2llb + +import ( + "testing" + + "github.com/moby/buildkit/solver/pb" + "github.com/moby/buildkit/util/appcontext" + "github.com/stretchr/testify/require" +) + +func sourceIdentifiers(t *testing.T, df string) []string { + t.Helper() + res, err := Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) + require.NoError(t, err) + def, err := res.State.Marshal(appcontext.Context()) + require.NoError(t, err) + var ids []string + for _, dt := range def.Def { + var op pb.Op + require.NoError(t, op.Unmarshal(dt)) + if src := op.GetSource(); src != nil { + ids = append(ids, src.Identifier) + } + } + return ids +} + +func TestAddKeepGitDirNonSuffixedURL(t *testing.T) { + t.Parallel() + // with --keep-git-dir, a non-".git" URL must become a git source + ids := sourceIdentifiers(t, ` +FROM scratch +ADD --keep-git-dir=true https://git.sr.ht/~foo/bar#main /dst +`) + require.Contains(t, ids, "git://git.sr.ht/~foo/bar#main") + require.Len(t, ids, 1) + + // same with --keep-git-dir=false + ids = sourceIdentifiers(t, ` +FROM scratch +ADD --keep-git-dir=false https://git.sr.ht/~foo/bar#main /dst +`) + require.Contains(t, ids, "git://git.sr.ht/~foo/bar#main") + + // without the flag, a non-".git" URL stays an HTTP source + ids = sourceIdentifiers(t, ` +FROM scratch +ADD https://git.sr.ht/~foo/bar /dst +`) + require.Contains(t, ids, "https://git.sr.ht/~foo/bar") + + // without the flag, a ".git" URL is still a git source + ids = sourceIdentifiers(t, ` +FROM scratch +ADD https://github.com/moby/buildkit.git#master /dst +`) + require.Contains(t, ids, "git://github.com/moby/buildkit.git#master") +} diff --git a/frontend/dockerfile/dockerfile2llb/epoch.go b/frontend/dockerfile/dockerfile2llb/epoch.go index 03e7d667d310..41b37a740722 100644 --- a/frontend/dockerfile/dockerfile2llb/epoch.go +++ b/frontend/dockerfile/dockerfile2llb/epoch.go @@ -177,7 +177,10 @@ func sourceDateEpochAddSource(cmd *instructions.AddCommand, env *llb.EnvList, sh return nil, err } - if isHTTPSource(src) { + // An explicit --keep-git-dir flag identifies the source as a git URL + knownGit := cmd.KeepGitDir != nil + + if cmd.KeepGitDir == nil && isHTTPSource(src, knownGit) { var checksum digest.Digest if cmd.Checksum != "" { expandedChecksum, _, err := shlex.ProcessWord(cmd.Checksum, env) @@ -193,7 +196,7 @@ func sourceDateEpochAddSource(cmd *instructions.AddCommand, env *llb.EnvList, sh return &st, nil } - gitRef, isGit, gitRefErr := dfgitutil.ParseGitRef(src) + gitRef, isGit, gitRefErr := dfgitutil.ParseGitRef(src, knownGit) if gitRefErr != nil && isGit { return nil, gitRefErr } diff --git a/frontend/dockerfile/dockerfile_addgit_test.go b/frontend/dockerfile/dockerfile_addgit_test.go index 26815ca968f8..32ecb01f3e6d 100644 --- a/frontend/dockerfile/dockerfile_addgit_test.go +++ b/frontend/dockerfile/dockerfile_addgit_test.go @@ -78,6 +78,7 @@ func testAddGit(t *testing.T, sb integration.Sandbox, format string) { gitCommands = append(gitCommands, makeCommit("v0.0.2")...) gitCommands = append(gitCommands, makeCommit("v0.0.3")...) gitCommands = append(gitCommands, "git update-server-info") + gitCommands = append(gitCommands, "ln -s .git/ bla") err = runShell(gitDir, gitCommands...) require.NoError(t, err) @@ -220,6 +221,39 @@ RUN [ ! -d /nogitdir/.git ] }, nil) require.NoError(t, err) + // Additional test: ADD from a git URL without the ".git" suffix. + // --keep-git-dir flag marks the URL as a git URL regardless of the suffix. + dockerfileNoSuffix, err := applyTemplate(` + FROM alpine + ARG REPO="{{.ServerURL}}/bla" + ADD ${REPO} /nogitanything + RUN [ ! -f /nogitanything/foo ] + RUN [ ! -d /nogitanything/.git ] + ADD --keep-git-dir=true ${REPO}#v0.0.2 /gitdir + RUN [ -f /gitdir/foo ] + RUN [ "$(cat /gitdir/foo)" = "foo of v0.0.2" ] + RUN [ -d /gitdir/.git ] + ADD --keep-git-dir=false ${REPO}#v0.0.3 /nogitdir + RUN [ -f /nogitdir/foo ] + RUN [ "$(cat /nogitdir/foo)" = "foo of v0.0.3" ] + RUN [ ! -d /nogitdir/.git ] + `, map[string]string{ + "ServerURL": serverURL, + }) + require.NoError(t, err) + + dirNoSuffix := integration.Tmpdir(t, + fstest.CreateFile("Dockerfile", []byte(dockerfileNoSuffix), 0600), + ) + + _, err = f.Solve(sb.Context(), c, client.SolveOpt{ + LocalMounts: map[string]fsutil.FS{ + dockerui.DefaultLocalNameDockerfile: dirNoSuffix, + dockerui.DefaultLocalNameContext: dirNoSuffix, + }, + }, nil) + require.NoError(t, err) + // checksum does not match dockerfile5, err := applyTemplate(` FROM alpine diff --git a/frontend/dockerfile/docs/reference.md b/frontend/dockerfile/docs/reference.md index 5a366a1b90e9..e0311db8fac2 100644 --- a/frontend/dockerfile/docs/reference.md +++ b/frontend/dockerfile/docs/reference.md @@ -1601,6 +1601,17 @@ FROM alpine ADD --keep-git-dir=true https://github.com/moby/buildkit.git#v0.10.1 /buildkit ``` +Passing the `--keep-git-dir` flag explicitly (whether `true` or `false`) also +marks `` as the URL of a Git repository, even when the URL doesn't end +with the `.git` suffix. This is useful for Git forges such as Azure DevOps +or SourceHut that don't support the `.git` suffix in clone URLs. + +```dockerfile +# syntax=docker/dockerfile:1 +FROM alpine +ADD --keep-git-dir=false https://git.sr.ht/~foo/bar#main /bar +``` + ### ADD --checksum ```dockerfile diff --git a/frontend/dockerui/context.go b/frontend/dockerui/context.go index 6fb3ebd1be60..3985452df7bc 100644 --- a/frontend/dockerui/context.go +++ b/frontend/dockerui/context.go @@ -291,7 +291,7 @@ func sourceOpFromState(ctx context.Context, st *llb.State, opts ...llb.Constrain } func DetectGitContext(ref string, keepGit *bool, opts ...llb.GitOption) (*llb.State, bool, error) { - g, isGit, err := dfgitutil.ParseGitRef(ref) + g, isGit, err := dfgitutil.ParseGitRef(ref, keepGit != nil) if err != nil { return nil, isGit, err }