Strategic merge support for CRDs

[prydonius]

I'm building a simple CRD that contains a PodTemplate under spec.template and I found that, unlike a Deployment which has the same spec, the strategic merge isn't handled correctly by Kustomize, leading to overwritten fields.

For example:

given the following base:

apiVersion: example.com/v1alpha1
kind: MyCRD
metadata:
  name: service
spec:
  template:
    spec:
      terminationGracePeriodSeconds: 20
      containers:
      - name: server
        image: server
        command: example
        ports:
        - name: grpc
          protocol: TCP
          containerPort: 8080
        envFrom:
        - configMapRef:
            name: configmap
        - secretRef:
            name: secret

applying the following overlay:

apiVersion: example.com/v1alpha1
kind: MyCRD
metadata:
  name: service
spec:
  template:
    spec:
      containers:
      - name: server
         image: nginx

the resulting manifest is:

apiVersion: example.com/v1alpha1
kind: MyCRD
metadata:
  name: service
spec:
  template:
    spec:
      containers:
      - image: nginx
        name: server
      terminationGracePeriodSeconds: 20

The spec.template.spec.containers array gets entirely replaced, rather than merged based on the name field value (such as for a Deployment).

I found that the issue here is that Kustomize does not have the schema for my CRD and so does not have the patch strategy and merge keys defined for it to be able to perform the correct strategic merge. If I fork Kustomize and call AddSchemaFromFileUsingField with the appropriate schema (in this case, pointing to the original PodTemplateSpec definition), I'm able to get Kustomize to correctly overlay my patch:

{
  "definitions": {
    "v1alpha1.MyCRD": {
      "description": "MyCRD is the Schema for the mycrd 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": "MyCRDSpec defines the desired state of MyCRD",
          "properties": {
            "template": {
              "$ref": "#/definitions/io.k8s.api.core.v1.PodTemplateSpec",
              "description": "Template describes the pods that will be created."
            }
          },
          "required": [
            "template"
          ],
          "type": "object"
        },
        "status": {
          "description": "MyCRDStatus defines the observed state of MyCRD",
          "properties": {
            "success": {
              "type": "boolean"
            }
          },
          "type": "object"
        }
      },
      "type": "object",
      "x-kubernetes-group-version-kind": [
        {
          "group": "example.com",
          "kind": "MyCRD",
          "version": "v1alpha1"
        }
      ]
    }
  }
}

result:

apiVersion: example.com/v1alpha1
kind: MyCRD
metadata:
  name: service
spec:
  template:
    spec:
      terminationGracePeriodSeconds: 20
      containers:
      - name: server
        image: nginx
        command: example
        ports:
        - name: grpc
          protocol: TCP
          containerPort: 8080
        envFrom:
        - configMapRef:
            name: configmap
        - secretRef:
            name: secret

Proposal

It would be great if there was a way to load in my CRD's schema with patch strategy and merge key information, so that Kustomize can apply patches as intended. There is an existing crds field in the Kustomize spec which appears to be for this use case, but as far as I can tell it's only used to create a TransformerConfig. Could we use this field for both the TransformerConfig and loading the OpenAPI schema for strategic merge?

A slight issue is that the structure that LoadConfigFromCRDs and AddSchemaFromFileUsingField has slightly different nesting, though the underlying schema is the same OpenAPI format (see this example for the TransformerConfig). We would need to choose one of these formats and load both the TransformerConfig and add the OpenAPI schema from the chosen format. Additionally, I've seen that some people have asked if the crds field could load the full CRD definition and the OpenAPI schema from the validation field. Perhaps changing it to load this would make the most sense?

I'd be happy to submit a PR to do this, if we think this is the correct approach.

[prydonius]

possible dupe of #1510

[pwittrock]

cc @eddiezane could we do this during the bug scrub?

[prydonius]

We're currently hacking around this by loading the CRDs schemas directly (our CRD schemas are in the format that AddSchemaFromFileUsingField expects): prydonius@7610688

[eddiezane]

/assign @monopole

[pwittrock]

This should be resolved as part of the migration to kyaml. Users will be able to override the merge keys on a per-field basis.

[prydonius]

@pwittrock that's great, although it would also be nice to just inherit merge keys from existing types (e.g. PodSpec) somehow. Are there any docs on how to set the merge keys with kyaml?

[Shell32-Natsu]

@monopole @natasha41575 This issue is related to #3111. Users may want to load their own schema.

[serhatcetinkaya]

@monopole In case you haven't started working on the issue, I'd like to work on it and hopefully submit a PR. Also, I might need some initial guidance regarding how to implement this.

[natasha41575]

@serhatcetinkaya I have a proposal for this and am working on an implementation, but I'm happy to hear your thoughts about it.

[natasha41575]

After some discussion with @monopole, the SIG-CLI group and others, we have created a KEP to allow users to specify their own custom schema files.

See kubernetes/enhancements#2206 and https://github.com/kubernetes/enhancements/tree/master/keps/sig-cli/2206-openapi-features-in-kustomize.

When this is implemented you will be able to fetch the openapi schema from your kubernetes cluster, change the merge keys, and use your custom schema via an openapi/path field in the kustomization file. Hopefully that is enough to resolve this issue for now.

[natasha41575]

You can now specify your own custom OpenAPI schema file to use with kustomize via an openapi field in the kustomization file, the example here is for your exact use case: https://github.com/kubernetes-sigs/kustomize/blob/master/examples/customOpenAPIschema.md.

You can also now fetch the OpenAPI schema from your kubernetes cluster with the command kustomize openapi fetch.

Closing this issue.