Merge pull request #3718 from rajatvig/fix-insteadof-url-mismatch

Author: CerealBoy

internal/job/checkout.go

@@ -538,15 +538,35 @@ func (e *Executor) updateRemoteURL(ctx context.Context, gitDir, repository strin
 
 	// First check what the existing remote is, for both logging and debugging
 	// purposes.
-	args := []string{"remote", "get-url", "origin"}
+
+	// Check if there are multiple URLs configured (e.g., via git remote set-url --add).
+	args := []string{"config", "--get-all", "remote.origin.url"}
 	if gitDir != "" {
 		args = append([]string{"--git-dir", gitDir}, args...)
 	}
-	gotURL, err := e.shell.Command("git", args...).RunAndCaptureStdout(ctx)
+	allURLs, err := e.shell.Command("git", args...).RunAndCaptureStdout(ctx)
 	if err != nil {
 		return false, err
 	}
 
+	var gotURL string
+	urls := strings.Split(strings.TrimSpace(allURLs), "\n")
+	if len(urls) > 1 {
+		// Multiple URLs configured - fall back to git remote get-url which
+		// handles this correctly (returns primary fetch URL).
+		args = []string{"remote", "get-url", "origin"}
+		if gitDir != "" {
+			args = append([]string{"--git-dir", gitDir}, args...)
+		}
+		gotURL, err = e.shell.Command("git", args...).RunAndCaptureStdout(ctx)
+		if err != nil {
+			return false, err
+		}
+	} else {
+		// Single URL - use config output directly to avoid insteadOf transformation.
+		gotURL = urls[0]
+	}
+
 	if gotURL == repository {
 		// No need to update anything
 		return false, nil

internal/job/integration/checkout_integration_test.go

@@ -297,7 +297,7 @@ func TestCheckingOutLocalGitProjectWithShortCommitHash(t *testing.T) {
 	// Git should attempt to fetch the shortHash, but fail. Then fallback to fetching
 	// all the heads and tags and checking out the short commit hash.
 	git.ExpectAll([][]any{
-		{"remote", "get-url", "origin"},
+		{"config", "--get-all", "remote.origin.url"},
 		{"clean", "-ffxdq"},
 		{"fetch", "--", "origin", shortCommitHash},
 		{"config", "remote.origin.fetch"},
@@ -615,7 +615,7 @@ func TestCheckoutErrorIsRetried(t *testing.T) {
 
 	// But assert which ones are called
 	git.ExpectAll([][]any{
-		{"remote", "get-url", "origin"},
+		{"config", "--get-all", "remote.origin.url"},
 		{"clean", "-fdq"},
 		{"fetch", "-v", "--", "origin", "main"},
 		{"checkout", "-f", "FETCH_HEAD"},
@@ -678,7 +678,7 @@ func TestFetchErrorIsRetried(t *testing.T) {
 
 	// But assert which ones are called
 	git.ExpectAll([][]any{
-		{"remote", "get-url", "origin"},
+		{"config", "--get-all", "remote.origin.url"},
 		{"clean", "-ffxdq"},
 		{"fetch", "-v", "--prune", "--depth=1", "--", "origin", "main"},
 		{"clone", "-v", "--depth=1", "--", tester.Repo.Path, "."},
@@ -1096,6 +1096,63 @@ func TestGitCheckoutWithoutCommitResolvedAndNoMetaData(t *testing.T) {
 	tester.RunAndCheck(t, env...)
 }
 
+func TestMultipleRemoteURLsFallsBackToGetURL(t *testing.T) {
+	t.Parallel()
+
+	tester, err := NewExecutorTester(mainCtx)
+	if err != nil {
+		t.Fatalf("NewExecutorTester() error = %v", err)
+	}
+	defer tester.Close()
+
+	env := []string{
+		"BUILDKITE_GIT_CLONE_FLAGS=-v",
+		"BUILDKITE_GIT_CLEAN_FLAGS=-fdq",
+		"BUILDKITE_GIT_FETCH_FLAGS=-v",
+	}
+
+	// Simulate state from a previous checkout
+	if err := os.MkdirAll(tester.CheckoutDir(), 0o755); err != nil {
+		t.Fatalf("error creating dir to clone from: %s", err)
+	}
+	cmd := exec.Command("git", "clone", "-v", "--", tester.Repo.Path, ".")
+	cmd.Dir = tester.CheckoutDir()
+	if _, err = cmd.Output(); err != nil {
+		t.Fatalf("error cloning test repo: %s", err)
+	}
+
+	// Add a second remote URL to simulate multi-URL configuration
+	cmd = exec.Command("git", "remote", "set-url", "--add", "origin", "https://example.com/extra.git")
+	cmd.Dir = tester.CheckoutDir()
+	if _, err = cmd.Output(); err != nil {
+		t.Fatalf("error adding second remote URL: %s", err)
+	}
+
+	// Actually execute git commands, but with expectations
+	git := tester.
+		MustMock(t, "git").
+		PassthroughToLocalCommand()
+
+	// Assert the expected git commands - should call config --get-all first,
+	// then fall back to remote get-url when multiple URLs are detected
+	git.ExpectAll([][]any{
+		{"config", "--get-all", "remote.origin.url"},
+		{"remote", "get-url", "origin"},
+		{"clean", "-fdq"},
+		{"fetch", "-v", "--", "origin", "main"},
+		{"checkout", "-f", "FETCH_HEAD"},
+		{"clean", "-fdq"},
+		{"--no-pager", "log", "-1", "HEAD", "-s", "--no-color", gitShowFormatArg},
+	})
+
+	// Mock out the meta-data calls to the agent after checkout
+	agent := tester.MockAgent(t)
+	agent.Expect("meta-data", "exists", job.CommitMetadataKey).AndExitWith(1)
+	agent.Expect("meta-data", "set", job.CommitMetadataKey).WithStdin(commitPattern)
+
+	tester.RunAndCheck(t, env...)
+}
+
 type subDirMatcher struct {
 	dir string
 }