Skip to content

Commit

Permalink
refactor git cli calls
Browse files Browse the repository at this point in the history 
* create hosts/resolv overrides once rather than on every call
* use an object to avoid repeating tons of args

Signed-off-by: Alex Suraci <suraci.alex@gmail.com>
  • Loading branch information
@vito
vito committed Jun 21, 2023
1 parent 410990b commit 49cff4f
Show file tree
Hide file tree
Showing 3 changed files with 194 additions and 109 deletions.
129 changes: 89 additions & 40 deletions source/git/gitsource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,16 +129,22 @@ func (gs *gitSource) mountRemote(ctx context.Context, remote string, auth []stri
}
}()

git, err := newGitSession(dir, "", "", nil, auth)
if err != nil {
return "", nil, err
}
defer git.cleanup()

if initializeRepo {
// Explicitly set the Git config 'init.defaultBranch' to the
// implied default to suppress "hint:" output about not having a
// default initial branch name set which otherwise spams unit
// test logs.
if _, err := gitWithinDir(ctx, dir, "", "", "", nil, auth, "-c", "init.defaultBranch=master", "init", "--bare"); err != nil {
if _, err := git.run(ctx, "-c", "init.defaultBranch=master", "init", "--bare"); err != nil {
return "", nil, errors.Wrapf(err, "failed to init repo at %s", dir)
}

if _, err := gitWithinDir(ctx, dir, "", "", "", nil, auth, "remote", "add", "origin", remote); err != nil {
if _, err := git.run(ctx, "remote", "add", "origin", remote); err != nil {
return "", nil, errors.Wrapf(err, "failed add origin repo at %s", dir)
}

Expand Down Expand Up @@ -371,17 +377,23 @@ func (gs *gitSourceHandler) CacheKey(ctx context.Context, g session.Group, index
return "", "", nil, false, err
}

git, err := newGitSession(gitDir, sock, knownHosts, netConf, gs.auth)
if err != nil {
return "", "", nil, false, err
}
defer git.cleanup()

ref := gs.src.Ref
if ref == "" {
ref, err = getDefaultBranch(ctx, gitDir, "", sock, knownHosts, netConf, gs.auth, gs.src.Remote)
ref, err = getDefaultBranch(ctx, git, gs.src.Remote)
if err != nil {
return "", "", nil, false, err
}
}

// TODO: should we assume that remote tag is immutable? add a timer?

buf, err := gitWithinDir(ctx, gitDir, "", sock, knownHosts, netConf, gs.auth, "ls-remote", "origin", ref)
buf, err := git.run(ctx, "ls-remote", "origin", ref)
if err != nil {
return "", "", nil, false, errors.Wrapf(err, "failed to fetch remote %s", urlutil.RedactCredentials(remote))
}
Expand Down Expand Up @@ -457,9 +469,15 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out
return nil, err
}

git, err := newGitSession(gitDir, sock, knownHosts, netConf, gs.auth)
if err != nil {
return nil, err
}
defer git.cleanup()

ref := gs.src.Ref
if ref == "" {
ref, err = getDefaultBranch(ctx, gitDir, "", sock, knownHosts, netConf, gs.auth, gs.src.Remote)
ref, err = getDefaultBranch(ctx, git, gs.src.Remote)
if err != nil {
return nil, err
}
Expand All @@ -468,7 +486,7 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out
doFetch := true
if isCommitSHA(ref) {
// skip fetch if commit already exists
if _, err := gitWithinDir(ctx, gitDir, "", sock, knownHosts, netConf, nil, "cat-file", "-e", ref+"^{commit}"); err == nil {
if _, err := git.run(ctx, "cat-file", "-e", ref+"^{commit}"); err == nil {
doFetch = false
}
}
Expand All @@ -492,10 +510,10 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out
// in case the ref is a branch and it now points to a different commit sha
// TODO: is there a better way to do this?
}
if _, err := gitWithinDir(ctx, gitDir, "", sock, knownHosts, netConf, gs.auth, args...); err != nil {
if _, err := git.run(ctx, args...); err != nil {
return nil, errors.Wrapf(err, "failed to fetch remote %s", urlutil.RedactCredentials(gs.src.Remote))
}
_, err = gitWithinDir(ctx, gitDir, "", sock, knownHosts, netConf, nil, "reflog", "expire", "--all", "--expire=now")
_, err = git.run(ctx, "reflog", "expire", "--all", "--expire=now")
if err != nil {
return nil, errors.Wrapf(err, "failed to expire reflog for remote %s", urlutil.RedactCredentials(gs.src.Remote))
}
Expand Down Expand Up @@ -537,19 +555,20 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out
if err := os.MkdirAll(checkoutDir, 0711); err != nil {
return nil, err
}
_, err = gitWithinDir(ctx, checkoutDirGit, "", sock, knownHosts, netConf, nil, "-c", "init.defaultBranch=master", "init")
checkoutGit := git.withinDir(checkoutDirGit, "")
_, err = checkoutGit.run(ctx, "-c", "init.defaultBranch=master", "init")
if err != nil {
return nil, err
}
// Defense-in-depth: clone using the file protocol to disable local-clone
// optimizations which can be abused on some versions of Git to copy unintended
// host files into the build context.
_, err = gitWithinDir(ctx, checkoutDirGit, "", sock, knownHosts, netConf, nil, "remote", "add", "origin", "file://"+gitDir)
_, err = checkoutGit.run(ctx, "remote", "add", "origin", "file://"+gitDir)
if err != nil {
return nil, err
}

gitCatFileBuf, err := gitWithinDir(ctx, gitDir, "", sock, knownHosts, netConf, gs.auth, "cat-file", "-t", ref)
gitCatFileBuf, err := git.run(ctx, "cat-file", "-t", ref)
if err != nil {
return nil, err
}
Expand All @@ -560,26 +579,26 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out
pullref += ":refs/tags/" + pullref
} else if isCommitSHA(ref) {
pullref = "refs/buildkit/" + identity.NewID()
_, err = gitWithinDir(ctx, gitDir, "", sock, knownHosts, netConf, gs.auth, "update-ref", pullref, ref)
_, err = git.run(ctx, "update-ref", pullref, ref)
if err != nil {
return nil, err
}
} else {
pullref += ":" + pullref
}
_, err = gitWithinDir(ctx, checkoutDirGit, "", sock, knownHosts, netConf, gs.auth, "fetch", "-u", "--depth=1", "origin", pullref)
_, err = checkoutGit.run(ctx, "fetch", "-u", "--depth=1", "origin", pullref)
if err != nil {
return nil, err
}
_, err = gitWithinDir(ctx, checkoutDirGit, checkoutDir, sock, knownHosts, netConf, nil, "checkout", "FETCH_HEAD")
_, err = checkoutGit.run(ctx, "checkout", "FETCH_HEAD")
if err != nil {
return nil, errors.Wrapf(err, "failed to checkout remote %s", urlutil.RedactCredentials(gs.src.Remote))
}
_, err = gitWithinDir(ctx, checkoutDirGit, "", sock, knownHosts, netConf, nil, "remote", "set-url", "origin", urlutil.RedactCredentials(gs.src.Remote))
_, err = checkoutGit.run(ctx, "remote", "set-url", "origin", urlutil.RedactCredentials(gs.src.Remote))
if err != nil {
return nil, errors.Wrapf(err, "failed to set remote origin to %s", urlutil.RedactCredentials(gs.src.Remote))
}
_, err = gitWithinDir(ctx, checkoutDirGit, "", sock, knownHosts, netConf, nil, "reflog", "expire", "--all", "--expire=now")
_, err = checkoutGit.run(ctx, "reflog", "expire", "--all", "--expire=now")
if err != nil {
return nil, errors.Wrapf(err, "failed to expire reflog for remote %s", urlutil.RedactCredentials(gs.src.Remote))
}
Expand All @@ -595,7 +614,7 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out
return nil, errors.Wrapf(err, "failed to create temporary checkout dir")
}
}
_, err = gitWithinDir(ctx, gitDir, cd, sock, knownHosts, netConf, nil, "checkout", ref, "--", ".")
_, err = git.withinDir(gitDir, cd).run(ctx, "checkout", ref, "--", ".")
if err != nil {
return nil, errors.Wrapf(err, "failed to checkout remote %s", urlutil.RedactCredentials(gs.src.Remote))
}
Expand Down Expand Up @@ -628,7 +647,7 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out
}
}

_, err = gitWithinDir(ctx, gitDir, checkoutDir, sock, knownHosts, netConf, gs.auth, "submodule", "update", "--init", "--recursive", "--depth=1")
_, err = git.withinDir(gitDir, checkoutDir).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))
}
Expand Down Expand Up @@ -669,14 +688,6 @@ func isCommitSHA(str string) bool {
return validHex.MatchString(str)
}

func gitWithinDir(ctx context.Context, gitDir, workDir, sshAuthSock, knownHosts string, netConf *networks.Config, auth []string, args ...string) (*bytes.Buffer, error) {
a := append([]string{"--git-dir", gitDir}, auth...)
if workDir != "" {
a = append(a, "--work-tree", workDir)
}
return git(ctx, workDir, sshAuthSock, knownHosts, netConf, append(a, args...)...)
}

func getGitSSHCommand(knownHosts string) string {
gitSSHCommand := "ssh -F /dev/null"
if knownHosts != "" {
Expand All @@ -687,7 +698,40 @@ func getGitSSHCommand(knownHosts string) string {
return gitSSHCommand
}

func git(ctx context.Context, dir, sshAuthSock, knownHosts string, netConf *networks.Config, args ...string) (_ *bytes.Buffer, err error) {
type gitSession struct {
gitDir string // --git-dir flag
sshAuthSock string
knownHosts string
auth []string

workDir string // command cwd

hostsPath string
resolvPath string
}

func newGitSession(gitDir, sshAuthSock, knownHosts string, netConf *networks.Config, auth []string) (*gitSession, error) {
session := &gitSession{
workDir: gitDir,
sshAuthSock: sshAuthSock,
knownHosts: knownHosts,
auth: auth,
}
if err := session.initConfig(netConf); err != nil {
session.cleanup()
return nil, err
}
return session, nil
}

func (s *gitSession) withinDir(gitDir, workDir string) *gitSession {
cp := *s
cp.gitDir = gitDir
cp.workDir = workDir
return &cp
}

func (s *gitSession) run(ctx context.Context, args ...string) (_ *bytes.Buffer, err error) {
for {
stdout, stderr, flush := logs.NewLogStreams(ctx, true)
defer stdout.Close()
Expand All @@ -697,9 +741,16 @@ func git(ctx context.Context, dir, sshAuthSock, knownHosts string, netConf *netw
flush()
}
}()
if s.gitDir != "" {
args = append([]string{"--git-dir", s.gitDir}, args...)
}
args = append([]string{"-c", "protocol.file.allow=user"}, args...) // Block sneaky repositories from using repos from the filesystem as submodules.
cmd := exec.Command("git", args...)
cmd.Dir = dir // some commands like submodule require this
if s.workDir != "" {
cmd.Dir = s.workDir // some commands like submodule require this
} else {
cmd.Dir = s.gitDir // set a sane default for commands where it doesn't matter
}
buf := bytes.NewBuffer(nil)
errbuf := bytes.NewBuffer(nil)
cmd.Stdin = nil
Expand All @@ -708,22 +759,20 @@ func git(ctx context.Context, dir, sshAuthSock, knownHosts string, netConf *netw
cmd.Env = []string{
"PATH=" + os.Getenv("PATH"),
"GIT_TERMINAL_PROMPT=0",
"GIT_SSH_COMMAND=" + getGitSSHCommand(knownHosts),
"GIT_SSH_COMMAND=" + getGitSSHCommand(s.knownHosts),
// "GIT_TRACE=1",
"GIT_CONFIG_NOSYSTEM=1", // Disable reading from system gitconfig.
"HOME=/dev/null", // Disable reading from user gitconfig.
"LC_ALL=C", // Ensure consistent output.
}
if netConf != nil {
if len(netConf.IpHosts) > 0 {
cmd.Env = append(cmd.Env, "EXTRA_HOSTS="+netConf.ExtraHosts())
}
if netConf.Dns != nil && len(netConf.Dns.SearchDomains) > 0 {
cmd.Env = append(cmd.Env, "SEARCH_DOMAINS="+strings.Join(netConf.Dns.SearchDomains, " "))
}
if s.hostsPath != "" {
cmd.Env = append(cmd.Env, "HOSTS_OVERRIDE="+s.hostsPath)
}
if s.resolvPath != "" {
cmd.Env = append(cmd.Env, "RESOLV_OVERRIDE="+s.resolvPath)
}
if sshAuthSock != "" {
cmd.Env = append(cmd.Env, "SSH_AUTH_SOCK="+sshAuthSock)
if s.sshAuthSock != "" {
cmd.Env = append(cmd.Env, "SSH_AUTH_SOCK="+s.sshAuthSock)
}
// remote git commands spawn helper processes that inherit FDs and don't
// handle parent death signal so exec.CommandContext can't be used
Expand Down Expand Up @@ -762,8 +811,8 @@ func tokenScope(remote string) string {
}

// getDefaultBranch gets the default branch of a repository using ls-remote
func getDefaultBranch(ctx context.Context, gitDir, workDir, sshAuthSock, knownHosts string, netConf *networks.Config, auth []string, remoteURL string) (string, error) {
buf, err := gitWithinDir(ctx, gitDir, workDir, sshAuthSock, knownHosts, netConf, auth, "ls-remote", "--symref", remoteURL, "HEAD")
func getDefaultBranch(ctx context.Context, git *gitSession, remoteURL string) (string, error) {
buf, err := git.run(ctx, "ls-remote", "--symref", remoteURL, "HEAD")
if err != nil {
return "", errors.Wrapf(err, "error fetching default branch for repository %s", urlutil.RedactCredentials(remoteURL))
}
Expand Down
10 changes: 7 additions & 3 deletions source/git/gitsource_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,13 +292,17 @@ func testFetchByTag(t *testing.T, tag, expectedCommitSubject string, isAnnotated
}

if keepGitDir {
git, err := newGitSession(dir, "", "", nil, nil)
require.NoError(t, err)
defer git.cleanup()

if isAnnotatedTag {
// get commit sha that the annotated tag points to
annotatedTagCommit, err := git(ctx, dir, "", "", nil, "", "rev-list", "-n", "1", tag)
annotatedTagCommit, err := git.run(ctx, "rev-list", "-n", "1", tag)
require.NoError(t, err)

// get current commit sha
headCommit, err := git(ctx, dir, "", "", nil, "", "rev-parse", "HEAD")
headCommit, err := git.run(ctx, "rev-parse", "HEAD")
require.NoError(t, err)

// HEAD should match the actual commit sha (and not the sha of the annotated tag,
Expand All @@ -309,7 +313,7 @@ func testFetchByTag(t *testing.T, tag, expectedCommitSubject string, isAnnotated
// test that we checked out the correct commit
// (in the case of an annotated tag, this message is of the commit the annotated tag points to
// and not the message of the tag)
gitLogOutput, err := git(ctx, dir, "", "", nil, "", "log", "-n", "1", "--format=%s")
gitLogOutput, err := git.run(ctx, "log", "-n", "1", "--format=%s")
require.NoError(t, err)
require.True(t, strings.Contains(strings.TrimSpace(gitLogOutput.String()), expectedCommitSubject))
}
Expand Down
Loading

0 comments on commit 49cff4f

Please sign in to comment.