Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions .claude/rules/webhook-validation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
---
paths:
- "hypershift-operator/controllers/hostedcluster/hostedcluster_webhook*.go"
---

# Do Not Extend the Webhook

The webhook in `hostedcluster_webhook.go` is **not a general-purpose extension point**. Do not add new validation or defaulting logic here.

HyperShift uses CRD CEL validation rules instead of webhooks. The webhook exists only for KubeVirt platform-specific needs (defaulting and JSON patch annotation validation).

## Why

Webhooks add latency and operational complexity that CEL avoids:

- **Konnectivity overhead.** In hosted-on-hosted topologies (e.g., IBM Cloud, where management clusters are themselves hosted clusters), a webhook request travels: apiserver → konnectivity server → konnectivity agent → node → webhook service (possibly on a different node). This adds significant per-request latency to every API write.
- **Availability coupling.** A down or slow webhook blocks all writes to the resource. CEL runs inside the API server with no external dependency.
- **Operational overhead.** Webhooks require TLS certificate management, webhook configurations, and a running service.

## What to Do Instead

Add new validation as CEL markers on the API types in `api/hypershift/v1beta1/` and cover them with envtests. See `api/AGENTS.md` and `test/envtest/README.md`.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this is valid for api/ not only api/hypershift/v1beta1


The exception is CAPI resources, where webhooks are used unconditionally during the v1beta2 migration period.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's only the conversion webhook that is required for CAPI

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — updated to "a conversion webhook is required" in 7339dfe.

4 changes: 4 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,10 @@ Please see /hypershift/.cursor/rules/code-formatting.mdc
### CRD API Machinery Fundamentals
See api/AGENTS.md

### Validation: CEL Over Webhooks

Do not add new validation logic to the admission webhook. Use CEL validation rules instead. See .claude/rules/webhook-validation.md for rationale and guidance.

### Design Invariants

See [docs/content/reference/goals-and-design-invariants.md](docs/content/reference/goals-and-design-invariants.md) for security and architectural invariants that all code changes must respect.
Expand Down
1 change: 1 addition & 0 deletions api/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ To avoid introducing new dependencies, do not add utils or methods to the API ty
- **OpenAPI schema validation only runs when a key is present in the serialized object.** if a field is omitted, the validation never executes. This is why `MinLength=1` on an optional field is safe: the constraint only fires when the user actually provides a value.
- **Optionality and min constraints are independent concerns.** An optional field with `MinLength=1` means "you don't have to set this, but if you do, it can't be empty." These do not conflict.
- **Admission-time validation rejects the write immediately.** Controller-time validation accepts the write, the user assumes success, and discovers the problem later via conditions or logs. Always prefer admission-time via CEL over controller-time validation.
- **Use CEL rules, not webhooks, for new validation.** HyperShift deliberately avoids admission webhooks because they add latency (especially in hosted-on-hosted topologies like IBM Cloud where requests traverse konnectivity), require TLS cert management, and couple resource availability to a running webhook service. The existing webhook is scoped to KubeVirt platform defaults and is not a general extension point. Add new validation as CEL markers on the API types in this module.

### Immutability

Expand Down
Loading