FieldValidation is missing from ApplyOptions

[erikgb]

In order to migrate away from the deprecated Apply patch, which we have been using to perform SSA requests with the controller-runtime client, we need the FieldValidation option on ApplyOptions. Is there any reason why it's missing, or is it just an oversight?

controller-runtime/pkg/client/options.go

Lines 768 to 784 in fc84a60

	// fieldValidation instructs the server on how to handle
	// objects in the request (POST/PUT/PATCH) containing unknown
	// or duplicate fields. Valid values are:
	// - Ignore: This will ignore any unknown fields that are silently
	// dropped from the object, and will ignore all but the last duplicate
	// field that the decoder encounters. This is the default behavior
	// prior to v1.23.
	// - Warn: This will send a warning via the standard warning response
	// header for each unknown field that is dropped from the object, and
	// for each duplicate field that is encountered. The request will
	// still succeed if there are no other errors, and will only persist
	// the last of any duplicate fields. This is the default in v1.23+
	// - Strict: This will fail the request with a BadRequest error if
	// any unknown fields would be dropped from the object, or if any
	// duplicate fields are present. The error returned from the server
	// will contain all unknown and duplicate fields encountered.
	FieldValidation string

This came up when I asked if Flux could set fieldValidation to Strict in its SSA requests, ref. fluxcd/pkg#1016.

[sbueringer]

Could it be that SSA just always uses strict?

[sbueringer]

Remembered this one

Just that I got it right. FieldValidation for other funcs can be set to Ignore, Warn, Strict. Upstream ApplyOptions don't have a FieldValidation field and it always behaves like Strict, correct?

Correct

#3253 (comment)

[erikgb]

This seems strange to me. Isn't the new client.Apply() equivalent to client.Patch() with type Apply?

[sbueringer]

IIRC, it should be. Was FieldValidation used there? i.e. what happened if client.Patch was called with FieldValidation set?

Here is the upstream metav1.ApplyOptions struct: https://github.com/kubernetes/kubernetes/blob/15ca38b521d94abfac00fcf57031fcaa28b8d7df/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go#L676-L698

[erikgb]

Maybe it was an oversight upstream? 🤔 We've been using it with our Patch calls for SSA requests, and it seems to work as expected: defaults to Warn and is possible to set to Strict (which we have done).

[erikgb]

No, reading the thread you referred to more closely, it seems to be always Strict when using Apply. I wonder if this is documented anywhere upstream?

[sbueringer]

We've been using it with our Patch calls for SSA requests, and it seems to work as expected: defaults to Warn and is possible to set to Strict (which we have done).

And you also got the corresponding warnings / validation errors from the apiserver accordingly, right?

(let's first try to figure out if the kube-apiserver actually supports it)

[alvaroaleman]

I was also wondering if this might be an oversight when I worked on this so I tried to manually set it to something other than strict in the patch request but that just gets ignored.

I think fundamentally the issue is that SSA will error out when given fields that do not exist in the schema of the object. To get around that, one would have to explicitly filter these out first which I guess no one bothered to implement. This is different from the proto/json/yaml case, where the deserializer just drops unknown fields by default without erroring, so there the strictness required explicit implementation.

It is also questionable if not being strict is a good idea in general and in particular with SSA, as there the meaning of the request really is "Set all these fields to those values".

[alvaroaleman]

That being said, I can see that this is confusing, maybe we can godoc this somewhere?

[erikgb]

I think fundamentally the issue is that SSA will error out when given fields that do not exist in the schema of the object. To get around that, one would have to explicitly filter these out first which I guess no one bothered to implement.

I think Flux has the logic to do this now, or @stefanprodan? This all started with https://cloud-native.slack.com/archives/CLAJ40HV3/p1756734262631239:

apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingAdmissionPolicyBinding
metadata:
  name: sane-prometheus-resources
spec:
  policyName: sane-prometheus-resources
  validationActions: [Warn, Audit]
  matchResources:
    matchExpressions:
      - key: openshift.io/cluster-monitoring
        operator: NotIn
        values:
          - "true"

Which is invalid/wrong, resulting in

kind: ValidatingAdmissionPolicyBinding
apiVersion: admissionregistration.k8s.io/v1
metadata:
  name: sane-prometheus-resources
  uid: 0e6ae234-cdc0-445a-8d04-ede2851a7e6f
  resourceVersion: '1919045421'
  generation: 2
  creationTimestamp: '2025-08-28T12:40:29Z'
  labels:
    kustomize.toolkit.fluxcd.io/name: validating-admission-policies
    kustomize.toolkit.fluxcd.io/namespace: flux-system
  annotations:
    config.kubernetes.io/origin: |
      path: sane-prometheus-resources/policy-binding.yamlspec:
  policyName: sane-prometheus-resources
  matchResources:
    namespaceSelector: {}
    objectSelector: {}
    matchPolicy: Equivalent
  validationActions:
    - Warn
    - Audit

in the cluster when applied by Flux using SSA.

[stefanprodan]

In Flux we drop unknown fields and we also set some of the defaults to avoid lots of issues we had with SSA over the years. This only happens for the native kinds.

[alvaroaleman]

I think Flux has the logic to do this now, or @stefanprodan? This all started with https://cloud-native.slack.com/archives/CLAJ40HV3/p1756734262631239:

Sorry, I don't have an account in that slack. If I try to apply this through controller-runtimes apply, I get the same error as with kubectl SSA:

$ cat /private/tmp/ap.yaml
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingAdmissionPolicyBinding
metadata:
  name: sane-prometheus-resources
spec:
  policyName: sane-prometheus-resources
  validationActions: [Warn, Audit]
  matchResources:
    matchExpressions:
      - key: openshift.io/cluster-monitoring
        operator: NotIn
        values:
          - "true"

$ cat main.go
package main

import (
        "context"
        "os"

        "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
        ctrl "sigs.k8s.io/controller-runtime"
        "sigs.k8s.io/controller-runtime/pkg/client"
        "sigs.k8s.io/yaml"
)

func main() {
        c, err := client.New(ctrl.GetConfigOrDie(), client.Options{})
        if err != nil {
                panic(err)
        }

        raw, err := os.ReadFile("/private/tmp/ap.yaml")
        if err != nil {
                panic(err)
        }

        u := &unstructured.Unstructured{}
        if err := yaml.Unmarshal(raw, u); err != nil {
                panic(err)
        }

        if err := c.Apply(context.Background(), client.ApplyConfigurationFromUnstructured(u), client.FieldOwner("foo")); err != nil {
                panic(err)
        }
}

$ go run ./
panic: failed to create typed patch object (/sane-prometheus-resources; admissionregistration.k8s.io/v1, Kind=ValidatingAdmissionPolicyBinding): .spec.matchResources.matchExpressions: field not declared in schema

goroutine 1 [running]:
main.main()
        /var/folders/9m/9c0mfmhj6377zwcl5t54xbqc0000gn/T/tmp.3xL8019AK1/main.go:30 +0x160
exit status 2

Which all seems expected to me. Could you link to the relevant code in flux and what it does?

[stefanprodan]

We have this normalize pkg, that uses the DefaultUnstructuredConverter, then for some of native kinds we also set defaults, like port protocol.

[alvaroaleman]

We have this normalize pkg, that uses the DefaultUnstructuredConverter, then for some of native kinds we also set defaults, like port protocol.

That makes it sound like these fields probably get dropped there rather than in controller-runtime? My guess would be that that happens here: https://github.com/fluxcd/pkg/blob/a5e02cec033a1cfc050231a1d00207350a71b386/ssa/normalize/normalize.go#L48

[erikgb]

Should we just close this issue as not an issue? Thanks for all your feedback here! I learned a lot!