Repository: rancher/fleet
Pull request #3267 (2025-01-28 13:01:06)

Add `templateValues` to HelmApp, Bundle, etc
<!-- Specify the issue ID that this pull request is solving -->
Refers to #3266 
<!-- Make sure that the referenced issue provides steps to reproduce it -->

<!-- Describe the changes introduced by this pull request -->
Implement new field inside `Helm` options: `templateValues`, which performs templating of each individual value before converting data to yaml. This allows to use more complex template expressions then currently available to template string data, like creation of maps and lists.

<!--
  Please provide a unit, integration (`./integrationtests/`) or e2e (`./e2e/`) test if possible.
-->

Danil-Grigorev: With this change I was able to deploy helm chart with proper modifications:
```yaml

apiversion: fleet.cattle.io/v1alpha1
kind: HelmApp
metadata:
  finalizers:
  - fleet.cattle.io/helmapp-finalizer
  generation: 3
  name: sample1
  namespace: default
  resourceVersion: "23694"
  uid: 511580b5-733a-4304-9cbb-229448989a3a
spec:
  helm:
    chart: tigera-operator
    releaseName: projectcalico
    repo: https://docs.tigera.io/calico/charts
    templateValues:
      installation: |-
        cni:
          type: Calico
          ipam:
            type: HostLocal
        calicoNetwork:
          bgp: Disabled
          mtu: 1350
          ipPools:
            ${- range $cidr := .ClusterValues.Cluster.spec.clusterNetwork.pods.cidrBlocks }
            - cidr: "${ $cidr }"
              encapsulation: None
              natOutgoing: Enabled
              nodeSelector: all()${- end}
  ignore: {}
  insecureSkipTLSVerify: true
  targets:
  - clusterName: docker-demo
```
created correct `BundleDeployment`:
```yaml

apiversion: fleet.cattle.io/v1alpha1
kind: BundleDeployment
metadata:
  creationTimestamp: "2025-01-28T15:00:20Z"
  finalizers:
  - fleet.cattle.io/bundle-deployment-finalizer
  generation: 1
  labels:
    fleet.cattle.io/bundle-name: sample1
    fleet.cattle.io/bundle-namespace: default
    fleet.cattle.io/cluster: docker-demo
    fleet.cattle.io/cluster-namespace: default
    fleet.cattle.io/fleet-helm-name: sample1
    fleet.cattle.io/managed: "true"
  name: sample1
  namespace: cluster-default-docker-demo-9a1d57cc2ad8
  resourceVersion: "22820"
  uid: 37600a86-c9e4-4445-a757-5611c5d27ace
spec:
  deploymentID: :fb788a089566f35d87c7d4b2ed4a499a2d3e36315b44cfa9446f66996b6a7c82
  helmChartOptions:
    helmAppInsecureSkipTLSVerify: true
  options:
    helm:
      chart: tigera-operator
      releaseName: projectcalico
      repo: https://docs.tigera.io/calico/charts
      templateValues:
        installation: |-
          cni:
            type: Calico
            ipam:
              type: HostLocal
          calicoNetwork:
            bgp: Disabled
            mtu: 1350
            ipPools:
              ${- range $cidr := .ClusterValues.Cluster.spec.clusterNetwork.pods.cidrBlocks }
              - cidr: "${ $cidr }"
                encapsulation: None
                natOutgoing: Enabled
                nodeSelector: all()${- end}
      values:
        installation:
          calicoNetwork:
            bgp: Disabled
            ipPools:
            - cidr: 10.1.0.0/16
              encapsulation: None
              natOutgoing: Enabled
              nodeSelector: all()
            mtu: 1350
          cni:
            ipam:
              type: HostLocal
            type: Calico
      version: v3.29.1
    ignore: {}
  stagedDeploymentID: :fb788a089566f35d87c7d4b2ed4a499a2d3e36315b44cfa9446f66996b6a7c82
…
```
Danil-Grigorev: > I like the idea, but I wonder if we _really_ need both _values_ and `templateValues`; perhaps we could template each member of the existing `value` field before serialising it to YAML, WDYT?

I also thought about it, but that has the same problem as previous solution. As the value in the map is interface{}, what type it actually contains is unknown. Template processing requires string data, which is only achievable after serialization - so it has to go through yaml decoder. These template expressions do not render as valid yaml, so we can't do anything.

Even if the interface is initially checked to be string, and we can skip yaml pre-processing, it is unclear if the templated result needs to stay a sting, or it has to be a map/list. `templatedValues` makes an assumption that the string inside will contain raw data (map/list). So I made a field addition to make it distinct in order to avoid breaking changes in API.
manno: > ```yaml
> installation: |-
> ```

I think this is supposed to be `installation:`, like in the issue. Otherwise, within a string value, the old approach should work.
Danil-Grigorev: Having the value specified as
```yaml
values:
  installation: |-
```
Results in error on HelmApp, which is only due to invalid type passed into `installation`, a string:
```yaml
status:
  conditions:
  - lastUpdateTime: "2025-01-31T15:55:58Z"
    message: 'ErrApplied(1) [Bundle sample1: template: tigera-operator/templates/tigera-operator/02-tigera-operator.yaml:81:17:
      executing "tigera-operator/templates/tigera-operator/02-tigera-operator.yaml"
      at <.Values.installation.kubernetesProvider>: can''t evaluate field kubernetesProvider
      in type interface {}]'
    status: "False"
    type: Ready
```

If I instead switch to just `installation:` in the template and apply again, it is invalid YAML:
```bash
dgrigorev@localhost:~/Documents/work/cluster-api-addon-provider-fleet> kubectl apply -f helm.yaml 
error: error parsing helm.yaml: error converting YAML to JSON: yaml: line 21: mapping values are not allowed in this context
```

Then if I go to generated `BundleDeployment` and edit it so it contains no `|-` in it, the rollout succeeds. It also succeeds with correct configuration (note the `cidr` field):
```yaml
kind: Installation
metadata:
  annotations:
    meta.helm.sh/release-name: projectcalico
    meta.helm.sh/release-namespace: default
    objectset.rio.cattle.io/id: default-sample1-fleet-addon-agent
  creationTimestamp: "2025-01-31T16:04:01Z"
  finalizers:
  - operator.tigera.io/installation-controller
  - tigera.io/operator-cleanup
  - operator.tigera.io/apiserver-controller
  generation: 4
  labels:
    app.kubernetes.io/managed-by: Helm
    objectset.rio.cattle.io/hash: cf1aceee4a248c0997592ec3509a41cb53a2fece
  name: default
  resourceVersion: "2589"
  uid: 9022fd0d-ea88-4d04-ac1a-ebc322adfc98
spec:
  calicoNetwork:
    bgp: Disabled
    hostPorts: Enabled
    ipPools:
    - allowedUses:
      - Workload
      - Tunnel
      blockSize: 26
      cidr: 10.1.0.0/16
```

None of this is needed in updated `templateValues` implementation.


Patch:
diff --git a/charts/fleet-crd/templates/crds.yaml b/charts/fleet-crd/templates/crds.yaml
index a6d752fe..a15cb88d 100644
--- a/charts/fleet-crd/templates/crds.yaml
+++ b/charts/fleet-crd/templates/crds.yaml
@@ -349,6 +349,24 @@ spec:
                           description: TakeOwnership makes helm skip the check for
                             its own annotations
                           type: boolean
+                        templateValues:
+                          additionalProperties:
+                            type: string
+                          description: 'Template Values passed to Helm. It is possible
+                            to specify the keys and values
+
+                            as go template strings. Unlike .values, content of each
+                            key will be templated
+
+                            first, before serializing to yaml. This allows to template
+                            complex values,
+
+                            like ranges and maps.
+
+                            templateValues keys have precedence over values keys in
+                            case of conflict.'
+                          nullable: true
+                          type: object
                         timeoutSeconds:
                           description: TimeoutSeconds is the time to wait for Helm
                             operations.
@@ -695,6 +713,24 @@ spec:
                           description: TakeOwnership makes helm skip the check for
                             its own annotations
                           type: boolean
+                        templateValues:
+                          additionalProperties:
+                            type: string
+                          description: 'Template Values passed to Helm. It is possible
+                            to specify the keys and values
+
+                            as go template strings. Unlike .values, content of each
+                            key will be templated
+
+                            first, before serializing to yaml. This allows to template
+                            complex values,
+
+                            like ranges and maps.
+
+                            templateValues keys have precedence over values keys in
+                            case of conflict.'
+                          nullable: true
+                          type: object
                         timeoutSeconds:
                           description: TimeoutSeconds is the time to wait for Helm
                             operations.
@@ -1568,6 +1604,24 @@ spec:
                       description: TakeOwnership makes helm skip the check for its
                         own annotations
                       type: boolean
+                    templateValues:
+                      additionalProperties:
+                        type: string
+                      description: 'Template Values passed to Helm. It is possible
+                        to specify the keys and values
+
+                        as go template strings. Unlike .values, content of each key
+                        will be templated
+
+                        first, before serializing to yaml. This allows to template
+                        complex values,
+
+                        like ranges and maps.
+
+                        templateValues keys have precedence over values keys in case
+                        of conflict.'
+                      nullable: true
+                      type: object
                     timeoutSeconds:
                       description: TimeoutSeconds is the time to wait for Helm operations.
                       type: integer
@@ -2433,6 +2487,24 @@ spec:
                             description: TakeOwnership makes helm skip the check for
                               its own annotations
                             type: boolean
+                          templateValues:
+                            additionalProperties:
+                              type: string
+                            description: 'Template Values passed to Helm. It is possible
+                              to specify the keys and values
+
+                              as go template strings. Unlike .values, content of each
+                              key will be templated
+
+                              first, before serializing to yaml. This allows to template
+                              complex values,
+
+                              like ranges and maps.
+
+                              templateValues keys have precedence over values keys
+                              in case of conflict.'
+                            nullable: true
+                            type: object
                           timeoutSeconds:
                             description: TimeoutSeconds is the time to wait for Helm
                               operations.
@@ -7307,6 +7379,24 @@ spec:
                       description: TakeOwnership makes helm skip the check for its
                         own annotations
                       type: boolean
+                    templateValues:
+                      additionalProperties:
+                        type: string
+                      description: 'Template Values passed to Helm. It is possible
+                        to specify the keys and values
+
+                        as go template strings. Unlike .values, content of each key
+                        will be templated
+
+                        first, before serializing to yaml. This allows to template
+                        complex values,
+
+                        like ranges and maps.
+
+                        templateValues keys have precedence over values keys in case
+                        of conflict.'
+                      nullable: true
+                      type: object
                     timeoutSeconds:
                       description: TimeoutSeconds is the time to wait for Helm operations.
                       type: integer
@@ -8191,6 +8281,24 @@ spec:
                             description: TakeOwnership makes helm skip the check for
                               its own annotations
                             type: boolean
+                          templateValues:
+                            additionalProperties:
+                              type: string
+                            description: 'Template Values passed to Helm. It is possible
+                              to specify the keys and values
+
+                              as go template strings. Unlike .values, content of each
+                              key will be templated
+
+                              first, before serializing to yaml. This allows to template
+                              complex values,
+
+                              like ranges and maps.
+
+                              templateValues keys have precedence over values keys
+                              in case of conflict.'
+                            nullable: true
+                            type: object
                           timeoutSeconds:
                             description: TimeoutSeconds is the time to wait for Helm
                               operations.
diff --git a/internal/cmd/controller/options/calculate.go b/internal/cmd/controller/options/calculate.go
index 2f240cf3..3c182777 100644
--- a/internal/cmd/controller/options/calculate.go
+++ b/internal/cmd/controller/options/calculate.go
@@ -5,6 +5,7 @@ import (
 	"crypto/sha256"
 	"encoding/hex"
 	"encoding/json"
+	"maps"
 
 	fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
 	"github.com/rancher/wrangler/v3/pkg/data"
@@ -52,6 +53,11 @@ func Merge(base, custom fleet.BundleDeploymentOptions) fleet.BundleDeploymentOpt
 		} else if custom.Helm.Values != nil {
 			result.Helm.Values.Data = data.MergeMaps(result.Helm.Values.Data, custom.Helm.Values.Data)
 		}
+		if result.Helm.TemplateValues == nil {
+			result.Helm.TemplateValues = custom.Helm.TemplateValues
+		} else if custom.Helm.TemplateValues != nil {
+			maps.Copy(result.Helm.TemplateValues, custom.Helm.TemplateValues)
+		}
 		if custom.Helm.ValuesFrom != nil {
 			result.Helm.ValuesFrom = append(result.Helm.ValuesFrom, custom.Helm.ValuesFrom...)
 		}
diff --git a/internal/cmd/controller/target/builder.go b/internal/cmd/controller/target/builder.go
index 27f52980..27e8e98a 100644
--- a/internal/cmd/controller/target/builder.go
+++ b/internal/cmd/controller/target/builder.go
@@ -2,8 +2,10 @@ package target
 
 import (
 	"bytes"
+	"cmp"
 	"context"
 	"fmt"
+	"maps"
 	"sort"
 	"strings"
 	"text/template"
@@ -182,10 +184,15 @@ func preprocessHelmValues(logger logr.Logger, opts *fleet.BundleDeploymentOption
 	}
 
 	opts.Helm = opts.Helm.DeepCopy()
-	if opts.Helm.Values == nil || opts.Helm.Values.Data == nil {
-		opts.Helm.Values = &fleet.GenericMap{
-			Data: map[string]interface{}{},
-		}
+	opts.Helm.Values = cmp.Or(opts.Helm.Values, &fleet.GenericMap{
+		Data: map[string]interface{}{},
+	})
+
+	if opts.Helm.Values.Data == nil {
+		opts.Helm.Values.Data = map[string]interface{}{}
+	}
+
+	if opts.Helm.TemplateValues == nil && len(opts.Helm.Values.Data) == 0 {
 		return nil
 	}
 
@@ -211,6 +218,14 @@ func preprocessHelmValues(logger logr.Logger, opts *fleet.BundleDeploymentOption
 		if err != nil {
 			return err
 		}
+
+		templatedData, err := processTemplateValuesData(opts.Helm.TemplateValues, values)
+		if err != nil {
+			return err
+		}
+
+		maps.Copy(opts.Helm.Values.Data, templatedData)
+
 		logger.V(4).Info("preProcess completed", "releaseName", opts.Helm.ReleaseName)
 	}
 
@@ -278,6 +293,38 @@ func tplFuncMap() template.FuncMap {
 	return f
 }
 
+func processTemplateValuesData(helmTemplateData map[string]string, templateContext map[string]interface{}) (map[string]interface{}, error) {
+	renderedValues := make(map[string]interface{}, len(helmTemplateData))
+
+	for k, v := range helmTemplateData {
+		// fleet.yaml must be valid yaml, however '{}[]' are YAML control
+		// characters and will be interpreted as JSON data structures. This
+		// causes issues when parsing the fleet.yaml so we change the delims
+		// for templating to '${ }'
+		tmpl := template.New("values").Funcs(tplFuncMap()).Option("missingkey=error").Delims("${", "}")
+		tmpl, err := tmpl.Parse(v)
+		if err != nil {
+			return nil, fmt.Errorf("failed to parse helm values template: %w", err)
+		}
+
+		var b bytes.Buffer
+		err = tmpl.Execute(&b, templateContext)
+		if err != nil {
+			return nil, fmt.Errorf("failed to render helm values template: %w", err)
+		}
+
+		var value interface{}
+		err = kyaml.Unmarshal(b.Bytes(), &value)
+		if err != nil {
+			return nil, fmt.Errorf("failed to interpret rendered template as helm values: %s, %v", b.String(), err)
+		}
+
+		renderedValues[k] = value
+	}
+
+	return renderedValues, nil
+}
+
 func processTemplateValues(helmValues map[string]interface{}, templateContext map[string]interface{}) (map[string]interface{}, error) {
 	data, err := kyaml.Marshal(helmValues)
 	if err != nil {
diff --git a/pkg/apis/fleet.cattle.io/v1alpha1/bundledeployment_types.go b/pkg/apis/fleet.cattle.io/v1alpha1/bundledeployment_types.go
index ecfb9f28..26af4d07 100644
--- a/pkg/apis/fleet.cattle.io/v1alpha1/bundledeployment_types.go
+++ b/pkg/apis/fleet.cattle.io/v1alpha1/bundledeployment_types.go
@@ -211,6 +211,14 @@ type HelmOptions struct {
 	// +kubebuilder:validation:XPreserveUnknownFields
 	Values *GenericMap `json:"values,omitempty"`
 
+	// Template Values passed to Helm. It is possible to specify the keys and values
+	// as go template strings. Unlike .values, content of each key will be templated
+	// first, before serializing to yaml. This allows to template complex values,
+	// like ranges and maps.
+	// templateValues keys have precedence over values keys in case of conflict.
+	// +nullable
+	TemplateValues map[string]string `json:"templateValues,omitempty"`
+
 	// +nullable
 	// ValuesFrom loads the values from configmaps and secrets.
 	ValuesFrom []ValuesFrom `json:"valuesFrom,omitempty"`
diff --git a/pkg/apis/fleet.cattle.io/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/fleet.cattle.io/v1alpha1/zz_generated.deepcopy.go
index 78dd5f62..d78d397a 100644
--- a/pkg/apis/fleet.cattle.io/v1alpha1/zz_generated.deepcopy.go
+++ b/pkg/apis/fleet.cattle.io/v1alpha1/zz_generated.deepcopy.go
@@ -1681,6 +1681,13 @@ func (in *HelmOptions) DeepCopyInto(out *HelmOptions) {
 		in, out := &in.Values, &out.Values
 		*out = (*in).DeepCopy()
 	}
+	if in.TemplateValues != nil {
+		in, out := &in.TemplateValues, &out.TemplateValues
+		*out = make(map[string]string, len(*in))
+		for key, val := range *in {
+			(*out)[key] = val
+		}
+	}
 	if in.ValuesFrom != nil {
 		in, out := &in.ValuesFrom, &out.ValuesFrom
 		*out = make([]ValuesFrom, len(*in))


Test patch:
diff --git a/internal/cmd/controller/target/target_test.go b/internal/cmd/controller/target/target_test.go
index da084c78..6ea1258e 100644
--- a/internal/cmd/controller/target/target_test.go
+++ b/internal/cmd/controller/target/target_test.go
@@ -112,6 +112,17 @@ func TestProcessLabelValues(t *testing.T) {
 const bundleYamlWithTemplate = `namespace: default
 helm:
   releaseName: labels
+  templateValues:
+    mapData: |
+      ${- range $key := .ClusterValues.items }
+      "${ $key }":
+        nested: "true"
+      ${- end}
+    listData: |
+      ${- range $key := .ClusterValues.items }
+      - "${ $key }":
+        nested: "true"
+      ${- end}
   values:
     clusterName: "${ .ClusterLabels.name }"
     fromAnnotation: "${ .ClusterAnnotations.testAnnotation }"
@@ -154,6 +165,10 @@ func TestProcessTemplateValues(t *testing.T) {
 				"thirdTier": "bar",
 			},
 		},
+		"items": []string{
+			"one",
+			"two",
+		},
 		"list": []string{
 			"alpha",
 			"beta",
@@ -187,7 +202,7 @@ func TestProcessTemplateValues(t *testing.T) {
 
 	templatedValues, err := processTemplateValues(bundle.Helm.Values.Data, values)
 	if err != nil {
-		t.Fatalf("error during label processing %v", err)
+		t.Fatalf("error during templated values processing %v", err)
 	}
 
 	clusterName, ok := templatedValues["clusterName"]
@@ -314,6 +329,70 @@ func TestProcessTemplateValues(t *testing.T) {
 		t.Fatal("join func was not right")
 	}
 
+	templatedValuesData, err := processTemplateValuesData(bundle.Helm.TemplateValues, values)
+	if err != nil {
+		t.Fatalf("error during templated values processing %v", err)
+	}
+
+	mapData, ok := templatedValuesData["mapData"].(map[string]interface{})
+	if !ok {
+		t.Fatal("mapData not found")
+	}
+
+	one, ok := mapData["one"]
+	if !ok {
+		t.Fatal("unable to find key one")
+	}
+
+	oneData, ok := one.(map[string]interface{})
+	if !ok {
+		t.Fatal("one key was not right")
+	}
+
+	if oneData["nested"].(string) != "true" {
+		t.Fatal("one value was not right")
+	}
+
+	two, ok := mapData["two"]
+	if !ok {
+		t.Fatal("unable to find key two")
+	}
+
+	twoData, ok := two.(map[string]interface{})
+	if !ok {
+		t.Fatal("two key was not right")
+	}
+
+	if twoData["nested"].(string) != "true" {
+		t.Fatal("two value was not right")
+	}
+
+	listData, ok := templatedValuesData["listData"].([]interface{})
+	if !ok {
+		t.Fatal("listData not found")
+	}
+
+	if len(listData) != 2 {
+		t.Fatal("unable to find all listData keys")
+	}
+
+	oneListData, ok := listData[0].(map[string]interface{})
+	if !ok {
+		t.Fatal("oneListData key is not right")
+	}
+
+	if oneListData["nested"] != "true" {
+		t.Fatal("oneListData item is missing")
+	}
+
+	twoListData, ok := listData[1].(map[string]interface{})
+	if !ok {
+		t.Fatal("twoListData key is not right")
+	}
+
+	if twoListData["nested"] != "true" {
+		t.Fatal("twoListData item is missing")
+	}
 }
 
 const clusterYamlWithTemplateValues = `apiVersion: fleet.cattle.io/v1alpha1
@@ -393,7 +472,7 @@ func TestDisablePreProcessFlagEnabled(t *testing.T) {
 			t.Fatalf("key %s not found", testCase.Key)
 		} else {
 			if field != testCase.ExpectedValue {
-				t.Fatalf("key %s was not the expected value. Expected: '%s' Actual: '%s'", testCase.Key, field, testCase.ExpectedValue)
+				t.Fatalf("key %s was not the expected value. Expected: '%s' Actual: '%s'", testCase.Key, testCase.ExpectedValue, field)
 			}
 		}
 
@@ -405,8 +484,11 @@ const bundleYamlWithDisablePreProcessDisabled = `namespace: default
 helm:
   disablePreprocess: false
   releaseName: labels
+  templateValues:
+    overridden: "something_templated"
   values:
     clusterName: "${ .ClusterName }"
+    overridden: ""
 `
 
 func TestDisablePreProcessFlagDisabled(t *testing.T) {
@@ -433,6 +515,17 @@ func TestDisablePreProcessFlagDisabled(t *testing.T) {
 		}
 	}
 
+	key = "overridden"
+	expectedValue = "something_templated"
+
+	if field, ok := valuesObj[key]; !ok {
+		t.Fatalf("key %s not found", key)
+	} else {
+		if field != expectedValue {
+			t.Fatalf("key %s was not the expected value. Expected: '%s' Actual: '%s'", key, field, expectedValue)
+		}
+	}
+
 }
 
 const bundleYamlWithDisablePreProcessMissing = `namespace: default