-
Notifications
You must be signed in to change notification settings - Fork 219
docs(values,templating): add reference and best practices pages #4368
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Kit Patella <[email protected]>
✅ Deploy Preview for zarf-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
Signed-off-by: Kit Patella <[email protected]>
…em rather than duplicating the same content Signed-off-by: Kit Patella <[email protected]>
Signed-off-by: Kit Patella <[email protected]>
Signed-off-by: Kit Patella <[email protected]>
Signed-off-by: Kit Patella <[email protected]>
…nd config file imports and --set-values Signed-off-by: Kit Patella <[email protected]>
Signed-off-by: Kit Patella <[email protected]>
Signed-off-by: Kit Patella <[email protected]>
a1994sc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
|
||
| See the [Templating Reference](/ref/templating/) for complete function documentation. | ||
|
|
||
| ## Avoid Sensitive Data in Values Files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally this is good advice but I do think it is worth calling out that values files will be the primary mechanism for orchestrating sensitive values. Given the declarative delivery model - all secrets have to originate from somewhere.
It will be important for sunsetting zarf variables as it will provide a way to break these out of zarf config files.
We may want to delineate between sensitive values defaults getting "baked" into the package - for which you had a few good examples of different environment overrides that would align to that story telling.
|
|
||
| ## Common Errors and Solutions | ||
|
|
||
| ### Template Parse Errors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++ I like that we are making errors in docs discoverable.
| targetPath: ".config.database.host" # Takes priority | ||
| ``` | ||
|
|
||
| #### Mapping All Values |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: we may want to refine this section at some point to help users understand the best practices for building a package with values files that aligns to broader zarf hardening goals.
said another way - I think we should encourage using sourcePath/targetPath as a best practice for package hardening.
|
|
||
| ::: | ||
|
|
||
| Zarf supports Go template syntax and `text/template` engine with powerful features including control flow and functions. Zarf includes the full library of [Sprig functions](http://masterminds.github.io/sprig/) and some builtins for dynamic configuration in manifests, files, and actions. This powerful templating system enables complex transformations, conditional logic, and data manipulation beyond simple string substitution. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may be a good place to identify what is different between zarf templating and helm templating.
AustinAbro321
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving comments on the review so far, I'll aim to do a review on the ref section at another point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Across this PR we reference zarf package inspect, zarf dev inspect and using the values schema however none of these features are merged into Zarf as of yet. IMO we should wait until a feature is merged before documenting it
|
|
||
| ## Provide Defaults for Optional Values | ||
|
|
||
| Use the `default` function to provide fallback values: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While defaults are generally a good practice, I don't think they really make sense in the context of Zarf since we always validate that there is a value for template. I could be missing scenarios but AFAIK there is no way for the default to come into play
|
|
||
| ## Leverage Templating Functions | ||
|
|
||
| Use [functions](/ref/templating/) for dynamic configuration: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should link to ref/templating/#template-functions
| data: | ||
| # Numbers must be strings in ConfigMaps | ||
| port: {{ .Values.app.port | toString | quote }} | ||
| replicas: {{ .Values.app.replicas | toString | quote }} | ||
|
|
||
| # Booleans to strings | ||
| debug: {{ .Values.app.debug | toString | quote }} | ||
|
|
||
| # ❌ RISKY: Type mismatch errors | ||
| data: | ||
| port: {{ .Values.app.port }} # May fail if port is a number |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Numbers don't have to be strings in configmaps, Kubernetes will store either
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thoughts on generalizing the example a bit then, like "explicit type conversion may be necessary?"
|
|
||
| ## Handle Missing Values Gracefully | ||
|
|
||
| Protect against missing or nil values: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this advice makes sense in the context of Zarf, the "good" example will still fail if .values.monitoring.enabled is not set
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I agree, I don't like encouraging conditional blocks like this either. Removed in latest
| **Solution:** | ||
| ```yaml | ||
| # Use default for optional values | ||
| value: {{ .Values.app.missing | default "fallback" }} | ||
|
|
||
| # Or check if exists first | ||
| {{- if .Values.app.missing }} | ||
| value: {{ .Values.app.missing }} | ||
| {{- end }} | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both of these solutions would not solve the error.
|
|
||
| ### Quoting Problems | ||
|
|
||
| **Problem:** YAML parsing fails with boolean-like strings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I understand why we would use the quote function at times, we wouldn't want a type to be interpreted as the incorrect type. Are there situations where YAML parsing would fail?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's a specific yaml spec version where this was a problem, I don't know if this specifically affects Zarf though. This is probably covered under the more general guidance to quote strings.
Description
WIP: Add reference documentation for package values and templating.
Related Issue
Relates to #3946
Checklist before merging