helm: Helm lint throws an error where a warning should be thrown instead
As a result of these 2 PRs:
- https://github.com/helm/helm/pull/7986 - feat: implement deprecation warnings in helm lint
- https://github.com/helm/helm/pull/8011 - feat: lint the names of templated resources
now helm lint validates whether resource metadata.name complies Kubernetes naming rules and whether the apiVersion is up to date. And these are great features, thank you for implementing them!
However, these rules are having ERROR severity instead of WARNING. These should be warnings because these are NOT errors from the Kubernetes perspective - Kubernetes API still accepts resources with deprecated API versions as well as resources with metadata names that, for example, contain : in them. If these are warnings instead of errors, then it will be up to a client to decide whether he wants to fail lint (via using --strict flag for helm lint command) or not.
Setting the error severity level for these issues also complicates the migration of automated deployments to helm 3.3, especially form Helm 2.x.
How to fix? - I believe these lines of code should be rewritten to:
linter.RunLinterRule(support.WarningSev, path, validateMetadataName(&yamlStruct))
linter.RunLinterRule(support.WarningSev, path, validateNoDeprecations(&yamlStruct))
About this issue
- Original URL
- State: closed
- Created 4 years ago
- Reactions: 19
- Comments: 19 (10 by maintainers)
Commits related to this issue
- Reduce linting severity for users of out-of-date kubernetes Fixes #8596 Signed-off-by: Joe Julian <me@joejulian.name> — committed to joejulian/helm by joejulian 4 years ago
- Reduce linting severity for users of out-of-date kubernetes Fixes #8596 Signed-off-by: Joe Julian <me@joejulian.name> — committed to joejulian/helm by joejulian 4 years ago
- Reduce linting severity for users of out-of-date kubernetes Fixes #8596 Signed-off-by: Joe Julian <me@joejulian.name> — committed to joejulian/helm by joejulian 4 years ago
- Reduce linting severity for users of out-of-date kubernetes Fixes #8596 Signed-off-by: Joe Julian <me@joejulian.name> — committed to joejulian/helm by joejulian 4 years ago
- Reduce linting severity for users of out-of-date kubernetes Fixes #8596 Signed-off-by: Joe Julian <me@joejulian.name> — committed to joejulian/helm by joejulian 4 years ago
- Reduce linting severity for users of out-of-date kubernetes Fixes #8596 Signed-off-by: Joe Julian <me@joejulian.name> — committed to joejulian/helm by joejulian 4 years ago
- Reduce linting severity for users of out-of-date kubernetes Fixes #8596 Signed-off-by: Joe Julian <me@joejulian.name> — committed to joejulian/helm by joejulian 4 years ago
- Reduce linting severity for users of out-of-date kubernetes Fixes #8596 Signed-off-by: Joe Julian <me@joejulian.name> — committed to joejulian/helm by joejulian 4 years ago
- Reduce linting severity for users of out-of-date kubernetes Fixes #8596 Signed-off-by: Joe Julian <me@joejulian.name> — committed to joejulian/helm by joejulian 4 years ago
- Reduce linting severity for users of out-of-date kubernetes Fixes #8596 Signed-off-by: Joe Julian <me@joejulian.name> — committed to joejulian/helm by joejulian 4 years ago
- Reduce linting severity for users of out-of-date kubernetes Fixes #8596 Signed-off-by: Joe Julian <me@joejulian.name> — committed to joejulian/helm by joejulian 4 years ago
- Reduce linting severity for users of out-of-date kubernetes Fixes #8596 Signed-off-by: Joe Julian <me@joejulian.name> — committed to joejulian/helm by joejulian 4 years ago
- Reduce linting severity for users of out-of-date kubernetes Fixes #8596 Signed-off-by: Joe Julian <me@joejulian.name> — committed to joejulian/helm by joejulian 4 years ago
- Reduce linting severity for users of out-of-date kubernetes Fixes #8596 Signed-off-by: Joe Julian <me@joejulian.name> — committed to joejulian/helm by joejulian 4 years ago
- Reduce linting severity for users of out-of-date kubernetes Fixes #8596 Signed-off-by: Joe Julian <me@joejulian.name> — committed to joejulian/helm by joejulian 4 years ago
- Reduce linting severity for users of out-of-date kubernetes Fixes #8596 Signed-off-by: Joe Julian <me@joejulian.name> — committed to joejulian/helm by joejulian 4 years ago
- Reduce linting severity for users of out-of-date kubernetes Fixes #8596 Signed-off-by: Joe Julian <me@joejulian.name> — committed to joejulian/helm by joejulian 4 years ago
- Reduce linting severity for users of out-of-date kubernetes (#8608) * Reduce linting severity for users of out-of-date kubernetes Fixes #8596 Signed-off-by: Joe Julian <me@joejulian.name> * ... — committed to helm/helm by joejulian 3 years ago
- Reduce linting severity for users of out-of-date kubernetes (#8608) * Reduce linting severity for users of out-of-date kubernetes Fixes #8596 Signed-off-by: Joe Julian <me@joejulian.name> * ... — committed to ztbrown/helm by joejulian 3 years ago
- fix: bump helm version to fix the linter issue deprecated api are now warnings instead of errors see https://github.com/helm/helm/issues/8596 — committed to vbehar/tekton-helm-chart by vbehar 3 years ago
It looks to me like this is an overly strict reading of the naming docs? While the docs mention three name constraints (DNS subdomain, DNS label and Path segment), Helm’s linter appears to only consider the subdomain constraint, which will be overly strict for some types, and too liberal for those that work with the label constraint. As an example of where this is too strict, Kubernetes built-ins roles and bindings use colon-separation for organization, which has thus been a pattern repeated in the community. This is now disallowed, breaking several charts.
@bacongobbler I’m reminded that numerous people are still running production systems older than 1.16. I know people using 1.13. This is going to mess with things like
NetworkPolicywhich have a much smaller deprecation window. Some people will have environments that don’t have the new version yet. I wonder about the deprecation policy being a warning for some and an error for others. To add more nuance.The name issue is one that’s been called out in a few places. Some resources have different allowed lengths. I wonder if we should try to catch for those when we know them.
Thoughts?
There is a solution for this in #8608 - it just needs to correct some merge conflicts and get released.
The PR that is fixing this is #8608 and specifically https://github.com/helm/helm/pull/8608/files#diff-c708830a32a0f702f8307935ad3c4137R133-R138. NOTE: the scope of that PR is larger than this issue in that it is trying to provide the user with more detailed information about when an API is deprecated and when it is removed vs. just reducing the level from error to warning.