-
Notifications
You must be signed in to change notification settings - Fork 181
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Signed-off-by: Philipp Stehle <[email protected]>
- Loading branch information
Showing
1 changed file
with
121 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,121 @@ | ||
--- | ||
hip: 9999 | ||
title: "Add `warn` template function" | ||
authors: ["Philipp Stehle <[email protected]>"] | ||
created: "2022-12-09" | ||
type: "feature" | ||
status: "draft" | ||
--- | ||
|
||
## Abstract | ||
|
||
When authoring charts it can be useful to warn users, when they deviate from best practices. | ||
E.g. when using a deprecated value or using the `latest` tag for images. | ||
|
||
There are already means to do this in Helm, for example by using the `NOTES.txt` feature, but they all come with drawbacks. | ||
Also, as long as there is no standard way to produce warnings, it is hard to build tools around this. | ||
|
||
This HIP proposes the addition of a `warn` template function, that Chart authors can use to signal warnings to Helm. | ||
It also includes several proposals of how Helm could make this available to the Chart consumers. | ||
|
||
## Motivation | ||
|
||
Some reasons why chart authors might want to produce warnings: | ||
|
||
- They have deprecated functionality, which they plan to remove later. E.g. we have recently renamed our `timeout` value to `timeoutSeconds`. Until the next major release we, will fallback to `timeout` if `timeoutSeconds` is not present. To ensure a smooth transition to the next major release, it would be helpful if we could warn our users ahead of time. | ||
- For backward compatibility, they allow that continue to allow setting they know are sub-optimal. | ||
- To ease non-productive (dev) setups, some settings that should not be used for productive environment might be allowed anyway, but produce a warning. E.g. using a replica count of `1`, which of course can't be used, if high availability is a concern. | ||
|
||
Some ways how users could want to consume warnings: | ||
|
||
- Via cli output | ||
- In automated PR validation (validation fails, if the PR introduces a new warning) | ||
|
||
## Specification | ||
|
||
### Creating Warnings | ||
|
||
Add a new template function called `warn`, with one of the following semantics (looking for feedback, which one we should go for): | ||
|
||
1. `warn` takes a single string argument, describing the Warning. | ||
1. `warn` takes a two arguments: a category (`depreaction`, `security`, `misc`, etc.) and a description: | ||
The categories would be pre-defined by Helm | ||
1. `warn` takes 1 or more arguments and interprets the first argument as a `printf` style string. | ||
1. `warn` takes 2 or more arguments, combining alternative 2 and 3. | ||
|
||
### Consuming Warnings | ||
|
||
There can be multiple ways to consume warnings for users, the most straight forward would be printing them to the console. | ||
|
||
Other ways could be: | ||
|
||
- add a `--fail-on-warning` flag to `template`, `install`, `upgrade` and `lint` (causing helm to exit with non-zero on warnings) | ||
- add a `--warning-output` flag that would produce a machine readable (e.g. `yaml` or `json`) collection of warning on the filesystem for further processing. | ||
- Expose the warnings to SDK users as Go-structs | ||
|
||
De-duplication: Helm could detect the same warning occuring multiple times and de-duplicate them, e.g. the following template: | ||
|
||
```plain | ||
{{ warn "my warning" }} | ||
{{ warn "my warning" }} | ||
``` | ||
|
||
Would produce the following output: | ||
|
||
```plain | ||
[WARNING]: my warning (occurred 2 times) | ||
``` | ||
|
||
Also, in the long term we should consolidate all other sources of warnings, e.g.: | ||
|
||
- [Usage of deprecated Charts](https://github.com/helm/helm/blob/v3.10.2/cmd/helm/install.go#L227-L229) | ||
- Usage of depreacted Kubernetes API object | ||
- Helm Lint warnings | ||
|
||
and provide them through a common interface (i.e. in machine readable form). | ||
|
||
## Backwards compatibility | ||
|
||
Following [HIP-0004](https://github.com/helm/community/blob/main/hips/hip-0004.md#specification) | ||
|
||
- exported Go APIs will remain compatible: | ||
This might be of concern, we just need to keep it in mind while implementing. Should be possible to do this non-breaking. | ||
- CLI commands and flags: | ||
No existing flags will be changed. | ||
- CLI output: | ||
Will not change, unless the `warn` feature is used by a chart author and therefore actively opting into the feature. | ||
- File formats: | ||
No change required. | ||
- Charts: | ||
No change required. | ||
- Templates (commands, functions, syntax, and variables): | ||
Only adding new functionality | ||
|
||
## Security implications | ||
|
||
As this allows for automatic scanning for (security) warnings, a malicious actor might be enabled to automatically scan for Helm Charts with flaws, putting users at risk. On the other hand, it also enables to users to do the same and therefore might even increase security. | ||
|
||
## How to teach this | ||
|
||
For chart authors: Document the `warn` funcion with example use-cases. | ||
For chart consumers: Document how to consume warnings. | ||
|
||
## Reference implementation | ||
|
||
[PoC PR to helm/helm](https://github.com/helm/helm/pull/11619) | ||
|
||
## Rejected ideas | ||
|
||
- Use `NOTES.txt` to produce warnings. This has several drawbacks: | ||
- Harder to consume for users, as there is no standardized way to transport warnings. | ||
- Harder to consume in automation (a CI job). | ||
- Notes of sub-chart are not rendered (at least not by default). | ||
- Call the template function `warning`: There is already the [`fail`](https://helm.sh/docs/chart_template_guide/function_list/#fail) function, so `warn` seems to be more consistent. | ||
|
||
## Open issues | ||
|
||
Let me know | ||
|
||
## References | ||
|
||
A collection of URLs or materials used as references through the HIP. |