Repository: helm/chart-testing
Pull request #283 (2020-10-05 15:38:11)

Support execution of additional commands during ct lint
<!--
Thank you for contributing to helm/chart-testing.

When updates to your PR are requested, please add new commits and do not squash the
history. This will make it easier to identify new changes. The PR will be squashed
anyways when it is merged. Thanks.

Please make sure you test your changes before you push them. Once pushed, a CircleCI
will run across your changes and do some initial checks and linting. These checks run
very quickly. Please check the results. We would like these checks to pass before we
even continue reviewing your changes.
-->

**What this PR does / why we need it**:

This is an alternative design to #279 which not only support `helm unittest`, but running any command as part of `ct lint`.
@unguiculus mentioned in slack that you would prefer that instead of pulling in additional dependencies.

I think that's totally valid and also solve many issue. When using #279 I noticed for example that it would be nice if I could pass additional parameters to the command. In that case I needed to add `--helm3`.

Given that helm unittest is installed locally or mounted into the chart testing container one could use this to run helm unittest for each chart with this configured in `ct.yaml`:

```
additional-commands:
 - helm unittest --helm3 -f tests/*.yaml {{ .Path }}
```

The command is treated as a go template. Like this it's possible to get the path to the chart as in the example. It would also be open for further extension if needed.

One idea would be to pass the information if it's an helm 2 or helm 3 chart.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #

**Special notes for your reviewer**:

This works fine when ct is run locally and the toll is installed on your machine. If you want it in the container then the tool needs to be present there. There are multiple option to achieve this:

- built a custom image, which contains the tool
- mount a directory in case the tool has no dependencies
  PR helm/chart-testing-action/pull/54 adds support for it in chart-testing-action.

torstenwalter: Here is an example usage:

- installing the plugin https://github.com/torstenwalter/jenkins-helm-charts/blob/unit-tests-improved/.github/workflows/lint-test.yaml#L19-L26
- mounting it into the container https://github.com/torstenwalter/jenkins-helm-charts/blob/unit-tests-improved/.github/workflows/lint-test.yaml#L35
- specifying the aditional command to run
https://github.com/torstenwalter/jenkins-helm-charts/blob/unit-tests-improved/ct.yaml#L7-L8


scottrigby: Question: why build this into charts-testing?

As opposed to say just adding that line (or any other command for that matter) as a separate step in your CI?

I understand the `quintush/helm-unittest` plugin literally relates to "chart testing", but it's currently not related to this project at all apart from that. So this PR seems to be using the chart-testing command as a passthrough for literally any other command with no benefit I can see yet over calling those other commands separately. I could be wrong though, please let me know if there are other benefits.

If we did want to expand the scope of `helm/chart-testing` to support unit tests via that plugin, I would prefer expanding on your other PR https://github.com/helm/chart-testing/pull/279. These are just my initial thoughts. What do you think @unguiculus?
torstenwalter: The benefit of putting it here is the logic to detect which charts have been modified. It allows to run commands for modified charts only.

Otherwise you would need to duplicate that in your CI pipeline.

Allowing commands over just supporting helm unittest allows users to extend even further. One idea would be to check for deprecated Kubernetes APIs.
scottrigby: That sounds like a good reason 👍 
torstenwalter: Is there anything else, which should be changed?

Patch:
diff --git a/Dockerfile b/Dockerfile
index 6d2781b..c2b6c57 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -1,6 +1,7 @@
 FROM alpine:3.12
 
 RUN apk --no-cache add \
+    bash \
     curl \
     git \
     libc6-compat \
diff --git a/README.md b/README.md
index aa15405..e69c3c6 100644
--- a/README.md
+++ b/README.md
@@ -51,6 +51,9 @@ See documentation for individual commands:
 * [ct list-changed](doc/ct_list-changed.md)
 * [ct version](doc/ct_version.md)
 
+For a more extensive how-to guide, please see:
+
+* [charts-repo-actions-demo](https://github.com/helm/charts-repo-actions-demo)
 
 ## Configuration
 
@@ -198,7 +201,6 @@ Run the script specifying the version for the new release.
 
 Versions must start with a lower-case `v`, e. g. `v3.1.1`.
 
-
 ## Supported versions
 
 The previous MAJOR version will be supported for three months after each new MAJOR release.
diff --git a/ct/cmd/lint.go b/ct/cmd/lint.go
index cc32142..6580db9 100644
--- a/ct/cmd/lint.go
+++ b/ct/cmd/lint.go
@@ -69,6 +69,11 @@ func addLintFlags(flags *flag.FlagSet) {
 			Enable schema validation of 'Chart.yaml' using Yamale (default: true)`))
 	flags.Bool("validate-yaml", true, heredoc.Doc(`
 			Enable linting of 'Chart.yaml' and values files (default: true)`))
+	flags.StringSlice("additional-commands", []string{}, heredoc.Doc(`
+            Additional commands to run per chart (default: [])
+            Commands will be executed in the same order as provided in the list and will
+            be rendered with go template before being executed.
+            Example: "helm unittest --helm3 -f tests/*.yaml {{ .Path }}"`))
 }
 
 func lint(cmd *cobra.Command, args []string) error {
diff --git a/go.mod b/go.mod
index a311720..a35e549 100644
--- a/go.mod
+++ b/go.mod
@@ -9,6 +9,7 @@ require (
 	github.com/goreleaser/goreleaser v0.129.0
 	github.com/hashicorp/go-multierror v1.0.0
 	github.com/hashicorp/go-retryablehttp v0.6.4
+	github.com/mattn/go-shellwords v1.0.10
 	github.com/mitchellh/go-homedir v1.1.0
 	github.com/mitchellh/mapstructure v1.2.2 // indirect
 	github.com/pelletier/go-toml v1.6.0 // indirect
diff --git a/go.sum b/go.sum
index d118684..64c1511 100644
--- a/go.sum
+++ b/go.sum
@@ -219,6 +219,8 @@ github.com/mattn/go-isatty v0.0.5/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hd
 github.com/mattn/go-isatty v0.0.8/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s=
 github.com/mattn/go-isatty v0.0.11 h1:FxPOTFNqGkuDUGi3H/qkUbQO4ZiBa2brKq5r0l8TGeM=
 github.com/mattn/go-isatty v0.0.11/go.mod h1:PhnuNfih5lzO57/f3n+odYbM4JtupLOxQOAqxQCu2WE=
+github.com/mattn/go-shellwords v1.0.10 h1:Y7Xqm8piKOO3v10Thp7Z36h4FYFjt5xB//6XvOrs2Gw=
+github.com/mattn/go-shellwords v1.0.10/go.mod h1:EZzvwXDESEeg03EKmM+RmDnNOPKG4lLtQsUlTZDWQ8Y=
 github.com/mattn/go-zglob v0.0.1 h1:xsEx/XUoVlI6yXjqBK062zYhRTZltCNmYPx6v+8DNaY=
 github.com/mattn/go-zglob v0.0.1/go.mod h1:9fxibJccNxU2cnpIKLRRFA7zX7qhkJIQWBb449FYHOo=
 github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=
diff --git a/pkg/chart/chart.go b/pkg/chart/chart.go
index 07c04f1..12021f4 100644
--- a/pkg/chart/chart.go
+++ b/pkg/chart/chart.go
@@ -131,6 +131,13 @@ type Linter interface {
 	Yamale(yamlFile string, schemaFile string) error
 }
 
+// CmdExecutor is the interface
+//
+// RunCommand renders cmdTemplate as go template using data and executes the resulting command
+type CmdExecutor interface {
+	RunCommand(cmdTemplate string, data interface{}) error
+}
+
 // DirectoryLister is the interface
 //
 // ListChildDirs lists direct child directories of parentDir given they pass the test function
@@ -224,6 +231,7 @@ type Testing struct {
 	kubectl                  Kubectl
 	git                      Git
 	linter                   Linter
+	cmdExecutor              CmdExecutor
 	accountValidator         AccountValidator
 	directoryLister          DirectoryLister
 	chartUtils               ChartUtils
@@ -253,6 +261,7 @@ func NewTesting(config config.Configuration) (Testing, error) {
 		git:              tool.NewGit(procExec),
 		kubectl:          tool.NewKubectl(procExec),
 		linter:           tool.NewLinter(procExec),
+		cmdExecutor:      tool.NewCmdTemplateExecutor(procExec),
 		accountValidator: tool.AccountValidator{},
 		directoryLister:  util.DirectoryLister{},
 		chartUtils:       util.ChartUtils{},
@@ -451,6 +460,13 @@ func (t *Testing) LintChart(chart *Chart) TestResult {
 		}
 	}
 
+	for _, cmd := range t.config.AdditionalCommands {
+		if err := t.cmdExecutor.RunCommand(cmd, chart); err != nil {
+			result.Error = err
+			return result
+		}
+	}
+
 	// Lint with defaults if no values files are specified.
 	if len(valuesFiles) == 0 {
 		valuesFiles = append(valuesFiles, "")
diff --git a/pkg/config/config.go b/pkg/config/config.go
index ec62abe..a88d402 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -50,6 +50,7 @@ type Configuration struct {
 	ValidateMaintainers   bool     `mapstructure:"validate-maintainers"`
 	ValidateChartSchema   bool     `mapstructure:"validate-chart-schema"`
 	ValidateYaml          bool     `mapstructure:"validate-yaml"`
+	AdditionalCommands    []string `mapstructure:"additional-commands"`
 	CheckVersionIncrement bool     `mapstructure:"check-version-increment"`
 	ProcessAllCharts      bool     `mapstructure:"all"`
 	Charts                []string `mapstructure:"charts"`
diff --git a/pkg/tool/cmdexecutor.go b/pkg/tool/cmdexecutor.go
new file mode 100644
index 0000000..0449088
--- /dev/null
+++ b/pkg/tool/cmdexecutor.go
@@ -0,0 +1,38 @@
+package tool
+
+import (
+	"strings"
+	"text/template"
+
+	"github.com/mattn/go-shellwords"
+)
+
+type ProcessExecutor interface {
+	RunProcess(executable string, execArgs ...interface{}) error
+}
+
+type CmdTemplateExecutor struct {
+	exec ProcessExecutor
+}
+
+func NewCmdTemplateExecutor(exec ProcessExecutor) CmdTemplateExecutor {
+	return CmdTemplateExecutor{
+		exec: exec,
+	}
+}
+
+func (t CmdTemplateExecutor) RunCommand(cmdTemplate string, data interface{}) error {
+	var template = template.Must(template.New("command").Parse(cmdTemplate))
+	var b strings.Builder
+	if err := template.Execute(&b, data); err != nil {
+		return err
+	}
+	rendered := b.String()
+
+	words, err := shellwords.Parse(rendered)
+	if err != nil {
+		return err
+	}
+	name, args := words[0], words[1:]
+	return t.exec.RunProcess(name, args)
+}


Test patch:
diff --git a/pkg/chart/chart_test.go b/pkg/chart/chart_test.go
index 13a1196..829ccf8 100644
--- a/pkg/chart/chart_test.go
+++ b/pkg/chart/chart_test.go
@@ -109,6 +109,15 @@ func (h fakeHelm) Version() (string, error) {
 	return "v3.0.0", nil
 }
 
+type fakeCmdExecutor struct {
+	mock.Mock
+}
+
+func (c *fakeCmdExecutor) RunCommand(cmdTemplate string, data interface{}) error {
+	c.Called(cmdTemplate, data)
+	return nil
+}
+
 var ct Testing
 
 func init() {
@@ -417,3 +426,47 @@ func TestChart_HasCIValuesFile(t *testing.T) {
 		})
 	}
 }
+
+func TestChart_AdditionalCommandsAreRun(t *testing.T) {
+	type testData struct {
+		name            string
+		cfg             config.Configuration
+		callsRunCommand int
+	}
+
+	testCases := []testData{
+		{
+			name:            "no additional commands",
+			cfg:             config.Configuration{},
+			callsRunCommand: 0,
+		},
+		{
+			name: "one command",
+			cfg: config.Configuration{
+				AdditionalCommands: []string{"helm unittest --helm3 -f tests/*.yaml {{ .Path }}"},
+			},
+			callsRunCommand: 1,
+		},
+		{
+			name: "multiple commands",
+			cfg: config.Configuration{
+				AdditionalCommands: []string{"echo", "helm unittest --helm3 -f tests/*.yaml {{ .Path }}"},
+			},
+			callsRunCommand: 2,
+		},
+	}
+
+	for _, testData := range testCases {
+		t.Run(testData.name, func(t *testing.T) {
+			fakeCmdExecutor := new(fakeCmdExecutor)
+			fakeCmdExecutor.On("RunCommand", mock.Anything, mock.Anything).Return(nil)
+
+			ct := newTestingMock(testData.cfg)
+			ct.cmdExecutor = fakeCmdExecutor
+
+			ct.LintChart(&Chart{})
+
+			fakeCmdExecutor.AssertNumberOfCalls(t, "RunCommand", testData.callsRunCommand)
+		})
+	}
+}
diff --git a/pkg/tool/cmdexecutor_test.go b/pkg/tool/cmdexecutor_test.go
new file mode 100644
index 0000000..21cb6ea
--- /dev/null
+++ b/pkg/tool/cmdexecutor_test.go
@@ -0,0 +1,76 @@
+package tool
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/mock"
+)
+
+type fakeProcessExecutor struct {
+	mock.Mock
+}
+
+func (c *fakeProcessExecutor) RunProcess(executable string, execArgs ...interface{}) error {
+	c.Called(executable, execArgs[0])
+	return nil
+}
+
+func TestCmdTemplateExecutor_RunCommand(t *testing.T) {
+	type args struct {
+		cmdTemplate string
+		data        interface{}
+	}
+	tests := []struct {
+		name     string
+		args     args
+		wantErr  bool
+		validate func(t *testing.T, executor *fakeProcessExecutor)
+	}{
+		{
+			name: "command without arguments",
+			args: args{
+				cmdTemplate: "echo",
+				data:        nil,
+			},
+			validate: func(t *testing.T, executor *fakeProcessExecutor) {
+				executor.AssertCalled(t, "RunProcess", "echo", []string{})
+			},
+			wantErr: false,
+		},
+		{
+			name: "command with args",
+			args: args{
+				cmdTemplate: "echo hello world",
+			},
+			validate: func(t *testing.T, executor *fakeProcessExecutor) {
+				executor.AssertCalled(t, "RunProcess", "echo", []string{"hello", "world"})
+			},
+			wantErr: false,
+		},
+		{
+			name: "interpolate args",
+			args: args{
+				cmdTemplate: "helm unittest --helm3 -f tests/*.yaml {{ .Path }}",
+				data:        map[string]string{"Path": "charts/my-chart"},
+			},
+			validate: func(t *testing.T, executor *fakeProcessExecutor) {
+				executor.AssertCalled(t, "RunProcess", "helm", []string{"unittest", "--helm3", "-f", "tests/*.yaml", "charts/my-chart"})
+			},
+			wantErr: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			processExecutor := new(fakeProcessExecutor)
+			processExecutor.On("RunProcess", mock.Anything, mock.Anything).Return(nil)
+			templateExecutor := CmdTemplateExecutor{
+				exec: processExecutor,
+			}
+			if err := templateExecutor.RunCommand(tt.args.cmdTemplate, tt.args.data); (err != nil) != tt.wantErr {
+				t.Errorf("RunCommand() error = %v, wantErr %v", err, tt.wantErr)
+			}
+			tt.validate(t, processExecutor)
+
+		})
+	}
+}