kubernetes-sigs / kubernetes-sigs/controller-tools
crd: nondeterministic x-kubernetes-validations order when +k8s: markers are mixed with +kubebuilder:validation:XValidation on one field
- Dominant language
- Go
- Stars
- 868
- Forks
- 482
- Avg merge
- 1d 11h
- Merged PRs (30d)
- 8
Description
## What happened
When a field mixes a `+k8s:` declarative validation marker with `+kubebuilder:validation:XValidation` markers, the relative order of the emitted `x-kubernetes-validations` rules is nondeterministic across runs of `controller-gen crd`. The rules contributed by each marker family keep their declared order internally, but the *relative* order of the two groups flips randomly between runs.
This makes committed CRD manifests flap: repos that commit generated CRDs (and verify them in CI with a `git diff --exit-code` style check) see spurious diffs/failures depending on which ordering a given run produces.
## Minimal reproduction
```go
// WidgetSpec defines the desired state of Widget.
type WidgetSpec struct {
// +k8s:immutable
// +kubebuilder:validation:XValidation:rule="!self.startsWith('-')",message="branch must not start with '-'"
// +kubebuilder:validation:XValidation:rule="!self.contains(':')",message="branch must not contain ':'"
// +kubebuilder:validation:Required
Branch string `json:"branch"`
}
```
```sh
for i in $(seq 1 20); do
controller-gen crd paths="./api/v1alpha1/..." output:crd:artifacts:config=out-$i
done
md5sum out-*/*.yaml | awk '{print $1}' | sort | uniq -c
```
Observed with v0.21.0 (20 runs produced 2 distinct outputs, 19/1 split):
```diff
x-kubernetes-validations:
+ - message: field is immutable
+ rule: self == oldSelf
- message: branch must not start with '-'
rule: '!self.startsWith(''-'')'
- message: branch must not contain ':'
rule: '!self.contains('':'')'
- - message: field is immutable
- rule: self == oldSelf
```
## Root cause
In `applyMarkers` (`pkg/crd/schema.go`), markers are collected by iterating the `markers.MarkerValues` map (keyed by marker name), then ordered with `slices.SortStableFunc` using only the apply-priority:
```go
for markerName, markerValues := range markerSet {
...
}
slices.SortStableFunc(markers, func(i, j schemaMarkerWithName) int { return cmpPriority(i, j) })
```
`Immutable` (from `+k8s:immutable`) doesn't implement `ApplyPriorityMarker` and `XValidation.ApplyPriority()` returns `ApplyPriorityDefault`, so both families sort equal — and the *stable* sort then preserves whatever order the randomized map iteration happened to produce. Both markers append to `schema.XValidations`, so the appended rule order is nondeterministic.
This only bites when two *different* marker names contribute rules to the same field's `x-kubernetes-validations`; repeated values under a single marker name keep their declared order, which is why this wasn't visible before the `+k8s:` validation markers landed. The code is unchanged on `main` as of today.
Related: #1324 attempted to fix this by sorting the emitted rules, and was (correctly, I think) rejected — CEL rules execute in order, so reordering them changes semantics. But the underlying nondeterminism it observed is real; it was closed as "fixed by the stable sort update for markers", which doesn't actually cover the cross-marker-name case since the stable sort has no deterministic key for equal priorities.
## Proposed fix
Give equal-priority markers a deterministic secondary sort key — e.g. sort by `(applyPriority, markerName)` while keeping the stable sort so repeated values of the same marker preserve declared order:
```go
slices.SortStableFunc(markers, func(i, j schemaMarkerWithName) int {
if c := cmpPriority(i, j); c != 0 {
return c
}
return strings.Compare(i.Name, j.Name)
})
```
This doesn't reorder rules within a marker family (so existing single-family ordering guarantees are untouched), it just makes the cross-family relative order deterministic. The one caveat is that any output generated since the `+k8s:` markers landed may churn once when this lands, since the current cross-family order is arbitrary. An alternative that avoids even that churn for most repos would be sorting by source position (file/line of the marker comment) instead of name, which would also match user intuition that rules apply in the order written.
Happy to send a PR for either approach if there's agreement on the desired key.
## Environment
- controller-gen v0.21.0 (also inspected `pkg/crd/schema.go` on `main` — same code)
- go1.26, darwin/arm64 (map iteration randomization makes it reproducible everywhere)
## Workaround
Don't mix marker families on a field: replace `+k8s:immutable` with an explicit `+kubebuilder:validation:XValidation:rule="self == oldSelf",message="field is immutable"` so all rules come from one marker name and are emitted in declared order.
Contributor guide
Research direction
Read pkg/crd/schema.go, especially applyMarkers and the marker ordering logic. Run the 20-iteration controller-gen crd reproduction to observe the differing outputs, then add regression coverage for mixed marker families; done means x-kubernetes-validations ordering is deterministic while repeated values within one marker family retain their declared order.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 65/100