git: Remove hack to conditionally ignore gitconfig in testspks-gitaly-introduce-ignore-gitconfig

When running our tests, we want Git to use a well-known configuration.
It is thus mandatory that it never picks up the user's gitconfig files,
or otherwise tests may fail due to misconfiguration. To fix this, we
have added a hack that checks whether the currently executing binary has
a `.test` suffix: if it does we assume to be running our tests, and thus
inject a set of environment variable to ignore the gitconfig files.

We now have a better alternative with the newly introduced configuration
to ignore gitconfig files. So let's remove the hack and set this entry
as required. This also proves that the configuration works as expected.

6 files changed, 60 insertions, 16 deletions

diff --git a/internal/git/command_factory.go b/internal/git/command_factory.go
index 8fe94a63e..2f2ded567 100644
--- a/internal/git/command_factory.go
+++ b/internal/git/command_factory.go
@@ -7,7 +7,6 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
-	"strings"
 	"sync"
 
 	"github.com/prometheus/client_golang/prometheus"
@@ -157,14 +156,11 @@ func setupGitExecutionEnvironments(cfg config.Cfg, factoryCfg execCommandFactory
 	}
 
 	// Prevent the environment from affecting git calls by ignoring the configuration files.
-	//
 	// This should be done always but we have to wait until 15.0 due to backwards compatibility
-	// concerns. To fix tests ahead to 15.0, we ignore the global configuration when the package
-	// has been built under tests. `go test` uses a `.test` suffix on the test binaries. We use
-	// that to check whether to ignore the globals or not.
+	// concerns.
 	//
 	// See https://gitlab.com/gitlab-org/gitaly/-/issues/3617.
-	if strings.HasSuffix(os.Args[0], ".test") || cfg.Git.IgnoreGitconfig {
+	if cfg.Git.IgnoreGitconfig {
 		sharedEnvironment = append(sharedEnvironment,
 			"GIT_CONFIG_GLOBAL=/dev/null",
 			"GIT_CONFIG_SYSTEM=/dev/null",
diff --git a/internal/git/command_factory_cgroup_test.go b/internal/git/command_factory_cgroup_test.go
index 0ef61fcd3..69c3126bf 100644
--- a/internal/git/command_factory_cgroup_test.go
+++ b/internal/git/command_factory_cgroup_test.go
@@ -40,7 +40,11 @@ func TestNewCommandAddsToCgroup(t *testing.T) {
 	root := testhelper.TempDir(t)
 
 	cfg := config.Cfg{
+		BinDir:     filepath.Join(root, "bin.d"),
 		SocketPath: "/path/to/socket",
+		Git: config.Git{
+			IgnoreGitconfig: true,
+		},
 		Cgroups: cgroups.Config{
 			Repositories: cgroups.Repositories{
 				Count: 1,
@@ -50,7 +54,6 @@ func TestNewCommandAddsToCgroup(t *testing.T) {
 			Name: "storage-1",
 			Path: root,
 		}},
-		BinDir: filepath.Join(root, "bin.d"),
 	}
 
 	require.NoError(t, os.MkdirAll(cfg.BinDir, 0o755))
diff --git a/internal/git/command_factory_test.go b/internal/git/command_factory_test.go
index 3c3aec4fb..d709dd130 100644
--- a/internal/git/command_factory_test.go
+++ b/internal/git/command_factory_test.go
@@ -165,9 +165,27 @@ func TestCommandFactory_ExecutionEnvironment(t *testing.T) {
 		require.Equal(t, expectedExecEnv.EnvironmentVariables, actualExecEnv.EnvironmentVariables)
 	}
 
+	t.Run("set in config without ignored gitconfig", func(t *testing.T) {
+		assertExecEnv(t, config.Cfg{
+			Git: config.Git{
+				BinPath:         "/path/to/myGit",
+				IgnoreGitconfig: false,
+			},
+		}, git.ExecutionEnvironment{
+			BinaryPath: "/path/to/myGit",
+			EnvironmentVariables: []string{
+				"LANG=en_US.UTF-8",
+				"GIT_TERMINAL_PROMPT=0",
+			},
+		})
+	})
+
 	t.Run("set in config", func(t *testing.T) {
 		assertExecEnv(t, config.Cfg{
-			Git: config.Git{BinPath: "/path/to/myGit"},
+			Git: config.Git{
+				BinPath:         "/path/to/myGit",
+				IgnoreGitconfig: true,
+			},
 		}, git.ExecutionEnvironment{
 			BinaryPath: "/path/to/myGit",
 			EnvironmentVariables: []string{
@@ -183,7 +201,11 @@ func TestCommandFactory_ExecutionEnvironment(t *testing.T) {
 	t.Run("set using GITALY_TESTING_GIT_BINARY", func(t *testing.T) {
 		testhelper.ModifyEnvironment(t, "GITALY_TESTING_GIT_BINARY", "/path/to/env_git")
 
-		assertExecEnv(t, config.Cfg{Git: config.Git{}}, git.ExecutionEnvironment{
+		assertExecEnv(t, config.Cfg{
+			Git: config.Git{
+				IgnoreGitconfig: true,
+			},
+		}, git.ExecutionEnvironment{
 			BinaryPath: "/path/to/env_git",
 			EnvironmentVariables: []string{
 				"LANG=en_US.UTF-8",
@@ -283,7 +305,11 @@ func TestCommandFactory_ExecutionEnvironment(t *testing.T) {
 		resolvedPath, err := exec.LookPath("git")
 		require.NoError(t, err)
 
-		assertExecEnv(t, config.Cfg{}, git.ExecutionEnvironment{
+		assertExecEnv(t, config.Cfg{
+			Git: config.Git{
+				IgnoreGitconfig: true,
+			},
+		}, git.ExecutionEnvironment{
 			BinaryPath: resolvedPath,
 			EnvironmentVariables: []string{
 				"LANG=en_US.UTF-8",
diff --git a/internal/git/command_options_test.go b/internal/git/command_options_test.go
index 884eeb35d..88f41d2d0 100644
--- a/internal/git/command_options_test.go
+++ b/internal/git/command_options_test.go
@@ -203,7 +203,12 @@ func TestGlobalOption(t *testing.T) {
 }
 
 func TestWithConfig(t *testing.T) {
-	cfg := config.Cfg{BinDir: testhelper.TempDir(t)}
+	cfg := config.Cfg{
+		BinDir: testhelper.TempDir(t),
+		Git: config.Git{
+			IgnoreGitconfig: true,
+		},
+	}
 
 	ctx := testhelper.Context(t)
 
@@ -274,11 +279,16 @@ func TestWithConfig(t *testing.T) {
 }
 
 func TestExecCommandFactoryGitalyConfigOverrides(t *testing.T) {
-	cfg := config.Cfg{BinDir: testhelper.TempDir(t)}
-
-	cfg.Git.Config = []config.GitConfig{
-		{Key: "foo.bar", Value: "from-gitaly-config"},
+	cfg := config.Cfg{
+		BinDir: testhelper.TempDir(t),
+		Git: config.Git{
+			Config: []config.GitConfig{
+				{Key: "foo.bar", Value: "from-gitaly-config"},
+			},
+			IgnoreGitconfig: true,
+		},
 	}
+
 	ctx := testhelper.Context(t)
 
 	gitCmdFactory := newCommandFactory(t, cfg, WithSkipHooks())
@@ -299,7 +309,12 @@ func TestExecCommandFactoryGitalyConfigOverrides(t *testing.T) {
 }
 
 func TestWithConfigEnv(t *testing.T) {
-	cfg := config.Cfg{BinDir: testhelper.TempDir(t)}
+	cfg := config.Cfg{
+		BinDir: testhelper.TempDir(t),
+		Git: config.Git{
+			IgnoreGitconfig: true,
+		},
+	}
 
 	ctx := testhelper.Context(t)
 
diff --git a/internal/git/gittest/testhelper_test.go b/internal/git/gittest/testhelper_test.go
index 0dc03b44b..2eebc49be 100644
--- a/internal/git/gittest/testhelper_test.go
+++ b/internal/git/gittest/testhelper_test.go
@@ -26,6 +26,7 @@ func setup(t testing.TB) (config.Cfg, *gitalypb.Repository, string) {
 	var cfg config.Cfg
 
 	cfg.SocketPath = "it is a stub to bypass Validate method"
+	cfg.Git.IgnoreGitconfig = true
 
 	cfg.Storages = []config.Storage{
 		{
diff --git a/internal/testhelper/testcfg/gitaly_builder.go b/internal/testhelper/testcfg/gitaly_builder.go
index 3894ffada..0159525ed 100644
--- a/internal/testhelper/testcfg/gitaly_builder.go
+++ b/internal/testhelper/testcfg/gitaly_builder.go
@@ -143,6 +143,9 @@ func (gc *GitalyCfgBuilder) Build(t testing.TB) config.Cfg {
 	}
 
 	cfg.PackObjectsCache.Enabled = gc.packObjectsCacheEnabled
+	// Ignore the gitconfig so that tests aren't impacted by any configuration the user happens
+	// to have lying around.
+	cfg.Git.IgnoreGitconfig = true
 
 	require.NoError(t, cfg.Validate())