From 79bc5aa79e491cb7e2645d875b9a5faef846be58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Sibiril?= <81782+aureliensibiril@users.noreply.github.com> Date: Sat, 23 May 2026 12:10:37 +0200 Subject: [PATCH] Guard github name resolver against empty organization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a GitHub access-source connector has no organization configured yet (user finished OAuth but abandoned the picker), the source-name worker called https://api.github.com/orgs/ and got a 404 every 10 seconds, flooding logs. All other picker resolvers (sentry, gitlab, bitbucket, heroku, asana, netlify, clickup, vercel) short-circuit to ("", nil) for empty settings -- this aligns github with them so the worker falls into its existing "empty instance name -> mark synced with generic name" branch instead of retrying forever. Signed-off-by: Aurélien Sibiril <81782+aureliensibiril@users.noreply.github.com> --- pkg/accessreview/drivers/name_resolver.go | 4 + .../drivers/name_resolver_test.go | 84 +++++++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/pkg/accessreview/drivers/name_resolver.go b/pkg/accessreview/drivers/name_resolver.go index 06d990193..5248e19e8 100644 --- a/pkg/accessreview/drivers/name_resolver.go +++ b/pkg/accessreview/drivers/name_resolver.go @@ -552,6 +552,10 @@ func NewGitHubNameResolver(httpClient *http.Client, org string) NameResolver { } func (r *githubNameResolver) ResolveInstanceName(ctx context.Context) (string, error) { + if r.org == "" { + return "", nil + } + endpoint, err := url.JoinPath("https://api.github.com", "orgs", url.PathEscape(r.org)) if err != nil { return "", fmt.Errorf("cannot build github organization URL: %w", err) diff --git a/pkg/accessreview/drivers/name_resolver_test.go b/pkg/accessreview/drivers/name_resolver_test.go index 85bbff4e0..3622f14d0 100644 --- a/pkg/accessreview/drivers/name_resolver_test.go +++ b/pkg/accessreview/drivers/name_resolver_test.go @@ -284,6 +284,90 @@ func TestHerokuNameResolver(t *testing.T) { }) } +func TestGitHubNameResolver(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + org string + status int + body string + want string + wantErr bool + wantNoReq bool + }{ + { + name: "empty org returns empty name without HTTP call", + org: "", + wantNoReq: true, + want: "", + }, + { + name: "200 with name", + org: "acme", + status: http.StatusOK, + body: `{"name":"Acme Inc"}`, + want: "Acme Inc", + }, + { + name: "200 with empty name falls back to org slug", + org: "acme", + status: http.StatusOK, + body: `{"name":""}`, + want: "acme", + }, + { + name: "404 errors", + org: "missing", + status: http.StatusNotFound, + body: `{"message":"Not Found"}`, + wantErr: true, + }, + { + name: "500 errors", + org: "acme", + status: http.StatusInternalServerError, + body: `{"message":"boom"}`, + wantErr: true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + var called bool + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true + assert.Equal(t, http.MethodGet, r.Method) + assert.Equal(t, "/orgs/"+tc.org, r.URL.Path) + assert.Equal(t, "application/vnd.github+json", r.Header.Get("Accept")) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(tc.status) + _, _ = w.Write([]byte(tc.body)) + })) + defer srv.Close() + + client := &http.Client{Transport: &hostRewriter{target: srv.URL}} + + got, err := NewGitHubNameResolver(client, tc.org).ResolveInstanceName(context.Background()) + if tc.wantErr { + require.Error(t, err) + return + } + + require.NoError(t, err) + assert.Equal(t, tc.want, got) + + if tc.wantNoReq { + assert.False(t, called, "expected no HTTP call when org is empty") + } else { + assert.True(t, called, "expected HTTP call when org is non-empty") + } + }) + } +} + // roundTripperFunc adapts a function into an http.RoundTripper, useful for // asserting that a resolver short-circuits before making any HTTP call. type roundTripperFunc func(*http.Request) (*http.Response, error)