-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Description
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.