knative: install: Distinguish CRD probe errors from absence

[Elvand-Lie]

Summary

Only classify a confirmed missing Knative CRD as evidence that Knative is not
installed.

Problem

The installation probe previously treated every CRD lookup failure as absence.

That included:

• RBAC denials
• transient server failures
• callback errors without an HTTP status

As a result, Headlamp could display "Knative was not detected" or hide Knative
navigation when the probe had not actually established that the CRD was absent.

Fix

The probe now treats only a confirmed HTTP 404 as absence.

Other API error callbacks are treated as non-confirmed absence. Normal Knative
resource requests still enforce the user's Kubernetes permissions and can surface
their own API errors.

The existing request-promise fallback, boolean API, sidebar TTL, routing,
multi-cluster aggregation, and RBAC behavior are unchanged.

Regression tests

Added coverage for:

• successful CRD lookup
• confirmed 404 absence
• forbidden response
• server error
• error callback without HTTP status
• null error callback
• empty cluster selection
• cluster option forwarding
• multi-cluster success
• partial multi-cluster absence
• settle-once behavior

Verification

• npm run test
• npm run tsc
• npm run lint
• npm run format -- --check
• npm run build
• git diff --check

Related work

PR #1075 adds tests for the helper's current behavior and touches the same new
test-file path. This PR additionally changes the production behavior so that
probe errors are not represented as confirmed absence.

The distinction follows the same error-versus-absence principle discussed for
the Flux installation probe in #972/#984.

Scope

No sidebar architecture, cache TTL, routes, permissions, or installation UI was
changed.

AI assistance

AI-assisted tooling was used during investigation and implementation. I reviewed
the final diff and ran the listed validation locally.

[Ralthos]

The .catch branch cannot fire here.

CustomResourceDefinition declares one apiVersion, so apiFactory takes the singleApiFactory path and get is streamResult. streamResult kicks off an unawaited run() and returns Promise.resolve(cancel), and every failure inside run() is caught there and handed to errCb. The promise that request() returns never rejects. Traced through KubeObject.js:307, factories.js:115 and :176, and streamingApi.js:41, in @kinvolk/headlamp-plugin 0.14.0.

So flipping settle(false) to settle(true) in the .catch is a no-op for this resource class, and the whole behaviour change rests on err => settle(err?.status !== 404). The change surface is one line.

The test does not classify a rejected request as Knative absence mocks apiGet into returning a rejecting function, a shape the real apiGet does not produce. It passes, and it pins a branch production cannot reach. Harmless, but it reads as coverage of the .catch decision when nothing is being covered.

For RBAC-restricted users the probe now returns true unconditionally.

I measured this while writing up #1083. A ServiceAccount without get on customresourcedefinitions gets 403 from /apis/apiextensions.k8s.io/v1/customresourcedefinitions/<name>, and authorization is checked before existence, so 403 comes back whether the CRD is there or not.

That user can no longer reach the absence branch, which is the correct failure direction and does fix the bug in the title. It also means the probe has stopped answering the question for them: true regardless of what the cluster serves. /apis/serving.knative.dev/v1 would answer it, since group discovery is not RBAC gated and system:discovery is bound to system:authenticated.

I would leave that alone here. It is the case #1083 argues, and the boolean return is why the choice had to be made at all. A third state would let the sidebar show Knative and let the pages surface their own errors, without encoding "could not tell" as "installed".

One more, and it predates this PR: hasCrdInCluster leaks a watch connection whenever the probe succeeds.

settle(true) runs synchronously inside cb(item) and calls cancelFn(), which sets isCancelled and cancels socket. socket is undefined at that moment. run() then resumes on the next statement and opens the watch:

cb(item);
const watchUrl = ...;
socket = stream(watchUrl, (x) => cb(x.object), { isJson: true, cluster: clusterName });

The isCancelled guard stops cancel() from running a second time, so that socket stays open. One per cluster on every successful probe, which is the common path. Probably belongs in its own issue.

[illume]

Thanks for this PR.

A few of the commits don't quite follow the project guidelines. We use Linux kernel style for git commits — have a look at the contributing guide and previous commits with git log.

Commits that need attention

• knative: distinguish CRD probe errors from absence — Description must start with a capital letter — e.g. frontend: HomeButton: Fix the button not frontend: HomeButton: fix the button.
• knative: keep CRD probe fallback unchanged — Description must start with a capital letter — e.g. frontend: HomeButton: Fix the button not frontend: HomeButton: fix the button.

Commit guidelines

• Use atomic commits focused on a single change.
• Use the title format <area>: <Description of changes> — description must start with a capital letter.
• Keep the title under 72 characters (soft requirement).
• Explain the intention and why the change is needed.
• Make commit titles meaningful and describe what changed.
• Do not add code that a later commit rewrites; squash or reorder commits instead.
• Do not include Fixes #NN in commit messages.

Good examples:

• frontend: HomeButton: Fix so it navigates to home
• backend: config: Add enable-dynamic-clusters flag

[Copilot]

Copilot review overview

🟢 Approval recommended

The implementation matches the stated semantics and is comprehensively tested.

Review effort: Balanced
Findings: None

What changed in this PR

Updates Knative detection so only HTTP 404 confirms absence.

Changes:

• Treats non-404 probe errors as inconclusive.
• Adds regression coverage for probe and multi-cluster behavior.

File	Description
knative/​src/​isKnativeInstalled.ts	Refines CRD error classification.
knative/​src/​isKnativeInstalled.test.ts	Tests detection behavior and edge cases.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Elvand-Lie]

@illume Rebased and revised to address the review feedback and commit-history requirements. The branch is now a single atomic commit and has been re-verified. Ready for another look.