Sprig Templating for Helm Values with Inputs from Cluster Resource #671

rajiteh · 2021-12-04T01:01:38Z

This PR introduces the ability for fleet.yaml's helm.values object to contain go template strings in either the keys or values. The context of the go template also includes the cluster labels (for convenient migration from the existing global.fleet.clusterLabels.FOO users.

Cluster specific template variables can be added to Cluster CR under a new optional key in spec.templateContext

Related issues/PRs

Allow helm values overrides to inject cluster labels via go templates #507 but adds the per-cluster custom template context
Global labels inside string #375
would it be possible to concatenate label value with global labels now? #355

gregsidelinger · 2022-02-12T16:57:19Z

pkg/target/target.go

+		return nil, err
+	}
+
+	tpl, err := template.New("values").Funcs(tplFuncMap()).Option("missingkey=error").Parse(string(valuesMapStr))


So I'm getting an error from the test suite when I started adding some longer lines to the test suite for variable substitution.

Here is the tested bundleYamlWithTemplate I was playing around with.

const bundleYamlWithTemplate = `namespace: default helm: releaseName: labels values: clusterName: "{{ .ClusterLabels.name }}" fromAnnotation: "{{ .ClusterAnnotations.testAnnotation }}" clusterNamespace: "{{ .ClusterNamespace }}" fleetClusterName: "{{ .ClusterName }}" workingLongClusterName: kubernets.io/cluster/{{ index .ClusterLabels "p-w" }} brokenLongClusterName: kubernets.io/cluster/{{ index .ClusterLabels "not-working-for-some-reason" }} customStruct: - name: "{{ .Values.topLevel }}" key1: value1 key2: value2 - element2: "{{ .Values.nested.secondTier.thirdTier }}" - "element3_{{ .ClusterLabels.envType }}": "{{ .ClusterLabels.name }}" funcs: upper: "{{ .Values.topLevel | upper }}_test" join: '{{ .Values.list | join "," }}' diff: comparePatches: - apiVersion: networking.k8s.io/v1 kind: Ingress name: labels-fleetlabelsdemo namespace: default operations: - op: remove path: /spec/rules/0/host `

And here is the error I'm getting from my brokenLongClusterName: kubernets.io/cluster/{{ index .ClusterLabels "not-working-for-some-reason" }} line.

--- FAIL: TestProcessTemplateValues (0.00s) /home/gate/src/fleet/pkg/target/target_test.go:179: error during label processing template: values:1: unclosed action FAIL

I've done a bit of digging and it appears to have something to do with the k8syam.Marshal conversion to a string. New lines are getting inserted into the line. So brokenLongClusterName: kubernets.io/cluster/{{ index .ClusterLabels "not-working-for-some-reason" }} will end up like brokenLongClusterName: kubernets.io/cluster/{{ index .ClusterLabels "not-working-for-some-reason"\n }}. I have not been able to explain why the new lines are getting inserted.

kubernetes-sigs/yaml#68 has details about what is happening with lines over 80 chars. I can confirm using go-yaml v3 instead of the k8s sigs yaml library resolves the issue.

That's an interesting catch. I will update the library and see if that will fix it. However, fundamentally, this issue is raised by the fact that I serialize the entire document and re-parse it as template string. Looking back at it, I can't help it but feel a little bit unhappy about this approach because a carefully crafted template variable could fundamentally change the yaml structure of the templated output. Perhaps a better approach would be to iterate on each individual key values and template them as strings instead. WDYT?

I really liked the idea of treating all of the values as a big template so you can start to do some fun if/then logic and not try to cram it all on one line. But this 80 char line break in the yaml libraries breaks that. My current use case is better variable substitute and not doing crazy logic inside the values so I'm happy with each line getting process separately like I'm doing in #507.

Indeed. That's what convinced me to consider it. However, an alternative approach would be to define a dedicated field in fleet.yaml like templatedValues: and let the user define it as a multi-line string. This way it is implied that the entire structure is templated as a whole.

Interestingly enough, I couldn't reproduce the new line behaviour on my local system (go 1.17.6) -- but I switched to goyamlv3 just to be safe. I also refactored the templating function to be a lot more "structure" safe, as in only templating within the context of a specific string rather than the entire object.

rajiteh · 2022-03-11T18:54:38Z

@gregsidelinger in case you're interested I am planning to propose an enhancement on top of this templating system to introduce secret management via vals. I have a PR here with a basic implementation, docs pending. IMO this gives an elegant way to template and as well as resolve secrets per release/cluster basis.

https://github.com/rajiteh/fleet/pull/3/files

prachidamle · 2022-03-25T01:13:41Z

docs/gitrepo-structure.md

      key: values.yaml
    secretKeyRef:
      name: secret-values
      namespace: default
      key: values.yaml
  # Override immutable resources. This could be dangerous.
  force: false
+  # Disable go template pre-prosessing on the fleet values


nit: typo pre-processing

pkg/target/target.go

pkg/target/target_test.go

prachidamle · 2022-03-25T06:44:18Z

@rajiteh I am not clear on how would the Cluster specific template variables be added to a Cluster CR under spec.templateContext? Is the idea that the cluster Admins need to add this data to the target clusters? I am wondering if this templateContext data added to the Fleet target cluster CRs will get overwritten when synced with the management.cattle.io.cluster objects in Rancher.

rajiteh · 2022-03-29T16:05:45Z

@rajiteh I am not clear on how would the Cluster specific template variables be added to a Cluster CR under spec.templateContext? Is the idea that the cluster Admins need to add this data to the target clusters? I am wondering if this templateContext data added to the Fleet target cluster CRs will get overwritten when synced with the management.cattle.io.cluster objects in Rancher.

Yes, the idea is that admins will add these values. I can't speak to Rancher use case unfortunately as I'm only using fleet standalone.

PS: I will address your feedback over the weekend!

gregsidelinger · 2022-03-29T16:34:10Z

I have not seen rancher touch the templateContext on the fleet cluster in my setup. Once set rancher left everything alone when it synced labels from the management cluster.

rajiteh · 2022-04-14T18:11:42Z

@prachidamle hello! sorry for the delay, please have a look when you get a chance. I believe I address all the requested feedback.

k0da · 2022-08-31T10:20:12Z

Hello, any updates?

manno · 2022-11-23T11:31:41Z

@rajiteh Let's get this merged. Do you want to rename the context variable and rebase this PR against current master, or should we try to schedule this?

rajiteh · 2022-11-25T19:10:44Z

@manno I made all the changes and fixed the build!

PS: Rebasing seems to be a little painful, we could squash the commits instead perhaps?

manno · 2022-11-29T09:48:18Z

@manno I made all the changes and fixed the build!

PS: Rebasing seems to be a little painful, we could squash the commits instead perhaps?

Yes, that would be great.

I also discussed this briefly with @mattfarina and we have some concerns regarding recursion depth. templateSubstitutions and processLabelValues should probably be limited to a depth of, I don't know, 50?
However, I don't think that has to happen in this PR.

pkg/target/target_test.go

thardeck · 2022-11-30T14:24:25Z

@rajiteh There is still a small linting issue: https://github.com/rancher/fleet/actions/runs/3576792840/jobs/6029393464

  Error: `getClusterAndBundle` - `clusterYaml` always receives `clusterYamlWithTemplateValues` (`"apiVersion: fleet.cattle.io/v1alpha1\nkind: Cluster\nmetadata:\n  na...`) (unparam)

manno · 2022-12-01T18:01:02Z

Thanks! 🎉

rajiteh · 2022-12-01T18:09:33Z

@manno the relevant documentation for this PR can be found here: rancher/fleet-docs#17

manno · 2022-12-02T11:32:10Z

Documentation is live under https://fleet.rancher.io/next/gitrepo-add

@thardeck created a new alpha release which includes the feature: https://github.com/rancher/fleet/releases/tag/v0.6.0-alpha4

rajiteh force-pushed the template_values branch from 61cb346 to 418dd05 Compare January 6, 2022 16:31

gregsidelinger reviewed Feb 12, 2022

View reviewed changes

gregsidelinger mentioned this pull request Feb 15, 2022

Allow helm values overrides to inject cluster labels via go templates #507

Closed

rajiteh force-pushed the template_values branch from ddb4e5e to b191284 Compare February 19, 2022 01:33

SheilaghM requested review from prachidamle, aiyengar2 and superseb February 23, 2022 19:41

rajiteh force-pushed the template_values branch from f4e269e to d2fd7ae Compare March 11, 2022 18:50

rajiteh mentioned this pull request Mar 14, 2022

Secrets with vals #526

Closed

prachidamle reviewed Mar 25, 2022

View reviewed changes

pkg/target/target.go Show resolved Hide resolved

prachidamle reviewed Mar 25, 2022

View reviewed changes

pkg/target/target_test.go Show resolved Hide resolved

superseb removed their request for review April 8, 2022 14:39

rajiteh force-pushed the template_values branch from 25168d5 to 1454c20 Compare April 11, 2022 18:53

MKlimuszka added this to the v2.6.6 milestone May 10, 2022

MKlimuszka added the area/fleet label May 10, 2022

mattfarina self-assigned this Jul 6, 2022

mattfarina added the [zube]: Review label Jul 6, 2022

zube bot added the team/fleet label Jul 26, 2022

dkeightley requested review from gregsidelinger and prachidamle and removed request for gregsidelinger October 6, 2022 04:16

dkeightley added the area/internal label Oct 17, 2022

rajiteh force-pushed the template_values branch 2 times, most recently from abe5228 to b531671 Compare November 29, 2022 18:06

rajiteh commented Nov 29, 2022

View reviewed changes

pkg/target/target_test.go Show resolved Hide resolved

manno previously approved these changes Nov 30, 2022

View reviewed changes

manno mentioned this pull request Nov 30, 2022

Limit recursion depth for cluster label replacement in values #1143

Merged

rajiteh dismissed manno’s stale review via 65a91b0 December 1, 2022 17:32

rajiteh force-pushed the template_values branch from b531671 to 65a91b0 Compare December 1, 2022 17:32

feat: templatable helm values from cluster

9ee1b90

rajiteh force-pushed the template_values branch from 65a91b0 to 9ee1b90 Compare December 1, 2022 17:35

manno approved these changes Dec 1, 2022

View reviewed changes

manno merged commit 20705b6 into rancher:master Dec 1, 2022

zube bot added [zube]: Done and removed [zube]: Review labels Dec 1, 2022

rajiteh deleted the template_values branch December 1, 2022 20:53

This was referenced Dec 2, 2022

would it be possible to concatenate label value with global labels now? #355

Closed

Global labels inside string #375

Closed

manno added the area/templating label Dec 7, 2022

This was referenced Dec 7, 2022

Cluster values full templating #1158

Merged

Templating Helm Values in fleet.yaml #1164

Closed

Fleet RBAC #385

Closed

zube bot removed the [zube]: Done label Mar 2, 2023

ngoeddel-openi mentioned this pull request Mar 2, 2023

global.fleet.clusterLabels not rendered in plain list items #921

Open

1 task

kkaempf mentioned this pull request Mar 10, 2023

Feature Request: variables in targetCustomizations? #1395

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sprig Templating for Helm Values with Inputs from Cluster Resource #671

Sprig Templating for Helm Values with Inputs from Cluster Resource #671

rajiteh commented Dec 4, 2021 •

edited by mattfarina

gregsidelinger Feb 12, 2022

gregsidelinger Feb 13, 2022

rajiteh Feb 16, 2022

gregsidelinger Feb 16, 2022

rajiteh Feb 16, 2022 •

edited

rajiteh Feb 19, 2022

rajiteh commented Mar 11, 2022

prachidamle Mar 25, 2022

rajiteh Apr 14, 2022

prachidamle commented Mar 25, 2022

rajiteh commented Mar 29, 2022

gregsidelinger commented Mar 29, 2022

rajiteh commented Apr 14, 2022 •

edited

k0da commented Aug 31, 2022

manno commented Nov 23, 2022

rajiteh commented Nov 25, 2022 •

edited

manno commented Nov 29, 2022 •

edited

thardeck commented Nov 30, 2022

manno commented Dec 1, 2022

rajiteh commented Dec 1, 2022

manno commented Dec 2, 2022

Sprig Templating for Helm Values with Inputs from Cluster Resource #671

Sprig Templating for Helm Values with Inputs from Cluster Resource #671

Conversation

rajiteh commented Dec 4, 2021 • edited by mattfarina

gregsidelinger Feb 12, 2022

Choose a reason for hiding this comment

gregsidelinger Feb 13, 2022

Choose a reason for hiding this comment

rajiteh Feb 16, 2022

Choose a reason for hiding this comment

gregsidelinger Feb 16, 2022

Choose a reason for hiding this comment

rajiteh Feb 16, 2022 • edited

Choose a reason for hiding this comment

rajiteh Feb 19, 2022

Choose a reason for hiding this comment

rajiteh commented Mar 11, 2022

prachidamle Mar 25, 2022

Choose a reason for hiding this comment

rajiteh Apr 14, 2022

Choose a reason for hiding this comment

prachidamle commented Mar 25, 2022

rajiteh commented Mar 29, 2022

gregsidelinger commented Mar 29, 2022

rajiteh commented Apr 14, 2022 • edited

k0da commented Aug 31, 2022

manno commented Nov 23, 2022

rajiteh commented Nov 25, 2022 • edited

manno commented Nov 29, 2022 • edited

thardeck commented Nov 30, 2022

manno commented Dec 1, 2022

rajiteh commented Dec 1, 2022

manno commented Dec 2, 2022

rajiteh commented Dec 4, 2021 •

edited by mattfarina

rajiteh Feb 16, 2022 •

edited

rajiteh commented Apr 14, 2022 •

edited

rajiteh commented Nov 25, 2022 •

edited

manno commented Nov 29, 2022 •

edited