Make Grafana/Metabase/Tailscale name errors terminal
The source-name worker keeps a generic name and marks the source synced only when a resolver reports ErrTerminalNameResolution. The Grafana and Metabase name resolvers, plus Tailscale via its shared fetchUsers, returned a plain error on any non-2xx, so a revoked credential (401/403) never reached the terminal path and the worker re-claimed the row every drain cycle with no delay. Route their non-2xx through nameStatusError so permanent 4xx wrap ErrTerminalNameResolution while 5xx stay retryable. The sentinel is inert on the ListAccounts sync path, which does not inspect it. Signed-off-by: Aurélien Sibiril <81782+aureliensibiril@users.noreply.github.com>
This commit is contained in:
@@ -208,7 +208,7 @@ func (r *grafanaNameResolver) ResolveInstanceName(ctx context.Context) (string,
|
||||
}()
|
||||
|
||||
if httpResp.StatusCode < 200 || httpResp.StatusCode >= 300 {
|
||||
return "", fmt.Errorf("cannot fetch grafana organization: unexpected status %d", httpResp.StatusCode)
|
||||
return "", nameStatusError("grafana organization", httpResp.StatusCode)
|
||||
}
|
||||
|
||||
var org grafanaOrg
|
||||
|
||||
@@ -236,7 +236,7 @@ func (r *metabaseNameResolver) ResolveInstanceName(ctx context.Context) (string,
|
||||
}()
|
||||
|
||||
if httpResp.StatusCode < 200 || httpResp.StatusCode >= 300 {
|
||||
return "", fmt.Errorf("cannot fetch metabase session properties: unexpected status %d", httpResp.StatusCode)
|
||||
return "", nameStatusError("metabase session properties", httpResp.StatusCode)
|
||||
}
|
||||
|
||||
var props metabaseSessionProperties
|
||||
|
||||
@@ -61,6 +61,44 @@ func TestNameStatusError(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestNameResolversTerminalOnClientError guards the fix that makes the
|
||||
// grafana, metabase and tailscale name resolvers surface a permanent 4xx as
|
||||
// ErrTerminalNameResolution. Without it a revoked-credential source hot-loops
|
||||
// the source-name worker, which never marks such a source synced.
|
||||
func TestNameResolversTerminalOnClientError(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
srv := httptest.NewServer(
|
||||
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusUnauthorized)
|
||||
_, _ = w.Write([]byte(`{"message":"unauthorized"}`))
|
||||
}),
|
||||
)
|
||||
t.Cleanup(srv.Close)
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
resolver NameResolver
|
||||
}{
|
||||
{name: "grafana", resolver: NewGrafanaNameResolver(srv.Client(), srv.URL)},
|
||||
{name: "metabase", resolver: NewMetabaseNameResolver(srv.Client(), srv.URL)},
|
||||
{name: "tailscale", resolver: NewTailscaleNameResolver(&http.Client{Transport: &hostRewriter{target: srv.URL}})},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(
|
||||
tc.name,
|
||||
func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
_, err := tc.resolver.ResolveInstanceName(context.Background())
|
||||
require.Error(t, err)
|
||||
assert.ErrorIs(t, err, ErrTerminalNameResolution)
|
||||
},
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// hostRewriter redirects requests to the configured target host so that
|
||||
// resolvers with hardcoded production URLs (api.notion.com, etc.) can be
|
||||
// pointed at an httptest server.
|
||||
|
||||
@@ -150,8 +150,12 @@ func (d *TailscaleDriver) fetchUsers(ctx context.Context) ([]tailscaleUser, erro
|
||||
_ = httpResp.Body.Close()
|
||||
}()
|
||||
|
||||
// Classify the status so the source-name worker (which reuses this via
|
||||
// tailscaleNameResolver) treats a 4xx as terminal instead of hot-looping.
|
||||
// The sentinel is inert on the ListAccounts sync path, which does not
|
||||
// inspect it.
|
||||
if httpResp.StatusCode < 200 || httpResp.StatusCode >= 300 {
|
||||
return nil, fmt.Errorf("cannot fetch tailscale users: unexpected status %d", httpResp.StatusCode)
|
||||
return nil, nameStatusError("tailscale users", httpResp.StatusCode)
|
||||
}
|
||||
|
||||
var resp tailscaleUsersResponse
|
||||
|
||||
Reference in New Issue
Block a user