Skip to content

Commit 6f607c2

Browse files
committed
feat(orchestrator): let a queue select the git change provider
## Summary ### Why? The previous change added a change provider that reads a git repository. Nothing could select it: `profiles.yaml` had no `type: git`, and the wiring had no case for one. This connects the two. ### What? `changeProvider: {type: git, git: {…}}` in `profiles.yaml`, nested under a named block like `github` and `phabricator` are, carrying what a copy of a repository needs: `remoteUrl`, `target`, `repoPath`, and optionally `tokenEnv`/`tokenUser`. `remote` defaults to `origin`, `tokenUser` to `x-access-token`. Three values are required because none has a default that could be right: there is no public git remote to fall back on, no universal trunk name, and nowhere obvious to keep a copy. The block deliberately reads like Runway's merger block — each service says for itself where its copy fetches from, so a queue's provider and its merger can name the same remote while being configured independently. **Two queues may share a `repoPath`, and should when they are on the same repository** — that is how they come to share one copy and one lock. Sharing a path while disagreeing about the remote or target is rejected at startup, because whichever queue was built first would silently decide what the other one reads. **The default `Auth` lives here, not in the extension.** `tokenAuth` reads a named environment variable and writes git an `http.extraheader` fragment inside the repository, mode 0600, included from its config — never into the remote URL, which git echoes back into error messages and from there into logs and dead-letter payloads. The extension only knows it has an `Auth` and calls it; a deployment that mints short-lived tokens or reads a secrets manager replaces this one file's worth of behaviour and changes nothing else. It is applied before each fetch rather than once, which is what lets an expiring credential be refreshed. A remote that needs no credential gets a nil `Auth` — a local path, or SSH served by the host's own configuration and agent. **Provisioning now fetches.** It runs at wiring time, and the reason given for that was to fail the misconfigured service rather than bury the failure in a queue's retry loop. Writing the test for it showed the claim was not yet true: initializing a directory and recording a remote succeeds whether or not the remote exists, so a wrong URL would have surfaced later, per message, as a validate failure. Provisioning fetches the target branch, which is the step that actually proves the remote is reachable and the credential works — and it warms the copy, so the first request does not pay for a clone. `newProfiles` takes a context so that provisioning can be cancelled: it reaches the network during startup, and a shutdown then should stop it rather than wait it out. Still nothing selects it — no demo queue is switched over and the orchestrator image has no git binary yet. Those are the next two steps. ## Test Plan - ✅ the seam test: configuration in, a provider out that reports `pkg/a/one.go` with 2 added lines from a repository the test built — which only passes if the config block, provisioning, the injected auth and the factory case all line up - ✅ an unreachable remote fails `newProfiles`. This is the test that found the gap above; before provisioning fetched, it passed startup and would have failed per message instead - ✅ defaults applied (`remote`, `tokenUser`) and each of the three required values rejected when missing - ✅ two queues sharing a `repoPath` with different remotes are rejected; two that agree are allowed, since sharing a copy is the point - ✅ `make test`, `make lint`, `make gazelle` The config block is nested under `git:` rather than flat as the plan sketched. Flat would have matched `merge.yaml` more literally, but this file's own grammar is a named block per provider, and consistency inside the file a reader is editing seemed worth more than symmetry with a file in another service.
1 parent 172c4fc commit 6f607c2

7 files changed

Lines changed: 475 additions & 7 deletions

File tree

service/submitqueue/orchestrator/server/BUILD.bazel

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ exports_files(
88
go_library(
99
name = "orchestrator_lib",
1010
srcs = [
11+
"changerepo.go",
1112
"config.go",
1213
"main.go",
1314
"profiles.go",
@@ -36,6 +37,7 @@ go_library(
3637
"//submitqueue/extension/buildrunner/githubactions:go_default_library",
3738
"//submitqueue/extension/changeprovider:go_default_library",
3839
"//submitqueue/extension/changeprovider/fake:go_default_library",
40+
"//submitqueue/extension/changeprovider/git:go_default_library",
3941
"//submitqueue/extension/changeprovider/github:go_default_library",
4042
"//submitqueue/extension/changeprovider/phabricator:go_default_library",
4143
"//submitqueue/extension/changeprovider/routing:go_default_library",
@@ -101,8 +103,16 @@ go_test(
101103
"config_test.go",
102104
"profiles_test.go",
103105
],
106+
# The pinned git, for the tests that resolve a git change provider against a
107+
# real repository.
108+
data = ["@git"],
104109
embed = [":orchestrator_lib"], # keep
110+
env = {
111+
"SUBMITQUEUE_TEST_GIT": "$(location @git//:git)",
112+
},
105113
deps = [
114+
"//platform/base/change:go_default_library",
115+
"//platform/gitexec/gitexectest:go_default_library",
106116
"//submitqueue/entity:go_default_library",
107117
"//submitqueue/extension/buildrunner:go_default_library",
108118
"//submitqueue/extension/changeprovider:go_default_library",
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
// Copyright (c) 2025 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 main
16+
17+
import (
18+
"context"
19+
"encoding/base64"
20+
"fmt"
21+
"os"
22+
"path/filepath"
23+
"strings"
24+
25+
gitprovider "github.com/uber/submitqueue/submitqueue/extension/changeprovider/git"
26+
)
27+
28+
// credentialFile holds the git configuration fragment carrying a token. It is
29+
// written inside the repository and included from its config, so the token
30+
// never appears in a remote URL — which git echoes back into error messages,
31+
// and from there into logs and dead-letter payloads.
32+
const credentialFile = "submitqueue-changeprovider-credentials.config"
33+
34+
// tokenAuth is the default gitprovider.Auth: a credential read from an
35+
// environment variable and presented to git as an HTTP header.
36+
//
37+
// It is deliberately here rather than in the extension. The extension takes an
38+
// Auth and calls it; what a credential is and where it comes from is a
39+
// deployment's business, so a deployment that mints short-lived tokens or
40+
// reads a secrets manager supplies its own implementation instead of this one
41+
// and changes nothing else.
42+
type tokenAuth struct {
43+
tokenEnv string
44+
tokenUser string
45+
}
46+
47+
// Apply writes the credential fragment, or removes it when the remote needs
48+
// none, so a repository that stops using a token stops carrying one.
49+
//
50+
// Called before every fetch rather than once, which is what lets a short-lived
51+
// credential be refreshed — reading the variable again each time is the whole
52+
// mechanism for that.
53+
func (a tokenAuth) Apply(ctx context.Context, repoPath, remoteURL string) error {
54+
path := filepath.Join(repoPath, credentialFile)
55+
56+
if !isHTTPRemote(remoteURL) {
57+
if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
58+
return fmt.Errorf("could not remove stale credential %q: %w", path, err)
59+
}
60+
return nil
61+
}
62+
63+
token, ok := os.LookupEnv(a.tokenEnv)
64+
if !ok || token == "" {
65+
return fmt.Errorf("environment variable %q named by tokenEnv is not set", a.tokenEnv)
66+
}
67+
68+
basic := base64.StdEncoding.EncodeToString([]byte(a.tokenUser + ":" + token))
69+
fragment := fmt.Sprintf("[http %q]\n\textraheader = Authorization: Basic %s\n", remoteURL, basic)
70+
if err := os.WriteFile(path, []byte(fragment), 0o600); err != nil {
71+
return fmt.Errorf("could not write credential %q: %w", path, err)
72+
}
73+
74+
// include.path resolves relative to the config file holding it, so the bare
75+
// filename lands beside it. A bare repository's config is at its root.
76+
return gitprovider.SetConfig(ctx, repoPath, "include.path", credentialFile)
77+
}
78+
79+
func isHTTPRemote(remoteURL string) bool {
80+
return strings.HasPrefix(remoteURL, "http://") || strings.HasPrefix(remoteURL, "https://")
81+
}
82+
83+
// newChangeRepo builds the local copy a git change provider reads through, and
84+
// provisions it.
85+
//
86+
// Provisioning happens here, at wiring time, rather than on first use: a change
87+
// provider is resolved once per message on the validate path, so a clone
88+
// started there would sit inside a retry loop and report an unreachable remote
89+
// as a queue processing failure instead of a service that is misconfigured.
90+
func newChangeRepo(ctx context.Context, cfg gitProviderConfig) (*gitprovider.Repo, error) {
91+
var auth gitprovider.Auth
92+
if cfg.TokenEnv != "" {
93+
auth = tokenAuth{tokenEnv: cfg.TokenEnv, tokenUser: cfg.TokenUser}
94+
}
95+
96+
repo, err := gitprovider.NewRepo(gitprovider.RepoConfig{
97+
Path: cfg.RepoPath,
98+
RemoteURL: cfg.RemoteURL,
99+
Remote: cfg.Remote,
100+
Target: cfg.Target,
101+
Auth: auth,
102+
})
103+
if err != nil {
104+
return nil, err
105+
}
106+
if err := repo.Provision(ctx); err != nil {
107+
return nil, fmt.Errorf("could not provision change repository %q: %w", cfg.RepoPath, err)
108+
}
109+
return repo, nil
110+
}

service/submitqueue/orchestrator/server/config.go

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
// Change provider types selectable from configuration.
2626
const (
2727
changeProviderTypeFake = "fake"
28+
changeProviderTypeGit = "git"
2829
changeProviderTypeGitHub = "github"
2930
changeProviderTypePhabricator = "phabricator"
3031
changeProviderTypeRouting = "routing"
@@ -74,6 +75,10 @@ const defaultBuildBudget = 4
7475

7576
// Defaults for the provider integrations, matching each vendor's convention.
7677
const (
78+
// defaultGitRemote and defaultGitTokenUser match git's own convention and
79+
// the username a forge expects a token to be presented under.
80+
defaultGitRemote = "origin"
81+
defaultGitTokenUser = "x-access-token"
7782
defaultGitHubTokenEnv = "GITHUB_TOKEN"
7883
defaultGitHubBaseURL = "https://api.github.com"
7984
defaultPhabTokenEnv = "PHAB_API_TOKEN"
@@ -121,10 +126,37 @@ type queueProfileConfig struct {
121126
// dispatches between them on the change URI's scheme.
122127
type changeProviderConfig struct {
123128
Type string `yaml:"type"`
129+
Git *gitProviderConfig `yaml:"git"`
124130
GitHub *githubProviderConfig `yaml:"github"`
125131
Phabricator *phabProviderConfig `yaml:"phabricator"`
126132
}
127133

134+
// gitProviderConfig configures the git change provider, which derives change
135+
// metadata from a repository rather than asking a service for it.
136+
//
137+
// It mirrors Runway's merger block deliberately: each service keeps its own
138+
// copy of a queue's repository and says for itself where that copy fetches
139+
// from, so the two are configured independently even when they name the same
140+
// remote.
141+
type gitProviderConfig struct {
142+
// RemoteURL is where this service's copy fetches from. A URL or a local
143+
// path; a bind-mounted bare repository and a remote host differ only here.
144+
RemoteURL string `yaml:"remoteUrl"`
145+
// Remote is the name the copy records RemoteURL under.
146+
Remote string `yaml:"remote"`
147+
// Target is the branch a change's first commit is measured against.
148+
Target string `yaml:"target"`
149+
// RepoPath is where this service keeps its copy. It belongs to this service
150+
// alone — another service reading the same remote keeps its own.
151+
RepoPath string `yaml:"repoPath"`
152+
// TokenEnv names the environment variable holding a credential for an
153+
// http(s) remote. Empty means the remote needs none, which is the case for a
154+
// local path and for SSH served by the host's own configuration.
155+
TokenEnv string `yaml:"tokenEnv"`
156+
// TokenUser is the username the credential is presented under.
157+
TokenUser string `yaml:"tokenUser"`
158+
}
159+
128160
// githubProviderConfig configures the GitHub change provider.
129161
type githubProviderConfig struct {
130162
// TokenEnv names the environment variable holding the API token.
@@ -269,6 +301,41 @@ func (c *profilesConfig) normalizeAndValidate() error {
269301
}
270302
}
271303
}
304+
return c.validateGitRepoPaths()
305+
}
306+
307+
// validateGitRepoPaths rejects two queues keeping their copies in one directory
308+
// while disagreeing about what that copy is.
309+
//
310+
// A shared path is legitimate — two queues on the same repository should share
311+
// one copy, and sharing it is what makes them share its lock. Sharing it with a
312+
// different remote or target is not: whichever queue was built first silently
313+
// decides what the other one reads.
314+
func (c profilesConfig) validateGitRepoPaths() error {
315+
type owner struct {
316+
queue string
317+
cfg gitProviderConfig
318+
}
319+
byPath := make(map[string]owner)
320+
321+
for _, q := range c.Queues {
322+
resolved := c.resolve(q)
323+
if resolved.ChangeProvider.Type != changeProviderTypeGit || resolved.ChangeProvider.Git == nil {
324+
continue
325+
}
326+
git := *resolved.ChangeProvider.Git
327+
previous, seen := byPath[git.RepoPath]
328+
if !seen {
329+
byPath[git.RepoPath] = owner{queue: q.Name, cfg: git}
330+
continue
331+
}
332+
if previous.cfg.RemoteURL != git.RemoteURL || previous.cfg.Target != git.Target {
333+
return fmt.Errorf(
334+
"queues %q and %q share repoPath %q but read different repositories (%s@%s vs %s@%s); give each its own path",
335+
previous.queue, q.Name, git.RepoPath,
336+
previous.cfg.RemoteURL, previous.cfg.Target, git.RemoteURL, git.Target)
337+
}
338+
}
272339
return nil
273340
}
274341

@@ -317,6 +384,8 @@ func (c *changeProviderConfig) normalizeAndValidate(where string) error {
317384
switch c.Type {
318385
case changeProviderTypeFake:
319386
return nil
387+
case changeProviderTypeGit:
388+
return c.ensureGit(where)
320389
case changeProviderTypeGitHub:
321390
c.ensureGitHub()
322391
case changeProviderTypePhabricator:
@@ -343,6 +412,31 @@ func (c *changeProviderConfig) normalizeAndValidate(where string) error {
343412
return nil
344413
}
345414

415+
// ensureGit defaults and checks the git block. Three values have no sensible
416+
// default and are required: there is no public git remote to fall back on, no
417+
// universal trunk name, and nowhere obvious to keep a copy.
418+
func (c *changeProviderConfig) ensureGit(where string) error {
419+
if c.Git == nil {
420+
c.Git = &gitProviderConfig{}
421+
}
422+
if c.Git.Remote == "" {
423+
c.Git.Remote = defaultGitRemote
424+
}
425+
if c.Git.TokenUser == "" {
426+
c.Git.TokenUser = defaultGitTokenUser
427+
}
428+
if c.Git.RemoteURL == "" {
429+
return fmt.Errorf("%s: git change provider requires remoteUrl", where)
430+
}
431+
if c.Git.Target == "" {
432+
return fmt.Errorf("%s: git change provider requires target", where)
433+
}
434+
if c.Git.RepoPath == "" {
435+
return fmt.Errorf("%s: git change provider requires repoPath", where)
436+
}
437+
return nil
438+
}
439+
346440
func (c *changeProviderConfig) ensureGitHub() {
347441
if c.GitHub == nil {
348442
c.GitHub = &githubProviderConfig{}

0 commit comments

Comments
 (0)