diff --git a/client/llb/source.go b/client/llb/source.go index 443a0cc6bcaf..06c27ec01f22 100644 --- a/client/llb/source.go +++ b/client/llb/source.go @@ -322,6 +322,15 @@ func Git(url, ref string, opts ...GitOption) State { addCap(&gi.Constraints, pb.CapSourceGitMountSSHSock) } + commitHash := gi.CommitHash + if commitHash == "" && remote != nil && remote.Fragment != nil { + commitHash = remote.Fragment.CommitHash + } + if commitHash != "" { + attrs[pb.AttrCommitHash] = commitHash + addCap(&gi.Constraints, pb.CapSourceGitCommitHash) + } + addCap(&gi.Constraints, pb.CapSourceGit) source := NewSource("git://"+id, attrs, gi.Constraints) @@ -345,6 +354,7 @@ type GitInfo struct { addAuthCap bool KnownSSHHosts string MountSSHSock string + CommitHash string } func KeepGitDir() GitOption { @@ -373,6 +383,12 @@ func MountSSHSock(sshID string) GitOption { }) } +func CommitHash(v string) GitOption { + return gitOptionFunc(func(gi *GitInfo) { + gi.CommitHash = v + }) +} + // AuthOption can be used with either HTTP or Git sources. type AuthOption interface { GitOption diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 3d82e82a07db..975cda4f6577 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -1464,7 +1464,11 @@ 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]) { + ok, err := isHTTPSource(cfg.params.SourcePaths[0]) + if err != nil { + return err + } + if !ok { return errors.New("checksum can't be specified for non-HTTP(S) sources") } } @@ -1509,7 +1513,7 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { if gitRef.SubDir != "" { commit += ":" + gitRef.SubDir } - gitOptions := []llb.GitOption{llb.WithCustomName(pgName)} + gitOptions := []llb.GitOption{llb.WithCustomName(pgName), llb.CommitHash(gitRef.CommitHash)} if cfg.keepGitDir { gitOptions = append(gitOptions, llb.KeepGitDir()) } @@ -1523,77 +1527,83 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { } else { a = a.Copy(st, "/", dest, opts...) } - } else if isHTTPSource(src) { - if !cfg.isAddCommand { - return errors.New("source can't be a URL for COPY") + } else { + isHTTPSourceV, err := isHTTPSource(src) + if err != nil { + return err } + if isHTTPSourceV { + if !cfg.isAddCommand { + return errors.New("source can't be a URL for COPY") + } - // Resources from remote URLs are not decompressed. - // https://docs.docker.com/engine/reference/builder/#add - // - // Note: mixing up remote archives and local archives in a single ADD instruction - // would result in undefined behavior: https://github.com/moby/buildkit/pull/387#discussion_r189494717 - u, err := url.Parse(src) - f := "__unnamed__" - if err == nil { - if base := path.Base(u.Path); base != "." && base != "/" { - f = base + // Resources from remote URLs are not decompressed. + // https://docs.docker.com/engine/reference/builder/#add + // + // Note: mixing up remote archives and local archives in a single ADD instruction + // would result in undefined behavior: https://github.com/moby/buildkit/pull/387#discussion_r189494717 + u, err := url.Parse(src) + f := "__unnamed__" + if err == nil { + if base := path.Base(u.Path); base != "." && base != "/" { + f = base + } } - } - st := llb.HTTP(src, llb.Filename(f), llb.WithCustomName(pgName), llb.Checksum(cfg.checksum), dfCmd(cfg.params)) + st := llb.HTTP(src, llb.Filename(f), llb.WithCustomName(pgName), llb.Checksum(cfg.checksum), dfCmd(cfg.params)) - opts := append([]llb.CopyOption{&llb.CopyInfo{ - Mode: chopt, - CreateDestPath: true, - }}, copyOpt...) + opts := append([]llb.CopyOption{&llb.CopyInfo{ + Mode: chopt, + CreateDestPath: true, + }}, copyOpt...) - if a == nil { - a = llb.Copy(st, f, dest, opts...) - } else { - a = a.Copy(st, f, dest, opts...) - } - } else { - validateCopySourcePath(src, &cfg) - var patterns []string - if cfg.parents { - // detect optional pivot point - parent, pattern, ok := strings.Cut(src, "/./") - if !ok { - pattern = src - src = "/" + if a == nil { + a = llb.Copy(st, f, dest, opts...) } else { - src = parent + a = a.Copy(st, f, dest, opts...) } + } else { + validateCopySourcePath(src, &cfg) + var patterns []string + if cfg.parents { + // detect optional pivot point + parent, pattern, ok := strings.Cut(src, "/./") + if !ok { + pattern = src + src = "/" + } else { + src = parent + } + + pattern, err = system.NormalizePath("/", pattern, d.platform.OS, false) + if err != nil { + return errors.Wrap(err, "removing drive letter") + } - pattern, err = system.NormalizePath("/", pattern, d.platform.OS, false) + patterns = []string{strings.TrimPrefix(pattern, "/")} + } + + src, err = system.NormalizePath("/", src, d.platform.OS, false) if err != nil { return errors.Wrap(err, "removing drive letter") } - patterns = []string{strings.TrimPrefix(pattern, "/")} - } - - src, err = system.NormalizePath("/", src, d.platform.OS, false) - if err != nil { - return errors.Wrap(err, "removing drive letter") - } - - opts := append([]llb.CopyOption{&llb.CopyInfo{ - Mode: chopt, - FollowSymlinks: true, - CopyDirContentsOnly: true, - IncludePatterns: patterns, - AttemptUnpack: cfg.isAddCommand, - CreateDestPath: true, - AllowWildcard: true, - AllowEmptyWildcard: true, - }}, copyOpt...) - - if a == nil { - a = llb.Copy(cfg.source, src, dest, opts...) - } else { - a = a.Copy(cfg.source, src, dest, opts...) + opts := append([]llb.CopyOption{&llb.CopyInfo{ + Mode: chopt, + FollowSymlinks: true, + CopyDirContentsOnly: true, + IncludePatterns: patterns, + AttemptUnpack: cfg.isAddCommand, + CreateDestPath: true, + AllowWildcard: true, + AllowEmptyWildcard: true, + }}, copyOpt...) + + if a == nil { + a = llb.Copy(cfg.source, src, dest, opts...) + } else { + a = a.Copy(cfg.source, src, dest, opts...) + } } } } @@ -2255,15 +2265,21 @@ func commonImageNames() []string { return out } -func isHTTPSource(src string) bool { +func isHTTPSource(src string) (bool, error) { if !strings.HasPrefix(src, "http://") && !strings.HasPrefix(src, "https://") { - return false + return false, nil } // https://github.com/ORG/REPO.git is a git source, not an http source - if gitRef, gitErr := gitutil.ParseGitRef(src); gitRef != nil && gitErr == nil { - return false + gitRef, gitErr := gitutil.ParseGitRef(src) + var eiuf *gitutil.ErrInvalidURLFragemnt + if errors.As(gitErr, &eiuf) { + // this is a git source, and it has an invalid URL fragment + return false, gitErr + } + if gitRef != nil && gitErr == nil { + return false, nil } - return true + return true, nil } func isEnabledForStage(stage string, value string) bool { diff --git a/frontend/dockerui/context.go b/frontend/dockerui/context.go index 3173558fd67f..46b120aae14b 100644 --- a/frontend/dockerui/context.go +++ b/frontend/dockerui/context.go @@ -149,7 +149,7 @@ func DetectGitContext(ref string, keepGit bool) (*llb.State, bool) { if g.SubDir != "" { commit += ":" + g.SubDir } - gitOpts := []llb.GitOption{WithInternalName("load git source " + ref)} + gitOpts := []llb.GitOption{WithInternalName("load git source " + ref), llb.CommitHash(g.CommitHash)} if keepGit { gitOpts = append(gitOpts, llb.KeepGitDir()) } diff --git a/frontend/gateway/grpcclient/client.go b/frontend/gateway/grpcclient/client.go index 615700f51c94..c1eef90f2f8b 100644 --- a/frontend/gateway/grpcclient/client.go +++ b/frontend/gateway/grpcclient/client.go @@ -272,6 +272,7 @@ func defaultLLBCaps() []*apicaps.PBCap { {ID: string(opspb.CapSourceGit), Enabled: true}, {ID: string(opspb.CapSourceGitKeepDir), Enabled: true}, {ID: string(opspb.CapSourceGitFullURL), Enabled: true}, + {ID: string(opspb.CapSourceGitCommitHash), Enabled: true}, {ID: string(opspb.CapSourceHTTP), Enabled: true}, {ID: string(opspb.CapSourceHTTPChecksum), Enabled: true}, {ID: string(opspb.CapSourceHTTPPerm), Enabled: true}, diff --git a/solver/pb/attr.go b/solver/pb/attr.go index b18223dcdc6a..24ea7f208ad5 100644 --- a/solver/pb/attr.go +++ b/solver/pb/attr.go @@ -6,6 +6,7 @@ const AttrAuthHeaderSecret = "git.authheadersecret" const AttrAuthTokenSecret = "git.authtokensecret" const AttrKnownSSHHosts = "git.knownsshhosts" const AttrMountSSHSock = "git.mountsshsock" +const AttrCommitHash = "git.commithash" const AttrLocalSessionID = "local.session" const AttrLocalUniqueID = "local.unique" const AttrIncludePatterns = "local.includepattern" diff --git a/solver/pb/caps.go b/solver/pb/caps.go index ce5b0d4ea9d2..256c427c4b2e 100644 --- a/solver/pb/caps.go +++ b/solver/pb/caps.go @@ -30,6 +30,7 @@ const ( CapSourceGitKnownSSHHosts apicaps.CapID = "source.git.knownsshhosts" CapSourceGitMountSSHSock apicaps.CapID = "source.git.mountsshsock" CapSourceGitSubdir apicaps.CapID = "source.git.subdir" + CapSourceGitCommitHash apicaps.CapID = "source.git.commithash" CapSourceHTTP apicaps.CapID = "source.http" CapSourceHTTPAuth apicaps.CapID = "source.http.auth" @@ -222,6 +223,12 @@ func init() { Status: apicaps.CapStatusExperimental, }) + Caps.Init(apicaps.Cap{ + ID: CapSourceGitCommitHash, + Enabled: true, + Status: apicaps.CapStatusExperimental, + }) + Caps.Init(apicaps.Cap{ ID: CapSourceHTTP, Enabled: true, diff --git a/source/git/identifier.go b/source/git/identifier.go index 77951399b08a..27714dc72074 100644 --- a/source/git/identifier.go +++ b/source/git/identifier.go @@ -13,6 +13,7 @@ import ( type GitIdentifier struct { Remote string Ref string + CommitHash string Subdir string KeepGitDir bool AuthTokenSecret string @@ -33,6 +34,7 @@ func NewGitIdentifier(remoteURL string) (*GitIdentifier, error) { repo := GitIdentifier{Remote: u.Remote} if u.Fragment != nil { repo.Ref = u.Fragment.Ref + repo.CommitHash = u.Fragment.CommitHash repo.Subdir = u.Fragment.Subdir } if sd := path.Clean(repo.Subdir); sd == "/" || sd == "." { diff --git a/source/git/source.go b/source/git/source.go index acba38551ea1..a04d34c7b727 100644 --- a/source/git/source.go +++ b/source/git/source.go @@ -92,6 +92,8 @@ func (gs *gitSource) Identifier(scheme, ref string, attrs map[string]string, pla id.KnownSSHHosts = v case pb.AttrMountSSHSock: id.MountSSHSock = v + case pb.AttrCommitHash: + id.CommitHash = v } } @@ -207,6 +209,9 @@ func (gs *gitSourceHandler) shaToCacheKey(sha, ref string) string { if gs.src.Subdir != "" { key += ":" + gs.src.Subdir } + if gs.src.CommitHash != "" { + key += ":commit-hash=" + gs.src.CommitHash + } return key } @@ -349,10 +354,18 @@ func (gs *gitSourceHandler) CacheKey(ctx context.Context, g session.Group, index gs.locker.Lock(remote) defer gs.locker.Unlock(remote) - if ref := gs.src.Ref; ref != "" && gitutil.IsCommitSHA(ref) { - cacheKey := gs.shaToCacheKey(ref, "") + var refCommitFullHash string + if gitutil.IsCommitSHA(gs.src.CommitHash) { + refCommitFullHash = gs.src.CommitHash + } + if refCommitFullHash == "" && gitutil.IsCommitSHA(gs.src.Ref) { + refCommitFullHash = gs.src.Ref + } + if refCommitFullHash != "" { + cacheKey := gs.shaToCacheKey(refCommitFullHash, "") gs.cacheKey = cacheKey - return cacheKey, ref, nil, true, nil + // gs.src.CommitHash is verified after checking out the commit + return cacheKey, refCommitFullHash, nil, true, nil } gs.getAuthToken(ctx, g) @@ -415,7 +428,9 @@ func (gs *gitSourceHandler) CacheKey(ctx context.Context, g session.Group, index if !gitutil.IsCommitSHA(sha) { return "", "", nil, false, errors.Errorf("invalid commit sha %q", sha) } - + if gs.src.CommitHash != "" && !strings.HasPrefix(sha, gs.src.CommitHash) { + return "", "", nil, false, errors.Errorf("expected commit hash to match %s, got %s", gs.src.CommitHash, sha) + } cacheKey := gs.shaToCacheKey(sha, usedRef) gs.cacheKey = cacheKey return cacheKey, sha, nil, true, nil @@ -536,6 +551,7 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out subdir = "." } + checkedoutRef := "HEAD" if gs.src.KeepGitDir && subdir == "." { checkoutDirGit := filepath.Join(checkoutDir, ".git") if err := os.MkdirAll(checkoutDir, 0711); err != nil { @@ -605,6 +621,7 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out if err != nil { return nil, errors.Wrapf(err, "failed to checkout remote %s", urlutil.RedactCredentials(gs.src.Remote)) } + checkedoutRef = ref // HEAD may not exist if subdir != "." { d, err := os.Open(filepath.Join(cd, subdir)) if err != nil { @@ -635,6 +652,16 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out } git = git.New(gitutil.WithWorkTree(checkoutDir), gitutil.WithGitDir(gitDir)) + if gs.src.CommitHash != "" { + actualHashBuf, err := git.Run(ctx, "rev-parse", checkedoutRef) + if err != nil { + return nil, errors.Wrapf(err, "failed to rev-parse %s for %s", checkedoutRef, urlutil.RedactCredentials(gs.src.Remote)) + } + actualHash := strings.TrimSpace(string(actualHashBuf)) + if !strings.HasPrefix(actualHash, gs.src.CommitHash) { + return nil, errors.Errorf("expected commit hash to match %s, got %s", gs.src.CommitHash, actualHash) + } + } _, err = git.Run(ctx, "submodule", "update", "--init", "--recursive", "--depth=1") if err != nil { return nil, errors.Wrapf(err, "failed to update submodules for %s", urlutil.RedactCredentials(gs.src.Remote)) diff --git a/source/git/source_test.go b/source/git/source_test.go index bdb765330999..3b90deb2c6d1 100644 --- a/source/git/source_test.go +++ b/source/git/source_test.go @@ -304,54 +304,75 @@ func testFetchUnreferencedRefSha(t *testing.T, ref string, keepGitDir bool) { } func TestFetchByTag(t *testing.T) { - testFetchByTag(t, "lightweight-tag", "third", false, true, false) + testFetchByTag(t, "lightweight-tag", "third", false, true, false, testCommitHashModeNone) } func TestFetchByTagKeepGitDir(t *testing.T) { - testFetchByTag(t, "lightweight-tag", "third", false, true, true) + testFetchByTag(t, "lightweight-tag", "third", false, true, true, testCommitHashModeNone) } func TestFetchByTagFull(t *testing.T) { - testFetchByTag(t, "refs/tags/lightweight-tag", "third", false, true, true) + testFetchByTag(t, "refs/tags/lightweight-tag", "third", false, true, true, testCommitHashModeNone) } func TestFetchByAnnotatedTag(t *testing.T) { - testFetchByTag(t, "v1.2.3", "second", true, false, false) + testFetchByTag(t, "v1.2.3", "second", true, false, false, testCommitHashModeNone) } func TestFetchByAnnotatedTagKeepGitDir(t *testing.T) { - testFetchByTag(t, "v1.2.3", "second", true, false, true) + testFetchByTag(t, "v1.2.3", "second", true, false, true, testCommitHashModeNone) } func TestFetchByAnnotatedTagFull(t *testing.T) { - testFetchByTag(t, "refs/tags/v1.2.3", "second", true, false, true) + testFetchByTag(t, "refs/tags/v1.2.3", "second", true, false, true, testCommitHashModeNone) } func TestFetchByBranch(t *testing.T) { - testFetchByTag(t, "feature", "withsub", false, true, false) + testFetchByTag(t, "feature", "withsub", false, true, false, testCommitHashModeNone) } func TestFetchByBranchKeepGitDir(t *testing.T) { - testFetchByTag(t, "feature", "withsub", false, true, true) + testFetchByTag(t, "feature", "withsub", false, true, true, testCommitHashModeNone) } func TestFetchByBranchFull(t *testing.T) { - testFetchByTag(t, "refs/heads/feature", "withsub", false, true, true) + testFetchByTag(t, "refs/heads/feature", "withsub", false, true, true, testCommitHashModeNone) } func TestFetchByRef(t *testing.T) { - testFetchByTag(t, "test", "feature", false, true, false) + testFetchByTag(t, "test", "feature", false, true, false, testCommitHashModeNone) } func TestFetchByRefKeepGitDir(t *testing.T) { - testFetchByTag(t, "test", "feature", false, true, true) + testFetchByTag(t, "test", "feature", false, true, true, testCommitHashModeNone) } func TestFetchByRefFull(t *testing.T) { - testFetchByTag(t, "refs/test", "feature", false, true, true) + testFetchByTag(t, "refs/test", "feature", false, true, true, testCommitHashModeNone) } -func testFetchByTag(t *testing.T, tag, expectedCommitSubject string, isAnnotatedTag, hasFoo13File, keepGitDir bool) { +func TestFetchByTagWithCommitHash(t *testing.T) { + testFetchByTag(t, "lightweight-tag", "third", false, true, false, testCommitHashModeValid) +} + +func TestFetchByTagWithCommitHashPartial(t *testing.T) { + testFetchByTag(t, "lightweight-tag", "third", false, true, false, testCommitHashModeValidPartial) +} + +func TestFetchByTagWithCommitHashInvalid(t *testing.T) { + testFetchByTag(t, "lightweight-tag", "third", false, true, false, testCommitHashModeInvalid) +} + +type testCommitHashMode int + +const ( + testCommitHashModeNone testCommitHashMode = iota + testCommitHashModeValid + testCommitHashModeValidPartial + testCommitHashModeInvalid +) + +func testFetchByTag(t *testing.T, tag, expectedCommitSubject string, isAnnotatedTag, hasFoo13File, keepGitDir bool, commitHashMode testCommitHashMode) { if runtime.GOOS == "windows" { t.Skip("Depends on unimplemented containerd bind-mount support on Windows") } @@ -366,6 +387,28 @@ func testFetchByTag(t *testing.T, tag, expectedCommitSubject string, isAnnotated id := &GitIdentifier{Remote: repo.mainURL, Ref: tag, KeepGitDir: keepGitDir} + if commitHashMode != testCommitHashModeNone { + cmd := exec.Command("git", "rev-parse", tag) + cmd.Dir = repo.mainPath + + out, err := cmd.Output() + require.NoError(t, err) + + sha := strings.TrimSpace(string(out)) + require.Equal(t, 40, len(sha)) + + switch commitHashMode { + case testCommitHashModeValid: + id.CommitHash = sha + case testCommitHashModeValidPartial: + id.CommitHash = sha[:8] + case testCommitHashModeInvalid: + id.CommitHash = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef" + default: + // NOTREACHED + } + } + g, err := gs.Resolve(ctx, id, nil, nil) require.NoError(t, err) @@ -376,6 +419,14 @@ func testFetchByTag(t *testing.T, tag, expectedCommitSubject string, isAnnotated expLen := 40 if keepGitDir { expLen += 4 + } + if id.Subdir != "" { + expLen += len(":" + id.Subdir) + } + if id.CommitHash != "" { + expLen += len(":commit-hash=" + id.CommitHash) + } + if keepGitDir { require.GreaterOrEqual(t, len(key1), expLen) } else { require.Equal(t, expLen, len(key1)) @@ -383,6 +434,10 @@ func testFetchByTag(t *testing.T, tag, expectedCommitSubject string, isAnnotated require.Equal(t, 40, len(pin1)) ref1, err := g.Snapshot(ctx, nil) + if commitHashMode == testCommitHashModeInvalid { + require.ErrorContains(t, err, "expected commit hash to match "+id.CommitHash) + return + } require.NoError(t, err) defer ref1.Release(context.TODO()) diff --git a/util/gitutil/git_ref.go b/util/gitutil/git_ref.go index 59d65826418a..37f18c0cea09 100644 --- a/util/gitutil/git_ref.go +++ b/util/gitutil/git_ref.go @@ -25,6 +25,10 @@ type GitRef struct { // Commit is optional. Commit string + // CommitHash verifies Commit if specified. + // CommitHash is optional. + CommitHash string + // SubDir is a directory path inside the repo. // SubDir is optional. SubDir string @@ -61,11 +65,14 @@ func ParseGitRef(ref string) (*GitRef, error) { return nil, cerrdefs.ErrInvalidArgument } else if strings.HasPrefix(ref, "github.com/") { res.IndistinguishableFromLocal = true // Deprecated - remote = fromURL(&url.URL{ + remote, err = fromURL(&url.URL{ Scheme: "https", Host: "github.com", Path: strings.TrimPrefix(ref, "github.com/"), }) + if err != nil { + return nil, err + } } else { remote, err = ParseURL(ref) if errors.Is(err, ErrUnknownProtocol) { @@ -94,7 +101,7 @@ func ParseGitRef(ref string) (*GitRef, error) { _, res.Remote, _ = strings.Cut(res.Remote, "://") } if remote.Fragment != nil { - res.Commit, res.SubDir = remote.Fragment.Ref, remote.Fragment.Subdir + res.Commit, res.CommitHash, res.SubDir = remote.Fragment.Ref, remote.Fragment.CommitHash, remote.Fragment.Subdir } repoSplitBySlash := strings.Split(res.Remote, "/") diff --git a/util/gitutil/git_ref_test.go b/util/gitutil/git_ref_test.go index fe4d094b651c..51812b310d3d 100644 --- a/util/gitutil/git_ref_test.go +++ b/util/gitutil/git_ref_test.go @@ -138,6 +138,15 @@ func TestParseGitRef(t *testing.T) { ref: ".git", expected: nil, }, + { + ref: "https://example.com/foo.git##ref=release/1.2,subdir=/subdir", + expected: &GitRef{ + Remote: "https://example.com/foo.git", + ShortName: "foo", + Commit: "release/1.2", + SubDir: "/subdir", + }, + }, } for _, tt := range cases { tt := tt diff --git a/util/gitutil/git_url.go b/util/gitutil/git_url.go index 0f1ff505c765..96e124d00207 100644 --- a/util/gitutil/git_url.go +++ b/util/gitutil/git_url.go @@ -7,6 +7,7 @@ import ( "github.com/moby/buildkit/util/sshutil" "github.com/pkg/errors" + "github.com/tonistiigi/go-csvvalue" ) const ( @@ -60,18 +61,61 @@ type GitURL struct { type GitURLFragment struct { // Ref is the git reference Ref string + // CommitHash is verified against Ref if specified + CommitHash string // Subdir is the sub-directory inside the git repository to use Subdir string } // splitGitFragment splits a git URL fragment into its respective git // reference and subdirectory components. -func splitGitFragment(fragment string) *GitURLFragment { +func splitGitFragment(fragment string) (*GitURLFragment, error) { + if strings.HasPrefix(fragment, "#") { + // Double-hash in the unparsed URL. + // e.g., https://github.com/user/repo.git##ref=tag,subdir=/dir + return splitGitFragmentCSVForm(fragment) + } + // Single-hash in the unparsed URL. + // e.g., https://github.com/user/repo.git#branch_or_tag_or_commit:dir if fragment == "" { - return nil + return nil, nil } ref, subdir, _ := strings.Cut(fragment, ":") - return &GitURLFragment{Ref: ref, Subdir: subdir} + return &GitURLFragment{Ref: ref, Subdir: subdir}, nil +} + +func splitGitFragmentCSVForm(fragment string) (*GitURLFragment, error) { + fragment = strings.TrimPrefix(fragment, "#") + if fragment == "" { + return nil, nil + } + fields, err := csvvalue.Fields(fragment, nil) + if err != nil { + return nil, errors.Wrapf(err, "failed to parse CSV %q", fragment) + } + + res := &GitURLFragment{} + for _, field := range fields { + key, value, ok := strings.Cut(field, "=") + if !ok { + return nil, errors.Errorf("invalid field '%s' must be a key=value pair", field) + } + key = strings.ToLower(key) + switch key { + case "ref": + res.Ref = value + case "commit": + res.CommitHash = value + case "subdir": + res.Subdir = value + default: + return nil, errors.Errorf("unexpected key '%s' in '%s' (supported keys: ref, commit, subdir)", key, field) + } + } + if res.CommitHash != "" && res.Ref == "" { + res.Ref = res.CommitHash + } + return res, nil } // ParseURL parses a BuildKit-style Git URL (that may contain additional @@ -86,11 +130,11 @@ func ParseURL(remote string) (*GitURL, error) { if err != nil { return nil, err } - return fromURL(url), nil + return fromURL(url) } if url, err := sshutil.ParseSCPStyleURL(remote); err == nil { - return fromSCPStyleURL(url), nil + return fromSCPStyleURL(url) } return nil, ErrUnknownProtocol @@ -105,28 +149,46 @@ func IsGitTransport(remote string) bool { return sshutil.IsImplicitSSHTransport(remote) } -func fromURL(url *url.URL) *GitURL { +// ErrInvalidURLFragemnt is returned for an invalid URL fragment +type ErrInvalidURLFragemnt struct { + error +} + +func fromURL(url *url.URL) (*GitURL, error) { withoutFragment := *url withoutFragment.Fragment = "" - return &GitURL{ + fragment, err := splitGitFragment(url.Fragment) + if err != nil { + return nil, &ErrInvalidURLFragemnt{ + error: errors.Wrapf(err, "failed to parse URL fragment %q", url.Fragment), + } + } + gitURL := &GitURL{ Scheme: url.Scheme, User: url.User, Host: url.Host, Path: url.Path, - Fragment: splitGitFragment(url.Fragment), + Fragment: fragment, Remote: withoutFragment.String(), } + return gitURL, nil } -func fromSCPStyleURL(url *sshutil.SCPStyleURL) *GitURL { +func fromSCPStyleURL(url *sshutil.SCPStyleURL) (*GitURL, error) { withoutFragment := *url withoutFragment.Fragment = "" + fragment, err := splitGitFragment(url.Fragment) + if err != nil { + return nil, &ErrInvalidURLFragemnt{ + error: errors.Wrapf(err, "failed to parse URL fragment %q", url.Fragment), + } + } return &GitURL{ Scheme: SSHProtocol, User: url.User, Host: url.Host, Path: url.Path, - Fragment: splitGitFragment(url.Fragment), + Fragment: fragment, Remote: withoutFragment.String(), - } + }, nil } diff --git a/util/gitutil/git_url_test.go b/util/gitutil/git_url_test.go index 63ee90aa1b8a..b884273e6945 100644 --- a/util/gitutil/git_url_test.go +++ b/util/gitutil/git_url_test.go @@ -151,6 +151,73 @@ func TestParseURL(t *testing.T) { Path: "/moby/buildkit", }, }, + { + url: "https://github.com/moby/buildkit##ref=v1.0.0,subdir=/subdir", + result: GitURL{ + Scheme: HTTPSProtocol, + Host: "github.com", + Path: "/moby/buildkit", + Fragment: &GitURLFragment{Ref: "v1.0.0", Subdir: "/subdir"}, + }, + }, + { + url: "https://github.com/moby/buildkit##ref=v1.0.0,subdir=/subdir,unknown-fragment=invalid", + err: true, + }, + { + url: `https://github.com/moby/buildkit##"ref=ref,with,comma",subdir=/subdir`, + result: GitURL{ + Scheme: HTTPSProtocol, + Host: "github.com", + Path: "/moby/buildkit", + Fragment: &GitURLFragment{Ref: "ref,with,comma", Subdir: "/subdir"}, + }, + }, + { + url: `https://github.com/moby/buildkit##ref=#ref-starts-with-sharp,subdir=/subdir`, + result: GitURL{ + Scheme: HTTPSProtocol, + Host: "github.com", + Path: "/moby/buildkit", + Fragment: &GitURLFragment{Ref: "#ref-starts-with-sharp", Subdir: "/subdir"}, + }, + }, + { + url: "https://github.com/moby/buildkit##ref=v1.0.0,commit=deadbeefdeadbeefdeadbeefdeadbeefdeadbeef", + result: GitURL{ + Scheme: HTTPSProtocol, + Host: "github.com", + Path: "/moby/buildkit", + Fragment: &GitURLFragment{Ref: "v1.0.0", CommitHash: "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"}, + }, + }, + { + url: "https://github.com/moby/buildkit##commit=deadbeefdeadbeefdeadbeefdeadbeefdeadbeef", + result: GitURL{ + Scheme: HTTPSProtocol, + Host: "github.com", + Path: "/moby/buildkit", + Fragment: &GitURLFragment{Ref: "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef", CommitHash: "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"}, + }, + }, + { + url: "https://github.com/moby/buildkit##ref=v1.0.0,commit=deadbeef", + result: GitURL{ + Scheme: HTTPSProtocol, + Host: "github.com", + Path: "/moby/buildkit", + Fragment: &GitURLFragment{Ref: "v1.0.0", CommitHash: "deadbeef"}, + }, + }, + { + url: "https://github.com/moby/buildkit##commit=deadbeef", + result: GitURL{ + Scheme: HTTPSProtocol, + Host: "github.com", + Path: "/moby/buildkit", + Fragment: &GitURLFragment{Ref: "deadbeef", CommitHash: "deadbeef"}, + }, + }, } for _, test := range tests { t.Run(test.url, func(t *testing.T) {