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

HIP: warn template function #280

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 121 additions & 0 deletions hips/hip-9999.md
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.