Skip to content

Commit

Permalink
Run helm lint rules ourselves
Browse files Browse the repository at this point in the history
Instead of calling the helm linter to make it run helm
rules, lets just import those rules into our linter and run them

Signed-off-by: Itxaka <[email protected]>
  • Loading branch information
Itxaka committed Apr 9, 2021
1 parent d929a52 commit b125f15
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 31 deletions.
35 changes: 17 additions & 18 deletions pkg/action/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ limitations under the License.
package action

import (
"github.com/pkg/errors"
"github.com/rancher-sandbox/hypper/pkg/lint"
"helm.sh/helm/v3/pkg/action"
helmAction "helm.sh/helm/v3/pkg/action"
"helm.sh/helm/v3/pkg/chartutil"
"helm.sh/helm/v3/pkg/lint/support"
"io/ioutil"
Expand All @@ -29,27 +30,27 @@ import (

// Lint is a composite type of Helm's Lint type
type Lint struct {
*action.Lint
*helmAction.Lint
}

// NewLint creates a new Lint object with the given configuration.
func NewLint() *Lint {
return &Lint{
action.NewLint(),
helmAction.NewLint(),
}
}

// Run executes 'helm Lint' against the given chart and then runs the chart against hypper lint rules
func (l *Lint) Run(paths []string, vals map[string]interface{}) *action.LintResult {
func (l *Lint) Run(paths []string, vals map[string]interface{}) *helmAction.LintResult {
lowestTolerance := support.ErrorSev
if l.Strict {
lowestTolerance = support.WarningSev
}
// run it against the helm linter rules
result := l.Lint.Run(paths, vals)
// run it against our linter rules

result := &helmAction.LintResult{}

for _, path := range paths {
linter, err := lintChart(path)
linter, err := lintChart(path, vals, l.Namespace, l.Strict)
if err != nil {
result.Errors = append(result.Errors, err)
continue
Expand All @@ -66,36 +67,34 @@ func (l *Lint) Run(paths []string, vals map[string]interface{}) *action.LintResu
return result
}

// lintChart extracts the chart to a temp dir, checks that is a valid chart (has a Chart.yaml) and runs our rules
// We dont add to errors if we cant open the file because the helm linter has already added those as it runs before
// us so no need to flag those errors or else we get them duplicated.
func lintChart(path string) (support.Linter, error) {
// lintChart extracts the chart to a temp dir, checks that is a valid chart (has a Chart.yaml) and all rules
func lintChart(path string, vals map[string]interface{}, namespace string, strict bool) (support.Linter, error) {
var chartPath string
linter := support.Linter{}

if strings.HasSuffix(path, ".tgz") || strings.HasSuffix(path, ".tar.gz") {
tempDir, err := ioutil.TempDir("", "hypper-lint")
if err != nil {
return linter, nil
return linter, errors.Wrap(err, "unable to create temp dir to extract tarball")
}
defer os.RemoveAll(tempDir)

file, err := os.Open(path)
if err != nil {
return linter, nil
return linter, errors.Wrap(err, "unable to open tarball")
}
defer file.Close()

if err = chartutil.Expand(tempDir, file); err != nil {
return linter, nil
return linter, errors.Wrap(err, "unable to extract tarball")
}

files, err := ioutil.ReadDir(tempDir)
if err != nil {
return linter, nil
return linter, errors.Wrapf(err, "unable to read temporary output directory %s", tempDir)
}
if !files[0].IsDir() {
return linter, nil
return linter, errors.Errorf("unexpected file %s in temporary output directory %s", files[0].Name(), tempDir)
}

chartPath = filepath.Join(tempDir, files[0].Name())
Expand All @@ -108,5 +107,5 @@ func lintChart(path string) (support.Linter, error) {
return linter, nil
}

return lint.All(chartPath), nil
return lint.All(chartPath, vals, namespace, strict), nil
}
4 changes: 3 additions & 1 deletion pkg/action/lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (

var (
values = make(map[string]interface{})
namespace = "testNamespace"
strict = false
chart1MultipleChartLint = "testdata/charts/multiplecharts-lint-chart-1"
chart2MultipleChartLint = "testdata/charts/multiplecharts-lint-chart-2"
corruptedTgzChart = "testdata/charts/corrupted-compressed-chart.tgz"
Expand Down Expand Up @@ -68,7 +70,7 @@ func TestLintChart(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, err := lintChart(tt.chartPath)
_, err := lintChart(tt.chartPath, values, namespace, strict)
switch {
case err != nil && !tt.err:
t.Errorf("%s", err)
Expand Down
7 changes: 6 additions & 1 deletion pkg/lint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,21 @@ package lint

import (
"github.com/rancher-sandbox/hypper/pkg/lint/rules"
helmRules "helm.sh/helm/v3/pkg/lint/rules"
"helm.sh/helm/v3/pkg/lint/support"
"path/filepath"
)

// All runs all of the available linters on the given base directory.
func All(basedir string) support.Linter {
func All(basedir string, values map[string]interface{}, namespace string, strict bool) support.Linter {
// Using abs path to get directory context
chartDir, _ := filepath.Abs(basedir)

linter := support.Linter{ChartDir: chartDir}
helmRules.Chartfile(&linter)
helmRules.ValuesWithOverrides(&linter, values)
helmRules.Templates(&linter, values, namespace, strict)
helmRules.Dependencies(&linter)
rules.Annotations(&linter)
return linter
}
37 changes: 26 additions & 11 deletions pkg/lint/lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,29 @@ import (
"helm.sh/helm/v3/pkg/lint/support"
)

var values map[string]interface{}

const namespace = "testNamespace"
const strict = false

const badChartDir = "rules/testdata/badchart"
const badChartDirWithBrokenHypperDeps = "rules/testdata/badchartbrokenhypperdeps"
const goodChartDir = "rules/testdata/goodchart"

func TestBadChart(t *testing.T) {
m := All(badChartDir).Messages
if len(m) != 3 {
m := All(badChartDir, values, namespace, strict).Messages
if len(m) != 4 {
t.Errorf("Number of errors %v", len(m))
t.Errorf("All didn't fail with expected errors, got %#v", m)
}
// There should be 3 WARNINGs, check for them
var w1, w2, w3 bool
// There should be 1 INFO and 3 WARNINGs, check for them
var i1, w1, w2, w3 bool
for _, msg := range m {
if msg.Severity == support.InfoSev {
if strings.Contains(msg.Err.Error(), "icon is recommended") {
i1 = true
}
}
if msg.Severity == support.WarningSev {
if strings.Contains(msg.Err.Error(), "Setting hypper.cattle.io/release-name in annotations is recommended") {
w1 = true
Expand All @@ -52,33 +62,38 @@ func TestBadChart(t *testing.T) {
}
}
}
if !w1 || !w2 || !w3 {
if !i1 || !w1 || !w2 || !w3 {
t.Errorf("Didn't find all the expected errors, got %#v", m)
}
}

func TestBadChartBrokenDeps(t *testing.T) {
m := All(badChartDirWithBrokenHypperDeps).Messages
if len(m) != 1 {
m := All(badChartDirWithBrokenHypperDeps, values, namespace, strict).Messages
if len(m) != 2 {
t.Errorf("Number of errors %v", len(m))
t.Errorf("All didn't fail with expected errors, got %#v", m)
}
// There should be 1 Error, check for it
var e1 bool
// There should be 1 INFO and 1 ERROR, check for it
var i1, e1 bool
for _, msg := range m {
if msg.Severity == support.InfoSev {
if strings.Contains(msg.Err.Error(), "icon is recommended") {
i1 = true
}
}
if msg.Severity == support.ErrorSev {
if strings.Contains(msg.Err.Error(), "Shared dependencies list is broken, please check the correct format") {
e1 = true
}
}
}
if !e1 {
if !i1 || !e1 {
t.Errorf("Didn't find all the expected errors, got %#v", m)
}
}

func TestGoodChart(t *testing.T) {
m := All(goodChartDir).Messages
m := All(goodChartDir, values, namespace, strict).Messages
if len(m) != 0 {
t.Error("All returned linter messages when it shouldn't have")
for i, msg := range m {
Expand Down
1 change: 1 addition & 0 deletions pkg/lint/rules/testdata/goodchart/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ description: A Helm chart for Kubernetes
type: application
version: 0.1.0
appVersion: "1.16.0"
icon: "http://example.com/icon.png"
annotations:
"hypper.cattle.io/release-name": "release"
"hypper.cattle.io/namespace": "namespace"
Expand Down

0 comments on commit b125f15

Please sign in to comment.