Skip to content

Commit 7602fd8

Browse files
manjari25manjari25
committed
refactor(platform): Move GitRuntime into gitworkspace/local as Runtime
1 parent 5f8e463 commit 7602fd8

11 files changed

Lines changed: 258 additions & 203 deletions

File tree

‎platform/extension/gitworkspace/local/BUILD.bazel‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,24 @@ load("@rules_go//go:def.bzl", "go_library", "go_test")
22

33
go_library(
44
name = "go_default_library",
5-
srcs = ["local.go"],
5+
srcs = [
6+
"local.go",
7+
"runtime.go",
8+
],
69
importpath = "github.com/uber/submitqueue/platform/extension/gitworkspace/local",
710
visibility = ["//visibility:public"],
8-
deps = ["//platform/extension/gitworkspace:go_default_library"],
11+
deps = [
12+
"//platform/extension/gitworkspace:go_default_library",
13+
"//platform/git/exec:go_default_library",
14+
],
915
)
1016

1117
go_test(
1218
name = "go_default_test",
13-
srcs = ["local_test.go"],
19+
srcs = [
20+
"local_test.go",
21+
"runtime_test.go",
22+
],
1423
embed = [":go_default_library"],
1524
deps = [
1625
"//platform/extension/gitworkspace:go_default_library",

‎platform/extension/gitworkspace/local/runtime.go‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
// Copyright (c) 2026 Uber Technologies, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package local
16+
17+
import (
18+
"context"
19+
"fmt"
20+
"os/exec"
21+
"path/filepath"
22+
23+
gitexec "github.com/uber/submitqueue/platform/git/exec"
24+
)
25+
26+
// Runtime identifies the explicitly provided Git runtime that commands run on.
27+
type Runtime struct {
28+
// Executable is the absolute path to the Git executable.
29+
Executable string
30+
// ExecPath is the absolute directory containing Git's helper executables.
31+
ExecPath string
32+
// TemplateDir is the absolute directory containing Git's repository
33+
// templates.
34+
TemplateDir string
35+
// PassthroughEnv names additional environment variables to inherit from
36+
// the parent process, on top of the auth and transport ones always passed
37+
// through. For a deployment whose remote needs something unusual; leave
38+
// empty otherwise. Names that could alter merge semantics do not belong
39+
// here — the scrubbed environment is what keeps a merge reproducible.
40+
PassthroughEnv []string
41+
}
42+
43+
// Validate reports whether every path of the runtime is set and absolute.
44+
func (r Runtime) Validate() error {
45+
for _, field := range []struct{ name, path string }{
46+
{"executable", r.Executable},
47+
{"exec path", r.ExecPath},
48+
{"template dir", r.TemplateDir},
49+
} {
50+
if field.path == "" {
51+
return fmt.Errorf("git runtime %s is required", field.name)
52+
}
53+
if !filepath.IsAbs(field.path) {
54+
return fmt.Errorf("git runtime %s must be absolute: %q", field.name, field.path)
55+
}
56+
}
57+
return nil
58+
}
59+
60+
// Command constructs a Git command without inheriting the caller's
61+
// environment. The executable and helper paths come from the pinned runtime;
62+
// repository-local configuration remains an intentional input.
63+
func (r Runtime) Command(ctx context.Context, dir string, args ...string) *exec.Cmd {
64+
gitArgs := make([]string, 0, len(args)+3)
65+
gitArgs = append(gitArgs,
66+
"--exec-path="+r.ExecPath,
67+
"-c", "init.templateDir="+r.TemplateDir,
68+
)
69+
gitArgs = append(gitArgs, args...)
70+
71+
cmd := exec.CommandContext(ctx, r.Executable, gitArgs...)
72+
cmd.Dir = dir
73+
// HOME and XDG_CONFIG_HOME are isolated to the checkout rather than inherited,
74+
// so the runtime's literals — appended last — override any HOME a deployment
75+
// passed through. The remaining literals pin git's runtime; the scrub set and
76+
// transport variables come from gitexec.Env.
77+
cmd.Env = gitexec.Env(gitexec.EnvOptions{
78+
Transport: true,
79+
Passthrough: r.PassthroughEnv,
80+
Literal: []string{
81+
"HOME=" + filepath.Join(dir, ".submitqueue-git-home"),
82+
"XDG_CONFIG_HOME=" + filepath.Join(dir, ".submitqueue-git-home", "xdg"),
83+
"GIT_EXEC_PATH=" + r.ExecPath,
84+
"GIT_TEMPLATE_DIR=" + r.TemplateDir,
85+
"LC_ALL=C",
86+
"LANG=C",
87+
},
88+
})
89+
return cmd
90+
}

‎platform/extension/gitworkspace/local/runtime_test.go‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
// Copyright (c) 2026 Uber Technologies, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package local
16+
17+
import (
18+
"context"
19+
"os"
20+
"path/filepath"
21+
"strings"
22+
"testing"
23+
24+
"github.com/stretchr/testify/assert"
25+
"github.com/stretchr/testify/require"
26+
)
27+
28+
func dummyRuntime(t *testing.T) Runtime {
29+
t.Helper()
30+
dir := t.TempDir()
31+
return Runtime{
32+
Executable: filepath.Join(dir, "git"),
33+
ExecPath: filepath.Join(dir, "git-core"),
34+
TemplateDir: filepath.Join(dir, "templates"),
35+
}
36+
}
37+
38+
func TestRuntimeValidate(t *testing.T) {
39+
valid := dummyRuntime(t)
40+
tests := []struct {
41+
name string
42+
mutate func(*Runtime)
43+
wantErr bool
44+
}{
45+
{name: "valid", mutate: func(*Runtime) {}},
46+
{name: "missing executable", mutate: func(r *Runtime) { r.Executable = "" }, wantErr: true},
47+
{name: "relative exec path", mutate: func(r *Runtime) { r.ExecPath = "git-core" }, wantErr: true},
48+
{name: "relative template dir", mutate: func(r *Runtime) { r.TemplateDir = "templates" }, wantErr: true},
49+
}
50+
for _, tt := range tests {
51+
t.Run(tt.name, func(t *testing.T) {
52+
runtime := valid
53+
tt.mutate(&runtime)
54+
err := runtime.Validate()
55+
if tt.wantErr {
56+
require.Error(t, err)
57+
return
58+
}
59+
require.NoError(t, err)
60+
})
61+
}
62+
}
63+
64+
func TestRuntimeCommandDoesNotInheritEnvironment(t *testing.T) {
65+
t.Setenv("SUBMITQUEUE_GIT_AMBIENT", "ambient")
66+
runtime := dummyRuntime(t)
67+
68+
cmd := runtime.Command(context.Background(), t.TempDir(), "--version")
69+
70+
assert.Equal(t, runtime.Executable, cmd.Path)
71+
assert.NotContains(t, cmd.Env, "SUBMITQUEUE_GIT_AMBIENT=ambient")
72+
assert.Contains(t, cmd.Env, "GIT_CONFIG_NOSYSTEM=1")
73+
assert.Contains(t, cmd.Env, "GIT_EXEC_PATH="+runtime.ExecPath)
74+
}
75+
76+
func TestRuntimeCommandPreservesAuthEnvironment(t *testing.T) {
77+
// Scrubbing denies git ambient configuration; it must not also deny it the
78+
// means to reach the remote. Without the agent socket an SSH remote cannot
79+
// authenticate, and without PATH git cannot even exec ssh.
80+
t.Setenv("SSH_AUTH_SOCK", "/tmp/agent.sock")
81+
t.Setenv("PATH", "/usr/bin:/bin")
82+
t.Setenv("HTTPS_PROXY", "http://proxy.example.com:3128")
83+
t.Setenv("SUBMITQUEUE_GIT_AMBIENT", "ambient")
84+
85+
cmd := dummyRuntime(t).Command(context.Background(), t.TempDir(), "--version")
86+
87+
assert.Contains(t, cmd.Env, "SSH_AUTH_SOCK=/tmp/agent.sock")
88+
assert.Contains(t, cmd.Env, "PATH=/usr/bin:/bin")
89+
assert.Contains(t, cmd.Env, "HTTPS_PROXY=http://proxy.example.com:3128")
90+
assert.NotContains(t, cmd.Env, "SUBMITQUEUE_GIT_AMBIENT=ambient")
91+
}
92+
93+
func TestRuntimeCommandOmitsUnsetAuthEnvironment(t *testing.T) {
94+
// An unset variable must be omitted rather than exported empty: an empty
95+
// SSH_AUTH_SOCK tells ssh there is no agent instead of letting it look.
96+
t.Setenv("SSH_AUTH_SOCK", "")
97+
require.NoError(t, os.Unsetenv("SSH_AUTH_SOCK"))
98+
99+
cmd := dummyRuntime(t).Command(context.Background(), t.TempDir(), "--version")
100+
101+
for _, entry := range cmd.Env {
102+
assert.False(t, strings.HasPrefix(entry, "SSH_AUTH_SOCK="), "unset variable leaked as %q", entry)
103+
}
104+
}
105+
106+
func TestRuntimeCommandPassesThroughExtraEnvironment(t *testing.T) {
107+
t.Setenv("SUBMITQUEUE_CUSTOM_TRANSPORT", "value")
108+
runtime := dummyRuntime(t)
109+
runtime.PassthroughEnv = []string{"SUBMITQUEUE_CUSTOM_TRANSPORT"}
110+
111+
cmd := runtime.Command(context.Background(), t.TempDir(), "--version")
112+
113+
assert.Contains(t, cmd.Env, "SUBMITQUEUE_CUSTOM_TRANSPORT=value")
114+
}
115+
116+
func TestRuntimeCommandIsolatesHome(t *testing.T) {
117+
t.Setenv("HOME", "/ambient/home")
118+
dir := t.TempDir()
119+
120+
cmd := dummyRuntime(t).Command(context.Background(), dir, "--version")
121+
122+
assert.Contains(t, cmd.Env, "HOME="+filepath.Join(dir, ".submitqueue-git-home"))
123+
assert.NotContains(t, cmd.Env, "HOME=/ambient/home")
124+
}

‎runway/extension/merger/git/BUILD.bazel‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ go_library(
1616
"//api/runway/messagequeue/protopb:go_default_library",
1717
"//platform/base/change/git:go_default_library",
1818
"//platform/base/change/github:go_default_library",
19+
"//platform/extension/gitworkspace/local:go_default_library",
1920
"//platform/git/exec:go_default_library",
2021
"//platform/metrics:go_default_library",
2122
"//runway/extension/merger:go_default_library",
@@ -45,6 +46,7 @@ go_test(
4546
"//api/base/mergestrategy/protopb:go_default_library",
4647
"//api/runway/messagequeue:go_default_library",
4748
"//api/runway/messagequeue/protopb:go_default_library",
49+
"//platform/extension/gitworkspace/local:go_default_library",
4850
"//platform/git/exec:go_default_library",
4951
"//platform/git/exectest:go_default_library",
5052
"//runway/extension/merger:go_default_library",

‎runway/extension/merger/git/git_merger.go‎

Lines changed: 5 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ import (
6969
"errors"
7070
"fmt"
7171
"os/exec"
72-
"path/filepath"
7372
"strings"
7473
"sync"
7574

@@ -79,6 +78,7 @@ import (
7978
mergestrategypb "github.com/uber/submitqueue/api/base/mergestrategy/protopb"
8079
runwaymq "github.com/uber/submitqueue/api/runway/messagequeue"
8180
runwaypb "github.com/uber/submitqueue/api/runway/messagequeue/protopb"
81+
gitlocal "github.com/uber/submitqueue/platform/extension/gitworkspace/local"
8282
gitexec "github.com/uber/submitqueue/platform/git/exec"
8383
coremetrics "github.com/uber/submitqueue/platform/metrics"
8484
"github.com/uber/submitqueue/runway/extension/merger"
@@ -97,23 +97,6 @@ const (
9797
defaultCommitterEmail = "runway@submitqueue.invalid"
9898
)
9999

100-
// GitRuntime identifies the explicitly provided Git runtime used by the Merger.
101-
type GitRuntime struct {
102-
// Executable is the absolute path to the Git executable.
103-
Executable string
104-
// ExecPath is the absolute directory containing Git's helper executables.
105-
ExecPath string
106-
// TemplateDir is the absolute directory containing Git's repository
107-
// templates.
108-
TemplateDir string
109-
// PassthroughEnv names additional environment variables to inherit from
110-
// the parent process, on top of the auth and transport ones always passed
111-
// through. For a deployment whose remote needs something unusual; leave
112-
// empty otherwise. Names that could alter merge semantics do not belong
113-
// here — the scrubbed environment is what keeps a merge reproducible.
114-
PassthroughEnv []string
115-
}
116-
117100
// Params holds the dependencies for the git Merger.
118101
type Params struct {
119102
// CheckoutPath is the absolute path to an existing git checkout that the
@@ -128,7 +111,7 @@ type Params struct {
128111
// concrete strategy (REBASE, SQUASH_REBASE, MERGE, or PROMOTE).
129112
DefaultStrategy mergestrategypb.Strategy
130113
// Runtime is the pinned Git runtime used for every invocation.
131-
Runtime GitRuntime
114+
Runtime gitlocal.Runtime
132115
// MaxPushAttempts caps how many times a committing merge retries the full
133116
// reset/apply/push cycle when the remote tip moves under it. Defaults to
134117
// defaultMaxPushAttempts when zero or negative.
@@ -166,7 +149,7 @@ type gitMerger struct {
166149
remote string
167150
target string
168151
defaultStrategy mergestrategypb.Strategy
169-
runtime GitRuntime
152+
runtime gitlocal.Runtime
170153
maxPushAttempts int
171154
fetchRefspecs []string
172155
checkStaleness bool
@@ -201,7 +184,7 @@ type resolvedStep struct {
201184
// The checkout must already exist and have the configured remote. Runtime paths
202185
// must be absolute and DefaultStrategy must be a concrete strategy.
203186
func NewMerger(params Params) (merger.Merger, error) {
204-
if err := params.Runtime.validate(); err != nil {
187+
if err := params.Runtime.Validate(); err != nil {
205188
return nil, err
206189
}
207190
if !isConcreteStrategy(params.DefaultStrategy) {
@@ -237,22 +220,6 @@ func NewMerger(params Params) (merger.Merger, error) {
237220
}, nil
238221
}
239222

240-
func (r GitRuntime) validate() error {
241-
for name, path := range map[string]string{
242-
"executable": r.Executable,
243-
"exec path": r.ExecPath,
244-
"template dir": r.TemplateDir,
245-
} {
246-
if path == "" {
247-
return fmt.Errorf("git runtime %s is required", name)
248-
}
249-
if !filepath.IsAbs(path) {
250-
return fmt.Errorf("git runtime %s must be absolute: %q", name, path)
251-
}
252-
}
253-
return nil
254-
}
255-
256223
// CheckMergeability applies the request's steps as a dry run: it verifies each
257224
// step applies cleanly without committing to the remote, and returns per-step
258225
// results with empty Outputs.
@@ -1025,43 +992,11 @@ func (m *gitMerger) commandAs(ctx context.Context, author authorIdent, args ...s
1025992
"-c", "commit.gpgsign=false",
1026993
)
1027994
withIdentity = append(withIdentity, args...)
1028-
cmd := newGitCommand(ctx, m.runtime, m.checkoutPath, withIdentity...)
995+
cmd := m.runtime.Command(ctx, m.checkoutPath, withIdentity...)
1029996
cmd.Env = append(cmd.Env, author.env()...)
1030997
return cmd
1031998
}
1032999

1033-
// newGitCommand constructs a Git command without inheriting the caller's
1034-
// environment. The executable and helper paths come from the pinned runtime;
1035-
// repository-local configuration remains an intentional input.
1036-
func newGitCommand(ctx context.Context, runtime GitRuntime, dir string, args ...string) *exec.Cmd {
1037-
gitArgs := make([]string, 0, len(args)+3)
1038-
gitArgs = append(gitArgs,
1039-
"--exec-path="+runtime.ExecPath,
1040-
"-c", "init.templateDir="+runtime.TemplateDir,
1041-
)
1042-
gitArgs = append(gitArgs, args...)
1043-
1044-
cmd := exec.CommandContext(ctx, runtime.Executable, gitArgs...)
1045-
cmd.Dir = dir
1046-
// HOME and XDG_CONFIG_HOME are isolated to the checkout rather than inherited,
1047-
// so the runtime's literals — appended last — override any HOME a deployment
1048-
// passed through. The remaining literals pin git's runtime; the scrub set and
1049-
// transport variables come from the shared composer.
1050-
cmd.Env = gitexec.Env(gitexec.EnvOptions{
1051-
Transport: true,
1052-
Passthrough: runtime.PassthroughEnv,
1053-
Literal: []string{
1054-
"HOME=" + filepath.Join(dir, ".submitqueue-git-home"),
1055-
"XDG_CONFIG_HOME=" + filepath.Join(dir, ".submitqueue-git-home", "xdg"),
1056-
"GIT_EXEC_PATH=" + runtime.ExecPath,
1057-
"GIT_TEMPLATE_DIR=" + runtime.TemplateDir,
1058-
"LC_ALL=C",
1059-
"LANG=C",
1060-
},
1061-
})
1062-
return cmd
1063-
}
1064-
10651000
// isConcreteStrategy reports whether s names a concrete integration strategy
10661001
// (i.e. not DEFAULT and not an unknown value).
10671002
func isConcreteStrategy(s mergestrategypb.Strategy) bool {

0 commit comments

Comments
 (0)