From 42c89c88ae8d17f6c898a0785793d4a1346e4c26 Mon Sep 17 00:00:00 2001 From: Preetam Dwivedi Date: Thu, 9 Jul 2026 15:31:08 -0700 Subject: [PATCH] feat(change): require host[:port] authority in change URIs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary ### Why? The Change URIs RFC (doc/rfc/change-uri.md, previous PR in this stack) makes every change URI carry the provider instance's `host[:port]` as its RFC 3986 authority, so a URI is globally unambiguous on its own: GitHub and Phabricator URIs previously carried no host (the same URI could name two different changes on two provider instances), and the `ghe`/`ghes` schemes stood in for an instance discriminator without actually being one. ### What? - `platform/base/change/github`: parser rewritten on `net/url`; format is now `github://{host[:port]}/{org}/{repo}/pull/{pr}/{sha}`; single `github` scheme — `ghe`/`ghes` are rejected; parsed form gains a `Host` field. - `platform/base/change/phabricator`: same treatment; format is now `phab://{host[:port]}/D{revision}/{diff}`; the host moves in-band from queue config into the URI authority. - `platform/base/change/git`: unchanged format (it already carried the authority); gains only the new lowercase-host validation. - `platform/base/change/changeutil`: new `IsLowercaseASCII` predicate — canonical form is validated, never normalized (uppercase hosts are rejected, not folded), matching the existing full-40-hex SHA rule. - Contract docs updated in `api/base/change` proto (comment-only, regenerated via `make proto`), `platform/base/change`, and entity comments; all test fixtures across gateway, orchestrator, extensions, runway, integration, and e2e suites moved to authority-bearing URIs (`github.example.com` / `phab.example.com`). No wire change: `Change.uris` stays a repeated string; only the accepted grammar of its contents changes. Pre-production, so this is a hard cutover with no dual-format window (per the RFC's rejected alternatives). ## Test Plan ✅ `make test` (65/65), ✅ `make build`, ✅ `make proto` / `make fmt` / `make gazelle` idempotent. Parser suites cover the new grammar: host and host:port round-trips, nested orgs, and rejections for missing host, uppercase host, host-less legacy URIs, and `ghe://`/`ghes://` schemes. --- api/base/change/proto/change.proto | 13 ++- api/base/change/protopb/change.pb.go | 13 ++- api/runway/messagequeue/merge_test.go | 4 +- platform/base/change/change.go | 16 ++- platform/base/change/changeutil/BUILD.bazel | 10 +- platform/base/change/changeutil/host.go | 30 +++++ platform/base/change/changeutil/host_test.go | 42 +++++++ platform/base/change/git/change_id.go | 3 + platform/base/change/git/change_id_test.go | 5 + platform/base/change/github/change_id.go | 58 ++++++---- platform/base/change/github/change_id_test.go | 109 ++++++++++-------- platform/base/change/phabricator/BUILD.bazel | 1 + platform/base/change/phabricator/change_id.go | 48 +++++--- .../base/change/phabricator/change_id_test.go | 65 +++++++---- runway/extension/merger/noop/noop_test.go | 4 +- submitqueue/core/fakemarker/fakemarker.go | 2 +- .../core/fakemarker/fakemarker_test.go | 22 ++-- submitqueue/entity/change_provider.go | 2 +- submitqueue/entity/change_record.go | 2 +- submitqueue/entity/land_request_test.go | 12 +- submitqueue/entity/request_test.go | 12 +- .../buildrunner/buildkite/buildkite_test.go | 14 +-- .../extension/buildrunner/fake/fake_test.go | 14 +-- .../githubactions/githubactions_test.go | 8 +- .../changeprovider/fake/fake_test.go | 8 +- .../changeprovider/github/provider_test.go | 22 ++-- .../changeprovider/github/validate_test.go | 2 +- .../phabricator/convert_test.go | 4 +- .../phabricator/provider_test.go | 20 ++-- .../changeprovider/routing/provider_test.go | 8 +- .../extension/mergechecker/fake/fake_test.go | 10 +- .../mergechecker/github/checker_test.go | 12 +- .../mergechecker/github/graphql_test.go | 2 +- .../extension/pusher/fake/fake_test.go | 8 +- .../extension/pusher/git/git_pusher_test.go | 2 +- .../extension/scorer/fake/fake_test.go | 4 +- submitqueue/gateway/controller/land_test.go | 18 +-- .../controller/batch/batch_test.go | 4 +- .../controller/merge/merge_test.go | 4 +- .../controller/score/score_test.go | 2 +- .../controller/start/start_test.go | 12 +- .../controller/validate/validate_test.go | 18 +-- test/e2e/submitqueue/suite_test.go | 4 +- .../submitqueue/extension/storage/suite.go | 14 +-- .../submitqueue/gateway/suite_test.go | 2 +- 45 files changed, 420 insertions(+), 269 deletions(-) create mode 100644 platform/base/change/changeutil/host.go create mode 100644 platform/base/change/changeutil/host_test.go diff --git a/api/base/change/proto/change.proto b/api/base/change/proto/change.proto index a86a58f1..784eaa1b 100644 --- a/api/base/change/proto/change.proto +++ b/api/base/change/proto/change.proto @@ -29,14 +29,15 @@ option java_package = "com.uber.submitqueue.base.change"; // Stovepipe, and other repo-local domains — the proto-level analog of the // platform/base/change Go entity. Domains import it rather than redefining their own. message Change { - // URIs identifying the change(s) (RFC 3986 compliant). The scheme identifies the - // change provider, and the path contains provider-specific resource identifiers. + // URIs identifying the change(s) (RFC 3986 compliant): scheme:///. + // The scheme identifies the change provider, the authority is the provider instance + // the change lives on, and the path contains provider-specific resource identifiers. // // Supported by default (other providers can be added): - // GitHub PR: "github:////pull//" - // ("ghe"/"ghes" schemes for GitHub Enterprise) - // git commit: "git://///" - // ( is a fully-qualified, percent-encoded git ref) + // GitHub PR: "github://///pull//" + // Phabricator Diff: "phab:///D/" + // git commit: "git://///" + // ( is a fully-qualified, percent-encoded git ref) // // The commit SHA must be the full 40-character lowercase hex SHA; abbreviated // SHAs are rejected because downstream staleness checks compare by strict equality. diff --git a/api/base/change/protopb/change.pb.go b/api/base/change/protopb/change.pb.go index 28416150..37c95cad 100644 --- a/api/base/change/protopb/change.pb.go +++ b/api/base/change/protopb/change.pb.go @@ -45,15 +45,16 @@ const ( // platform/base/change Go entity. Domains import it rather than redefining their own. type Change struct { state protoimpl.MessageState `protogen:"open.v1"` - // URIs identifying the change(s) (RFC 3986 compliant). The scheme identifies the - // change provider, and the path contains provider-specific resource identifiers. + // URIs identifying the change(s) (RFC 3986 compliant): scheme:///. + // The scheme identifies the change provider, the authority is the provider instance + // the change lives on, and the path contains provider-specific resource identifiers. // // Supported by default (other providers can be added): // - // GitHub PR: "github:////pull//" - // ("ghe"/"ghes" schemes for GitHub Enterprise) - // git commit: "git://///" - // ( is a fully-qualified, percent-encoded git ref) + // GitHub PR: "github://///pull//" + // Phabricator Diff: "phab:///D/" + // git commit: "git://///" + // ( is a fully-qualified, percent-encoded git ref) // // The commit SHA must be the full 40-character lowercase hex SHA; abbreviated // SHAs are rejected because downstream staleness checks compare by strict equality. diff --git a/api/runway/messagequeue/merge_test.go b/api/runway/messagequeue/merge_test.go index 72eb9e0b..5be5c368 100644 --- a/api/runway/messagequeue/merge_test.go +++ b/api/runway/messagequeue/merge_test.go @@ -33,12 +33,12 @@ func TestMergeRequestRoundTrip(t *testing.T) { Steps: []*MergeStep{ { StepId: "queue-a/1", - Changes: []*changepb.Change{{Uris: []string{"github://uber/repo/pull/1/0123456789abcdef0123456789abcdef01234567"}}}, + Changes: []*changepb.Change{{Uris: []string{"github://github.example.com/uber/repo/pull/1/0123456789abcdef0123456789abcdef01234567"}}}, Strategy: strategypb.Strategy_REBASE, }, { StepId: "queue-a/2", - Changes: []*changepb.Change{{Uris: []string{"github://uber/repo/pull/2/89abcdef0123456789abcdef0123456789abcdef"}}}, + Changes: []*changepb.Change{{Uris: []string{"github://github.example.com/uber/repo/pull/2/89abcdef0123456789abcdef0123456789abcdef"}}}, Strategy: strategypb.Strategy_MERGE, }, }, diff --git a/platform/base/change/change.go b/platform/base/change/change.go index ff86c272..5848948d 100644 --- a/platform/base/change/change.go +++ b/platform/base/change/change.go @@ -20,13 +20,17 @@ package change // Change represents a code change identified by URIs from a code change provider (e.g., GitHub Pull Request, Phabricator Diff). // The provider is extracted from the URI scheme. The object is immutable after creation. type Change struct { - // URIs identifies the change(s) to land (RFC 3986 compliant). - // The scheme identifies the change provider, and the path contains provider-specific resource identifiers. + // URIs identifies the change(s) to land (RFC 3986 compliant): scheme:///. + // The scheme identifies the change provider, the authority is the provider instance the + // change lives on, and the path contains provider-specific resource identifiers. // - // GitHub is supported by default (though other providers can be added): - // Template: ":////pull//" - // Example: "github://uber/submitqueue/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab" - // Schemes: "github", "ghe", "ghes". Head commit SHA must be full 40-char lowercase hex. + // Supported formats: + // GitHub PR: "github://///pull//" + // Phabricator Diff: "phab:///D/" + // git ref/commit: "git://///" + // Example: "github://github.example.com/uber/submitqueue/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab" + // + // Head/commit SHAs must be the full 40-char lowercase hex form. // URIs []string `json:"uris"` } diff --git a/platform/base/change/changeutil/BUILD.bazel b/platform/base/change/changeutil/BUILD.bazel index 36211060..1165707f 100644 --- a/platform/base/change/changeutil/BUILD.bazel +++ b/platform/base/change/changeutil/BUILD.bazel @@ -2,14 +2,20 @@ load("@rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", - srcs = ["hex.go"], + srcs = [ + "hex.go", + "host.go", + ], importpath = "github.com/uber/submitqueue/platform/base/change/changeutil", visibility = ["//visibility:public"], ) go_test( name = "go_default_test", - srcs = ["hex_test.go"], + srcs = [ + "hex_test.go", + "host_test.go", + ], embed = [":go_default_library"], deps = ["@com_github_stretchr_testify//assert:go_default_library"], ) diff --git a/platform/base/change/changeutil/host.go b/platform/base/change/changeutil/host.go new file mode 100644 index 00000000..758c6a08 --- /dev/null +++ b/platform/base/change/changeutil/host.go @@ -0,0 +1,30 @@ +// 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 changeutil + +// IsLowercaseASCII reports whether s contains no uppercase ASCII letters (A-Z). +// Providers validate the host segment of a change URI with this: DNS is +// case-insensitive, so an uppercase variant of a host would alias one +// provider instance into many identities if let through. Canonical form +// rejects uppercase hosts rather than folding them. +func IsLowercaseASCII(s string) bool { + for i := 0; i < len(s); i++ { + c := s[i] + if c >= 'A' && c <= 'Z' { + return false + } + } + return true +} diff --git a/platform/base/change/changeutil/host_test.go b/platform/base/change/changeutil/host_test.go new file mode 100644 index 00000000..f9340e60 --- /dev/null +++ b/platform/base/change/changeutil/host_test.go @@ -0,0 +1,42 @@ +// 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 changeutil + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestIsLowercaseASCII(t *testing.T) { + tests := []struct { + name string + s string + want bool + }{ + {name: "plain lowercase host", s: "github.example.com", want: true}, + {name: "lowercase host with port", s: "github.example.com:8443", want: true}, + {name: "empty string", s: "", want: true}, + {name: "digits and hyphens", s: "git-01.example-corp.com", want: true}, + {name: "uppercase letter rejected", s: "GitHub.example.com", want: false}, + {name: "all uppercase rejected", s: "GITHUB.EXAMPLE.COM", want: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, IsLowercaseASCII(tt.s)) + }) + } +} diff --git a/platform/base/change/git/change_id.go b/platform/base/change/git/change_id.go index 73df17eb..e354f61c 100644 --- a/platform/base/change/git/change_id.go +++ b/platform/base/change/git/change_id.go @@ -72,6 +72,9 @@ func ParseChangeID(raw string) (ChangeID, error) { if u.Host == "" { return ChangeID{}, fmt.Errorf("invalid change ID %q: missing remote (expected format: %s)", raw, changeIDFormat) } + if !changeutil.IsLowercaseASCII(u.Hostname()) { + return ChangeID{}, fmt.Errorf("invalid change ID %q: remote %q must be lowercase (expected format: %s)", raw, u.Hostname(), changeIDFormat) + } // Split on the escaped path so the percent-encoded ref stays a single // segment (url.URL.Path decodes %2F to "/", which would split it apart). diff --git a/platform/base/change/git/change_id_test.go b/platform/base/change/git/change_id_test.go index f8bbdc76..c06cea67 100644 --- a/platform/base/change/git/change_id_test.go +++ b/platform/base/change/git/change_id_test.go @@ -106,6 +106,11 @@ func TestParseChangeID(t *testing.T) { raw: "git:///uber/monorepo/refs%2Fheads%2Fmain/" + sha, wantErr: true, }, + { + name: "uppercase host", + raw: "git://Git.example.com/uber/monorepo/refs%2Fheads%2Fmain/" + sha, + wantErr: true, + }, { name: "missing commit SHA", raw: "git://git.example.com/uber/monorepo/refs%2Fheads%2Fmain", diff --git a/platform/base/change/github/change_id.go b/platform/base/change/github/change_id.go index c0097719..d6e75f46 100644 --- a/platform/base/change/github/change_id.go +++ b/platform/base/change/github/change_id.go @@ -16,14 +16,21 @@ package github import ( "fmt" + "net/url" "strconv" "strings" "github.com/uber/submitqueue/platform/base/change/changeutil" ) +// scheme is the canonical URI scheme for GitHub change identifiers. Deployment +// flavors (github.com, GitHub Enterprise, GitHub Enterprise Server) share this +// one scheme; the flavor is derivable from the host in the authority, so it +// does not get its own scheme. +const scheme = "github" + // changeIDFormat is the expected format for change IDs, included in error messages. -const changeIDFormat = "{scheme}://{owner}/{repo}/pull/{pr_number}/{head_commit_sha}" +const changeIDFormat = "github://{host[:port]}/{owner}/{repo}/pull/{pr_number}/{head_commit_sha}" // pullSegment is the literal segment separating the repo path from the pull // request number. Mirrors the path layout of an actual GitHub PR URL @@ -38,13 +45,16 @@ const pullSegment = "pull" // Validate up-front to fail fast at the gateway with a clearer error. const shaLength = 40 -// ChangeID represents a parsed GitHub-family change identifier. -// Covers GitHub.com, GitHub Enterprise (GHE), and GitHub Enterprise Server (GHES) -// since they share the same pull request model. -// Format: {scheme}://{owner}/{repo}/pull/{pr_number}/{head_commit_sha} +// ChangeID represents a parsed GitHub change identifier. The authority names +// the GitHub instance the change lives on, so github.com, GitHub Enterprise, +// and GitHub Enterprise Server all parse under the single "github" scheme. +// Format: github://{host[:port]}/{owner}/{repo}/pull/{pr_number}/{head_commit_sha} type ChangeID struct { - // Scheme captures the source variant (e.g., "github", "ghe", "ghes"). + // Scheme captures the URI scheme (always "github" in current implementation). Scheme string + // Host is the host or host:port of the GitHub instance the change lives + // on, e.g. "github.example.com" or "github.example.com:8443". + Host string // Org is the organization or owner of the repository. Org string // Repo is the repository name. @@ -56,29 +66,30 @@ type ChangeID struct { } // ParseChangeID parses a raw change ID string into a ChangeID. -// Expected format: {scheme}://{owner}/{repo}/pull/{pr_number}/{head_commit_sha} +// Expected format: github://{host[:port]}/{owner}/{repo}/pull/{pr_number}/{head_commit_sha} // The parser works from the end: SHA (last), PR number (second-to-last), // the literal "pull" segment (third-to-last), and everything before is the // repo path (split into owner and repo). func ParseChangeID(raw string) (ChangeID, error) { - // Split on "://" to get scheme and path - schemeSplit := strings.SplitN(raw, "://", 2) - if len(schemeSplit) != 2 { - return ChangeID{}, fmt.Errorf("invalid change ID %q: missing '://' separator (expected format: %s)", raw, changeIDFormat) + u, err := url.Parse(raw) + if err != nil { + return ChangeID{}, fmt.Errorf("invalid change ID %q: %w (expected format: %s)", raw, err, changeIDFormat) } - - scheme := schemeSplit[0] - if scheme == "" { - return ChangeID{}, fmt.Errorf("invalid change ID %q: empty scheme (expected format: %s)", raw, changeIDFormat) + if u.Scheme != scheme { + return ChangeID{}, fmt.Errorf("invalid change ID %q: scheme must be %q, got %q (expected format: %s)", raw, scheme, u.Scheme, changeIDFormat) + } + if u.Host == "" { + return ChangeID{}, fmt.Errorf("invalid change ID %q: missing host (expected format: %s)", raw, changeIDFormat) + } + if !changeutil.IsLowercaseASCII(u.Hostname()) { + return ChangeID{}, fmt.Errorf("invalid change ID %q: host %q must be lowercase (expected format: %s)", raw, u.Hostname(), changeIDFormat) } - path := schemeSplit[1] - - // Split the path into segments and parse from the end. - segments := strings.Split(path, "/") - // Need at least 5 segments: {owner}/{repo}/pull/{pr_number}/{sha} + // Split on the escaped path so any percent-encoded segments stay intact. + segments := strings.Split(strings.TrimPrefix(u.EscapedPath(), "/"), "/") + // Need at least 5 segments: {owner}/{repo}/pull/{pr_number}/{sha}. if len(segments) < 5 { - return ChangeID{}, fmt.Errorf("invalid change ID %q: need at least owner/repo/pull/pr/sha, got %d segments (expected format: %s)", raw, len(segments), changeIDFormat) + return ChangeID{}, fmt.Errorf("invalid change ID %q: need at least owner/repo/pull/pr/sha, got %d path segments (expected format: %s)", raw, len(segments), changeIDFormat) } sha := segments[len(segments)-1] @@ -118,7 +129,8 @@ func ParseChangeID(raw string) (ChangeID, error) { } return ChangeID{ - Scheme: scheme, + Scheme: u.Scheme, + Host: u.Host, Org: org, Repo: repo, PRNumber: prNumber, @@ -128,7 +140,7 @@ func ParseChangeID(raw string) (ChangeID, error) { // String returns the string representation of the change ID. func (c ChangeID) String() string { - return fmt.Sprintf("%s://%s/%s/%s/%d/%s", c.Scheme, c.Org, c.Repo, pullSegment, c.PRNumber, c.HeadCommitSHA) + return fmt.Sprintf("%s://%s/%s/%s/%s/%d/%s", c.Scheme, c.Host, c.Org, c.Repo, pullSegment, c.PRNumber, c.HeadCommitSHA) } // OwnerRepo returns the "{org}/{repo}" string. diff --git a/platform/base/change/github/change_id_test.go b/platform/base/change/github/change_id_test.go index 7f8c90ea..58d7a247 100644 --- a/platform/base/change/github/change_id_test.go +++ b/platform/base/change/github/change_id_test.go @@ -38,10 +38,11 @@ func TestParseChangeID(t *testing.T) { wantErr bool }{ { - name: "valid github scheme", - raw: "github://uber/submitqueue/pull/123/" + shaAFull, + name: "plain host", + raw: "github://github.example.com/uber/submitqueue/pull/123/" + shaAFull, want: ChangeID{ Scheme: "github", + Host: "github.example.com", Org: "uber", Repo: "submitqueue", PRNumber: 123, @@ -49,32 +50,23 @@ func TestParseChangeID(t *testing.T) { }, }, { - name: "valid ghe scheme", - raw: "ghe://uber/monorepo/pull/456/" + shaCFull, + name: "host with port", + raw: "github://github.example.com:8443/uber/monorepo/pull/456/" + shaCFull, want: ChangeID{ - Scheme: "ghe", + Scheme: "github", + Host: "github.example.com:8443", Org: "uber", Repo: "monorepo", PRNumber: 456, HeadCommitSHA: shaCFull, }, }, - { - name: "valid ghes scheme", - raw: "ghes://org/repo/pull/1/" + sha1Full, - want: ChangeID{ - Scheme: "ghes", - Org: "org", - Repo: "repo", - PRNumber: 1, - HeadCommitSHA: sha1Full, - }, - }, { name: "nested org path", - raw: "github://uber/frontend/webapp/pull/42/" + shaAFull, + raw: "github://github.example.com/uber/frontend/webapp/pull/42/" + shaAFull, want: ChangeID{ Scheme: "github", + Host: "github.example.com", Org: "uber/frontend", Repo: "webapp", PRNumber: 42, @@ -83,72 +75,87 @@ func TestParseChangeID(t *testing.T) { }, { name: "missing pull segment", - raw: "github://uber/submitqueue/123/" + shaAFull, + raw: "github://github.example.com/uber/submitqueue/123/" + shaAFull, wantErr: true, }, { name: "wrong literal segment (issues instead of pull)", - raw: "github://uber/submitqueue/issues/123/" + shaAFull, + raw: "github://github.example.com/uber/submitqueue/issues/123/" + shaAFull, + wantErr: true, + }, + { + name: "missing host", + raw: "github:///uber/submitqueue/pull/123/" + shaAFull, + wantErr: true, + }, + { + name: "uppercase host", + raw: "github://GitHub.example.com/uber/submitqueue/pull/123/" + shaAFull, wantErr: true, }, { - name: "missing separator", - raw: "github/uber/submitqueue/pull/123/" + shaAFull, + name: "old host-less format now parses host as owner and fails on path length", + raw: "github://uber/submitqueue/pull/123/" + shaAFull, wantErr: true, }, { - name: "empty scheme", - raw: "://uber/submitqueue/pull/123/" + shaAFull, + name: "ghe scheme rejected", + raw: "ghe://github.example.com/uber/monorepo/pull/456/" + shaCFull, + wantErr: true, + }, + { + name: "ghes scheme rejected", + raw: "ghes://github.example.com/org/repo/pull/1/" + sha1Full, wantErr: true, }, { name: "too few segments", - raw: "github://uber/pull/123/" + shaAFull, + raw: "github://github.example.com/uber/pull/123/" + shaAFull, wantErr: true, }, { name: "only one segment", - raw: "github://" + shaAFull, + raw: "github://github.example.com", wantErr: true, }, { name: "empty owner", - raw: "github:///submitqueue/pull/123/" + shaAFull, + raw: "github://github.example.com//submitqueue/pull/123/" + shaAFull, wantErr: true, }, { name: "empty repo", - raw: "github://uber//pull/123/" + shaAFull, + raw: "github://github.example.com/uber//pull/123/" + shaAFull, wantErr: true, }, { name: "non-numeric PR number", - raw: "github://uber/submitqueue/pull/abc/" + shaAFull, + raw: "github://github.example.com/uber/submitqueue/pull/abc/" + shaAFull, wantErr: true, }, { name: "empty SHA", - raw: "github://uber/submitqueue/pull/123/", + raw: "github://github.example.com/uber/submitqueue/pull/123/", wantErr: true, }, { name: "abbreviated SHA", - raw: "github://uber/submitqueue/pull/123/abc123def", + raw: "github://github.example.com/uber/submitqueue/pull/123/abc123def", wantErr: true, }, { name: "uppercase SHA", - raw: "github://uber/submitqueue/pull/123/ABCDEF0123456789ABCDEF0123456789ABCDEF01", + raw: "github://github.example.com/uber/submitqueue/pull/123/ABCDEF0123456789ABCDEF0123456789ABCDEF01", wantErr: true, }, { name: "non-hex SHA", - raw: "github://uber/submitqueue/pull/123/zzzzzz0123456789abcdef0123456789abcdef01", + raw: "github://github.example.com/uber/submitqueue/pull/123/zzzzzz0123456789abcdef0123456789abcdef01", wantErr: true, }, { name: "SHA too long", - raw: "github://uber/submitqueue/pull/123/" + shaAFull + "ab", + raw: "github://github.example.com/uber/submitqueue/pull/123/" + shaAFull + "ab", wantErr: true, }, { @@ -178,37 +185,40 @@ func TestChangeID_String(t *testing.T) { want string }{ { - name: "github", + name: "plain host", id: ChangeID{ Scheme: "github", + Host: "github.example.com", Org: "uber", Repo: "submitqueue", PRNumber: 123, HeadCommitSHA: shaAFull, }, - want: "github://uber/submitqueue/pull/123/" + shaAFull, + want: "github://github.example.com/uber/submitqueue/pull/123/" + shaAFull, }, { - name: "ghe", + name: "host with port", id: ChangeID{ - Scheme: "ghe", + Scheme: "github", + Host: "github.example.com:8443", Org: "corp", Repo: "app", PRNumber: 99, HeadCommitSHA: shaCFull, }, - want: "ghe://corp/app/pull/99/" + shaCFull, + want: "github://github.example.com:8443/corp/app/pull/99/" + shaCFull, }, { - name: "ghes", + name: "nested org", id: ChangeID{ - Scheme: "ghes", - Org: "org", - Repo: "repo", - PRNumber: 1, - HeadCommitSHA: sha1Full, + Scheme: "github", + Host: "github.example.com", + Org: "uber/frontend", + Repo: "webapp", + PRNumber: 42, + HeadCommitSHA: shaAFull, }, - want: "ghes://org/repo/pull/1/" + sha1Full, + want: "github://github.example.com/uber/frontend/webapp/pull/42/" + shaAFull, }, } @@ -222,6 +232,7 @@ func TestChangeID_String(t *testing.T) { func TestChangeID_OwnerRepo(t *testing.T) { id := ChangeID{ Scheme: "github", + Host: "github.example.com", Org: "uber", Repo: "submitqueue", PRNumber: 1, @@ -232,10 +243,10 @@ func TestChangeID_OwnerRepo(t *testing.T) { func TestParseChangeID_RoundTrip(t *testing.T) { originals := []string{ - "github://uber/submitqueue/pull/123/" + shaAFull, - "ghe://corp/monorepo/pull/99/" + shaCFull, - "ghes://org/repo/pull/1/" + sha2Full, - "github://uber/frontend/webapp/pull/42/" + shaBFull, + "github://github.example.com/uber/submitqueue/pull/123/" + shaAFull, + "github://github.example.com:8443/corp/monorepo/pull/99/" + shaCFull, + "github://github.example.com/org/repo/pull/1/" + sha2Full, + "github://github.example.com/uber/frontend/webapp/pull/42/" + shaBFull, } for _, raw := range originals { diff --git a/platform/base/change/phabricator/BUILD.bazel b/platform/base/change/phabricator/BUILD.bazel index 2aa4fc24..24bfa7f8 100644 --- a/platform/base/change/phabricator/BUILD.bazel +++ b/platform/base/change/phabricator/BUILD.bazel @@ -5,6 +5,7 @@ go_library( srcs = ["change_id.go"], importpath = "github.com/uber/submitqueue/platform/base/change/phabricator", visibility = ["//visibility:public"], + deps = ["//platform/base/change/changeutil:go_default_library"], ) go_test( diff --git a/platform/base/change/phabricator/change_id.go b/platform/base/change/phabricator/change_id.go index 535672e8..a2670a7f 100644 --- a/platform/base/change/phabricator/change_id.go +++ b/platform/base/change/phabricator/change_id.go @@ -16,17 +16,21 @@ package phabricator import ( "fmt" + "net/url" "regexp" "strconv" "strings" + + "github.com/uber/submitqueue/platform/base/change/changeutil" ) // changeIDFormat is the expected format for change IDs, included in error messages. -const changeIDFormat = "{scheme}://D{revision_id}/{diff_id}" +const changeIDFormat = "phab://{host[:port]}/D{revision_id}/{diff_id}" // scheme is the canonical URI scheme for Phabricator change identifiers. -// Unlike GitHub (which has multiple variants: github / ghe / ghes), Phabricator -// has a single scheme; the host is encoded out-of-band via queue config. +// Phabricator has a single scheme (there is no per-flavor variant like GitHub's +// github.com vs. GitHub Enterprise); the host in the authority names the +// specific Phabricator instance the change lives on. const scheme = "phab" // revisionPrefix is the literal "D" character that prefixes Phabricator revision @@ -45,7 +49,7 @@ var revisionPattern = regexp.MustCompile(`^D([1-9]\d*)$`) var diffPattern = regexp.MustCompile(`^[1-9]\d*$`) // ChangeID represents a parsed Phabricator change identifier. -// Format: phab://D{revision_id}/{diff_id} +// Format: phab://{host[:port]}/D{revision_id}/{diff_id} // // Revision and diff are Phabricator's two-level identifier model: // - RevisionID names the logical review (stable across updates, e.g., D12345). @@ -57,6 +61,9 @@ type ChangeID struct { // the parsed form mirrors entity/change/github.ChangeID and so future variants // (e.g., a separate scheme per Phabricator install) are a non-breaking add. Scheme string + // Host is the host or host:port of the Phabricator instance the change + // lives on (e.g. "phabricator.example.com" or "phabricator.example.com:443"). + Host string // RevisionID is the numeric portion of the Phabricator revision identifier // (the digits after the leading "D"). For example, "D12345" parses to 12345. RevisionID int @@ -66,23 +73,25 @@ type ChangeID struct { } // ParseChangeID parses a raw change ID string into a ChangeID. -// Expected format: phab://D{revision_id}/{diff_id} -// The parser splits on "://" to separate scheme from path, then the path into -// exactly two segments: the D-prefixed revision and the diff ID. +// Expected format: phab://{host[:port]}/D{revision_id}/{diff_id} +// The parser requires a non-empty, lowercase host, then splits the remaining +// path into exactly two segments: the D-prefixed revision and the diff ID. func ParseChangeID(raw string) (ChangeID, error) { - schemeSplit := strings.SplitN(raw, "://", 2) - if len(schemeSplit) != 2 { - return ChangeID{}, fmt.Errorf("invalid change ID %q: missing '://' separator (expected format: %s)", raw, changeIDFormat) + u, err := url.Parse(raw) + if err != nil { + return ChangeID{}, fmt.Errorf("invalid change ID %q: %w (expected format: %s)", raw, err, changeIDFormat) } - - gotScheme := schemeSplit[0] - if gotScheme != scheme { - return ChangeID{}, fmt.Errorf("invalid change ID %q: scheme must be %q, got %q (expected format: %s)", raw, scheme, gotScheme, changeIDFormat) + if u.Scheme != scheme { + return ChangeID{}, fmt.Errorf("invalid change ID %q: scheme must be %q, got %q (expected format: %s)", raw, scheme, u.Scheme, changeIDFormat) + } + if u.Host == "" { + return ChangeID{}, fmt.Errorf("invalid change ID %q: missing host (expected format: %s)", raw, changeIDFormat) + } + if !changeutil.IsLowercaseASCII(u.Hostname()) { + return ChangeID{}, fmt.Errorf("invalid change ID %q: host %q must be lowercase (expected format: %s)", raw, u.Hostname(), changeIDFormat) } - path := schemeSplit[1] - - segments := strings.Split(path, "/") + segments := strings.Split(strings.TrimPrefix(u.EscapedPath(), "/"), "/") if len(segments) != 2 { return ChangeID{}, fmt.Errorf("invalid change ID %q: path must have exactly 2 segments (revision/diff), got %d (expected format: %s)", raw, len(segments), changeIDFormat) } @@ -108,7 +117,8 @@ func ParseChangeID(raw string) (ChangeID, error) { } return ChangeID{ - Scheme: gotScheme, + Scheme: u.Scheme, + Host: u.Host, RevisionID: revisionID, DiffID: diffID, }, nil @@ -116,7 +126,7 @@ func ParseChangeID(raw string) (ChangeID, error) { // String returns the string representation of the change ID. func (c ChangeID) String() string { - return fmt.Sprintf("%s://%s%d/%d", c.Scheme, revisionPrefix, c.RevisionID, c.DiffID) + return fmt.Sprintf("%s://%s/%s%d/%d", c.Scheme, c.Host, revisionPrefix, c.RevisionID, c.DiffID) } // Revision returns the Phabricator revision identifier in its canonical diff --git a/platform/base/change/phabricator/change_id_test.go b/platform/base/change/phabricator/change_id_test.go index 61c53856..6ab4743f 100644 --- a/platform/base/change/phabricator/change_id_test.go +++ b/platform/base/change/phabricator/change_id_test.go @@ -34,23 +34,40 @@ func TestParseChangeID(t *testing.T) { }{ { name: "valid", - raw: "phab://D123/456", - want: ChangeID{Scheme: "phab", RevisionID: 123, DiffID: 456}, + raw: "phab://phab.example.com/D123/456", + want: ChangeID{Scheme: "phab", Host: "phab.example.com", RevisionID: 123, DiffID: 456}, + }, + { + name: "valid with port", + raw: "phab://phab.example.com:443/D123/456", + want: ChangeID{Scheme: "phab", Host: "phab.example.com:443", RevisionID: 123, DiffID: 456}, }, {name: "missing separator", raw: "phab:D123/456", wantErr: true}, - {name: "wrong scheme", raw: "github://D123/456", wantErr: true}, - {name: "wrong segment count", raw: "phab://D123", wantErr: true}, - {name: "missing revision prefix", raw: "phab://123/456", wantErr: true}, - {name: "revision prefix without digits", raw: "phab://D/456", wantErr: true}, - {name: "non-numeric revision", raw: "phab://Dabc/456", wantErr: true}, - {name: "non-positive revision", raw: "phab://D0/456", wantErr: true}, - {name: "empty diff", raw: "phab://D123/", wantErr: true}, - {name: "non-numeric diff", raw: "phab://D123/abc", wantErr: true}, - {name: "non-positive diff", raw: "phab://D123/0", wantErr: true}, - {name: "leading zero revision", raw: "phab://D01/2", wantErr: true}, - {name: "leading zero diff", raw: "phab://D1/02", wantErr: true}, - {name: "overflow revision", raw: "phab://D99999999999999999999/1", wantErr: true}, - {name: "overflow diff", raw: "phab://D1/99999999999999999999", wantErr: true}, + {name: "wrong scheme", raw: "github://phab.example.com/D123/456", wantErr: true}, + {name: "missing host", raw: "phab:///D123/456", wantErr: true}, + {name: "uppercase host", raw: "phab://Phab.example.com/D123/456", wantErr: true}, + { + name: "old host-less format rejected: D12345 parses as an uppercase host", + raw: "phab://D12345/67890", + wantErr: true, + }, + { + name: "lowercase host variant of old format fails on segment count", + raw: "phab://d12345/67890", + wantErr: true, + }, + {name: "wrong segment count", raw: "phab://phab.example.com/D123", wantErr: true}, + {name: "missing revision prefix", raw: "phab://phab.example.com/123/456", wantErr: true}, + {name: "revision prefix without digits", raw: "phab://phab.example.com/D/456", wantErr: true}, + {name: "non-numeric revision", raw: "phab://phab.example.com/Dabc/456", wantErr: true}, + {name: "non-positive revision", raw: "phab://phab.example.com/D0/456", wantErr: true}, + {name: "empty diff", raw: "phab://phab.example.com/D123/", wantErr: true}, + {name: "non-numeric diff", raw: "phab://phab.example.com/D123/abc", wantErr: true}, + {name: "non-positive diff", raw: "phab://phab.example.com/D123/0", wantErr: true}, + {name: "leading zero revision", raw: "phab://phab.example.com/D01/2", wantErr: true}, + {name: "leading zero diff", raw: "phab://phab.example.com/D1/02", wantErr: true}, + {name: "overflow revision", raw: "phab://phab.example.com/D99999999999999999999/1", wantErr: true}, + {name: "overflow diff", raw: "phab://phab.example.com/D1/99999999999999999999", wantErr: true}, } for _, tt := range tests { @@ -67,13 +84,21 @@ func TestParseChangeID(t *testing.T) { } func TestChangeID_Revision(t *testing.T) { - id := ChangeID{Scheme: "phab", RevisionID: 12345, DiffID: 67890} + id := ChangeID{Scheme: "phab", Host: "phab.example.com", RevisionID: 12345, DiffID: 67890} assert.Equal(t, "D12345", id.Revision()) } func TestParseChangeID_RoundTrip(t *testing.T) { - raw := "phab://D123/456" - parsed, err := ParseChangeID(raw) - require.NoError(t, err) - assert.Equal(t, raw, parsed.String()) + originals := []string{ + "phab://phab.example.com/D123/456", + "phab://phab.example.com:443/D123/456", + } + + for _, raw := range originals { + t.Run(raw, func(t *testing.T) { + parsed, err := ParseChangeID(raw) + require.NoError(t, err) + assert.Equal(t, raw, parsed.String()) + }) + } } diff --git a/runway/extension/merger/noop/noop_test.go b/runway/extension/merger/noop/noop_test.go index e3f54cf3..740281aa 100644 --- a/runway/extension/merger/noop/noop_test.go +++ b/runway/extension/merger/noop/noop_test.go @@ -33,12 +33,12 @@ func testRequest() *runwaymq.MergeRequest { Steps: []*runwaymq.MergeStep{ { StepId: "queue-a/1", - Changes: []*changepb.Change{{Uris: []string{"github://uber/repo/pull/1/abcdef0123456789abcdef0123456789abcdef01"}}}, + Changes: []*changepb.Change{{Uris: []string{"github://github.example.com/uber/repo/pull/1/abcdef0123456789abcdef0123456789abcdef01"}}}, Strategy: strategypb.Strategy_REBASE, }, { StepId: "queue-a/2", - Changes: []*changepb.Change{{Uris: []string{"github://uber/repo/pull/2/89abcdef0123456789abcdef0123456789abcdef"}}}, + Changes: []*changepb.Change{{Uris: []string{"github://github.example.com/uber/repo/pull/2/89abcdef0123456789abcdef0123456789abcdef"}}}, Strategy: strategypb.Strategy_MERGE, }, }, diff --git a/submitqueue/core/fakemarker/fakemarker.go b/submitqueue/core/fakemarker/fakemarker.go index f729b626..741c5976 100644 --- a/submitqueue/core/fakemarker/fakemarker.go +++ b/submitqueue/core/fakemarker/fakemarker.go @@ -32,7 +32,7 @@ const Prefix = "sq-fake=" // Token returns the marker token embedded in the first URI that carries one, or // "" if none do. The token ends at the first "&" or "#" delimiter, so a marker // may sit among other query parameters or a fragment (e.g. -// "github://o/r/pull/1/a?sq-fake=build-fail&attempt=2"). +// "github://github.example.com/o/r/pull/1/a?sq-fake=build-fail&attempt=2"). func Token(uris []string) string { for _, u := range uris { if i := strings.Index(u, Prefix); i >= 0 { diff --git a/submitqueue/core/fakemarker/fakemarker_test.go b/submitqueue/core/fakemarker/fakemarker_test.go index 8e827f5d..90e35478 100644 --- a/submitqueue/core/fakemarker/fakemarker_test.go +++ b/submitqueue/core/fakemarker/fakemarker_test.go @@ -34,37 +34,37 @@ func TestToken(t *testing.T) { }, { name: "no marker", - uris: []string{"github://o/r/pull/1/a"}, + uris: []string{"github://github.example.com/o/r/pull/1/a"}, want: "", }, { name: "marker at end of uri", - uris: []string{"github://o/r/pull/1/a?sq-fake=build-fail"}, + uris: []string{"github://github.example.com/o/r/pull/1/a?sq-fake=build-fail"}, want: "build-fail", }, { name: "marker trimmed at & delimiter", - uris: []string{"github://o/r/pull/1/a?sq-fake=build-fail&attempt=2"}, + uris: []string{"github://github.example.com/o/r/pull/1/a?sq-fake=build-fail&attempt=2"}, want: "build-fail", }, { name: "marker trimmed at # delimiter", - uris: []string{"github://o/r/pull/1/a?sq-fake=build-fail#frag"}, + uris: []string{"github://github.example.com/o/r/pull/1/a?sq-fake=build-fail#frag"}, want: "build-fail", }, { name: "marker before a query param it precedes", - uris: []string{"github://o/r/pull/1/a?sq-fake=push-error&foo=bar#frag"}, + uris: []string{"github://github.example.com/o/r/pull/1/a?sq-fake=push-error&foo=bar#frag"}, want: "push-error", }, { name: "marker on a later uri", - uris: []string{"github://o/r/pull/1/a", "github://o/r/pull/2/b?sq-fake=conflict"}, + uris: []string{"github://github.example.com/o/r/pull/1/a", "github://github.example.com/o/r/pull/2/b?sq-fake=conflict"}, want: "conflict", }, { name: "first marker wins", - uris: []string{"github://o/r/pull/1/a?sq-fake=first", "github://o/r/pull/2/b?sq-fake=second"}, + uris: []string{"github://github.example.com/o/r/pull/1/a?sq-fake=first", "github://github.example.com/o/r/pull/2/b?sq-fake=second"}, want: "first", }, } @@ -89,19 +89,19 @@ func TestTokenInChanges(t *testing.T) { }, { name: "no marker", - changes: []change.Change{{URIs: []string{"github://o/r/pull/1/a"}}}, + changes: []change.Change{{URIs: []string{"github://github.example.com/o/r/pull/1/a"}}}, want: "", }, { name: "marker on first change", - changes: []change.Change{{URIs: []string{"github://o/r/pull/1/a?sq-fake=build-fail&attempt=2"}}}, + changes: []change.Change{{URIs: []string{"github://github.example.com/o/r/pull/1/a?sq-fake=build-fail&attempt=2"}}}, want: "build-fail", }, { name: "marker on later change", changes: []change.Change{ - {URIs: []string{"github://o/r/pull/1/a"}}, - {URIs: []string{"github://o/r/pull/2/b?sq-fake=push-error"}}, + {URIs: []string{"github://github.example.com/o/r/pull/1/a"}}, + {URIs: []string{"github://github.example.com/o/r/pull/2/b?sq-fake=push-error"}}, }, want: "push-error", }, diff --git a/submitqueue/entity/change_provider.go b/submitqueue/entity/change_provider.go index 9ffacc0a..a363e2a6 100644 --- a/submitqueue/entity/change_provider.go +++ b/submitqueue/entity/change_provider.go @@ -69,7 +69,7 @@ func (d ChangeDetails) FileCount() int { // returns one ChangeInfo per URI so callers can correlate results to inputs by URI. type ChangeInfo struct { // URI is the full change URI for correlation with the input request - // (e.g., "github://uber/repo/pull/98/c3a4d5e6f7890123456789abcdef0123456789ab"). + // (e.g., "github://github.example.com/uber/repo/pull/98/c3a4d5e6f7890123456789abcdef0123456789ab"). URI string `json:"uri"` // Details is the provider-supplied facts for this URI. Details ChangeDetails `json:"details"` diff --git a/submitqueue/entity/change_record.go b/submitqueue/entity/change_record.go index 00730d10..d78c53a5 100644 --- a/submitqueue/entity/change_record.go +++ b/submitqueue/entity/change_record.go @@ -20,7 +20,7 @@ package entity // change provider. There is no update path. type ChangeRecord struct { // URI identifies the change (RFC 3986). Same scheme/format as change.Change.URIs. - // Example: "github://uber/submitqueue/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab". + // Example: "github://github.example.com/uber/submitqueue/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab". URI string `json:"uri"` // RequestID is the owning land request that claimed this URI. diff --git a/submitqueue/entity/land_request_test.go b/submitqueue/entity/land_request_test.go index 7ca75a77..8820aea6 100644 --- a/submitqueue/entity/land_request_test.go +++ b/submitqueue/entity/land_request_test.go @@ -28,8 +28,8 @@ func TestLandRequest_ToBytes(t *testing.T) { ID: "test-queue/123", Queue: "test-queue", Change: change.Change{URIs: []string{ - "github://uber/submitqueue/pull/456/abcdef0123456789abcdef0123456789abcdef01", - "github://uber/submitqueue/pull/789/0123456789abcdef0123456789abcdef01234567", + "github://github.example.com/uber/submitqueue/pull/456/abcdef0123456789abcdef0123456789abcdef01", + "github://github.example.com/uber/submitqueue/pull/789/0123456789abcdef0123456789abcdef01234567", }}, LandStrategy: mergestrategy.MergeStrategyRebase, } @@ -41,7 +41,7 @@ func TestLandRequest_ToBytes(t *testing.T) { // Verify JSON contains expected fields jsonStr := string(data) assert.Contains(t, jsonStr, "test-queue/123") - assert.Contains(t, jsonStr, "github://uber/submitqueue/pull/456/abcdef0123456789abcdef0123456789abcdef01") + assert.Contains(t, jsonStr, "github://github.example.com/uber/submitqueue/pull/456/abcdef0123456789abcdef0123456789abcdef01") assert.Contains(t, jsonStr, "rebase") } @@ -98,9 +98,9 @@ func TestLandRequest_SerializationRoundTrip(t *testing.T) { ID: "queue1/100", Queue: "queue1", Change: change.Change{URIs: []string{ - "github://uber/repo-a/pull/101/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - "github://uber/repo-a/pull/102/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", - "github://uber/repo-a/pull/103/cccccccccccccccccccccccccccccccccccccccc", + "github://github.example.com/uber/repo-a/pull/101/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "github://github.example.com/uber/repo-a/pull/102/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + "github://github.example.com/uber/repo-a/pull/103/cccccccccccccccccccccccccccccccccccccccc", }}, LandStrategy: mergestrategy.MergeStrategySquashRebase, }, diff --git a/submitqueue/entity/request_test.go b/submitqueue/entity/request_test.go index 5b8d02ba..4415fb20 100644 --- a/submitqueue/entity/request_test.go +++ b/submitqueue/entity/request_test.go @@ -28,8 +28,8 @@ func TestRequest_ToBytes(t *testing.T) { ID: "test-queue/123", Queue: "test-queue", Change: change.Change{URIs: []string{ - "github://uber/submitqueue/pull/456/abcdef0123456789abcdef0123456789abcdef01", - "github://uber/submitqueue/pull/789/0123456789abcdef0123456789abcdef01234567", + "github://github.example.com/uber/submitqueue/pull/456/abcdef0123456789abcdef0123456789abcdef01", + "github://github.example.com/uber/submitqueue/pull/789/0123456789abcdef0123456789abcdef01234567", }}, LandStrategy: mergestrategy.MergeStrategyRebase, State: RequestStateStarted, @@ -43,7 +43,7 @@ func TestRequest_ToBytes(t *testing.T) { // Verify JSON contains expected fields jsonStr := string(data) assert.Contains(t, jsonStr, "test-queue/123") - assert.Contains(t, jsonStr, "github://uber/submitqueue/pull/456/abcdef0123456789abcdef0123456789abcdef01") + assert.Contains(t, jsonStr, "github://github.example.com/uber/submitqueue/pull/456/abcdef0123456789abcdef0123456789abcdef01") assert.Contains(t, jsonStr, "rebase") assert.Contains(t, jsonStr, "started") } @@ -149,9 +149,9 @@ func TestRequest_SerializationRoundTrip(t *testing.T) { ID: "queue1/100", Queue: "queue1", Change: change.Change{URIs: []string{ - "github://uber/repo-a/pull/101/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - "github://uber/repo-a/pull/102/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", - "github://uber/repo-a/pull/103/cccccccccccccccccccccccccccccccccccccccc", + "github://github.example.com/uber/repo-a/pull/101/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "github://github.example.com/uber/repo-a/pull/102/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + "github://github.example.com/uber/repo-a/pull/103/cccccccccccccccccccccccccccccccccccccccc", }}, LandStrategy: mergestrategy.MergeStrategySquashRebase, State: RequestStateLanded, diff --git a/submitqueue/extension/buildrunner/buildkite/buildkite_test.go b/submitqueue/extension/buildrunner/buildkite/buildkite_test.go index 60de0abd..90dcfa86 100644 --- a/submitqueue/extension/buildrunner/buildkite/buildkite_test.go +++ b/submitqueue/extension/buildrunner/buildkite/buildkite_test.go @@ -81,8 +81,8 @@ func TestTrigger_SubmitsCorrectPayloadAndReturnsBuildkiteNumber(t *testing.T) { var capturedBody []byte resolver := changesetfake.New(). - Set("base-batch", change.Change{URIs: []string{"github://org/repo/pull/1/aaa111"}}). - Set("head-batch", change.Change{URIs: []string{"github://org/repo/pull/2/bbb222"}}) + Set("base-batch", change.Change{URIs: []string{"github://github.example.com/org/repo/pull/1/aaa111"}}). + Set("head-batch", change.Change{URIs: []string{"github://github.example.com/org/repo/pull/2/bbb222"}}) r := newTestRunner(t, http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { capturedMethod = req.Method @@ -99,8 +99,8 @@ func TestTrigger_SubmitsCorrectPayloadAndReturnsBuildkiteNumber(t *testing.T) { var req createBuildRequest require.NoError(t, json.Unmarshal(capturedBody, &req)) - assert.Equal(t, `["github://org/repo/pull/1/aaa111"]`, req.Env[EnvKeyBaseURIs]) - assert.Equal(t, `["github://org/repo/pull/2/bbb222"]`, req.Env[EnvKeyHeadURIs]) + assert.Equal(t, `["github://github.example.com/org/repo/pull/1/aaa111"]`, req.Env[EnvKeyBaseURIs]) + assert.Equal(t, `["github://github.example.com/org/repo/pull/2/bbb222"]`, req.Env[EnvKeyHeadURIs]) assert.Equal(t, "my-queue", req.Env[EnvKeyQueue]) } @@ -125,8 +125,8 @@ func TestTrigger_EmptyBase_ProducesJSONArray(t *testing.T) { func TestTrigger_MultipleChangesFlattened(t *testing.T) { var capturedBody []byte resolver := changesetfake.New().Set("head-batch", - change.Change{URIs: []string{"github://org/repo/pull/1/aaa"}}, - change.Change{URIs: []string{"github://org/repo/pull/2/bbb", "github://org/repo/pull/3/ccc"}}, + change.Change{URIs: []string{"github://github.example.com/org/repo/pull/1/aaa"}}, + change.Change{URIs: []string{"github://github.example.com/org/repo/pull/2/bbb", "github://github.example.com/org/repo/pull/3/ccc"}}, ) r := newTestRunner(t, http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { capturedBody, _ = io.ReadAll(req.Body) @@ -140,7 +140,7 @@ func TestTrigger_MultipleChangesFlattened(t *testing.T) { var req createBuildRequest require.NoError(t, json.Unmarshal(capturedBody, &req)) assert.Equal(t, - `["github://org/repo/pull/1/aaa","github://org/repo/pull/2/bbb","github://org/repo/pull/3/ccc"]`, + `["github://github.example.com/org/repo/pull/1/aaa","github://github.example.com/org/repo/pull/2/bbb","github://github.example.com/org/repo/pull/3/ccc"]`, req.Env[EnvKeyHeadURIs], ) } diff --git a/submitqueue/extension/buildrunner/fake/fake_test.go b/submitqueue/extension/buildrunner/fake/fake_test.go index e342b97f..2a9980f3 100644 --- a/submitqueue/extension/buildrunner/fake/fake_test.go +++ b/submitqueue/extension/buildrunner/fake/fake_test.go @@ -41,7 +41,7 @@ func TestNew_ImplementsInterface(t *testing.T) { func TestRunner_Trigger_UniqueIDs(t *testing.T) { ctx := context.Background() - id1, err := newFake("github://o/r/pull/1/a").Trigger(ctx, nil, entity.Batch{ID: headBatchID}, nil) + id1, err := newFake("github://github.example.com/o/r/pull/1/a").Trigger(ctx, nil, entity.Batch{ID: headBatchID}, nil) require.NoError(t, err) assert.NotEmpty(t, id1.ID) @@ -59,7 +59,7 @@ func TestRunner_Trigger_UniqueIDs(t *testing.T) { } func TestRunner_TriggerError(t *testing.T) { - r := newFake("github://o/r/pull/1/a?sq-fake=trigger-error") + r := newFake("github://github.example.com/o/r/pull/1/a?sq-fake=trigger-error") _, err := r.Trigger(context.Background(), nil, entity.Batch{ID: headBatchID}, nil) require.Error(t, err) } @@ -75,22 +75,22 @@ func TestRunner_Status(t *testing.T) { }{ { name: "no marker succeeds", - headURIs: []string{"github://o/r/pull/1/a"}, + headURIs: []string{"github://github.example.com/o/r/pull/1/a"}, wantStatus: entity.BuildStatusSucceeded, }, { name: "build-fail marker fails", - headURIs: []string{"github://o/r/pull/1/a?sq-fake=build-fail"}, + headURIs: []string{"github://github.example.com/o/r/pull/1/a?sq-fake=build-fail"}, wantStatus: entity.BuildStatusFailed, }, { name: "build-fail marker among other query params", - headURIs: []string{"github://o/r/pull/1/a?ref=main&sq-fake=build-fail&attempt=2"}, + headURIs: []string{"github://github.example.com/o/r/pull/1/a?ref=main&sq-fake=build-fail&attempt=2"}, wantStatus: entity.BuildStatusFailed, }, { name: "build-error marker errors", - headURIs: []string{"github://o/r/pull/1/a?sq-fake=build-error"}, + headURIs: []string{"github://github.example.com/o/r/pull/1/a?sq-fake=build-error"}, wantErr: true, }, } @@ -124,7 +124,7 @@ func TestRunner_Status_UnknownBuildSucceeds(t *testing.T) { // back correctly by a different runner instance. func TestStatus_StatelessAcrossInstances(t *testing.T) { ctx := context.Background() - id, err := newFake("github://o/r/pull/1/a?sq-fake=build-fail").Trigger(ctx, nil, entity.Batch{ID: headBatchID}, nil) + id, err := newFake("github://github.example.com/o/r/pull/1/a?sq-fake=build-fail").Trigger(ctx, nil, entity.Batch{ID: headBatchID}, nil) require.NoError(t, err) status, _, err := New(changesetfake.New()).Status(ctx, id) diff --git a/submitqueue/extension/buildrunner/githubactions/githubactions_test.go b/submitqueue/extension/buildrunner/githubactions/githubactions_test.go index 882984a0..d1067947 100644 --- a/submitqueue/extension/buildrunner/githubactions/githubactions_test.go +++ b/submitqueue/extension/buildrunner/githubactions/githubactions_test.go @@ -94,8 +94,8 @@ func TestTrigger_DispatchesWorkflowAndReturnsRunID(t *testing.T) { var capturedBody []byte resolver := changesetfake.New(). - Set("base-batch", change.Change{URIs: []string{"github://org/repo/pull/1/aaa111"}}). - Set("head-batch", change.Change{URIs: []string{"github://org/repo/pull/2/bbb222"}}) + Set("base-batch", change.Change{URIs: []string{"github://github.example.com/org/repo/pull/1/aaa111"}}). + Set("head-batch", change.Change{URIs: []string{"github://github.example.com/org/repo/pull/2/bbb222"}}) r := newTestRunner(t, http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { capturedMethod = req.Method @@ -122,8 +122,8 @@ func TestTrigger_DispatchesWorkflowAndReturnsRunID(t *testing.T) { assert.Equal(t, "main", req.Ref) assert.True(t, req.ReturnRunDetails) assert.NotContains(t, req.Inputs, "sq_build_id") - assert.Equal(t, `["github://org/repo/pull/1/aaa111"]`, req.Inputs[InputKeyBaseURIs]) - assert.Equal(t, `["github://org/repo/pull/2/bbb222"]`, req.Inputs[InputKeyHeadURIs]) + assert.Equal(t, `["github://github.example.com/org/repo/pull/1/aaa111"]`, req.Inputs[InputKeyBaseURIs]) + assert.Equal(t, `["github://github.example.com/org/repo/pull/2/bbb222"]`, req.Inputs[InputKeyHeadURIs]) assert.Equal(t, "my-queue", req.Inputs[InputKeyQueue]) assert.Equal(t, "value", req.Inputs["custom"]) diff --git a/submitqueue/extension/changeprovider/fake/fake_test.go b/submitqueue/extension/changeprovider/fake/fake_test.go index 9a9c75f5..aaf941be 100644 --- a/submitqueue/extension/changeprovider/fake/fake_test.go +++ b/submitqueue/extension/changeprovider/fake/fake_test.go @@ -35,12 +35,12 @@ func TestProvider_Get_OnePerURI(t *testing.T) { uris []string }{ {name: "nil URIs", uris: nil}, - {name: "single URI", uris: []string{"github://owner/repo/pull/1/abc"}}, + {name: "single URI", uris: []string{"github://github.example.com/owner/repo/pull/1/abc"}}, { name: "multiple URIs (stack)", uris: []string{ - "github://owner/repo/pull/1/abc", - "github://owner/repo/pull/2/def", + "github://github.example.com/owner/repo/pull/1/abc", + "github://github.example.com/owner/repo/pull/2/def", }, }, } @@ -61,7 +61,7 @@ func TestProvider_Get_OnePerURI(t *testing.T) { func TestProvider_Get_ErrorMarker(t *testing.T) { p := New() _, err := p.Get(context.Background(), entity.Request{Change: change.Change{ - URIs: []string{"github://owner/repo/pull/1/abc?sq-fake=provider-error"}, + URIs: []string{"github://github.example.com/owner/repo/pull/1/abc?sq-fake=provider-error"}, }}) require.Error(t, err) } diff --git a/submitqueue/extension/changeprovider/github/provider_test.go b/submitqueue/extension/changeprovider/github/provider_test.go index 43f72d00..f114f9eb 100644 --- a/submitqueue/extension/changeprovider/github/provider_test.go +++ b/submitqueue/extension/changeprovider/github/provider_test.go @@ -68,7 +68,7 @@ func TestProvider_Get(t *testing.T) { }, }) }, - uris: []string{"github://uber/submitqueue/pull/123/" + shaA}, + uris: []string{"github://github.example.com/uber/submitqueue/pull/123/" + shaA}, }, { name: "invalid URI returns error", @@ -78,8 +78,8 @@ func TestProvider_Get(t *testing.T) { { name: "inconsistent change set returns error", uris: []string{ - "github://uber/submitqueue/pull/123/" + shaA, - "github://uber/different-repo/pull/456/" + shaB, + "github://github.example.com/uber/submitqueue/pull/123/" + shaA, + "github://github.example.com/uber/different-repo/pull/456/" + shaB, }, wantErr: true, }, @@ -92,7 +92,7 @@ func TestProvider_Get(t *testing.T) { Files: filesData{Nodes: []fileNode{{Path: "main.go"}}}, }) }, - uris: []string{"github://uber/submitqueue/pull/123/" + shaOld}, + uris: []string{"github://github.example.com/uber/submitqueue/pull/123/" + shaOld}, wantErr: true, }, } @@ -149,7 +149,7 @@ func TestProvider_Get_Pagination(t *testing.T) { p := newTestProvider(t, server.URL) infos, err := p.Get(context.Background(), entity.Request{Change: change.Change{ - URIs: []string{"github://uber/submitqueue/pull/456/" + shaXYZ}, + URIs: []string{"github://github.example.com/uber/submitqueue/pull/456/" + shaXYZ}, }}) require.NoError(t, err) @@ -173,16 +173,16 @@ func TestProvider_Get_MultiplePRs(t *testing.T) { p := newTestProvider(t, server.URL) infos, err := p.Get(context.Background(), entity.Request{Change: change.Change{ URIs: []string{ - "github://uber/submitqueue/pull/123/" + shaA, - "github://uber/submitqueue/pull/456/" + shaB, + "github://github.example.com/uber/submitqueue/pull/123/" + shaA, + "github://github.example.com/uber/submitqueue/pull/456/" + shaB, }, }}) require.NoError(t, err) assert.Equal(t, 2, callCount) require.Len(t, infos, 2) - assert.Equal(t, "github://uber/submitqueue/pull/123/"+shaA, infos[0].URI) - assert.Equal(t, "github://uber/submitqueue/pull/456/"+shaB, infos[1].URI) + assert.Equal(t, "github://github.example.com/uber/submitqueue/pull/123/"+shaA, infos[0].URI) + assert.Equal(t, "github://github.example.com/uber/submitqueue/pull/456/"+shaB, infos[1].URI) } func TestProvider_Get_FetchError_StopsOnFirstFailure(t *testing.T) { @@ -205,8 +205,8 @@ func TestProvider_Get_FetchError_StopsOnFirstFailure(t *testing.T) { p := newTestProvider(t, server.URL) _, err := p.Get(context.Background(), entity.Request{Change: change.Change{ URIs: []string{ - "github://uber/submitqueue/pull/123/" + shaA, - "github://uber/submitqueue/pull/456/" + shaB, + "github://github.example.com/uber/submitqueue/pull/123/" + shaA, + "github://github.example.com/uber/submitqueue/pull/456/" + shaB, }, }}) diff --git a/submitqueue/extension/changeprovider/github/validate_test.go b/submitqueue/extension/changeprovider/github/validate_test.go index c1ca05e2..529fac1e 100644 --- a/submitqueue/extension/changeprovider/github/validate_test.go +++ b/submitqueue/extension/changeprovider/github/validate_test.go @@ -47,7 +47,7 @@ func TestValidateChangeConsistency(t *testing.T) { name: "different scheme", changeIDs: []entitygithub.ChangeID{ {Scheme: "github", Org: "uber", Repo: "submitqueue", PRNumber: 1}, - {Scheme: "ghe", Org: "uber", Repo: "submitqueue", PRNumber: 2}, + {Scheme: "other", Org: "uber", Repo: "submitqueue", PRNumber: 2}, }, wantErr: true, }, diff --git a/submitqueue/extension/changeprovider/phabricator/convert_test.go b/submitqueue/extension/changeprovider/phabricator/convert_test.go index de4bb6a0..dc618bfe 100644 --- a/submitqueue/extension/changeprovider/phabricator/convert_test.go +++ b/submitqueue/extension/changeprovider/phabricator/convert_test.go @@ -10,7 +10,7 @@ import ( ) func TestConvertToChangeInfo(t *testing.T) { - parsed := changephab.ChangeID{Scheme: "phab", RevisionID: 12345, DiffID: 100} + parsed := changephab.ChangeID{Scheme: "phab", Host: "phab.example.com", RevisionID: 12345, DiffID: 100} diff := &diffResult{ AuthorName: "Alice", AuthorEmail: "alice@example.com", @@ -22,7 +22,7 @@ func TestConvertToChangeInfo(t *testing.T) { info := convertToChangeInfo(parsed, diff) - assert.Equal(t, "phab://D12345/100", info.URI) + assert.Equal(t, "phab://phab.example.com/D12345/100", info.URI) assert.Equal(t, entity.Author{Name: "Alice", Email: "alice@example.com"}, info.Details.Author) assert.Len(t, info.Details.ChangedFiles, 2) } diff --git a/submitqueue/extension/changeprovider/phabricator/provider_test.go b/submitqueue/extension/changeprovider/phabricator/provider_test.go index 743b18f8..1a594b3a 100644 --- a/submitqueue/extension/changeprovider/phabricator/provider_test.go +++ b/submitqueue/extension/changeprovider/phabricator/provider_test.go @@ -52,7 +52,7 @@ func TestProvider_Get(t *testing.T) { handler: func(w http.ResponseWriter, r *http.Request) { serveConduit(t, w, validDiffResponse()) }, - uris: []string{"phab://D200/100"}, + uris: []string{"phab://phab.example.com/D200/100"}, }, { name: "invalid URI returns error", @@ -64,7 +64,7 @@ func TestProvider_Get(t *testing.T) { handler: func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusInternalServerError) }, - uris: []string{"phab://D200/100"}, + uris: []string{"phab://phab.example.com/D200/100"}, wantErr: "Conduit API returned status 500", }, } @@ -90,7 +90,7 @@ func TestProvider_Get(t *testing.T) { } require.NoError(t, err) require.Len(t, infos, 1) - assert.Equal(t, "phab://D200/100", infos[0].URI) + assert.Equal(t, "phab://phab.example.com/D200/100", infos[0].URI) assert.Equal(t, "Test Author", infos[0].Details.Author.Name) assert.Len(t, infos[0].Details.ChangedFiles, 2) }) @@ -119,21 +119,21 @@ func TestProvider_Get_MultipleDiffs(t *testing.T) { client := &http.Client{Transport: &testTransport{baseURL: server.URL}} p := newTestProvider(t, client) infos, err := p.Get(context.Background(), entity.Request{Change: change.Change{ - URIs: []string{"phab://D200/100", "phab://D201/101"}, + URIs: []string{"phab://phab.example.com/D200/100", "phab://phab.example.com/D201/101"}, }}) require.NoError(t, err) assert.Equal(t, 1, callCount) require.Len(t, infos, 2) - assert.Equal(t, "phab://D200/100", infos[0].URI) - assert.Equal(t, "phab://D201/101", infos[1].URI) + assert.Equal(t, "phab://phab.example.com/D200/100", infos[0].URI) + assert.Equal(t, "phab://phab.example.com/D201/101", infos[1].URI) } func TestProvider_Get_ConnectionError(t *testing.T) { client := &http.Client{Transport: &testTransport{baseURL: "http://127.0.0.1:0"}} p := newTestProvider(t, client) _, err := p.Get(context.Background(), entity.Request{Change: change.Change{ - URIs: []string{"phab://D200/100"}, + URIs: []string{"phab://phab.example.com/D200/100"}, }}) require.Error(t, err) @@ -163,7 +163,7 @@ func TestProvider_Get_FileDetails(t *testing.T) { client := &http.Client{Transport: &testTransport{baseURL: server.URL}} p := newTestProvider(t, client) infos, err := p.Get(context.Background(), entity.Request{Change: change.Change{ - URIs: []string{"phab://D200/100"}, + URIs: []string{"phab://phab.example.com/D200/100"}, }}) require.NoError(t, err) @@ -209,7 +209,7 @@ func TestProvider_Get_Batching(t *testing.T) { for i := range uris { diffID := i + 1 revisionID := 1000 + i - uris[i] = fmt.Sprintf("phab://D%d/%d", revisionID, diffID) + uris[i] = fmt.Sprintf("phab://phab.example.com/D%d/%d", revisionID, diffID) } client := &http.Client{Transport: &testTransport{baseURL: server.URL}} @@ -245,7 +245,7 @@ func TestProvider_Get_BatchingStopsOnError(t *testing.T) { for i := range uris { diffID := i + 1 revisionID := 1000 + i - uris[i] = fmt.Sprintf("phab://D%d/%d", revisionID, diffID) + uris[i] = fmt.Sprintf("phab://phab.example.com/D%d/%d", revisionID, diffID) } client := &http.Client{Transport: &testTransport{baseURL: server.URL}} diff --git a/submitqueue/extension/changeprovider/routing/provider_test.go b/submitqueue/extension/changeprovider/routing/provider_test.go index 2cc15190..8caa47a9 100644 --- a/submitqueue/extension/changeprovider/routing/provider_test.go +++ b/submitqueue/extension/changeprovider/routing/provider_test.go @@ -30,10 +30,10 @@ import ( ) const ( - githubURI1 = "github://uber/repo/pull/1/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" - githubURI2 = "github://uber/repo/pull/2/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" - phabURI1 = "phab://D123/456" - phabURI2 = "phab://D789/101" + githubURI1 = "github://github.example.com/uber/repo/pull/1/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + githubURI2 = "github://github.example.com/uber/repo/pull/2/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + phabURI1 = "phab://phab.example.com/D123/456" + phabURI2 = "phab://phab.example.com/D789/101" gitURI1 = "git://remote.example.com/uber/repo/refs%2Fheads%2Fmain/cccccccccccccccccccccccccccccccccccccccc" ) diff --git a/submitqueue/extension/mergechecker/fake/fake_test.go b/submitqueue/extension/mergechecker/fake/fake_test.go index 8bbbe465..7357c80c 100644 --- a/submitqueue/extension/mergechecker/fake/fake_test.go +++ b/submitqueue/extension/mergechecker/fake/fake_test.go @@ -38,7 +38,7 @@ func TestChecker_Check(t *testing.T) { }{ { name: "no marker is mergeable", - uris: []string{"github://owner/repo/pull/1/abc"}, + uris: []string{"github://github.example.com/owner/repo/pull/1/abc"}, wantMergeable: true, }, { @@ -48,18 +48,18 @@ func TestChecker_Check(t *testing.T) { }, { name: "unmergeable marker", - uris: []string{"github://owner/repo/pull/1/abc?sq-fake=unmergeable"}, + uris: []string{"github://github.example.com/owner/repo/pull/1/abc?sq-fake=unmergeable"}, }, { name: "error marker", - uris: []string{"github://owner/repo/pull/1/abc?sq-fake=mergecheck-error"}, + uris: []string{"github://github.example.com/owner/repo/pull/1/abc?sq-fake=mergecheck-error"}, wantErr: true, }, { name: "marker on second uri", uris: []string{ - "github://owner/repo/pull/1/abc", - "github://owner/repo/pull/2/def?sq-fake=unmergeable", + "github://github.example.com/owner/repo/pull/1/abc", + "github://github.example.com/owner/repo/pull/2/def?sq-fake=unmergeable", }, }, } diff --git a/submitqueue/extension/mergechecker/github/checker_test.go b/submitqueue/extension/mergechecker/github/checker_test.go index 263f162f..e38f1b38 100644 --- a/submitqueue/extension/mergechecker/github/checker_test.go +++ b/submitqueue/extension/mergechecker/github/checker_test.go @@ -94,7 +94,7 @@ func TestMergeChecker_Check(t *testing.T) { handler: graphQLHandler(t, []PRInfo{ {Number: 1, Mergeable: PRMergeableStateMergeable, BaseRefName: "main", HeadRefName: "feature-1", HeadRefOid: shaAFull, State: PRStateOpen}, }), - change: change.Change{URIs: []string{"github://uber/repo/pull/1/" + shaAFull}}, + change: change.Change{URIs: []string{"github://github.example.com/uber/repo/pull/1/" + shaAFull}}, wantMergeable: true, }, { @@ -102,7 +102,7 @@ func TestMergeChecker_Check(t *testing.T) { handler: graphQLHandler(t, []PRInfo{ {Number: 1, Mergeable: PRMergeableStateConflicting, BaseRefName: "main", HeadRefName: "feature-1", HeadRefOid: shaAFull, State: PRStateOpen}, }), - change: change.Change{URIs: []string{"github://uber/repo/pull/1/" + shaAFull}}, + change: change.Change{URIs: []string{"github://github.example.com/uber/repo/pull/1/" + shaAFull}}, wantMergeable: false, wantReason: "PR #1 has merge conflicts", }, @@ -112,7 +112,7 @@ func TestMergeChecker_Check(t *testing.T) { {Number: 1, Mergeable: PRMergeableStateMergeable, BaseRefName: "main", HeadRefName: "feature-1", HeadRefOid: sha1Full, State: PRStateOpen}, {Number: 2, Mergeable: PRMergeableStateMergeable, BaseRefName: "feature-1", HeadRefName: "feature-2", HeadRefOid: sha2Full, State: PRStateOpen}, }), - change: change.Change{URIs: []string{"github://uber/repo/pull/1/" + sha1Full, "github://uber/repo/pull/2/" + sha2Full}}, + change: change.Change{URIs: []string{"github://github.example.com/uber/repo/pull/1/" + sha1Full, "github://github.example.com/uber/repo/pull/2/" + sha2Full}}, wantMergeable: true, }, { @@ -120,7 +120,7 @@ func TestMergeChecker_Check(t *testing.T) { handler: graphQLHandler(t, []PRInfo{ {Number: 1, Mergeable: PRMergeableStateUnknown, BaseRefName: "main", HeadRefName: "feature-1", HeadRefOid: shaAFull, State: PRStateOpen}, }), - change: change.Change{URIs: []string{"github://uber/repo/pull/1/" + shaAFull}}, + change: change.Change{URIs: []string{"github://github.example.com/uber/repo/pull/1/" + shaAFull}}, wantErr: true, }, { @@ -128,7 +128,7 @@ func TestMergeChecker_Check(t *testing.T) { handler: graphQLHandler(t, []PRInfo{ {Number: 1, Mergeable: PRMergeableStateMergeable, BaseRefName: "main", HeadRefName: "feature-1", HeadRefOid: shaNewFull, State: PRStateOpen}, }), - change: change.Change{URIs: []string{"github://uber/repo/pull/1/" + shaOldFull}}, + change: change.Change{URIs: []string{"github://github.example.com/uber/repo/pull/1/" + shaOldFull}}, wantMergeable: false, wantReason: fmt.Sprintf("PR #1 head SHA changed: expected %s, got %s", shaOldFull, shaNewFull), }, @@ -146,7 +146,7 @@ func TestMergeChecker_Check(t *testing.T) { w.WriteHeader(http.StatusInternalServerError) _, _ = w.Write([]byte("internal server error")) }), - change: change.Change{URIs: []string{"github://uber/repo/pull/1/" + shaAFull}}, + change: change.Change{URIs: []string{"github://github.example.com/uber/repo/pull/1/" + shaAFull}}, wantErr: true, }, } diff --git a/submitqueue/extension/mergechecker/github/graphql_test.go b/submitqueue/extension/mergechecker/github/graphql_test.go index ecef12a2..909114fc 100644 --- a/submitqueue/extension/mergechecker/github/graphql_test.go +++ b/submitqueue/extension/mergechecker/github/graphql_test.go @@ -46,7 +46,7 @@ func TestBuildGraphQLQuery(t *testing.T) { changeIDs: []entitygithub.ChangeID{ {Scheme: "github", Org: "uber", Repo: "repo", PRNumber: 1, HeadCommitSHA: "sha1"}, {Scheme: "github", Org: "uber", Repo: "repo", PRNumber: 2, HeadCommitSHA: "sha2"}, - {Scheme: "ghe", Org: "corp", Repo: "app", PRNumber: 99, HeadCommitSHA: "sha99"}, + {Scheme: "github", Org: "corp", Repo: "app", PRNumber: 99, HeadCommitSHA: "sha99"}, }, wantParts: []string{ `pr0: repository(owner: "uber", name: "repo")`, diff --git a/submitqueue/extension/pusher/fake/fake_test.go b/submitqueue/extension/pusher/fake/fake_test.go index 9f57f88a..d91b2727 100644 --- a/submitqueue/extension/pusher/fake/fake_test.go +++ b/submitqueue/extension/pusher/fake/fake_test.go @@ -33,8 +33,8 @@ func TestNew_ImplementsInterface(t *testing.T) { func TestPusher_Push_Committed(t *testing.T) { changes := []change.Change{ - {URIs: []string{"github://owner/repo/pull/1/abc"}}, - {URIs: []string{"github://owner/repo/pull/2/def"}}, + {URIs: []string{"github://github.example.com/owner/repo/pull/1/abc"}}, + {URIs: []string{"github://github.example.com/owner/repo/pull/2/def"}}, } p := New(changesetfake.New().Set("b", changes...)) @@ -54,13 +54,13 @@ func TestPusher_Push_Committed(t *testing.T) { } func TestPusher_Push_ConflictMarker(t *testing.T) { - p := New(changesetfake.New().Set("b", change.Change{URIs: []string{"github://owner/repo/pull/1/abc?sq-fake=conflict"}})) + p := New(changesetfake.New().Set("b", change.Change{URIs: []string{"github://github.example.com/owner/repo/pull/1/abc?sq-fake=conflict"}})) _, err := p.Push(context.Background(), []entity.Batch{{ID: "b"}}) assert.True(t, errors.Is(err, pusher.ErrConflict)) } func TestPusher_Push_ErrorMarker(t *testing.T) { - p := New(changesetfake.New().Set("b", change.Change{URIs: []string{"github://owner/repo/pull/1/abc?sq-fake=push-error"}})) + p := New(changesetfake.New().Set("b", change.Change{URIs: []string{"github://github.example.com/owner/repo/pull/1/abc?sq-fake=push-error"}})) res, err := p.Push(context.Background(), []entity.Batch{{ID: "b"}}) require.Error(t, err) // Atomicity: on error no outcomes are reported. diff --git a/submitqueue/extension/pusher/git/git_pusher_test.go b/submitqueue/extension/pusher/git/git_pusher_test.go index ca25a831..40b5428f 100644 --- a/submitqueue/extension/pusher/git/git_pusher_test.go +++ b/submitqueue/extension/pusher/git/git_pusher_test.go @@ -155,7 +155,7 @@ func (f gitFixture) landOnMain(t *testing.T, sha string) { // uri builds a github-format URI ending in `sha` so the Pusher's parser // resolves it to that SHA. func uri(sha string) string { - return fmt.Sprintf("github://uber/submitqueue/pull/1/%s", sha) + return fmt.Sprintf("github://github.example.com/uber/submitqueue/pull/1/%s", sha) } func (f gitFixture) newPusher(t *testing.T) pusher.Pusher { diff --git a/submitqueue/extension/scorer/fake/fake_test.go b/submitqueue/extension/scorer/fake/fake_test.go index 7c09ed51..d98e3b08 100644 --- a/submitqueue/extension/scorer/fake/fake_test.go +++ b/submitqueue/extension/scorer/fake/fake_test.go @@ -55,7 +55,7 @@ func delegate(resolver changeset.Resolver, want float64) scorer.Scorer { } func TestScore_DelegatesWhenUnmarked(t *testing.T) { - r := resolverFor("github://o/r/pull/1/a") + r := resolverFor("github://github.example.com/o/r/pull/1/a") s := New(r, delegate(r, 0.7)) got, err := s.Score(context.Background(), entity.Batch{ID: batchID}) require.NoError(t, err) @@ -63,7 +63,7 @@ func TestScore_DelegatesWhenUnmarked(t *testing.T) { } func TestScore_ErrorMarker(t *testing.T) { - r := resolverFor("github://o/r/pull/1/a?sq-fake=score-error") + r := resolverFor("github://github.example.com/o/r/pull/1/a?sq-fake=score-error") s := New(r, delegate(r, 0.7)) _, err := s.Score(context.Background(), entity.Batch{ID: batchID}) require.Error(t, err) diff --git a/submitqueue/gateway/controller/land_test.go b/submitqueue/gateway/controller/land_test.go index 01252d61..63df3c11 100644 --- a/submitqueue/gateway/controller/land_test.go +++ b/submitqueue/gateway/controller/land_test.go @@ -102,7 +102,7 @@ func TestLand_ReturnsSqid(t *testing.T) { req := &pb.LandRequest{ Queue: "test-queue", - Change: &changepb.Change{Uris: []string{"github://uber/test-repo/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab"}}, + Change: &changepb.Change{Uris: []string{"github://github.example.com/uber/test-repo/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab"}}, } resp, err := controller.Land(ctx, req) @@ -120,7 +120,7 @@ func TestLand_ReturnsErrorOnCounterFailure(t *testing.T) { req := &pb.LandRequest{ Queue: "test-queue", - Change: &changepb.Change{Uris: []string{"github://uber/test-repo/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab"}}, + Change: &changepb.Change{Uris: []string{"github://github.example.com/uber/test-repo/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab"}}, } _, err := controller.Land(ctx, req) @@ -144,7 +144,7 @@ func TestLand_CounterDomainIncludesQueue(t *testing.T) { req := &pb.LandRequest{ Queue: "my-queue", - Change: &changepb.Change{Uris: []string{"github://uber/test-repo/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab"}}, + Change: &changepb.Change{Uris: []string{"github://github.example.com/uber/test-repo/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab"}}, } _, err := controller.Land(ctx, req) @@ -161,7 +161,7 @@ func TestLand_ReturnsErrorOnEmptyQueue(t *testing.T) { req := &pb.LandRequest{ Queue: "", - Change: &changepb.Change{Uris: []string{"github://uber/test-repo/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab"}}, + Change: &changepb.Change{Uris: []string{"github://github.example.com/uber/test-repo/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab"}}, } _, err := controller.Land(ctx, req) @@ -215,7 +215,7 @@ func TestLand_ReturnsUnrecognizedQueueWhenStoreReportsNotFound(t *testing.T) { req := &pb.LandRequest{ Queue: "missing-queue", - Change: &changepb.Change{Uris: []string{"github://uber/test-repo/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab"}}, + Change: &changepb.Change{Uris: []string{"github://github.example.com/uber/test-repo/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab"}}, } _, err := controller.Land(ctx, req) @@ -241,7 +241,7 @@ func TestLand_PropagatesQueueConfigStoreError(t *testing.T) { req := &pb.LandRequest{ Queue: "test-queue", - Change: &changepb.Change{Uris: []string{"github://uber/test-repo/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab"}}, + Change: &changepb.Change{Uris: []string{"github://github.example.com/uber/test-repo/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab"}}, } _, err := controller.Land(ctx, req) @@ -273,7 +273,7 @@ func TestLand_PublishesToQueue(t *testing.T) { req := &pb.LandRequest{ Queue: "test-queue", - Change: &changepb.Change{Uris: []string{"github://uber/backend/pull/456/fedcba9876543210fedcba9876543210fedcba98"}}, + Change: &changepb.Change{Uris: []string{"github://github.example.com/uber/backend/pull/456/fedcba9876543210fedcba9876543210fedcba98"}}, Strategy: mergestrategypb.Strategy_REBASE, } resp, err := controller.Land(ctx, req) @@ -291,7 +291,7 @@ func TestLand_PublishesToQueue(t *testing.T) { require.NoError(t, err) assert.Equal(t, "test-queue/123", deserializedReq.ID) assert.Equal(t, "test-queue", deserializedReq.Queue) - assert.Equal(t, []string{"github://uber/backend/pull/456/fedcba9876543210fedcba9876543210fedcba98"}, deserializedReq.Change.URIs) + assert.Equal(t, []string{"github://github.example.com/uber/backend/pull/456/fedcba9876543210fedcba9876543210fedcba98"}, deserializedReq.Change.URIs) assert.Equal(t, mergestrategy.MergeStrategyRebase, deserializedReq.LandStrategy) } @@ -309,7 +309,7 @@ func TestLand_ContinuesWhenPublishFails(t *testing.T) { req := &pb.LandRequest{ Queue: "test-queue", - Change: &changepb.Change{Uris: []string{"github://uber/service/pull/1/c3a4d5e6f7890123456789abcdef0123456789ab"}}, + Change: &changepb.Change{Uris: []string{"github://github.example.com/uber/service/pull/1/c3a4d5e6f7890123456789abcdef0123456789ab"}}, } _, err := controller.Land(ctx, req) diff --git a/submitqueue/orchestrator/controller/batch/batch_test.go b/submitqueue/orchestrator/controller/batch/batch_test.go index f1651c5d..fe02f62f 100644 --- a/submitqueue/orchestrator/controller/batch/batch_test.go +++ b/submitqueue/orchestrator/controller/batch/batch_test.go @@ -65,7 +65,7 @@ func testRequest() entity.Request { return entity.Request{ ID: "test-queue/123", Queue: "test-queue", - Change: change.Change{URIs: []string{"github://uber/service/pull/456/abcdef0123456789abcdef0123456789abcdef01"}}, + Change: change.Change{URIs: []string{"github://github.example.com/uber/service/pull/456/abcdef0123456789abcdef0123456789abcdef01"}}, LandStrategy: mergestrategy.MergeStrategyRebase, State: entity.RequestStateStarted, Version: 1, @@ -284,7 +284,7 @@ func TestController_Process_WithDependencies(t *testing.T) { request := entity.Request{ ID: "test-queue/456", Queue: "test-queue", - Change: change.Change{URIs: []string{"github://uber/service/pull/789/789abc1234567890abcdef1234567890abcdef12"}}, + Change: change.Change{URIs: []string{"github://github.example.com/uber/service/pull/789/789abc1234567890abcdef1234567890abcdef12"}}, LandStrategy: mergestrategy.MergeStrategyRebase, State: entity.RequestStateStarted, Version: 1, diff --git a/submitqueue/orchestrator/controller/merge/merge_test.go b/submitqueue/orchestrator/controller/merge/merge_test.go index 1b8eab35..dd9eabaa 100644 --- a/submitqueue/orchestrator/controller/merge/merge_test.go +++ b/submitqueue/orchestrator/controller/merge/merge_test.go @@ -90,13 +90,13 @@ func TestProcess_PublishesFullPayloadToRunway(t *testing.T) { req1 := entity.Request{ ID: "test-queue/1", Queue: "test-queue", - Change: change.Change{URIs: []string{"github://uber/repo/pull/1/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}}, + Change: change.Change{URIs: []string{"github://github.example.com/uber/repo/pull/1/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}}, LandStrategy: mergestrategy.MergeStrategySquashRebase, } req2 := entity.Request{ ID: "test-queue/2", Queue: "test-queue", - Change: change.Change{URIs: []string{"github://uber/repo/pull/2/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"}}, + Change: change.Change{URIs: []string{"github://github.example.com/uber/repo/pull/2/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"}}, LandStrategy: mergestrategy.MergeStrategyRebase, } batch := entity.Batch{ diff --git a/submitqueue/orchestrator/controller/score/score_test.go b/submitqueue/orchestrator/controller/score/score_test.go index 63671aff..87325dcc 100644 --- a/submitqueue/orchestrator/controller/score/score_test.go +++ b/submitqueue/orchestrator/controller/score/score_test.go @@ -59,7 +59,7 @@ func testRequest() entity.Request { ID: "test-queue/1", Queue: "test-queue", Change: change.Change{ - URIs: []string{"github://uber/repo/pull/1/abcdef0123456789abcdef0123456789abcdef01"}, + URIs: []string{"github://github.example.com/uber/repo/pull/1/abcdef0123456789abcdef0123456789abcdef01"}, }, State: entity.RequestStateStarted, Version: 1, diff --git a/submitqueue/orchestrator/controller/start/start_test.go b/submitqueue/orchestrator/controller/start/start_test.go index 9a11e5ac..733ae9f6 100644 --- a/submitqueue/orchestrator/controller/start/start_test.go +++ b/submitqueue/orchestrator/controller/start/start_test.go @@ -106,7 +106,7 @@ func TestController_Process_Success(t *testing.T) { delivery := makeDelivery(t, ctrl, entity.LandRequest{ ID: "test-queue/123", Queue: "test-queue", - Change: change.Change{URIs: []string{"github://uber/service/pull/456/abcdef0123456789abcdef0123456789abcdef01"}}, + Change: change.Change{URIs: []string{"github://github.example.com/uber/service/pull/456/abcdef0123456789abcdef0123456789abcdef01"}}, LandStrategy: mergestrategy.MergeStrategyRebase, }) @@ -147,7 +147,7 @@ func TestController_Process_ConstructsRequestWithStateAndVersion(t *testing.T) { landRequest := entity.LandRequest{ ID: "test-queue/42", Queue: "test-queue", - Change: change.Change{URIs: []string{"github://uber/service/pull/1/abcdef0123456789abcdef0123456789abcdef01"}}, + Change: change.Change{URIs: []string{"github://github.example.com/uber/service/pull/1/abcdef0123456789abcdef0123456789abcdef01"}}, LandStrategy: mergestrategy.MergeStrategySquashRebase, } delivery := makeDelivery(t, ctrl, landRequest) @@ -179,7 +179,7 @@ func TestController_Process_AllStrategies(t *testing.T) { delivery := makeDelivery(t, ctrl, entity.LandRequest{ ID: fmt.Sprintf("queue/%s", tt.strategy), Queue: "test-queue", - Change: change.Change{URIs: []string{"github://uber/service/pull/1/aaa111bbbaaa111bbbaaa111bbbaaa111bbbaaa1"}}, + Change: change.Change{URIs: []string{"github://github.example.com/uber/service/pull/1/aaa111bbbaaa111bbbaaa111bbbaaa111bbbaaa1"}}, LandStrategy: tt.strategy, }) @@ -195,7 +195,7 @@ func TestController_Process_PublishFailure(t *testing.T) { delivery := makeDelivery(t, ctrl, entity.LandRequest{ ID: "test-queue/123", Queue: "test-queue", - Change: change.Change{URIs: []string{"github://uber/service/pull/1/789abc1234567890abcdef1234567890abcdef12"}}, + Change: change.Change{URIs: []string{"github://github.example.com/uber/service/pull/1/789abc1234567890abcdef1234567890abcdef12"}}, LandStrategy: mergestrategy.MergeStrategyRebase, }) @@ -215,7 +215,7 @@ func TestController_Process_StorageFailure(t *testing.T) { delivery := makeDelivery(t, ctrl, entity.LandRequest{ ID: "test-queue/123", Queue: "test-queue", - Change: change.Change{URIs: []string{"github://uber/service/pull/1/789abc1234567890abcdef1234567890abcdef12"}}, + Change: change.Change{URIs: []string{"github://github.example.com/uber/service/pull/1/789abc1234567890abcdef1234567890abcdef12"}}, LandStrategy: mergestrategy.MergeStrategyRebase, }) @@ -237,7 +237,7 @@ func TestController_Process_AlreadyExistsSucceeds(t *testing.T) { delivery := makeDelivery(t, ctrl, entity.LandRequest{ ID: "test-queue/123", Queue: "test-queue", - Change: change.Change{URIs: []string{"github://uber/service/pull/1/789abc1234567890abcdef1234567890abcdef12"}}, + Change: change.Change{URIs: []string{"github://github.example.com/uber/service/pull/1/789abc1234567890abcdef1234567890abcdef12"}}, LandStrategy: mergestrategy.MergeStrategyRebase, }) diff --git a/submitqueue/orchestrator/controller/validate/validate_test.go b/submitqueue/orchestrator/controller/validate/validate_test.go index d2c57efa..c4526c5d 100644 --- a/submitqueue/orchestrator/controller/validate/validate_test.go +++ b/submitqueue/orchestrator/controller/validate/validate_test.go @@ -54,7 +54,7 @@ type mockChangeProvider struct{} func (m *mockChangeProvider) Get(ctx context.Context, request entity.Request) ([]entity.ChangeInfo, error) { return []entity.ChangeInfo{ { - URI: "github://org/repo/pull/123/abcdef0123456789abcdef0123456789abcdef01", + URI: "github://github.example.com/org/repo/pull/123/abcdef0123456789abcdef0123456789abcdef01", Details: entity.ChangeDetails{ Author: entity.Author{ Name: "Test User", @@ -129,7 +129,7 @@ func TestNewController(t *testing.T) { request := entity.Request{ ID: "test-queue/123", Queue: "test-queue", - Change: change.Change{URIs: []string{"github://uber/service/pull/456/abcdef0123456789abcdef0123456789abcdef01"}}, + Change: change.Change{URIs: []string{"github://github.example.com/uber/service/pull/456/abcdef0123456789abcdef0123456789abcdef01"}}, LandStrategy: mergestrategy.MergeStrategyRebase, State: entity.RequestStateStarted, Version: 1, @@ -149,7 +149,7 @@ func TestController_Process_Success(t *testing.T) { request := entity.Request{ ID: "test-queue/123", Queue: "test-queue", - Change: change.Change{URIs: []string{"github://uber/service/pull/456/abcdef0123456789abcdef0123456789abcdef01"}}, + Change: change.Change{URIs: []string{"github://github.example.com/uber/service/pull/456/abcdef0123456789abcdef0123456789abcdef01"}}, LandStrategy: mergestrategy.MergeStrategyRebase, State: entity.RequestStateStarted, Version: 1, @@ -174,7 +174,7 @@ func TestController_Process_PublishesCheckToRunway(t *testing.T) { request := entity.Request{ ID: "test-queue/123", Queue: "test-queue", - Change: change.Change{URIs: []string{"github://uber/service/pull/456/abcdef0123456789abcdef0123456789abcdef01"}}, + Change: change.Change{URIs: []string{"github://github.example.com/uber/service/pull/456/abcdef0123456789abcdef0123456789abcdef01"}}, LandStrategy: mergestrategy.MergeStrategyRebase, State: entity.RequestStateStarted, Version: 1, @@ -233,7 +233,7 @@ func TestController_Process_ClaimsChangeRecordsWithDetails(t *testing.T) { // The request's URI matches the URI the mock change provider returns, so the // claim carries that change's details. - const uri = "github://org/repo/pull/123/abcdef0123456789abcdef0123456789abcdef01" + const uri = "github://github.example.com/org/repo/pull/123/abcdef0123456789abcdef0123456789abcdef01" request := entity.Request{ ID: "test-queue/123", Queue: "test-queue", @@ -298,7 +298,7 @@ func TestController_Process_PublishFailure(t *testing.T) { request := entity.Request{ ID: "test-queue/123", Queue: "test-queue", - Change: change.Change{URIs: []string{"github://uber/service/pull/1/789abc1234567890abcdef1234567890abcdef12"}}, + Change: change.Change{URIs: []string{"github://github.example.com/uber/service/pull/1/789abc1234567890abcdef1234567890abcdef12"}}, LandStrategy: mergestrategy.MergeStrategyRebase, State: entity.RequestStateStarted, Version: 1, @@ -330,8 +330,8 @@ func TestController_Process_DuplicateDetection(t *testing.T) { queueName = "test-queue" newRequestID = queueName + "/123" dupRequestID = queueName + "/100" - uriA = "github://uber/service/pull/1/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" - uriB = "github://uber/service/pull/2/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + uriA = "github://github.example.com/uber/service/pull/1/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + uriB = "github://github.example.com/uber/service/pull/2/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" anotherReqID = queueName + "/050" orphanReqID = queueName + "/999" terminalReqID = queueName + "/200" @@ -505,7 +505,7 @@ func TestController_Process_ChangeStoreQueryFailure(t *testing.T) { request := entity.Request{ ID: "test-queue/123", Queue: "test-queue", - Change: change.Change{URIs: []string{"github://uber/service/pull/1/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}}, + Change: change.Change{URIs: []string{"github://github.example.com/uber/service/pull/1/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}}, LandStrategy: mergestrategy.MergeStrategyRebase, State: entity.RequestStateStarted, Version: 1, diff --git a/test/e2e/submitqueue/suite_test.go b/test/e2e/submitqueue/suite_test.go index eeb87826..19d27bc3 100644 --- a/test/e2e/submitqueue/suite_test.go +++ b/test/e2e/submitqueue/suite_test.go @@ -191,7 +191,7 @@ func (s *E2EIntegrationSuite) TestPingOrchestrator() { // storage only via that cross-service publish→consume→persist path, so its // presence in the timeline proves the path works. func (s *E2EIntegrationSuite) TestLand_HappyPath_ReachesLanded() { - sqid := s.land("e2e-test-queue", "github://uber/e2e-service/pull/123/abcdef0123456789abcdef0123456789abcdef01") + sqid := s.land("e2e-test-queue", "github://github.example.com/uber/e2e-service/pull/123/abcdef0123456789abcdef0123456789abcdef01") s.log.Logf("Land (happy path) succeeded: sqid=%s; waiting for landed", sqid) // Black-box: the customer-facing status reaches landed. @@ -246,7 +246,7 @@ func (s *E2EIntegrationSuite) TestCancelRequest_InvalidSqid() { func (s *E2EIntegrationSuite) TestCancel_RecordsIntent() { t := s.T() - sqid := s.land("e2e-cancel-queue", "github://uber/e2e-cancel/pull/9999/abcdef0123456789abcdef0123456789abcdef01") + sqid := s.land("e2e-cancel-queue", "github://github.example.com/uber/e2e-cancel/pull/9999/abcdef0123456789abcdef0123456789abcdef01") s.log.Logf("Land (cancel path) succeeded: sqid=%s; cancelling", sqid) _, err := s.gatewayClient.Cancel(s.ctx, &gatewaypb.CancelRequest{Sqid: sqid, Reason: "e2e cancel test"}) diff --git a/test/integration/submitqueue/extension/storage/suite.go b/test/integration/submitqueue/extension/storage/suite.go index 933dd96e..83842032 100644 --- a/test/integration/submitqueue/extension/storage/suite.go +++ b/test/integration/submitqueue/extension/storage/suite.go @@ -63,7 +63,7 @@ func (s *StorageContractSuite) TestStorage_CreateAndGet() { Queue: "test-queue", State: entity.RequestStateStarted, Change: change.Change{ - URIs: []string{"github://uber/storage-test/pull/123/abcdef0123456789abcdef0123456789abcdef01"}, + URIs: []string{"github://github.example.com/uber/storage-test/pull/123/abcdef0123456789abcdef0123456789abcdef01"}, }, LandStrategy: mergestrategy.MergeStrategyMerge, Version: 1, @@ -95,10 +95,10 @@ func (s *StorageContractSuite) TestStorage_CreateAndGet_StackedPRs() { // Stacked PRs as separate URIs stackedURIs := []string{ - "github://uber/monorepo/pull/101/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - "github://uber/monorepo/pull/102/cccccccccccccccccccccccccccccccccccccccc", - "github://uber/monorepo/pull/103/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee", - "github://uber/monorepo/pull/104/0000000000000000000000000000000000000004", + "github://github.example.com/uber/monorepo/pull/101/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "github://github.example.com/uber/monorepo/pull/102/cccccccccccccccccccccccccccccccccccccccc", + "github://github.example.com/uber/monorepo/pull/103/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee", + "github://github.example.com/uber/monorepo/pull/104/0000000000000000000000000000000000000004", } request := entity.Request{ @@ -232,7 +232,7 @@ func (s *StorageContractSuite) TestStorage_CreateDuplicate() { } // changeURI is a representative change URI reused across change-store contract tests. -const changeURI = "github://uber/x/pull/1/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +const changeURI = "github://github.example.com/uber/x/pull/1/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" // Change-store contract tests scope each case to a distinct queue so they stay // isolated without truncation (GetByURI is scoped by (queue, uri)). @@ -247,7 +247,7 @@ func (s *StorageContractSuite) TestStorage_ChangeCreateAndGet_NoMatch() { URI: changeURI, RequestID: queue + "/1", Queue: queue, CreatedAt: 1, UpdatedAt: 1, Version: 1, })) - got, err := s.storage.GetChangeStore().GetByURI(ctx, queue, "github://uber/x/pull/2/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb") + got, err := s.storage.GetChangeStore().GetByURI(ctx, queue, "github://github.example.com/uber/x/pull/2/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb") require.NoError(t, err) assert.Empty(t, got) } diff --git a/test/integration/submitqueue/gateway/suite_test.go b/test/integration/submitqueue/gateway/suite_test.go index 20e22820..f14dd03a 100644 --- a/test/integration/submitqueue/gateway/suite_test.go +++ b/test/integration/submitqueue/gateway/suite_test.go @@ -146,7 +146,7 @@ func (s *GatewayIntegrationSuite) TestLandAPI() { req := &pb.LandRequest{ Queue: "test-queue", - Change: &changepb.Change{Uris: []string{"github://uber/integration-test/pull/123/abcdef0123456789abcdef0123456789abcdef01"}}, + Change: &changepb.Change{Uris: []string{"github://github.example.com/uber/integration-test/pull/123/abcdef0123456789abcdef0123456789abcdef01"}}, Strategy: mergestrategypb.Strategy_REBASE, }