Skip to content

HIP: Add Exclude File Option to Helm Lint Command #353

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.

Sign up for GitHub

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

HIP: Add Exclude File Option to Helm Lint Command #353

Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1385741
add hip-0019
danilo-patrucco-sev1tech Jul 3, 2024
b8e3b42
Update hip-0019.md
danilo-patrucco Aug 13, 2024
168c2e5
change some wording and add more information
danilo-patrucco Sep 19, 2024
00809c1
modifying the file additionaly after feedback
danilo-patrucco Sep 23, 2024
23e2488
initial changes as suggested by Daniel Pritchett
danilo-patrucco Sep 24, 2024
534e367
include some more changes
danilo-patrucco Sep 24, 2024
4e07227
fix some changes that were supposed to go in the rejected ideas.
danilo-patrucco Oct 10, 2024
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
61 changes: 61 additions & 0 deletions hips/hip-0019.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
---
hip: 0019
title: "Add Exclude File Option to Helm Lint Command"
authors: Danilo Patrucco
created: "2024-07-03"
type: "feature"
status: "draft"
---

## Abstract
This proposal introduces enhancements to the `helm lint` command, allowing the exclusion of specific files or patterns through a `.helmlintconfig.yaml` file, analogous to `.gitignore` or `.helmignore`. Additionally, a `--lint-config-file` flag will be added to specify alternative locations for the `.helmlintconfig.yaml` file. This feature is particularly useful in projects with multiple sub-charts. An environment variable `HELM_LINT_CONFIG_FILE` will also be introduced to mirror the functionality of the flag.
Copy link

Choose a reason for hiding this comment

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

Can we reword analogous to .gitignore or .helmignore now that we're targeting a general-purpose config file format?

Copy link
Author

Choose a reason for hiding this comment

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

done

Copy link

Choose a reason for hiding this comment

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

Suggested change
This proposal introduces enhancements to the `helm lint` command, allowing the exclusion of specific files or patterns through a `.helmlintconfig.yaml` file, analogous to `.gitignore` or `.helmignore`. Additionally, a `--lint-config-file` flag will be added to specify alternative locations for the `.helmlintconfig.yaml` file. This feature is particularly useful in projects with multiple sub-charts. An environment variable `HELM_LINT_CONFIG_FILE` will also be introduced to mirror the functionality of the flag.
This proposal introduces enhancements to the `helm lint` command, allowing the exclusion of specific files or patterns through a `.helmlintignore` file, analogous to `.gitignore` or `.helmignore`. Additionally, a `--lint-config-file` flag will be added to specify alternative locations for the `.helmlintignore` file. This feature is particularly useful in projects with multiple sub-charts. An environment variable `HELM_LINT_IGNORE_FILE` will also be introduced to mirror the functionality of the flag.

Copy link
Author

Choose a reason for hiding this comment

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

Hi Terry, Following our discussion with George, we've changed the .helmlintignore file to .helmlintconfig.yaml. The reason for this change is that the new config file will be in YAML format, and it will not only allow us to ignore specific linting errors but in the future we will also add more options/rules for linting. This is similar to the hadolint.yaml file that supports ignoring errors and setting up specific linting rules and options.

Copy link
Author

@danilo-patrucco danilo-patrucco Sep 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Also, We did put the PR for helm/helm back in Draft, since we are waiting to have a finalized version of the HIP to continue the development work on the helm repo. Please let me know If you have any additional reservation, and thank you for the review


## Motivation
In large Helm charts, certain files or configurations may not conform to standard linting rules but are accepted by the maintainers. The current `helm lint` process evaluates all files within a chart, often leading to irrelevant warnings or errors. An option to exclude specific files and the ability to specify config file locations would streamline the linting process.
Copy link

Choose a reason for hiding this comment

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

Potential phrasing: this can put helm users in situations where imported subcharts are flagged with low-priority lint errors that they'd prefer to suppress

Copy link
Author

Choose a reason for hiding this comment

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

done


## Rationale
The aim of this proposal is to grant developers and integrators more control over the linting process by enabling:
- Exclusions for files imported or managed by third parties that are not directly editable.
- Exclusions for files that may not adhere to static linting rules.
- Exclusions for files containing intentional deviations from standard practices due to specific deployment scenarios.

The `.helmlintconfig.yaml` file will support simple patterns for matching files and directories, enhancing ease of management. The `--lint-config-file` flag will facilitate centralized management of lint exclusions in complex projects.

## Specification
### `.helmlintconfig.yaml` File Format
The `.helmlintconfig.yaml` file enables chart developers to specify filenames or glob patterns to exclude from linting. The format is YAML, with patterns specified under a `lintIgnore` key, making future extensions for additional configurations feasible.
Copy link
Member

Choose a reason for hiding this comment

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

glob patterns for both file paths and error strings

Copy link
Author

Choose a reason for hiding this comment

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

done !


#### Example
```
# .helmlintconfig.yaml file example
lintIgnore:
Copy link
Member

Choose a reason for hiding this comment

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

perhaps a format more like (bikeshedding hard here):

# .helmlintconfig.yaml file example
pathIgnore:
- "migrations/templates/job.yaml"
- "gitlab/templates/shared-secrets/self-signed-cert-job.yml"
- "gitlab/templates/*.yaml"
errorIgnore:
- "chart metadata is missing these dependencies*"
- "<include 'fullname' .>"

Copy link
Author

@danilo-patrucco danilo-patrucco Sep 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I modified this section a little, but kept it similar to our suggestion because an error (the same error specifically) can occur in multiple charts / sub-charts, and connecting it to a file path would be beneficial (sorry for the bikeshedding here)

- fileMask: "migrations/templates/job.yaml" # Ignores all errors from this file
- errorMask: "chart metadata is missing these dependencies**" # Ignores all icon errors
- fileMask: "gitlab/templates/shared-secrets/self-signed-cert-job.yml"
errorMask: "<include 'fullname' .>" # Ignores this specific error but not other errors in the file
```

### Command Behavior
When `helm lint` is executed, it checks for the presence of a `.helmlintconfig.yaml` file in the chart directory or at a location specified by the `--lint-config-file` flag. The command then parses this file to exclude any files or directories matching the patterns specified, including specific error lines.
Copy link

Choose a reason for hiding this comment

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

or wherever the env var says to look

Copy link
Author

Choose a reason for hiding this comment

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

done


## Backwards Compatibility
This proposal introduces no breaking changes. Charts without a `.helmlintconfig.yaml` file or those not using the `--lint-config-file` flag will function as before.

## Security Implications
No significant security implications are expected as the `.helmlintconfig.yaml` file is processed locally by `helm lint`, without modifying chart content or involving network activity.

## How to Teach This
Documentation for the `.helmlintconfig.yaml` file and the `--lint-config-file` flag will be integrated into the official Helm documentation under the `helm lint` section. Detailed examples and common patterns will be provided to assist new users.

## Reference Implementation
PR #[13257](https://github.com/helm/helm/pull/13257) created in the Helm/helm repository in gitlab
Copy link

Choose a reason for hiding this comment

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

n.b. this PR will need some significant changes to catch up with the current design as described here in HIP-0019

Copy link
Author

Choose a reason for hiding this comment

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

this i will leave untouched if it's ok with you, since we put the PR in draft


## Open Issues
- Developing an effective and user-friendly pattern syntax for the `.helmlintconfig.yaml` file.
- Considering the potential for in-file annotations to temporarily exclude specific chart sections from linting.

## Rejected Ideas
Copy link
Member

Choose a reason for hiding this comment

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

Providing rule specific excludes and configurations, and composing configuration from "default" configuration e.g. similar to https://yamllint.readthedocs.io/en/stable/configuration.html#extending-the-default-configuration

Helm today doesn't "name" its lint rules, and changing the code/structure to support the above can be extended later

Copy link
Author

Choose a reason for hiding this comment

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

Done, let me know if it's ok

Copy link
Member

Choose a reason for hiding this comment

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

oops, sorry, I mean we should add these to the "rejected ideas":

None at this time.

## References
- This proposal aligns with practices outlined in [HIP-0001](https://github.com/helm/community/blob/master/hips/hip-0001.md).