Background
Raised during review of #851 (by @afritzler).
The current PR #851 adds a validating admission webhook to enforce that spec.serverRef.name is set and that the referenced Server exists. After review, this approach was identified as unnecessary and potentially problematic. This issue captures the preferred alternatives.
1. Enforce non-empty serverRef.name via a CRD validation marker
ServerRef in api/v1alpha1/servermaintenance_types.go is already marked +required with no omitempty, so the CRD schema already rejects objects missing the field entirely. The remaining gap — an empty name string — can be closed declaratively using a CEL validation marker directly on the field, without any webhook:
// +kubebuilder:validation:Required
// +kubebuilder:validation:XValidation:rule="self.name != ''",message="serverRef.name must be set"
ServerRef *corev1.LocalObjectReference `json:"serverRef"`
Tasks:
- Add the
XValidation marker to ServerRef in api/v1alpha1/servermaintenance_types.go.
- Regenerate CRD manifests.
2. Handle a missing referenced Server gracefully in the reconciler
Cross-object existence checks at admission time are an anti-pattern in Kubernetes for several reasons:
- GitOps / kustomize bundles often apply a
Server and a ServerMaintenance together; apply order is not guaranteed, so a valid bundle could be rejected.
- The webhook read is not transactional with the subsequent create, making it inherently racy.
- If the
Server appears moments later, the controller handles it naturally via requeue anyway.
The idiomatic approach is to handle a missing Server in the reconciler by requeuing and surfacing the situation via a status condition, consistent with how the rest of the operator works.
Tasks:
- Verify whether the
ServerMaintenance reconciler already handles a not-found Server gracefully.
- If not, add the appropriate requeue logic and a status condition (e.g.
ServerNotFound) so operators get early, clear feedback without blocking eventual consistency.
References
Background
Raised during review of #851 (by @afritzler).
The current PR #851 adds a validating admission webhook to enforce that
spec.serverRef.nameis set and that the referencedServerexists. After review, this approach was identified as unnecessary and potentially problematic. This issue captures the preferred alternatives.1. Enforce non-empty
serverRef.namevia a CRD validation markerServerRefinapi/v1alpha1/servermaintenance_types.gois already marked+requiredwith noomitempty, so the CRD schema already rejects objects missing the field entirely. The remaining gap — an emptynamestring — can be closed declaratively using a CEL validation marker directly on the field, without any webhook:Tasks:
XValidationmarker toServerRefinapi/v1alpha1/servermaintenance_types.go.2. Handle a missing referenced
Servergracefully in the reconcilerCross-object existence checks at admission time are an anti-pattern in Kubernetes for several reasons:
Serverand aServerMaintenancetogether; apply order is not guaranteed, so a valid bundle could be rejected.Serverappears moments later, the controller handles it naturally via requeue anyway.The idiomatic approach is to handle a missing
Serverin the reconciler by requeuing and surfacing the situation via a status condition, consistent with how the rest of the operator works.Tasks:
ServerMaintenancereconciler already handles a not-foundServergracefully.ServerNotFound) so operators get early, clear feedback without blocking eventual consistency.References
serverRefonserverMaintenanceson creation #629