From 13aa5fa027597b36e10491d0b413d90314a81e68 Mon Sep 17 00:00:00 2001 From: manjari Date: Fri, 2 Oct 2026 19:21:16 +0000 Subject: [PATCH] refactor(platform): Move GitRuntime into platform/git/exec as Runtime --- platform/git/exec/BUILD.bazel | 6 +- platform/git/exec/runtime.go | 88 +++++++++++++ platform/git/exec/runtime_test.go | 124 ++++++++++++++++++ runway/extension/merger/git/git_merger.go | 74 +---------- .../extension/merger/git/git_merger_test.go | 116 +--------------- service/runway/server/BUILD.bazel | 1 - service/runway/server/checkout.go | 9 +- service/runway/server/checkout_test.go | 8 +- service/runway/server/gitruntime.go | 12 +- service/runway/server/main.go | 5 +- 10 files changed, 242 insertions(+), 201 deletions(-) create mode 100644 platform/git/exec/runtime.go create mode 100644 platform/git/exec/runtime_test.go diff --git a/platform/git/exec/BUILD.bazel b/platform/git/exec/BUILD.bazel index a834acc9b..0df02c8e9 100644 --- a/platform/git/exec/BUILD.bazel +++ b/platform/git/exec/BUILD.bazel @@ -5,6 +5,7 @@ go_library( srcs = [ "command_error.go", "gitexec.go", + "runtime.go", ], importpath = "github.com/uber/submitqueue/platform/git/exec", visibility = ["//visibility:public"], @@ -12,7 +13,10 @@ go_library( go_test( name = "go_default_test", - srcs = ["gitexec_test.go"], + srcs = [ + "gitexec_test.go", + "runtime_test.go", + ], embed = [":go_default_library"], deps = [ "@com_github_stretchr_testify//assert:go_default_library", diff --git a/platform/git/exec/runtime.go b/platform/git/exec/runtime.go new file mode 100644 index 000000000..c29930694 --- /dev/null +++ b/platform/git/exec/runtime.go @@ -0,0 +1,88 @@ +// Copyright (c) 2026 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package gitexec + +import ( + "context" + "fmt" + "os/exec" + "path/filepath" +) + +// Runtime identifies the explicitly provided Git runtime that commands run on. +type Runtime struct { + // Executable is the absolute path to the Git executable. + Executable string + // ExecPath is the absolute directory containing Git's helper executables. + ExecPath string + // TemplateDir is the absolute directory containing Git's repository + // templates. + TemplateDir string + // PassthroughEnv names additional environment variables to inherit from + // the parent process, on top of the auth and transport ones always passed + // through. For a deployment whose remote needs something unusual; leave + // empty otherwise. Names that could alter merge semantics do not belong + // here — the scrubbed environment is what keeps a merge reproducible. + PassthroughEnv []string +} + +// Validate reports whether every path of the runtime is set and absolute. +func (r Runtime) Validate() error { + for _, field := range []struct{ name, path string }{ + {"executable", r.Executable}, + {"exec path", r.ExecPath}, + {"template dir", r.TemplateDir}, + } { + if field.path == "" { + return fmt.Errorf("git runtime %s is required", field.name) + } + if !filepath.IsAbs(field.path) { + return fmt.Errorf("git runtime %s must be absolute: %q", field.name, field.path) + } + } + return nil +} + +// Command constructs a Git command without inheriting the caller's +// environment. The executable and helper paths come from the pinned runtime; +// repository-local configuration remains an intentional input. +func (r Runtime) Command(ctx context.Context, dir string, args ...string) *exec.Cmd { + gitArgs := make([]string, 0, len(args)+3) + gitArgs = append(gitArgs, + "--exec-path="+r.ExecPath, + "-c", "init.templateDir="+r.TemplateDir, + ) + gitArgs = append(gitArgs, args...) + + cmd := exec.CommandContext(ctx, r.Executable, gitArgs...) + cmd.Dir = dir + // HOME and XDG_CONFIG_HOME are isolated to the checkout rather than inherited, + // so the runtime's literals — appended last — override any HOME a deployment + // passed through. The remaining literals pin git's runtime; the scrub set and + // transport variables come from Env. + cmd.Env = Env(EnvOptions{ + Transport: true, + Passthrough: r.PassthroughEnv, + Literal: []string{ + "HOME=" + filepath.Join(dir, ".submitqueue-git-home"), + "XDG_CONFIG_HOME=" + filepath.Join(dir, ".submitqueue-git-home", "xdg"), + "GIT_EXEC_PATH=" + r.ExecPath, + "GIT_TEMPLATE_DIR=" + r.TemplateDir, + "LC_ALL=C", + "LANG=C", + }, + }) + return cmd +} diff --git a/platform/git/exec/runtime_test.go b/platform/git/exec/runtime_test.go new file mode 100644 index 000000000..62b3b03d0 --- /dev/null +++ b/platform/git/exec/runtime_test.go @@ -0,0 +1,124 @@ +// Copyright (c) 2026 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package gitexec + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func dummyRuntime(t *testing.T) Runtime { + t.Helper() + dir := t.TempDir() + return Runtime{ + Executable: filepath.Join(dir, "git"), + ExecPath: filepath.Join(dir, "git-core"), + TemplateDir: filepath.Join(dir, "templates"), + } +} + +func TestRuntimeValidate(t *testing.T) { + valid := dummyRuntime(t) + tests := []struct { + name string + mutate func(*Runtime) + wantErr bool + }{ + {name: "valid", mutate: func(*Runtime) {}}, + {name: "missing executable", mutate: func(r *Runtime) { r.Executable = "" }, wantErr: true}, + {name: "relative exec path", mutate: func(r *Runtime) { r.ExecPath = "git-core" }, wantErr: true}, + {name: "relative template dir", mutate: func(r *Runtime) { r.TemplateDir = "templates" }, wantErr: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + runtime := valid + tt.mutate(&runtime) + err := runtime.Validate() + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + }) + } +} + +func TestRuntimeCommandDoesNotInheritEnvironment(t *testing.T) { + t.Setenv("SUBMITQUEUE_GIT_AMBIENT", "ambient") + runtime := dummyRuntime(t) + + cmd := runtime.Command(context.Background(), t.TempDir(), "--version") + + assert.Equal(t, runtime.Executable, cmd.Path) + assert.NotContains(t, cmd.Env, "SUBMITQUEUE_GIT_AMBIENT=ambient") + assert.Contains(t, cmd.Env, "GIT_CONFIG_NOSYSTEM=1") + assert.Contains(t, cmd.Env, "GIT_EXEC_PATH="+runtime.ExecPath) +} + +func TestRuntimeCommandPreservesAuthEnvironment(t *testing.T) { + // Scrubbing denies git ambient configuration; it must not also deny it the + // means to reach the remote. Without the agent socket an SSH remote cannot + // authenticate, and without PATH git cannot even exec ssh. + t.Setenv("SSH_AUTH_SOCK", "/tmp/agent.sock") + t.Setenv("PATH", "/usr/bin:/bin") + t.Setenv("HTTPS_PROXY", "http://proxy.example.com:3128") + t.Setenv("SUBMITQUEUE_GIT_AMBIENT", "ambient") + + cmd := dummyRuntime(t).Command(context.Background(), t.TempDir(), "--version") + + assert.Contains(t, cmd.Env, "SSH_AUTH_SOCK=/tmp/agent.sock") + assert.Contains(t, cmd.Env, "PATH=/usr/bin:/bin") + assert.Contains(t, cmd.Env, "HTTPS_PROXY=http://proxy.example.com:3128") + assert.NotContains(t, cmd.Env, "SUBMITQUEUE_GIT_AMBIENT=ambient") +} + +func TestRuntimeCommandOmitsUnsetAuthEnvironment(t *testing.T) { + // An unset variable must be omitted rather than exported empty: an empty + // SSH_AUTH_SOCK tells ssh there is no agent instead of letting it look. + t.Setenv("SSH_AUTH_SOCK", "") + require.NoError(t, os.Unsetenv("SSH_AUTH_SOCK")) + + cmd := dummyRuntime(t).Command(context.Background(), t.TempDir(), "--version") + + for _, entry := range cmd.Env { + assert.False(t, strings.HasPrefix(entry, "SSH_AUTH_SOCK="), "unset variable leaked as %q", entry) + } +} + +func TestRuntimeCommandPassesThroughExtraEnvironment(t *testing.T) { + t.Setenv("SUBMITQUEUE_CUSTOM_TRANSPORT", "value") + runtime := dummyRuntime(t) + runtime.PassthroughEnv = []string{"SUBMITQUEUE_CUSTOM_TRANSPORT"} + + cmd := runtime.Command(context.Background(), t.TempDir(), "--version") + + assert.Contains(t, cmd.Env, "SUBMITQUEUE_CUSTOM_TRANSPORT=value") +} + +func TestRuntimeCommandIsolatesHome(t *testing.T) { + t.Setenv("HOME", "/ambient/home") + dir := t.TempDir() + + cmd := dummyRuntime(t).Command(context.Background(), dir, "--version") + + assert.Contains(t, cmd.Env, "HOME="+filepath.Join(dir, ".submitqueue-git-home")) + assert.NotContains(t, cmd.Env, "HOME=/ambient/home") +} diff --git a/runway/extension/merger/git/git_merger.go b/runway/extension/merger/git/git_merger.go index af46eac03..ec1f91bd1 100644 --- a/runway/extension/merger/git/git_merger.go +++ b/runway/extension/merger/git/git_merger.go @@ -69,7 +69,6 @@ import ( "errors" "fmt" "os/exec" - "path/filepath" "strings" "sync" @@ -97,23 +96,6 @@ const ( defaultCommitterEmail = "runway@submitqueue.invalid" ) -// GitRuntime identifies the explicitly provided Git runtime used by the Merger. -type GitRuntime struct { - // Executable is the absolute path to the Git executable. - Executable string - // ExecPath is the absolute directory containing Git's helper executables. - ExecPath string - // TemplateDir is the absolute directory containing Git's repository - // templates. - TemplateDir string - // PassthroughEnv names additional environment variables to inherit from - // the parent process, on top of the auth and transport ones always passed - // through. For a deployment whose remote needs something unusual; leave - // empty otherwise. Names that could alter merge semantics do not belong - // here — the scrubbed environment is what keeps a merge reproducible. - PassthroughEnv []string -} - // Params holds the dependencies for the git Merger. type Params struct { // CheckoutPath is the absolute path to an existing git checkout that the @@ -128,7 +110,7 @@ type Params struct { // concrete strategy (REBASE, SQUASH_REBASE, MERGE, or PROMOTE). DefaultStrategy mergestrategypb.Strategy // Runtime is the pinned Git runtime used for every invocation. - Runtime GitRuntime + Runtime gitexec.Runtime // MaxPushAttempts caps how many times a committing merge retries the full // reset/apply/push cycle when the remote tip moves under it. Defaults to // defaultMaxPushAttempts when zero or negative. @@ -166,7 +148,7 @@ type gitMerger struct { remote string target string defaultStrategy mergestrategypb.Strategy - runtime GitRuntime + runtime gitexec.Runtime maxPushAttempts int fetchRefspecs []string checkStaleness bool @@ -201,7 +183,7 @@ type resolvedStep struct { // The checkout must already exist and have the configured remote. Runtime paths // must be absolute and DefaultStrategy must be a concrete strategy. func NewMerger(params Params) (merger.Merger, error) { - if err := params.Runtime.validate(); err != nil { + if err := params.Runtime.Validate(); err != nil { return nil, err } if !isConcreteStrategy(params.DefaultStrategy) { @@ -237,22 +219,6 @@ func NewMerger(params Params) (merger.Merger, error) { }, nil } -func (r GitRuntime) validate() error { - for name, path := range map[string]string{ - "executable": r.Executable, - "exec path": r.ExecPath, - "template dir": r.TemplateDir, - } { - if path == "" { - return fmt.Errorf("git runtime %s is required", name) - } - if !filepath.IsAbs(path) { - return fmt.Errorf("git runtime %s must be absolute: %q", name, path) - } - } - return nil -} - // CheckMergeability applies the request's steps as a dry run: it verifies each // step applies cleanly without committing to the remote, and returns per-step // results with empty Outputs. @@ -1025,43 +991,11 @@ func (m *gitMerger) commandAs(ctx context.Context, author authorIdent, args ...s "-c", "commit.gpgsign=false", ) withIdentity = append(withIdentity, args...) - cmd := newGitCommand(ctx, m.runtime, m.checkoutPath, withIdentity...) + cmd := m.runtime.Command(ctx, m.checkoutPath, withIdentity...) cmd.Env = append(cmd.Env, author.env()...) return cmd } -// newGitCommand constructs a Git command without inheriting the caller's -// environment. The executable and helper paths come from the pinned runtime; -// repository-local configuration remains an intentional input. -func newGitCommand(ctx context.Context, runtime GitRuntime, dir string, args ...string) *exec.Cmd { - gitArgs := make([]string, 0, len(args)+3) - gitArgs = append(gitArgs, - "--exec-path="+runtime.ExecPath, - "-c", "init.templateDir="+runtime.TemplateDir, - ) - gitArgs = append(gitArgs, args...) - - cmd := exec.CommandContext(ctx, runtime.Executable, gitArgs...) - cmd.Dir = dir - // HOME and XDG_CONFIG_HOME are isolated to the checkout rather than inherited, - // so the runtime's literals — appended last — override any HOME a deployment - // passed through. The remaining literals pin git's runtime; the scrub set and - // transport variables come from the shared composer. - cmd.Env = gitexec.Env(gitexec.EnvOptions{ - Transport: true, - Passthrough: runtime.PassthroughEnv, - Literal: []string{ - "HOME=" + filepath.Join(dir, ".submitqueue-git-home"), - "XDG_CONFIG_HOME=" + filepath.Join(dir, ".submitqueue-git-home", "xdg"), - "GIT_EXEC_PATH=" + runtime.ExecPath, - "GIT_TEMPLATE_DIR=" + runtime.TemplateDir, - "LC_ALL=C", - "LANG=C", - }, - }) - return cmd -} - // isConcreteStrategy reports whether s names a concrete integration strategy // (i.e. not DEFAULT and not an unknown value). func isConcreteStrategy(s mergestrategypb.Strategy) bool { diff --git a/runway/extension/merger/git/git_merger_test.go b/runway/extension/merger/git/git_merger_test.go index 7314db6a4..d2fde7961 100644 --- a/runway/extension/merger/git/git_merger_test.go +++ b/runway/extension/merger/git/git_merger_test.go @@ -91,114 +91,6 @@ func setupGitFixture(t *testing.T) gitFixture { } } -func TestGitRuntimeValidate(t *testing.T) { - abs, err := filepath.Abs("git") - require.NoError(t, err) - tests := []struct { - name string - runtime GitRuntime - wantErr bool - }{ - { - name: "valid", - runtime: GitRuntime{ - Executable: abs, - ExecPath: abs, - TemplateDir: abs, - }, - }, - { - name: "missing executable", - runtime: GitRuntime{ - ExecPath: abs, - TemplateDir: abs, - }, - wantErr: true, - }, - { - name: "relative exec path", - runtime: GitRuntime{ - Executable: abs, - ExecPath: "git-core", - TemplateDir: abs, - }, - wantErr: true, - }, - { - name: "relative template dir", - runtime: GitRuntime{ - Executable: abs, - ExecPath: abs, - TemplateDir: "templates", - }, - wantErr: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - err := tt.runtime.validate() - if tt.wantErr { - require.Error(t, err) - return - } - require.NoError(t, err) - }) - } -} - -func TestNewGitCommandDoesNotInheritEnvironment(t *testing.T) { - t.Setenv("SUBMITQUEUE_GIT_AMBIENT", "ambient") - runtime := testGitRuntime(t) - - cmd := newGitCommand(context.Background(), runtime, t.TempDir(), "--version") - - assert.Equal(t, runtime.Executable, cmd.Path) - assert.NotContains(t, cmd.Env, "SUBMITQUEUE_GIT_AMBIENT=ambient") - assert.Contains(t, cmd.Env, "GIT_CONFIG_NOSYSTEM=1") - assert.Contains(t, cmd.Env, "GIT_EXEC_PATH="+runtime.ExecPath) -} - -func TestNewGitCommandPreservesAuthEnvironment(t *testing.T) { - // Scrubbing denies git ambient configuration; it must not also deny it the - // means to reach the remote. Without the agent socket an SSH remote cannot - // authenticate, and without PATH git cannot even exec ssh. - t.Setenv("SSH_AUTH_SOCK", "/tmp/agent.sock") - t.Setenv("PATH", "/usr/bin:/bin") - t.Setenv("HTTPS_PROXY", "http://proxy.example.com:3128") - t.Setenv("SUBMITQUEUE_GIT_AMBIENT", "ambient") - - cmd := newGitCommand(context.Background(), testGitRuntime(t), t.TempDir(), "--version") - - assert.Contains(t, cmd.Env, "SSH_AUTH_SOCK=/tmp/agent.sock") - assert.Contains(t, cmd.Env, "PATH=/usr/bin:/bin") - assert.Contains(t, cmd.Env, "HTTPS_PROXY=http://proxy.example.com:3128") - assert.NotContains(t, cmd.Env, "SUBMITQUEUE_GIT_AMBIENT=ambient") -} - -func TestNewGitCommandOmitsUnsetAuthEnvironment(t *testing.T) { - // An unset variable must be omitted rather than exported empty: an empty - // SSH_AUTH_SOCK tells ssh there is no agent instead of letting it look. - t.Setenv("SSH_AUTH_SOCK", "") - require.NoError(t, os.Unsetenv("SSH_AUTH_SOCK")) - - cmd := newGitCommand(context.Background(), testGitRuntime(t), t.TempDir(), "--version") - - for _, entry := range cmd.Env { - assert.False(t, strings.HasPrefix(entry, "SSH_AUTH_SOCK="), "unset variable leaked as %q", entry) - } -} - -func TestNewGitCommandPassesThroughExtraEnvironment(t *testing.T) { - t.Setenv("SUBMITQUEUE_CUSTOM_TRANSPORT", "value") - runtime := testGitRuntime(t) - runtime.PassthroughEnv = []string{"SUBMITQUEUE_CUSTOM_TRANSPORT"} - - cmd := newGitCommand(context.Background(), runtime, t.TempDir(), "--version") - - assert.Contains(t, cmd.Env, "SUBMITQUEUE_CUSTOM_TRANSPORT=value") -} - func TestCherryPickRange_NonConflictFailureIsRetryable(t *testing.T) { // A cherry-pick can exit non-zero for reasons that have nothing to do with // the change colliding — a bad revision here, standing in for a missing @@ -1745,7 +1637,7 @@ func (f gitFixture) pushMultiCommitPRAs(t *testing.T, branch string, commits ... func mustGit(t *testing.T, dir string, args ...string) { t.Helper() - cmd := newGitCommand(context.Background(), testGitRuntime(t), dir, args...) + cmd := testGitRuntime(t).Command(context.Background(), dir, args...) var stderr bytes.Buffer cmd.Stderr = &stderr require.NoError(t, cmd.Run(), "git %s: %s", strings.Join(args, " "), stderr.String()) @@ -1753,7 +1645,7 @@ func mustGit(t *testing.T, dir string, args ...string) { func mustGitOutput(t *testing.T, dir string, args ...string) []byte { t.Helper() - cmd := newGitCommand(context.Background(), testGitRuntime(t), dir, args...) + cmd := testGitRuntime(t).Command(context.Background(), dir, args...) var stdout, stderr bytes.Buffer cmd.Stdout = &stdout cmd.Stderr = &stderr @@ -1761,11 +1653,11 @@ func mustGitOutput(t *testing.T, dir string, args ...string) []byte { return stdout.Bytes() } -func testGitRuntime(t *testing.T) GitRuntime { +func testGitRuntime(t *testing.T) gitexec.Runtime { t.Helper() executable := gitexectest.Git(t) templateDescription := gitexectest.Runfile(t, "SUBMITQUEUE_TEST_GIT_TEMPLATE_DESCRIPTION") - return GitRuntime{ + return gitexec.Runtime{ Executable: executable, ExecPath: filepath.Dir(executable), TemplateDir: filepath.Dir(templateDescription), diff --git a/service/runway/server/BUILD.bazel b/service/runway/server/BUILD.bazel index 8cdde8575..1c6aad0df 100644 --- a/service/runway/server/BUILD.bazel +++ b/service/runway/server/BUILD.bazel @@ -110,7 +110,6 @@ go_test( "//platform/git/exec:go_default_library", "//platform/git/exectest:go_default_library", "//runway/controller/dlq:go_default_library", - "//runway/extension/merger/git:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", "@com_github_stretchr_testify//require:go_default_library", "@com_github_uber_go_tally//:go_default_library", diff --git a/service/runway/server/checkout.go b/service/runway/server/checkout.go index 4c12992a5..20f99f348 100644 --- a/service/runway/server/checkout.go +++ b/service/runway/server/checkout.go @@ -27,7 +27,6 @@ import ( "go.uber.org/zap" gitexec "github.com/uber/submitqueue/platform/git/exec" - gitmerger "github.com/uber/submitqueue/runway/extension/merger/git" ) // credentialFile is the name, inside the checkout's .git directory, of the @@ -50,7 +49,7 @@ const credentialFile = "submitqueue-credentials.config" // It is idempotent: an existing checkout has its remote URL and credential // brought back in line rather than being recreated, so a restart against a // persisted volume costs nothing and a rotated token takes effect. -func provisionCheckout(ctx context.Context, logger *zap.SugaredLogger, runtime gitmerger.GitRuntime, cfg mergerConfig) error { +func provisionCheckout(ctx context.Context, logger *zap.SugaredLogger, runtime gitexec.Runtime, cfg mergerConfig) error { if err := os.MkdirAll(cfg.CheckoutPath, 0o755); err != nil { return fmt.Errorf("create checkout dir %q: %w", cfg.CheckoutPath, err) } @@ -98,7 +97,7 @@ func provisionCheckout(ctx context.Context, logger *zap.SugaredLogger, runtime g // initRepository creates the repository if the path does not already hold one, // reporting whether it had to. An existing repository is left in place: it may // carry fetched objects worth keeping, and re-creating it would discard them. -func initRepository(ctx context.Context, runtime gitmerger.GitRuntime, cfg mergerConfig) (bool, error) { +func initRepository(ctx context.Context, runtime gitexec.Runtime, cfg mergerConfig) (bool, error) { if info, err := os.Stat(filepath.Join(cfg.CheckoutPath, ".git")); err == nil && info.IsDir() { return false, nil } @@ -110,7 +109,7 @@ func initRepository(ctx context.Context, runtime gitmerger.GitRuntime, cfg merge // configureRemote points the configured remote name at the configured URL, // adding it when absent and correcting it when it has drifted. -func configureRemote(ctx context.Context, runtime gitmerger.GitRuntime, cfg mergerConfig) error { +func configureRemote(ctx context.Context, runtime gitexec.Runtime, cfg mergerConfig) error { out, err := runGit(ctx, runtime, cfg.CheckoutPath, "remote") if err != nil { return fmt.Errorf("list remotes in %q: %w", cfg.CheckoutPath, err) @@ -194,7 +193,7 @@ func setLocalConfig(checkoutPath, key, value string) error { // configuration but retaining what is needed to reach a remote. It composes that // environment through gitexec.Env, the same source the merger uses, so // provisioning and merging authenticate — and behave — identically. -func runGit(ctx context.Context, runtime gitmerger.GitRuntime, dir string, args ...string) ([]byte, error) { +func runGit(ctx context.Context, runtime gitexec.Runtime, dir string, args ...string) ([]byte, error) { full := append([]string{ "--exec-path=" + runtime.ExecPath, "-c", "init.templateDir=" + runtime.TemplateDir, diff --git a/service/runway/server/checkout_test.go b/service/runway/server/checkout_test.go index d85de4536..bf1a74edd 100644 --- a/service/runway/server/checkout_test.go +++ b/service/runway/server/checkout_test.go @@ -26,17 +26,17 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/zap/zaptest" + gitexec "github.com/uber/submitqueue/platform/git/exec" gitexectest "github.com/uber/submitqueue/platform/git/exectest" - gitmerger "github.com/uber/submitqueue/runway/extension/merger/git" ) // testRuntime resolves the pinned git the Bazel target supplies. Provisioning // runs real git, so these tests use the same runtime the merger will. -func testRuntime(t *testing.T) gitmerger.GitRuntime { +func testRuntime(t *testing.T) gitexec.Runtime { t.Helper() executable := gitexectest.Git(t) templateDescription := gitexectest.Runfile(t, "SUBMITQUEUE_TEST_GIT_TEMPLATE_DESCRIPTION") - return gitmerger.GitRuntime{ + return gitexec.Runtime{ Executable: executable, ExecPath: filepath.Dir(executable), TemplateDir: filepath.Dir(templateDescription), @@ -68,7 +68,7 @@ func seedBareRepo(t *testing.T, branch string) string { return bare } -func mustRunGit(t *testing.T, ctx context.Context, runtime gitmerger.GitRuntime, dir string, args ...string) string { +func mustRunGit(t *testing.T, ctx context.Context, runtime gitexec.Runtime, dir string, args ...string) string { t.Helper() out, err := runGit(ctx, runtime, dir, args...) require.NoError(t, err, "git %s", strings.Join(args, " ")) diff --git a/service/runway/server/gitruntime.go b/service/runway/server/gitruntime.go index 8742ec677..7c2f24cd9 100644 --- a/service/runway/server/gitruntime.go +++ b/service/runway/server/gitruntime.go @@ -23,7 +23,7 @@ import ( "path/filepath" "strings" - gitmerger "github.com/uber/submitqueue/runway/extension/merger/git" + gitexec "github.com/uber/submitqueue/platform/git/exec" ) // resolveGitRuntime determines the pinned git runtime the merger invokes. @@ -36,23 +36,23 @@ import ( // override is unset. GIT_EXECUTABLE, GIT_EXEC_PATH, and GIT_TEMPLATE_DIR still // take precedence, which is how a deployment pins a git other than the one on // PATH. -func resolveGitRuntime(ctx context.Context) (gitmerger.GitRuntime, error) { +func resolveGitRuntime(ctx context.Context) (gitexec.Runtime, error) { executable, err := resolveGitExecutable() if err != nil { - return gitmerger.GitRuntime{}, err + return gitexec.Runtime{}, err } execPath, err := resolveGitExecPath(ctx, executable) if err != nil { - return gitmerger.GitRuntime{}, err + return gitexec.Runtime{}, err } templateDir, err := resolveGitTemplateDir(execPath) if err != nil { - return gitmerger.GitRuntime{}, err + return gitexec.Runtime{}, err } - return gitmerger.GitRuntime{ + return gitexec.Runtime{ Executable: executable, ExecPath: execPath, TemplateDir: templateDir, diff --git a/service/runway/server/main.go b/service/runway/server/main.go index 64ed86859..12092d835 100644 --- a/service/runway/server/main.go +++ b/service/runway/server/main.go @@ -44,6 +44,7 @@ import ( consumergatenoop "github.com/uber/submitqueue/platform/extension/consumergate/noop" extqueue "github.com/uber/submitqueue/platform/extension/messagequeue" queueMySQL "github.com/uber/submitqueue/platform/extension/messagequeue/mysql" + gitexec "github.com/uber/submitqueue/platform/git/exec" "github.com/uber/submitqueue/runway/controller" "github.com/uber/submitqueue/runway/controller/dlq" "github.com/uber/submitqueue/runway/controller/merge" @@ -362,7 +363,7 @@ func newMergerFactory(ctx context.Context, logger *zap.Logger, scope tally.Scope // The git runtime is resolved only when something actually needs it, so a // deployment running nothing but the noop merger does not require git to be // installed at all. - var runtime gitmerger.GitRuntime + var runtime gitexec.Runtime if cfg.usesGit() { runtime, err = resolveGitRuntime(ctx) if err != nil { @@ -500,7 +501,7 @@ type mergerBuilder struct { ctx context.Context logger *zap.Logger scope tally.Scope - runtime gitmerger.GitRuntime + runtime gitexec.Runtime // byTarget caches one merger per checkout path. Keying on the path alone is // safe only because validation has already rejected two queues that share a // checkout and disagree anywhere in their merger config: the cached instance