From 036cd3306eef1a9c846149da375d270b914342b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Sibiril?= <81782+aureliensibiril@users.noreply.github.com> Date: Thu, 28 May 2026 19:21:05 +0200 Subject: [PATCH] Reuse ListSentryOrganizations in Sentry driver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SentryDriver.resolveOrgSlug duplicated the same /organizations/?member=true call already implemented in ListSentryOrganizations, which is consumed by the OAuth org picker. Delegating to the shared helper prevents the two call sites from drifting (response shape, header set, pagination) and keeps the driver focused on member listing. Pure refactor: no behavior change. Add an httptest-backed smoke test covering the empty-stored-slug path end-to-end through ListAccounts so the auto-discovery flow stays exercised after the refactor. Signed-off-by: Aurélien Sibiril <81782+aureliensibiril@users.noreply.github.com> --- pkg/accessreview/drivers/sentry.go | 24 ++---------------- pkg/accessreview/drivers/sentry_test.go | 33 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/pkg/accessreview/drivers/sentry.go b/pkg/accessreview/drivers/sentry.go index 4bd3a29c7..88328bb79 100644 --- a/pkg/accessreview/drivers/sentry.go +++ b/pkg/accessreview/drivers/sentry.go @@ -64,29 +64,9 @@ func NewSentryDriver(httpClient *http.Client, orgSlug string) *SentryDriver { } func (d *SentryDriver) resolveOrgSlug(ctx context.Context) (string, error) { - req, err := http.NewRequestWithContext(ctx, http.MethodGet, "https://sentry.io/api/0/organizations/?member=true", nil) + orgs, err := ListSentryOrganizations(ctx, d.httpClient) if err != nil { - return "", fmt.Errorf("cannot create sentry organizations request: %w", err) - } - - req.Header.Set("Accept", "application/json") - - resp, err := d.httpClient.Do(req) - if err != nil { - return "", fmt.Errorf("cannot fetch sentry organizations: %w", err) - } - - defer func() { _ = resp.Body.Close() }() - - if resp.StatusCode != http.StatusOK { - return "", fmt.Errorf("cannot fetch sentry organizations: status %d", resp.StatusCode) - } - - var orgs []struct { - Slug string `json:"slug"` - } - if err := json.NewDecoder(resp.Body).Decode(&orgs); err != nil { - return "", fmt.Errorf("cannot decode sentry organizations response: %w", err) + return "", fmt.Errorf("cannot resolve sentry organization slug: %w", err) } if len(orgs) == 0 { diff --git a/pkg/accessreview/drivers/sentry_test.go b/pkg/accessreview/drivers/sentry_test.go index e3e28e8c2..01e99d6c6 100644 --- a/pkg/accessreview/drivers/sentry_test.go +++ b/pkg/accessreview/drivers/sentry_test.go @@ -16,6 +16,8 @@ package drivers import ( "context" + "net/http" + "net/http/httptest" "os" "testing" @@ -45,3 +47,34 @@ func TestSentryDriver(t *testing.T) { assert.NotEmpty(t, r.ExternalID) assert.NotEmpty(t, r.Role) } + +func TestSentryDriverListAccountsAutoDiscoversSlug(t *testing.T) { + t.Parallel() + + const discoveredSlug = "discovered-org" + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + + switch r.URL.Path { + case "/api/0/organizations/": + assert.Equal(t, "true", r.URL.Query().Get("member")) + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`[{"slug":"` + discoveredSlug + `","name":"Discovered Org"}]`)) + case "/api/0/organizations/" + discoveredSlug + "/members": + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`[{"id":"42","email":"alice@example.com","name":"Alice","orgRole":"member"}]`)) + default: + t.Errorf("unexpected request to %s", r.URL.Path) + w.WriteHeader(http.StatusNotFound) + } + })) + defer srv.Close() + + client := &http.Client{Transport: &hostRewriter{target: srv.URL}} + + records, err := NewSentryDriver(client, "").ListAccounts(context.Background()) + require.NoError(t, err) + require.Len(t, records, 1) + assert.Equal(t, "alice@example.com", records[0].Email) +}