Skip to content

Commit a6407d0

Browse files
committed
Flag a 401 on the Helm read and explain the flag to agents
A 401 means Radar's own credentials failed, not that the caller may not read release Secrets, so it now sets helm_issues_unchecked and is logged as a failure. Only a Kubernetes Forbidden answer still counts as the caller's permission boundary. The MCP issues tool description now says what helm_issues_unchecked means, since agents read the description, not the docs.
1 parent e90513a commit a6407d0

4 files changed

Lines changed: 39 additions & 7 deletions

File tree

‎internal/helm/issue_releases.go‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,20 @@ func (c *Client) ListIssueReleasesAcrossNamespaces(ctx context.Context, namespac
6161
return listIssueReleasesWithClients(ctx, client, labelsClient, contextName, namespaces)
6262
}
6363

64+
// IssueReadDenied reports whether a failed ListIssueReleasesAcrossNamespaces
65+
// was Kubernetes refusing the caller's release Secret list. A 401 is not: it
66+
// is Radar's own credentials failing, not the caller's permissions.
67+
func IssueReadDenied(err error) bool {
68+
return err != nil && isReleaseReadForbidden(err)
69+
}
70+
6471
// IssueReadUnchecked reports whether a failed ListIssueReleasesAcrossNamespaces
65-
// leaves the caller's Helm issues unknown. A denial is not: the caller may not
66-
// read release Secrets, so it sees no Helm issues, like any other kind it can't
67-
// list. Neither is a caller that left, since nobody gets the answer. Radar's
68-
// own request deadline is a failure: the caller is still waiting.
72+
// leaves the caller's Helm issues unknown. A denial does not: the caller may
73+
// not read release Secrets, so it sees no Helm issues, like any other kind it
74+
// can't list. Neither does a caller that left, since nobody gets the answer.
75+
// Radar's own request deadline does: the caller is still waiting.
6976
func IssueReadUnchecked(ctx context.Context, err error) bool {
70-
return err != nil && !IsForbiddenError(err) && !errors.Is(ctx.Err(), context.Canceled)
77+
return err != nil && !IssueReadDenied(err) && !errors.Is(ctx.Err(), context.Canceled)
7178
}
7279

7380
func listIssueReleasesWithClients(ctx context.Context, client kubernetes.Interface, labelsClient metadata.Interface, contextName string, namespaces []string) ([]HelmRelease, error) {

‎internal/mcp/helm_issues_unchecked_test.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"k8s.io/client-go/rest"
1616

1717
"github.com/skyhook-io/radar/internal/helm"
18+
"github.com/skyhook-io/radar/internal/issues"
1819
"github.com/skyhook-io/radar/internal/k8s"
1920
)
2021

@@ -38,6 +39,13 @@ func TestIssuesToolSaysWhenHelmReleasesCouldNotBeChecked(t *testing.T) {
3839
status: http.StatusForbidden,
3940
body: `{"kind":"Status","apiVersion":"v1","status":"Failure","message":"secrets is forbidden","reason":"Forbidden","code":403}`,
4041
},
42+
{
43+
// Radar's own credentials failing is not the caller's permissions.
44+
name: "API server rejects Radar's credentials",
45+
status: http.StatusUnauthorized,
46+
body: `{"kind":"Status","apiVersion":"v1","status":"Failure","message":"Unauthorized","reason":"Unauthorized","code":401}`,
47+
wantUnchecked: true,
48+
},
4149
{
4250
name: "read succeeded",
4351
status: http.StatusOK,
@@ -86,3 +94,18 @@ func TestIssuesToolSaysWhenHelmReleasesCouldNotBeChecked(t *testing.T) {
8694
})
8795
}
8896
}
97+
98+
// A caller that already left gets no answer, so its read cut short is not
99+
// a failed check.
100+
func TestNativeHelmIssuesDoNotSayUncheckedWhenCallerLeft(t *testing.T) {
101+
helm.ResetClient()
102+
if err := helm.InitializeWithRESTConfig(&rest.Config{Host: "http://127.0.0.1:1"}); err != nil {
103+
t.Fatal(err)
104+
}
105+
t.Cleanup(helm.ResetClient)
106+
ctx, cancel := context.WithCancel(context.Background())
107+
cancel()
108+
if _, unchecked := nativeHelmIssuesForContext(ctx, []string{"shop"}, issues.Filters{}); unchecked {
109+
t.Error("a caller that left was told Helm releases couldn't be checked")
110+
}
111+
}

‎internal/mcp/tools.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,8 @@ func registerTools(server *mcp.Server, includeWrites bool, paramRegistry *toolPa
389389
"grouped critical/warning failures across workloads, Jobs/CronJobs, HPAs, " +
390390
"PVCs, Nodes, references, pod startup, native Helm, and False controller conditions. " +
391391
"Use CEL `filter` for source/taxonomy slices; no source parameter. " +
392+
"`helm_issues_unchecked: true` means Helm releases couldn't be read, so failed " +
393+
"Helm releases may be missing; don't call Helm healthy. " +
392394
"`diagnostic_context.role` marks roots, rollups, symptoms, or context; " +
393395
"`related_issues[].count` is the root's affected subset, not the linked issue total. " +
394396
"Confidence: `high` declared edge, `medium` lead to verify, `low` heuristic. " +
@@ -3158,7 +3160,7 @@ func nativeHelmIssuesForContext(ctx context.Context, namespaces []string, filter
31583160
}
31593161
releases, err := helmClient.ListIssueReleasesAcrossNamespaces(ctx, helmNamespaces, username, groups)
31603162
if err != nil {
3161-
if !errors.Is(ctx.Err(), context.Canceled) && !helm.IsForbiddenError(err) {
3163+
if !errors.Is(ctx.Err(), context.Canceled) && !helm.IssueReadDenied(err) {
31623164
log.Printf("[mcp] Failed to list Helm releases for issue stream: %v", err)
31633165
}
31643166
return nil, helm.IssueReadUnchecked(ctx, err)

‎internal/server/issues_handler.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ func (s *Server) nativeHelmIssuesForRequest(r *http.Request, namespaces []string
231231
switch {
232232
case errors.Is(ctx.Err(), context.Canceled):
233233
// The caller went away; nobody is left to tell.
234-
case !helm.IsForbiddenError(err):
234+
case !helm.IssueReadDenied(err):
235235
log.Printf("[issues] Failed to list Helm releases for issue stream: %v", err)
236236
default:
237237
if _, seen := helmIssuesDeniedLogged.LoadOrStore(pkgauth.IdentityCacheKey(username, groups), struct{}{}); !seen {

0 commit comments

Comments
 (0)