Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cluster values full templating #1158

Merged
merged 5 commits into from
Dec 7, 2022
Merged

Conversation

manno
Copy link
Member

@manno manno commented Dec 6, 2022

PR #671 added templating to the values section in fleet.yaml.

Templating in fleet.yaml is not the same as in the templates/ dir of Helm charts. fleet.yaml must be valid YAML. Using the default templating delims {{, }} leads to errors, if they are not quoted, as the YAML parser interprets them as JSON. Therefore this PR switches the delims to ${, }. This will also help to differentiate one kind of templating from the other, as they don't behave the same in practice, e.g. when using range or with.

To be able to generate lists and to get rid of the recursion and reduce the number of times template.Execute is called, this PR treats the whole values: section from fleet.yaml as one template. With regards to the inputs from the cluster (TemplateValues) resource, I see no problem, because being able to change a Cluster resource requires much more privileges than changing a GitRepo resource.
The fleet.yaml however is already under control of the user, so any structure change in the values is likely intentional.

Updated docs: rancher/fleet-docs#30
Examples branch for e2e test: https://github.com/rancher/fleet-examples/tree/test-cluster-values/single-cluster/helm-cluster-values

Mario Manno added 4 commits December 7, 2022 11:00
Tests were using "Values", but code switched to "ClusterValues".
The helm values in the fleet.yaml are map[string]any, however unquoted
{{}} leads to a "yaml to json" error, thus templating can only produce
strings.
@manno manno force-pushed the cluster-values-full-templating branch from dcab6f0 to 00ef89c Compare December 7, 2022 10:00
@manno manno marked this pull request as ready for review December 7, 2022 10:45
@manno manno requested a review from a team as a code owner December 7, 2022 10:45
@manno manno self-assigned this Dec 7, 2022
@kkaempf kkaempf mentioned this pull request Dec 7, 2022
3 tasks
Copy link
Collaborator

@mattfarina mattfarina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just the slightest of nits in an error message.

pkg/target/target.go Show resolved Hide resolved
pkg/target/target.go Outdated Show resolved Hide resolved
@manno manno merged commit 296fed1 into master Dec 7, 2022
@manno manno deleted the cluster-values-full-templating branch December 7, 2022 17:47
This was referenced Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants