From 8deb92d65398dd37a7daa1c8dd90b37aa9312097 Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Wed, 27 Jul 2022 15:46:46 -0400 Subject: [PATCH 1/6] builder: modernize TestCheckoutGit Make the test more debuggable by logging all git command output and running each table-driven test case as a subtest. Signed-off-by: Cory Snider --- builder/remotecontext/git/gitutils_test.go | 139 ++++++++------------- 1 file changed, 54 insertions(+), 85 deletions(-) diff --git a/builder/remotecontext/git/gitutils_test.go b/builder/remotecontext/git/gitutils_test.go index 17df6fa867..47b897fa2a 100644 --- a/builder/remotecontext/git/gitutils_test.go +++ b/builder/remotecontext/git/gitutils_test.go @@ -170,9 +170,7 @@ func gitGetConfig(name string) string { } func TestCheckoutGit(t *testing.T) { - root, err := os.MkdirTemp("", "docker-build-git-checkout") - assert.NilError(t, err) - defer os.RemoveAll(root) + root := t.TempDir() autocrlf := gitGetConfig("core.autocrlf") if !(autocrlf == "true" || autocrlf == "false" || @@ -184,88 +182,57 @@ func TestCheckoutGit(t *testing.T) { eol = "\r\n" } + must := func(out []byte, err error) { + t.Helper() + if len(out) > 0 { + t.Logf("%s", out) + } + assert.NilError(t, err) + } + gitDir := filepath.Join(root, "repo") - _, err = git("init", gitDir) - assert.NilError(t, err) - - _, err = gitWithinDir(gitDir, "config", "user.email", "test@docker.com") - assert.NilError(t, err) - - _, err = gitWithinDir(gitDir, "config", "user.name", "Docker test") - assert.NilError(t, err) - - err = os.WriteFile(filepath.Join(gitDir, "Dockerfile"), []byte("FROM scratch"), 0644) - assert.NilError(t, err) + must(git("-c", "init.defaultBranch=master", "init", gitDir)) + must(gitWithinDir(gitDir, "config", "user.email", "test@docker.com")) + must(gitWithinDir(gitDir, "config", "user.name", "Docker test")) + assert.NilError(t, os.WriteFile(filepath.Join(gitDir, "Dockerfile"), []byte("FROM scratch"), 0644)) subDir := filepath.Join(gitDir, "subdir") assert.NilError(t, os.Mkdir(subDir, 0755)) - - err = os.WriteFile(filepath.Join(subDir, "Dockerfile"), []byte("FROM scratch\nEXPOSE 5000"), 0644) - assert.NilError(t, err) + assert.NilError(t, os.WriteFile(filepath.Join(subDir, "Dockerfile"), []byte("FROM scratch\nEXPOSE 5000"), 0644)) if runtime.GOOS != "windows" { - if err = os.Symlink("../subdir", filepath.Join(gitDir, "parentlink")); err != nil { - t.Fatal(err) - } - - if err = os.Symlink("/subdir", filepath.Join(gitDir, "absolutelink")); err != nil { - t.Fatal(err) - } + assert.NilError(t, os.Symlink("../subdir", filepath.Join(gitDir, "parentlink"))) + assert.NilError(t, os.Symlink("/subdir", filepath.Join(gitDir, "absolutelink"))) } - _, err = gitWithinDir(gitDir, "add", "-A") - assert.NilError(t, err) + must(gitWithinDir(gitDir, "add", "-A")) + must(gitWithinDir(gitDir, "commit", "-am", "First commit")) + must(gitWithinDir(gitDir, "checkout", "-b", "test")) - _, err = gitWithinDir(gitDir, "commit", "-am", "First commit") - assert.NilError(t, err) + assert.NilError(t, os.WriteFile(filepath.Join(gitDir, "Dockerfile"), []byte("FROM scratch\nEXPOSE 3000"), 0644)) + assert.NilError(t, os.WriteFile(filepath.Join(subDir, "Dockerfile"), []byte("FROM busybox\nEXPOSE 5000"), 0644)) - _, err = gitWithinDir(gitDir, "checkout", "-b", "test") - assert.NilError(t, err) - - err = os.WriteFile(filepath.Join(gitDir, "Dockerfile"), []byte("FROM scratch\nEXPOSE 3000"), 0644) - assert.NilError(t, err) - - err = os.WriteFile(filepath.Join(subDir, "Dockerfile"), []byte("FROM busybox\nEXPOSE 5000"), 0644) - assert.NilError(t, err) - - _, err = gitWithinDir(gitDir, "add", "-A") - assert.NilError(t, err) - - _, err = gitWithinDir(gitDir, "commit", "-am", "Branch commit") - assert.NilError(t, err) - - _, err = gitWithinDir(gitDir, "checkout", "master") - assert.NilError(t, err) + must(gitWithinDir(gitDir, "add", "-A")) + must(gitWithinDir(gitDir, "commit", "-am", "Branch commit")) + must(gitWithinDir(gitDir, "checkout", "master")) // set up submodule subrepoDir := filepath.Join(root, "subrepo") - _, err = git("init", subrepoDir) - assert.NilError(t, err) + must(git("-c", "init.defaultBranch=master", "init", subrepoDir)) + must(gitWithinDir(subrepoDir, "config", "user.email", "test@docker.com")) + must(gitWithinDir(subrepoDir, "config", "user.name", "Docker test")) - _, err = gitWithinDir(subrepoDir, "config", "user.email", "test@docker.com") - assert.NilError(t, err) + assert.NilError(t, os.WriteFile(filepath.Join(subrepoDir, "subfile"), []byte("subcontents"), 0644)) - _, err = gitWithinDir(subrepoDir, "config", "user.name", "Docker test") - assert.NilError(t, err) - - err = os.WriteFile(filepath.Join(subrepoDir, "subfile"), []byte("subcontents"), 0644) - assert.NilError(t, err) - - _, err = gitWithinDir(subrepoDir, "add", "-A") - assert.NilError(t, err) - - _, err = gitWithinDir(subrepoDir, "commit", "-am", "Subrepo initial") - assert.NilError(t, err) + must(gitWithinDir(subrepoDir, "add", "-A")) + must(gitWithinDir(subrepoDir, "commit", "-am", "Subrepo initial")) cmd := exec.Command("git", "submodule", "add", subrepoDir, "sub") // this command doesn't work with --work-tree cmd.Dir = gitDir - assert.NilError(t, cmd.Run()) + must(cmd.CombinedOutput()) - _, err = gitWithinDir(gitDir, "add", "-A") - assert.NilError(t, err) - - _, err = gitWithinDir(gitDir, "commit", "-am", "With submodule") - assert.NilError(t, err) + must(gitWithinDir(gitDir, "add", "-A")) + must(gitWithinDir(gitDir, "commit", "-am", "With submodule")) type singleCase struct { frag string @@ -299,28 +266,30 @@ func TestCheckoutGit(t *testing.T) { } for _, c := range cases { - ref, subdir := getRefAndSubdir(c.frag) - r, err := cloneGitRepo(gitRepo{remote: gitDir, ref: ref, subdir: subdir}) + t.Run(c.frag, func(t *testing.T) { + ref, subdir := getRefAndSubdir(c.frag) + r, err := cloneGitRepo(gitRepo{remote: gitDir, ref: ref, subdir: subdir}) - if c.fail { - assert.Check(t, is.ErrorContains(err, "")) - continue - } - assert.NilError(t, err) - defer os.RemoveAll(r) - if c.submodule { - b, err := os.ReadFile(filepath.Join(r, "sub/subfile")) + if c.fail { + assert.Check(t, is.ErrorContains(err, "")) + return + } assert.NilError(t, err) - assert.Check(t, is.Equal("subcontents", string(b))) - } else { - _, err := os.Stat(filepath.Join(r, "sub/subfile")) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Assert(t, os.IsNotExist(err)) - } + defer os.RemoveAll(r) + if c.submodule { + b, err := os.ReadFile(filepath.Join(r, "sub/subfile")) + assert.NilError(t, err) + assert.Check(t, is.Equal("subcontents", string(b))) + } else { + _, err := os.Stat(filepath.Join(r, "sub/subfile")) + assert.Assert(t, is.ErrorContains(err, "")) + assert.Assert(t, os.IsNotExist(err)) + } - b, err := os.ReadFile(filepath.Join(r, "Dockerfile")) - assert.NilError(t, err) - assert.Check(t, is.Equal(c.exp, string(b))) + b, err := os.ReadFile(filepath.Join(r, "Dockerfile")) + assert.NilError(t, err) + assert.Check(t, is.Equal(c.exp, string(b))) + }) } } From 0f7b0897ccfefb6cf5f4d024adbe8d419fb74773 Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Wed, 27 Jul 2022 16:05:13 -0400 Subject: [PATCH 2/6] builder: explicitly set CWD for all git commands Keep It Simple! Set the working directory for git commands by...setting the git process's working directory. Git commands can be run in the parent process's working directory by passing the empty string. Signed-off-by: Cory Snider --- builder/remotecontext/git/gitutils.go | 9 +++------ builder/remotecontext/git/gitutils_test.go | 12 ++++-------- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/builder/remotecontext/git/gitutils.go b/builder/remotecontext/git/gitutils.go index 1dd07851ed..bfcb6ab086 100644 --- a/builder/remotecontext/git/gitutils.go +++ b/builder/remotecontext/git/gitutils.go @@ -192,12 +192,9 @@ func checkoutGit(root, ref, subdir string) (string, error) { } func gitWithinDir(dir string, args ...string) ([]byte, error) { - a := []string{"--work-tree", dir, "--git-dir", filepath.Join(dir, ".git")} - return git(append(a, args...)...) -} - -func git(args ...string) ([]byte, error) { - return exec.Command("git", args...).CombinedOutput() + cmd := exec.Command("git", args...) + cmd.Dir = dir + return cmd.CombinedOutput() } // isGitTransport returns true if the provided str is a git transport by inspecting diff --git a/builder/remotecontext/git/gitutils_test.go b/builder/remotecontext/git/gitutils_test.go index 47b897fa2a..a91a6a084e 100644 --- a/builder/remotecontext/git/gitutils_test.go +++ b/builder/remotecontext/git/gitutils_test.go @@ -6,7 +6,6 @@ import ( "net/http/httptest" "net/url" "os" - "os/exec" "path/filepath" "runtime" "strings" @@ -160,7 +159,7 @@ func TestCloneArgsGit(t *testing.T) { } func gitGetConfig(name string) string { - b, err := git([]string{"config", "--get", name}...) + b, err := gitWithinDir("", "config", "--get", name) if err != nil { // since we are interested in empty or non empty string, // we can safely ignore the err here. @@ -191,7 +190,7 @@ func TestCheckoutGit(t *testing.T) { } gitDir := filepath.Join(root, "repo") - must(git("-c", "init.defaultBranch=master", "init", gitDir)) + must(gitWithinDir(root, "-c", "init.defaultBranch=master", "init", gitDir)) must(gitWithinDir(gitDir, "config", "user.email", "test@docker.com")) must(gitWithinDir(gitDir, "config", "user.name", "Docker test")) assert.NilError(t, os.WriteFile(filepath.Join(gitDir, "Dockerfile"), []byte("FROM scratch"), 0644)) @@ -218,7 +217,7 @@ func TestCheckoutGit(t *testing.T) { // set up submodule subrepoDir := filepath.Join(root, "subrepo") - must(git("-c", "init.defaultBranch=master", "init", subrepoDir)) + must(gitWithinDir(root, "-c", "init.defaultBranch=master", "init", subrepoDir)) must(gitWithinDir(subrepoDir, "config", "user.email", "test@docker.com")) must(gitWithinDir(subrepoDir, "config", "user.name", "Docker test")) @@ -227,10 +226,7 @@ func TestCheckoutGit(t *testing.T) { must(gitWithinDir(subrepoDir, "add", "-A")) must(gitWithinDir(subrepoDir, "commit", "-am", "Subrepo initial")) - cmd := exec.Command("git", "submodule", "add", subrepoDir, "sub") // this command doesn't work with --work-tree - cmd.Dir = gitDir - must(cmd.CombinedOutput()) - + must(gitWithinDir(gitDir, "submodule", "add", subrepoDir, "sub")) must(gitWithinDir(gitDir, "add", "-A")) must(gitWithinDir(gitDir, "commit", "-am", "With submodule")) From 72119f5d9bcce3f4901b7ef2bf4b8181fc1b8062 Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Thu, 28 Jul 2022 13:17:42 -0400 Subject: [PATCH 3/6] builder: isolate git from local system Prevent git commands we run from reading the user or system configuration, or cloning submodules from the local filesystem. Signed-off-by: Cory Snider --- builder/remotecontext/git/gitutils.go | 6 +++ builder/remotecontext/git/gitutils_test.go | 51 +++++++++++++++++++++- 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/builder/remotecontext/git/gitutils.go b/builder/remotecontext/git/gitutils.go index bfcb6ab086..b24dd52bc8 100644 --- a/builder/remotecontext/git/gitutils.go +++ b/builder/remotecontext/git/gitutils.go @@ -192,8 +192,14 @@ func checkoutGit(root, ref, subdir string) (string, error) { } func gitWithinDir(dir string, args ...string) ([]byte, error) { + args = append([]string{"-c", "protocol.file.allow=never"}, args...) // Block sneaky repositories from using repos from the filesystem as submodules. cmd := exec.Command("git", args...) cmd.Dir = dir + cmd.Env = append(cmd.Env, + "GIT_PROTOCOL_FROM_USER=0", // Disable unsafe remote protocols. + "GIT_CONFIG_NOSYSTEM=1", // Disable reading from system gitconfig. + "HOME=/dev/null", // Disable reading from user gitconfig. + ) return cmd.CombinedOutput() } diff --git a/builder/remotecontext/git/gitutils_test.go b/builder/remotecontext/git/gitutils_test.go index a91a6a084e..929030e178 100644 --- a/builder/remotecontext/git/gitutils_test.go +++ b/builder/remotecontext/git/gitutils_test.go @@ -1,11 +1,14 @@ package git // import "github.com/docker/docker/builder/remotecontext/git" import ( + "bytes" "fmt" "net/http" + "net/http/cgi" "net/http/httptest" "net/url" "os" + "os/exec" "path/filepath" "runtime" "strings" @@ -171,6 +174,49 @@ func gitGetConfig(name string) string { func TestCheckoutGit(t *testing.T) { root := t.TempDir() + gitpath, err := exec.LookPath("git") + assert.NilError(t, err) + gitversion, _ := exec.Command(gitpath, "version").CombinedOutput() + t.Logf("%s", gitversion) // E.g. "git version 2.30.2" + + // Serve all repositories under root using the Smart HTTP protocol so + // they can be cloned. The Dumb HTTP protocol is incompatible with + // shallow cloning but we unconditionally shallow-clone submodules, and + // we explicitly disable the file protocol. + // (Another option would be to use `git daemon` and the Git protocol, + // but that listens on a fixed port number which is a recipe for + // disaster in CI. Funnily enough, `git daemon --port=0` works but there + // is no easy way to discover which port got picked!) + + // Associate git-http-backend logs with the current (sub)test. + // Incompatible with parallel subtests. + currentSubtest := t + githttp := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var logs bytes.Buffer + (&cgi.Handler{ + Path: gitpath, + Args: []string{"http-backend"}, + Dir: root, + Env: []string{ + "GIT_PROJECT_ROOT=" + root, + "GIT_HTTP_EXPORT_ALL=1", + }, + Stderr: &logs, + }).ServeHTTP(w, r) + if logs.Len() == 0 { + return + } + for { + line, err := logs.ReadString('\n') + currentSubtest.Log("git-http-backend: " + line) + if err != nil { + break + } + } + }) + server := httptest.NewServer(&githttp) + defer server.Close() + autocrlf := gitGetConfig("core.autocrlf") if !(autocrlf == "true" || autocrlf == "false" || autocrlf == "input" || autocrlf == "") { @@ -226,7 +272,7 @@ func TestCheckoutGit(t *testing.T) { must(gitWithinDir(subrepoDir, "add", "-A")) must(gitWithinDir(subrepoDir, "commit", "-am", "Subrepo initial")) - must(gitWithinDir(gitDir, "submodule", "add", subrepoDir, "sub")) + must(gitWithinDir(gitDir, "submodule", "add", server.URL+"/subrepo", "sub")) must(gitWithinDir(gitDir, "add", "-A")) must(gitWithinDir(gitDir, "commit", "-am", "With submodule")) @@ -263,8 +309,9 @@ func TestCheckoutGit(t *testing.T) { for _, c := range cases { t.Run(c.frag, func(t *testing.T) { + currentSubtest = t ref, subdir := getRefAndSubdir(c.frag) - r, err := cloneGitRepo(gitRepo{remote: gitDir, ref: ref, subdir: subdir}) + r, err := cloneGitRepo(gitRepo{remote: server.URL + "/repo", ref: ref, subdir: subdir}) if c.fail { assert.Check(t, is.ErrorContains(err, "")) From 61acc9939fb5bbff6c63aa9399dff6cc9539b178 Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Thu, 13 Oct 2022 17:28:02 -0400 Subject: [PATCH 4/6] builder: make git config isolation opt-in While it is undesirable for the system or user git config to be used when the daemon clones a Git repo, it could break workflows if it was unconditionally applied to docker/cli as well. Signed-off-by: Cory Snider --- builder/remotecontext/git.go | 2 +- builder/remotecontext/git/gitutils.go | 61 +++++++++++++++------- builder/remotecontext/git/gitutils_test.go | 38 +++++++------- 3 files changed, 61 insertions(+), 40 deletions(-) diff --git a/builder/remotecontext/git.go b/builder/remotecontext/git.go index 1583ca28d0..85efba24f3 100644 --- a/builder/remotecontext/git.go +++ b/builder/remotecontext/git.go @@ -11,7 +11,7 @@ import ( // MakeGitContext returns a Context from gitURL that is cloned in a temporary directory. func MakeGitContext(gitURL string) (builder.Source, error) { - root, err := git.Clone(gitURL) + root, err := git.Clone(gitURL, git.WithIsolatedConfig(true)) if err != nil { return nil, err } diff --git a/builder/remotecontext/git/gitutils.go b/builder/remotecontext/git/gitutils.go index b24dd52bc8..e042e23971 100644 --- a/builder/remotecontext/git/gitutils.go +++ b/builder/remotecontext/git/gitutils.go @@ -16,21 +16,37 @@ type gitRepo struct { remote string ref string subdir string + + isolateConfig bool +} + +type CloneOption func(*gitRepo) + +// WithIsolatedConfig disables reading the user or system gitconfig files when +// performing Git operations. +func WithIsolatedConfig(v bool) CloneOption { + return func(gr *gitRepo) { + gr.isolateConfig = v + } } // Clone clones a repository into a newly created directory which // will be under "docker-build-git" -func Clone(remoteURL string) (string, error) { +func Clone(remoteURL string, opts ...CloneOption) (string, error) { repo, err := parseRemoteURL(remoteURL) if err != nil { return "", err } - return cloneGitRepo(repo) + for _, opt := range opts { + opt(&repo) + } + + return repo.clone() } -func cloneGitRepo(repo gitRepo) (checkoutDir string, err error) { +func (repo gitRepo) clone() (checkoutDir string, err error) { fetch := fetchArgs(repo.remote, repo.ref) root, err := os.MkdirTemp("", "docker-build-git") @@ -44,21 +60,21 @@ func cloneGitRepo(repo gitRepo) (checkoutDir string, err error) { } }() - if out, err := gitWithinDir(root, "init"); err != nil { + if out, err := repo.gitWithinDir(root, "init"); err != nil { return "", errors.Wrapf(err, "failed to init repo at %s: %s", root, out) } // Add origin remote for compatibility with previous implementation that // used "git clone" and also to make sure local refs are created for branches - if out, err := gitWithinDir(root, "remote", "add", "origin", repo.remote); err != nil { + if out, err := repo.gitWithinDir(root, "remote", "add", "origin", repo.remote); err != nil { return "", errors.Wrapf(err, "failed add origin repo at %s: %s", repo.remote, out) } - if output, err := gitWithinDir(root, fetch...); err != nil { + if output, err := repo.gitWithinDir(root, fetch...); err != nil { return "", errors.Wrapf(err, "error fetching: %s", output) } - checkoutDir, err = checkoutGit(root, repo.ref, repo.subdir) + checkoutDir, err = repo.checkout(root) if err != nil { return "", err } @@ -162,20 +178,20 @@ func supportsShallowClone(remoteURL string) bool { return true } -func checkoutGit(root, ref, subdir string) (string, error) { +func (repo gitRepo) checkout(root string) (string, error) { // Try checking out by ref name first. This will work on branches and sets // .git/HEAD to the current branch name - if output, err := gitWithinDir(root, "checkout", ref); err != nil { + if output, err := repo.gitWithinDir(root, "checkout", repo.ref); err != nil { // If checking out by branch name fails check out the last fetched ref - if _, err2 := gitWithinDir(root, "checkout", "FETCH_HEAD"); err2 != nil { - return "", errors.Wrapf(err, "error checking out %s: %s", ref, output) + if _, err2 := repo.gitWithinDir(root, "checkout", "FETCH_HEAD"); err2 != nil { + return "", errors.Wrapf(err, "error checking out %s: %s", repo.ref, output) } } - if subdir != "" { - newCtx, err := symlink.FollowSymlinkInScope(filepath.Join(root, subdir), root) + if repo.subdir != "" { + newCtx, err := symlink.FollowSymlinkInScope(filepath.Join(root, repo.subdir), root) if err != nil { - return "", errors.Wrapf(err, "error setting git context, %q not within git root", subdir) + return "", errors.Wrapf(err, "error setting git context, %q not within git root", repo.subdir) } fi, err := os.Stat(newCtx) @@ -191,15 +207,20 @@ func checkoutGit(root, ref, subdir string) (string, error) { return root, nil } -func gitWithinDir(dir string, args ...string) ([]byte, error) { +func (repo gitRepo) gitWithinDir(dir string, args ...string) ([]byte, error) { args = append([]string{"-c", "protocol.file.allow=never"}, args...) // Block sneaky repositories from using repos from the filesystem as submodules. cmd := exec.Command("git", args...) cmd.Dir = dir - cmd.Env = append(cmd.Env, - "GIT_PROTOCOL_FROM_USER=0", // Disable unsafe remote protocols. - "GIT_CONFIG_NOSYSTEM=1", // Disable reading from system gitconfig. - "HOME=/dev/null", // Disable reading from user gitconfig. - ) + // Disable unsafe remote protocols. + cmd.Env = append(cmd.Env, "GIT_PROTOCOL_FROM_USER=0") + + if repo.isolateConfig { + cmd.Env = append(cmd.Env, + "GIT_CONFIG_NOSYSTEM=1", // Disable reading from system gitconfig. + "HOME=/dev/null", // Disable reading from user gitconfig. + ) + } + return cmd.CombinedOutput() } diff --git a/builder/remotecontext/git/gitutils_test.go b/builder/remotecontext/git/gitutils_test.go index 929030e178..f3c67b3021 100644 --- a/builder/remotecontext/git/gitutils_test.go +++ b/builder/remotecontext/git/gitutils_test.go @@ -162,7 +162,7 @@ func TestCloneArgsGit(t *testing.T) { } func gitGetConfig(name string) string { - b, err := gitWithinDir("", "config", "--get", name) + b, err := gitRepo{}.gitWithinDir("", "config", "--get", name) if err != nil { // since we are interested in empty or non empty string, // we can safely ignore the err here. @@ -236,9 +236,9 @@ func TestCheckoutGit(t *testing.T) { } gitDir := filepath.Join(root, "repo") - must(gitWithinDir(root, "-c", "init.defaultBranch=master", "init", gitDir)) - must(gitWithinDir(gitDir, "config", "user.email", "test@docker.com")) - must(gitWithinDir(gitDir, "config", "user.name", "Docker test")) + must(gitRepo{}.gitWithinDir(root, "-c", "init.defaultBranch=master", "init", gitDir)) + must(gitRepo{}.gitWithinDir(gitDir, "config", "user.email", "test@docker.com")) + must(gitRepo{}.gitWithinDir(gitDir, "config", "user.name", "Docker test")) assert.NilError(t, os.WriteFile(filepath.Join(gitDir, "Dockerfile"), []byte("FROM scratch"), 0644)) subDir := filepath.Join(gitDir, "subdir") @@ -250,31 +250,31 @@ func TestCheckoutGit(t *testing.T) { assert.NilError(t, os.Symlink("/subdir", filepath.Join(gitDir, "absolutelink"))) } - must(gitWithinDir(gitDir, "add", "-A")) - must(gitWithinDir(gitDir, "commit", "-am", "First commit")) - must(gitWithinDir(gitDir, "checkout", "-b", "test")) + must(gitRepo{}.gitWithinDir(gitDir, "add", "-A")) + must(gitRepo{}.gitWithinDir(gitDir, "commit", "-am", "First commit")) + must(gitRepo{}.gitWithinDir(gitDir, "checkout", "-b", "test")) assert.NilError(t, os.WriteFile(filepath.Join(gitDir, "Dockerfile"), []byte("FROM scratch\nEXPOSE 3000"), 0644)) assert.NilError(t, os.WriteFile(filepath.Join(subDir, "Dockerfile"), []byte("FROM busybox\nEXPOSE 5000"), 0644)) - must(gitWithinDir(gitDir, "add", "-A")) - must(gitWithinDir(gitDir, "commit", "-am", "Branch commit")) - must(gitWithinDir(gitDir, "checkout", "master")) + must(gitRepo{}.gitWithinDir(gitDir, "add", "-A")) + must(gitRepo{}.gitWithinDir(gitDir, "commit", "-am", "Branch commit")) + must(gitRepo{}.gitWithinDir(gitDir, "checkout", "master")) // set up submodule subrepoDir := filepath.Join(root, "subrepo") - must(gitWithinDir(root, "-c", "init.defaultBranch=master", "init", subrepoDir)) - must(gitWithinDir(subrepoDir, "config", "user.email", "test@docker.com")) - must(gitWithinDir(subrepoDir, "config", "user.name", "Docker test")) + must(gitRepo{}.gitWithinDir(root, "-c", "init.defaultBranch=master", "init", subrepoDir)) + must(gitRepo{}.gitWithinDir(subrepoDir, "config", "user.email", "test@docker.com")) + must(gitRepo{}.gitWithinDir(subrepoDir, "config", "user.name", "Docker test")) assert.NilError(t, os.WriteFile(filepath.Join(subrepoDir, "subfile"), []byte("subcontents"), 0644)) - must(gitWithinDir(subrepoDir, "add", "-A")) - must(gitWithinDir(subrepoDir, "commit", "-am", "Subrepo initial")) + must(gitRepo{}.gitWithinDir(subrepoDir, "add", "-A")) + must(gitRepo{}.gitWithinDir(subrepoDir, "commit", "-am", "Subrepo initial")) - must(gitWithinDir(gitDir, "submodule", "add", server.URL+"/subrepo", "sub")) - must(gitWithinDir(gitDir, "add", "-A")) - must(gitWithinDir(gitDir, "commit", "-am", "With submodule")) + must(gitRepo{}.gitWithinDir(gitDir, "submodule", "add", server.URL+"/subrepo", "sub")) + must(gitRepo{}.gitWithinDir(gitDir, "add", "-A")) + must(gitRepo{}.gitWithinDir(gitDir, "commit", "-am", "With submodule")) type singleCase struct { frag string @@ -311,7 +311,7 @@ func TestCheckoutGit(t *testing.T) { t.Run(c.frag, func(t *testing.T) { currentSubtest = t ref, subdir := getRefAndSubdir(c.frag) - r, err := cloneGitRepo(gitRepo{remote: server.URL + "/repo", ref: ref, subdir: subdir}) + r, err := gitRepo{remote: server.URL + "/repo", ref: ref, subdir: subdir}.clone() if c.fail { assert.Check(t, is.ErrorContains(err, "")) From 94672c89cc7355e166ee198f2664644300ba9d5b Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Wed, 19 Oct 2022 20:11:27 -0400 Subject: [PATCH 5/6] builder: fix running git commands on Windows Setting cmd.Env overrides the default of passing through the parent process' environment, which works out fine most of the time, except when it doesn't. For whatever reason, leaving out all the environment causes git-for-windows sh.exe subprocesses to enter an infinite loop of access violations during Cygwin initialization in certain environments (specifically, our very own dev container image). Signed-off-by: Cory Snider --- builder/remotecontext/git/gitutils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder/remotecontext/git/gitutils.go b/builder/remotecontext/git/gitutils.go index e042e23971..25c6107bb6 100644 --- a/builder/remotecontext/git/gitutils.go +++ b/builder/remotecontext/git/gitutils.go @@ -212,7 +212,7 @@ func (repo gitRepo) gitWithinDir(dir string, args ...string) ([]byte, error) { cmd := exec.Command("git", args...) cmd.Dir = dir // Disable unsafe remote protocols. - cmd.Env = append(cmd.Env, "GIT_PROTOCOL_FROM_USER=0") + cmd.Env = append(cmd.Environ(), "GIT_PROTOCOL_FROM_USER=0") if repo.isolateConfig { cmd.Env = append(cmd.Env, From 67d010bd2c31a07bda06a279e163cdc2bd7d5c29 Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Thu, 20 Oct 2022 14:03:36 -0400 Subject: [PATCH 6/6] builder: add missing doc comment Signed-off-by: Cory Snider --- builder/remotecontext/git/gitutils.go | 1 + 1 file changed, 1 insertion(+) diff --git a/builder/remotecontext/git/gitutils.go b/builder/remotecontext/git/gitutils.go index 25c6107bb6..6af957c40f 100644 --- a/builder/remotecontext/git/gitutils.go +++ b/builder/remotecontext/git/gitutils.go @@ -20,6 +20,7 @@ type gitRepo struct { isolateConfig bool } +// CloneOption changes the behaviour of Clone(). type CloneOption func(*gitRepo) // WithIsolatedConfig disables reading the user or system gitconfig files when