FR: Add `spec.values` to the HelmChart CRD for structured YAML support
aofei opened this issue · 10 comments
Introduction
Currently, the HelmChart CRD supports specifying Chart values via spec.valuesContent
as a raw string. While effective, this method lacks the ability to leverage YAML syntax checking when editing manifests with editors. This proposal suggests an enhancement to the HelmChart CRD to address this limitation.
The primary goal of this proposal is to introduce a new field, spec.values
, to the HelmChart CRD. This field will allow users to provide Chart values as a YAML document rather than a raw string. This enhancement aims to improve the user experience by enabling YAML syntax validation during the editing process.
Proposed Changes
1. Introduction of spec.values
Add a new field spec.values
to the HelmChart CRD. This field should accept a YAML document representing the Chart values.
2. Precedence rule
In cases where both spec.values
and spec.valuesContent
are provided, spec.valuesContent
will take precedence (just like spec.chart
and spec.chartContent
). This ensures backward compatibility and provides flexibility for users who prefer the raw string format.
3. Implementation
Perhaps introduce a new field Values
to the pkg/apis/helm.cattle.io/v1. HelmChartSpec struct:
import "encoding/json"
type HelmChartSpec struct {
// ...
Values *json.RawMessage `json:"values,omitempty"`
}
this method lacks the ability to leverage YAML syntax checking when editing manifests with editors
Values *json.RawMessage `json:"values,omitempty"`
How is this handled in the generated OpenAPI spec? What specific editors handle this field type, and how does that enhance the display and validation of the resource as a whole?
+1. This would be particularly useful when using Kustomize to generate/override complex host/cluster-specific configurations (it can merge YAML documents, but not strings containing YAML). The existing CRDs only cover up to two levels of YAML-level merging, at HelmChart.spec.valuesContent
and HelmChartConfig.spec.valuesContent
respectively.
One more thing: this should apply not just to helm.cattle.io/v1.HelmChart
but to HelmChartConfig
too.
Can Kustomize merge arbitrary yaml, or does it need the document to have an OpenAPI Spec? If it requires a spec to know how to handle merging, then just changing it to json.RawMessage
won't improve anything, as the values yaml does not have a fixed schema.
@brandond Kustomize can merge arbitrary YAML, provided it's YAML, and not a string. While having specs improves strategic merge for complex cases, it is in no way necessary and Kustomize will happily do without it.
How is this handled in the generated OpenAPI spec?
Yeah, I was wrong. json.RawMessage
will be represented as a string
type in the generated OpenAPI spec:
helmcharts.helm.cattle.io
spec:
versions:
- name: v1
schema:
openAPIV3Schema:
properties:
spec:
type: object
properties:
values:
nullable: true
type: string
Referring to this, it seems that HelmChartSpec.Values
should be of type runtime.RawExtension
. However, I don't know how to set x-kubernetes-preserve-unknown-fields: true
for it.
What specific editors handle this field type, and how does that enhance the display and validation of the resource as a whole?
Actually my motivation stemmed from a YAML syntax error in spec.valuesContent
that I overlooked, as raw string values lacked syntax checking. So all I want initially was basic YAML syntax checking and highlighting. But @tdemin came up with something more interesting.
I'm not sure there is an unstructured type that will get you exactly what you want. Can you provide an example of another customer resource definition that does what you want in your editor of choice?
Cert-manager has something similar. Their webhook Issuer
has a config
field that supports arbitrary YAML. But instead of using runtime.RawExtension
, it uses k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1.JSON
. (webhook Issuer
related code in cert-manager)
I believe this is what we want as x-kubernetes-preserve-unknown-fields: true
will be added automatically.
import apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
type HelmChartSpec struct {
// ...
Values *apiextensionsv1.JSON `json:"values,omitempty"`
}
spec:
versions:
- name: v1
schema:
openAPIV3Schema:
properties:
spec:
type: object
properties:
values:
nullable: true
type: object
x-kubernetes-preserve-unknown-fields: true
ArgoCD does it like that for their argoproj.io/v1alpha1.Application.spec.source.helm.valuesObject
(and a multitude of fields elsewhere):
valuesObject:
description: ValuesObject specifies Helm values
to be passed to helm template, defined as
a map. This takes precedence over Values.
type: object
x-kubernetes-preserve-unknown-fields: true
They seem to be using *runtime.RawExtension
:
type ApplicationSourceHelm struct {
...
// ValuesObject specifies Helm values to be passed to helm template, defined as a map. This takes precedence over Values.
// +kubebuilder:pruning:PreserveUnknownFields
ValuesObject *runtime.RawExtension `json:"valuesObject,omitempty" protobuf:"bytes,10,opt,name=valuesObject"`
}
From my testing, the biggest problem with runtime.RawExtension
is that it requires the +kubebuilder:pruning:PreserveUnknownFields
annotation to work, which is not supported by helm-controller.
So k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1.JSON
should be what we're looking for.
We use rancher/wrangler to generate the CRDs, and there is a way to get that annotation set on the OpenAPI spec for a field using tags. I will take a look at it when I have some time.