📖 Add doc about Strict Server-Side Field Validation

[camilamacedo86]

Closes: #5208

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [camilamacedo86]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[camilamacedo86]

Hi @sbueringer and @alvaroaleman 👋

Since we are documenting a feature of controller-runtime — how to use it, when to use it, and related details — could you please give a hand with the review?
Your feedback would be very much appreciated! 🙏

Also, could you please let me know what you think?

I am ping @dlipovetsky since seems to be the author of the feature as well.
And who request this one: @guettli

Could you please folks approve this one if you are OK with.
PS.: I am adding a hold to allow everybody LGTM and etc and give proper time to everybody review

/hold

Thanks a lot! 💛

[alvaroaleman]

What does this have to do with upgrade ordering? Is this referring to CRDs being upgraded before controllers? That is always expected to happen IMHO.

Could also mention that this has no effect for server-side apply, there validation is always strict.

[camilamacedo86]

Hi @alvaroaleman,

Thank you for the quick reply. The above dev statement wasn’t entirely accurate — it was added by mistake, so please ignore it. I’ve already fixed it.

However, please note:
While CRDs should be updated first, in many scenarios this is still best-effort rather than guaranteed. This can become a concern, especially during the lifecycle and upgrade process.

I’ve updated the documentation to address this properly.
I hope this helps clarify the concerns as well.

[alvaroaleman]

While CRDs should be updated first, in many scenarios this is still best-effort rather than guaranteed.

IMHO if that fails, it is much preferrable to have errors rather than the apisever silently dropping the fields

[camilamacedo86]

IMHO if that fails, it is much preferrable to have errors rather than the apisever silently dropping the fields

Depends on the scenario, right? It really varies based on how the project is distributed, configured, and used (Helm, GitOps, DevOps setups, etc.).

The docs aim to clarify:

• What this feature is and what it solves;
• Where it works well and where it may not;
• What users need to test themselves — especially upgrades and lifecycle — when using Argo CD, Flux, Helm, third-party CRDs etc. Any caveats/concerns they should be aware of;
• How it plays with the Kubebuilder scaffold and which options might cause issues.

This way users/maintainers know it exists, see it as a valid option and make an informed decision.

Also: is there anything about the feature that we’re not explaining, missing, or getting wrong? Any key detail we should add?

Unless I misunderstood something — otherwise this wouldn’t be a feature and would just be the default behavior, right? Why wasn’t it added by default? What concerns were raised? Any other key concerns/considerations I might not be thinking about?

[guettli]

@camilamacedo86 @alvaroaleman maybe we should make it easier for developers to use WithFieldValidation before documenting it.

I started a thread about that in #controller-runtime

[sbueringer]

Sorry was just quickly reading over it so I might have missed this. This seems to focus only on CRDs?

If strict validation is used with builtin-resources this can mean that the controller is only compatible with exactly the Kubernetes version it was compiled against, which is often not desired

[camilamacedo86]

That is exactly the kind of amazing help that I am looking for !!! 🙌
It is something that we must clarify.

Thank you, I will try to add it.
You are amazing 🚀 ⭐

[guettli]

If strict validation is used with builtin-resources this can mean that the controller is only compatible with exactly the Kubernetes version it was compiled against, which is often not desired

I heard that Server-Side Apply always uses strict field validation. How can that work? I guess that SSA does a Patch, not an Update.

I guess that strict field validation works (even with other Kubernetes versions), when you use Patch, and the schema matches for the fields of that patch.

Are there docs about that? I would like to learn more about that.

[sbueringer]

SSA does neither Patch nor Update it does Apply. If you send a field that doesn't exist it will fail

No idea about docs to be honest, maybe somewhere on the Kubernetes website

[sbueringer]

But good point, maybe it's not too bad to use it with builtin types. You shouldn't try to set fields that might not exist on the Kubernetes versions you run against anyway. And Update is a bad idea in general anyway.

[camilamacedo86]

/hold

[guettli]

@camilamacedo86 What about adding this?

<aside class="warning">
<h1>Why Update is a bad idea</h1>

`client.Update(...)` sends the full object back to the API server. In controllers, this is usually a
poor fit because it can overwrite fields owned by other actors, increase conflict frequency, and
make version skew harder to debug.

Prefer targeted writes with `client.Patch(...)` and `client.Status().Patch(...)` so the reconciler
only changes the fields it owns.

This guidance is separate from strict field validation, but they work well together: patch-based
writes reduce the blast radius, and strict validation turns unknown-field warnings into hard errors.

</aside>

[alvaroaleman]

This is bad advice. Optimistic concurrency is there for a reason and it should not be recommended to disable it by default.

[guettli]

@alvaroaleman I am looking for a written guideline which helps me and other people new to controller development to understand when it is better to use Update/Patch/Apply. Afaik there is no such guide. Can you please help us? What is your recommendation?

[alvaroaleman]

There are no written guidelines which is why a PR like this exists in the first place. The correct answer to your question is "it depends". If you are looking for a single, never completely-wrong answer, Patch with optimistic concurrency is the safest option.