Cannot run patchesStrategicMerge on CRDs schema under Versions array

[gaelgatelement]

What happened?

When trying to use patchesStrategicMerge on CRD schema under spec/versions array, it fails and replace the whole array because it is missing a mergeKey and patchStrategy on Versions.

patchesStrategicMerge used to work with spec.version and spec.validation fields but those are now deprecated.

What did you expect to happen?

patchesStrategicMerge should be usable with new versions array.

How can we reproduce it (as minimally and precisely as possible)?

base.yaml :

---
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  name: foo.bug.local
spec:
  group: bug.local
  names:
    kind: Foo
    listKind: FooList
    plural: foos
    singular: foo
  scope: Namespaced
  versions:
  - name: v1alpha1
    schema:
      openAPIV3Schema:
        description: Foo is the Schema for the foos API
        properties:
          apiVersion:
            description: 'APIVersion defines the versioned schema of this representation
              of an object. Servers should convert recognized schemas to the latest
              internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
            type: string
          kind:
            description: 'Kind is a string value representing the REST resource this
              object represents. Servers may infer this from the endpoint the client
              submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
            type: string
          metadata:
            type: object
          spec:
            description: Spec defines the desired state of Foo
            type: object
            properties:
              bar:
                description: This should be present in the result
                type: object
              k8s:
                description: This field will be replaced by kustomize

kustomization.yaml :

resources:
- ./base.yaml

patchesStrategicMerge:
- |-
  apiVersion: apiextensions.k8s.io/v1
  kind: CustomResourceDefinition
  metadata:
    name: foo.bug.local
  spec:
    versions:
    - name: v1alpha1
      schema:
        openAPIV3Schema:
          properties:
            spec:
                k8s:
                  description: This should be present in the result
                  type: string

bar is missing from result : kustomize build bug/

apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  name: foo.bug.local
spec:
  group: bug.local
  names:
    kind: Foo
    listKind: FooList
    plural: foos
    singular: foo
  scope: Namespaced
  versions:
  - name: v1alpha1
    schema:
      openAPIV3Schema:
        properties:
          spec:
            k8s:
              description: This should be present in the result
              type: string

Anything else we need to know?

This might be a breaking change, if people relies on patchesStrategicMerge to replace their CRDs versions content instead of adding/merging.

Kubernetes version

All versions supporting apiextensions.k8s.io/v1.

Cloud provider

--

OS version

No response

Install tools

No response

Container runtime (CRI) and version (if applicable)

No response

Related plugins (CNI, CSI, ...) and versions (if applicable)

No response

[gaelgatelement]

/sig api-machinery

[k8s-ci-robot]

@gaelgatelement: The label(s) wg/api-machinery, committee/api-machinery cannot be applied, because the repository doesn't have them.

In response to [this](https://github.com/kubernetes/kubernetes/issues/113223#issuecomment-1286059728): >/sig api-machinery > >/wg api-machinery >/committee api-machinery > > Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.

[gaelgatelement]

/wg api-expression

[gaelgatelement]

Let me know, if this issue is sensible enough, if I should go down writing a KEP about this API change.

[liggitt]

I don't think it's a breaking change, since it did not change behavior for the v1beta1 endpoint. I would expect a single actor to define the versions for the CRD, since they also have to define the conversion between them. Independent ownership of different versions or of parts of the schema don't seem like a good thing to encourage.

[gaelgatelement]

I don't think it's a breaking change, since it did not change behavior for the v1beta1 endpoint. I would expect a single actor to define the versions for the CRD, since they also have to define the conversion between them. Independent ownership of different versions or of parts of the schema don't seem like a good thing to encourage.

I understand your reluctance to this change. It's indeed a good point that partial ownership would become possible.

We are actually using this schema change locally in our repository to be able to build complex schemas in multiple kustomize steps before pushing it to the kubernetes API. I think this is a valid usecase, but I see that there would have unwanted side-effects if we pushed it to the main kubernetes schema aswell.

I feel like this possibility will have to stay local to our buildchain ?

[liggitt]

If you're applying patches locally to construct a schema that is written to the server by a single actor, I'd suggest using a different patch type than strategic merge patch. You can get very fine-grained control of where and how the patch is applied with the json-patch type (https://kubectl.docs.kubernetes.io/references/kustomize/builtins/#_patchesjson6902_)

[gaelgatelement]

Yes, we have been using this as well, but here we are trying to include metadata in every field of our CRD.

To be more precise, we are trying to add openapi specifications extensions which are not supported by k8s yet. So we build 2 distincts schema file : first the CRD which will be injected into k8s, and then we attach some x-ui-type field everywhere in the tree using a strategic merge patch.

Using the Json patch, it would be very verbose to do so, while with the strategic merge patch the patch is readable and easy to manage.

[alexzielenski]

/triage accepted

thanks jordan

[k8s-triage-robot]

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

• Confirm that this issue is still relevant with /triage accepted (org members only)
• Close this issue with /close

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

[seans3]

/triage accepted

[liggitt]

/close

as working as intended