feat: add GitLab support to coderd/externalauth/gitprovider

Fixes CODAGT-146

Add GitLab support to the gitprovider package for gitsync/chatd PR
diff flows. This is a squashed stack of 3 PRs:

#25651 - refactor(coderd/externalauth): prepare gitprovider for multi-provider support
- Change gitprovider.New to return (Provider, error)
- Extract shared helpers (parseRetryAfter, checkRateLimitError,
  countDiffLines, escapePathPreserveSlashes) from github.go
- Update all callers (db2sdk, exp_chats, gitsync) for new signature
- Add error logging for provider construction failures
- Thread context through provider resolution

#25652 - feat(coderd/externalauth/gitprovider): add GitLab provider
- Implement full Provider interface: FetchPullRequestStatus,
  FetchPullRequestDiff, FetchBranchDiff, ResolveBranchPullRequest
- Handle nested groups, forks, and self-hosted instances
- Rate limit detection on both library and raw HTTP paths
- URL parsing/building with NormalizePullRequestURL support
- Unit tests covering error paths, URL parsing, state mapping
- Document GitLab configuration and known limitations

#25653 - test(coderd/externalauth/gitprovider): add GitLab VCR integration tests
- FetchPullRequestStatus: 4 fixtures (open, conflicts, merged, closed)
- FetchPullRequestDiff: 4 fixtures
- FetchBranchDiff: 3 fixtures (open, deleted, fork)
- ResolveBranchPullRequest: 3 fixtures
- go-vcr cassettes with sanitized GitLab API responses
This commit is contained in:
Cian Johnston
2026-05-25 17:41:02 +01:00
committed by GitHub
parent 2ad2f7869d
commit 579daaff70
34 changed files with 4146 additions and 179 deletions
+64 -84
View File
@@ -7,7 +7,6 @@ import (
"fmt"
"net/http"
"net/http/httptest"
"strconv"
"strings"
"testing"
"time"
@@ -16,12 +15,12 @@ import (
"github.com/stretchr/testify/require"
"github.com/coder/coder/v2/coderd/externalauth/gitprovider"
"github.com/coder/quartz"
)
func TestGitHubParseRepositoryOrigin(t *testing.T) {
t.Parallel()
gp := gitprovider.New("github", "", nil)
gp, err := gitprovider.New("github", "", nil)
require.NoError(t, err)
require.NotNil(t, gp)
tests := []struct {
@@ -121,7 +120,8 @@ func TestGitHubParseRepositoryOrigin(t *testing.T) {
func TestGitHubParsePullRequestURL(t *testing.T) {
t.Parallel()
gp := gitprovider.New("github", "", nil)
gp, err := gitprovider.New("github", "", nil)
require.NoError(t, err)
require.NotNil(t, gp)
tests := []struct {
@@ -194,7 +194,8 @@ func TestGitHubParsePullRequestURL(t *testing.T) {
func TestGitHubNormalizePullRequestURL(t *testing.T) {
t.Parallel()
gp := gitprovider.New("github", "", nil)
gp, err := gitprovider.New("github", "", nil)
require.NoError(t, err)
require.NotNil(t, gp)
tests := []struct {
@@ -245,7 +246,8 @@ func TestGitHubNormalizePullRequestURL(t *testing.T) {
func TestGitHubBuildBranchURL(t *testing.T) {
t.Parallel()
gp := gitprovider.New("github", "", nil)
gp, err := gitprovider.New("github", "", nil)
require.NoError(t, err)
require.NotNil(t, gp)
tests := []struct {
@@ -310,7 +312,8 @@ func TestGitHubBuildBranchURL(t *testing.T) {
func TestGitHubBuildPullRequestURL(t *testing.T) {
t.Parallel()
gp := gitprovider.New("github", "", nil)
gp, err := gitprovider.New("github", "", nil)
require.NoError(t, err)
require.NotNil(t, gp)
tests := []struct {
@@ -356,7 +359,8 @@ func TestGitHubBuildPullRequestURL(t *testing.T) {
func TestGitHubEnterpriseURLs(t *testing.T) {
t.Parallel()
gp := gitprovider.New("github", "https://ghes.corp.com/api/v3", nil)
gp, err := gitprovider.New("github", "https://ghes.corp.com/api/v3", nil)
require.NoError(t, err)
require.NotNil(t, gp)
t.Run("ParseRepositoryOrigin HTTPS", func(t *testing.T) {
@@ -419,7 +423,8 @@ func TestGitHubEnterpriseURLs(t *testing.T) {
func TestNewUnsupportedProvider(t *testing.T) {
t.Parallel()
gp := gitprovider.New("unsupported", "", nil)
gp, err := gitprovider.New("unsupported", "", nil)
require.NoError(t, err)
assert.Nil(t, gp, "unsupported provider type should return nil")
}
@@ -434,10 +439,11 @@ func TestGitHubRatelimit_403WithResetHeader(t *testing.T) {
}))
defer srv.Close()
gp := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
gp, err := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
require.NoError(t, err)
require.NotNil(t, gp)
_, err := gp.FetchPullRequestStatus(
_, err = gp.FetchPullRequestStatus(
context.Background(),
"test-token",
gitprovider.PRRef{Owner: "org", Repo: "repo", Number: 1},
@@ -459,10 +465,11 @@ func TestGitHubRatelimit_429WithRetryAfter(t *testing.T) {
}))
defer srv.Close()
gp := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
gp, err := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
require.NoError(t, err)
require.NotNil(t, gp)
_, err := gp.FetchPullRequestStatus(
_, err = gp.FetchPullRequestStatus(
context.Background(),
"test-token",
gitprovider.PRRef{Owner: "org", Repo: "repo", Number: 1},
@@ -486,10 +493,11 @@ func TestGitHubRatelimit_403NormalError(t *testing.T) {
}))
defer srv.Close()
gp := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
gp, err := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
require.NoError(t, err)
require.NotNil(t, gp)
_, err := gp.FetchPullRequestStatus(
_, err = gp.FetchPullRequestStatus(
context.Background(),
"bad-token",
gitprovider.PRRef{Owner: "org", Repo: "repo", Number: 1},
@@ -515,7 +523,9 @@ func TestGitHubFetchPullRequestDiff(t *testing.T) {
}))
defer srv.Close()
gp := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
gp, err := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
require.NoError(t, err)
require.NotNil(t, gp)
diff, err := gp.FetchPullRequestDiff(
@@ -537,7 +547,9 @@ func TestGitHubFetchPullRequestDiff(t *testing.T) {
}))
defer srv.Close()
gp := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
gp, err := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
require.NoError(t, err)
require.NotNil(t, gp)
diff, err := gp.FetchPullRequestDiff(
@@ -559,10 +571,12 @@ func TestGitHubFetchPullRequestDiff(t *testing.T) {
}))
defer srv.Close()
gp := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
gp, err := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
require.NoError(t, err)
require.NotNil(t, gp)
_, err := gp.FetchPullRequestDiff(
_, err = gp.FetchPullRequestDiff(
context.Background(),
"test-token",
gitprovider.PRRef{Owner: "org", Repo: "repo", Number: 1},
@@ -581,10 +595,11 @@ func TestFetchPullRequestDiff_Ratelimit(t *testing.T) {
}))
defer srv.Close()
gp := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
gp, err := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
require.NoError(t, err)
require.NotNil(t, gp)
_, err := gp.FetchPullRequestDiff(
_, err = gp.FetchPullRequestDiff(
context.Background(),
"test-token",
gitprovider.PRRef{Owner: "org", Repo: "repo", Number: 1},
@@ -614,10 +629,11 @@ func TestFetchBranchDiff_Ratelimit(t *testing.T) {
}))
defer srv.Close()
gp := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
gp, err := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
require.NoError(t, err)
require.NotNil(t, gp)
_, err := gp.FetchBranchDiff(
_, err = gp.FetchBranchDiff(
context.Background(),
"test-token",
gitprovider.BranchRef{Owner: "org", Repo: "repo", Branch: "feat"},
@@ -747,7 +763,9 @@ func TestFetchPullRequestStatus(t *testing.T) {
srv := httptest.NewServer(mux)
defer srv.Close()
gp := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
gp, err := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
require.NoError(t, err)
require.NotNil(t, gp)
before := time.Now().UTC()
@@ -793,7 +811,9 @@ func TestResolveBranchPullRequest(t *testing.T) {
defer srv.Close()
srvURL = srv.URL
gp := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
gp, err := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
require.NoError(t, err)
require.NotNil(t, gp)
prRef, err := gp.ResolveBranchPullRequest(
@@ -817,7 +837,9 @@ func TestResolveBranchPullRequest(t *testing.T) {
}))
defer srv.Close()
gp := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
gp, err := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
require.NoError(t, err)
require.NotNil(t, gp)
prRef, err := gp.ResolveBranchPullRequest(
@@ -840,7 +862,9 @@ func TestResolveBranchPullRequest(t *testing.T) {
}))
defer srv.Close()
gp := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
gp, err := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
require.NoError(t, err)
require.NotNil(t, gp)
prRef, err := gp.ResolveBranchPullRequest(
@@ -873,7 +897,9 @@ func TestFetchBranchDiff(t *testing.T) {
}))
defer srv.Close()
gp := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
gp, err := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
require.NoError(t, err)
require.NotNil(t, gp)
diff, err := gp.FetchBranchDiff(
@@ -894,10 +920,12 @@ func TestFetchBranchDiff(t *testing.T) {
}))
defer srv.Close()
gp := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
gp, err := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
require.NoError(t, err)
require.NotNil(t, gp)
_, err := gp.FetchBranchDiff(
_, err = gp.FetchBranchDiff(
context.Background(),
"test-token",
gitprovider.BranchRef{Owner: "org", Repo: "repo", Branch: "feat"},
@@ -921,10 +949,12 @@ func TestFetchBranchDiff(t *testing.T) {
}))
defer srv.Close()
gp := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
gp, err := gitprovider.New("github", srv.URL+"/api/v3", srv.Client())
require.NoError(t, err)
require.NotNil(t, gp)
_, err := gp.FetchBranchDiff(
_, err = gp.FetchBranchDiff(
context.Background(),
"test-token",
gitprovider.BranchRef{Owner: "org", Repo: "repo", Branch: "feat"},
@@ -937,59 +967,9 @@ func TestEscapePathPreserveSlashes(t *testing.T) {
t.Parallel()
// The function is unexported, so test it indirectly via BuildBranchURL.
// A branch with a space in a segment should be escaped, but slashes preserved.
gp := gitprovider.New("github", "", nil)
gp, err := gitprovider.New("github", "", nil)
require.NoError(t, err)
require.NotNil(t, gp)
got := gp.BuildBranchURL("owner", "repo", "feat/my thing")
assert.Equal(t, "https://github.com/owner/repo/tree/feat/my%20thing", got)
}
func TestParseRetryAfter(t *testing.T) {
t.Parallel()
clk := quartz.NewMock(t)
clk.Set(time.Now())
t.Run("RetryAfterSeconds", func(t *testing.T) {
t.Parallel()
h := http.Header{}
h.Set("Retry-After", "120")
d := gitprovider.ParseRetryAfter(h, clk)
assert.Equal(t, 120*time.Second, d)
})
t.Run("XRatelimitReset", func(t *testing.T) {
t.Parallel()
future := clk.Now().Add(90 * time.Second)
t.Logf("now: %d future: %d", clk.Now().Unix(), future.Unix())
h := http.Header{}
h.Set("X-Ratelimit-Reset", strconv.FormatInt(future.Unix(), 10))
d := gitprovider.ParseRetryAfter(h, clk)
assert.WithinDuration(t, future, clk.Now().Add(d), time.Second)
})
t.Run("NoHeaders", func(t *testing.T) {
t.Parallel()
h := http.Header{}
d := gitprovider.ParseRetryAfter(h, clk)
assert.Equal(t, time.Duration(0), d)
})
t.Run("InvalidValue", func(t *testing.T) {
t.Parallel()
h := http.Header{}
h.Set("Retry-After", "not-a-number")
d := gitprovider.ParseRetryAfter(h, clk)
assert.Equal(t, time.Duration(0), d)
})
t.Run("RetryAfterTakesPrecedence", func(t *testing.T) {
t.Parallel()
h := http.Header{}
h.Set("Retry-After", "60")
h.Set("X-Ratelimit-Reset", strconv.FormatInt(
clk.Now().Unix()+120, 10,
))
d := gitprovider.ParseRetryAfter(h, clk)
assert.Equal(t, 60*time.Second, d)
})
}