diff --git a/Gopkg.lock b/Gopkg.lock index ba7bcea..22f88ed 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -178,11 +178,20 @@ revision = "2c12c60302a5a0e62ee102ca9bc996277c2f64f5" version = "v1.2.1" +[[projects]] + digest = "1:711eebe744c0151a9d09af2315f0bb729b2ec7637ef4c410fa90a18ef74b65b6" + name = "github.com/stretchr/objx" + packages = ["."] + pruneopts = "" + revision = "477a77ecc69700c7cdeb1fa9e129548e1c1c393c" + version = "v0.1.1" + [[projects]] digest = "1:c587772fb8ad29ad4db67575dad25ba17a51f072ff18a22b4f0257a4d9c24f75" name = "github.com/stretchr/testify" packages = [ "assert", + "mock", "require", ] pruneopts = "" @@ -233,6 +242,7 @@ "github.com/spf13/pflag", "github.com/spf13/viper", "github.com/stretchr/testify/assert", + "github.com/stretchr/testify/mock", "github.com/stretchr/testify/require", "gopkg.in/yaml.v2", ] diff --git a/app/cmd/lint.go b/app/cmd/lint.go index 369be0e..c9b253c 100644 --- a/app/cmd/lint.go +++ b/app/cmd/lint.go @@ -19,7 +19,6 @@ import ( "os" "github.com/MakeNowJust/heredoc" - "github.com/helm/chart-testing/pkg/chart" "github.com/helm/chart-testing/pkg/config" "github.com/spf13/cobra" @@ -65,9 +64,13 @@ func addLintFlags(flags *flag.FlagSet) { is searched in the current directory, '$HOME/.ct', and '/etc/ct', in that order.`)) flags.Bool("validate-maintainers", true, heredoc.Doc(` - Enabled validation of maintainer account names in chart.yml (default: true). + Enable validation of maintainer account names in chart.yml (default: true). Works for GitHub, GitLab, and Bitbucket`)) flags.Bool("check-version-increment", true, "Activates a check for chart version increments (default: true)") + flags.Bool("validate-chart-schema", true, heredoc.Doc(` + 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)`)) } func lint(cmd *cobra.Command, args []string) { @@ -95,6 +98,6 @@ func lint(cmd *cobra.Command, args []string) { } func bindLintFlags(flagSet *flag.FlagSet, v *viper.Viper) error { - options := []string{"lint-conf", "chart-yaml-schema", "validate-maintainers", "check-version-increment"} + options := []string{"lint-conf", "chart-yaml-schema", "validate-maintainers", "check-version-increment", "validate-chart-schema", "validate-yaml"} return bindFlags(options, flagSet, v) } diff --git a/doc/ct.md b/doc/ct.md index 4de1db1..9ba72c6 100644 --- a/doc/ct.md +++ b/doc/ct.md @@ -25,4 +25,4 @@ in given chart directories. * [ct lint-and-install](ct_lint-and-install.md) - Lint, install, and test a chart * [ct version](ct_version.md) - Print version information -###### Auto generated by spf13/cobra on 17-Nov-2018 +###### Auto generated by spf13/cobra on 19-Dec-2018 diff --git a/doc/ct_install.md b/doc/ct_install.md index 3655711..f265286 100644 --- a/doc/ct_install.md +++ b/doc/ct_install.md @@ -57,4 +57,4 @@ ct install [flags] * [ct](ct.md) - The Helm chart testing tool -###### Auto generated by spf13/cobra on 17-Nov-2018 +###### Auto generated by spf13/cobra on 19-Dec-2018 diff --git a/doc/ct_lint-and-install.md b/doc/ct_lint-and-install.md index 4035bfb..ececce6 100644 --- a/doc/ct_lint-and-install.md +++ b/doc/ct_lint-and-install.md @@ -46,12 +46,14 @@ ct lint-and-install [flags] This is only used if namespace is specified. (default "app.kubernetes.io/instance") --remote string The name of the Git remote used to identify changed charts (default "origin") --target-branch string The name of the target branch used to identify changed charts (default "master") - --validate-maintainers Enabled validation of maintainer account names in chart.yml (default: true). + --validate-chart-schema Enable schema validation of 'Chart.yaml' using Yamale (default: true) (default true) + --validate-maintainers Enable validation of maintainer account names in chart.yml (default: true). Works for GitHub, GitLab, and Bitbucket (default true) + --validate-yaml Enable linting of 'Chart.yaml' and values files (default: true) (default true) ``` ### SEE ALSO * [ct](ct.md) - The Helm chart testing tool -###### Auto generated by spf13/cobra on 17-Nov-2018 +###### Auto generated by spf13/cobra on 19-Dec-2018 diff --git a/doc/ct_lint.md b/doc/ct_lint.md index f1bd5c6..97d7153 100644 --- a/doc/ct_lint.md +++ b/doc/ct_lint.md @@ -50,12 +50,14 @@ ct lint [flags] that order --remote string The name of the Git remote used to identify changed charts (default "origin") --target-branch string The name of the target branch used to identify changed charts (default "master") - --validate-maintainers Enabled validation of maintainer account names in chart.yml (default: true). + --validate-chart-schema Enable schema validation of 'Chart.yaml' using Yamale (default: true) (default true) + --validate-maintainers Enable validation of maintainer account names in chart.yml (default: true). Works for GitHub, GitLab, and Bitbucket (default true) + --validate-yaml Enable linting of 'Chart.yaml' and values files (default: true) (default true) ``` ### SEE ALSO * [ct](ct.md) - The Helm chart testing tool -###### Auto generated by spf13/cobra on 17-Nov-2018 +###### Auto generated by spf13/cobra on 19-Dec-2018 diff --git a/doc/ct_version.md b/doc/ct_version.md index a8c6f68..e8a2f1b 100644 --- a/doc/ct_version.md +++ b/doc/ct_version.md @@ -20,4 +20,4 @@ ct version [flags] * [ct](ct.md) - The Helm chart testing tool -###### Auto generated by spf13/cobra on 17-Nov-2018 +###### Auto generated by spf13/cobra on 19-Dec-2018 diff --git a/pkg/chart/chart.go b/pkg/chart/chart.go index e093dfa..be34118 100644 --- a/pkg/chart/chart.go +++ b/pkg/chart/chart.go @@ -280,19 +280,23 @@ func (t *Testing) LintChart(chart string, valuesFiles []string) TestResult { chartYaml := path.Join(chart, "Chart.yaml") valuesYaml := path.Join(chart, "values.yaml") - if err := t.linter.Yamale(chartYaml, t.config.ChartYamlSchema); err != nil { - result.Error = err - return result - } - - yamlFiles := append([]string{chartYaml, valuesYaml}, valuesFiles...) - for _, yamlFile := range yamlFiles { - if err := t.linter.YamlLint(yamlFile, t.config.LintConf); err != nil { + if t.config.ValidateChartSchema { + if err := t.linter.Yamale(chartYaml, t.config.ChartYamlSchema); err != nil { result.Error = err return result } } + if t.config.ValidateYaml { + yamlFiles := append([]string{chartYaml, valuesYaml}, valuesFiles...) + for _, yamlFile := range yamlFiles { + if err := t.linter.YamlLint(yamlFile, t.config.LintConf); err != nil { + result.Error = err + return result + } + } + } + if t.config.ValidateMaintainers { if err := t.ValidateMaintainers(chart); err != nil { result.Error = err diff --git a/pkg/chart/chart_test.go b/pkg/chart/chart_test.go index f9ca9e7..519f547 100644 --- a/pkg/chart/chart_test.go +++ b/pkg/chart/chart_test.go @@ -19,12 +19,11 @@ import ( "strings" "testing" - "github.com/pkg/errors" - - "github.com/helm/chart-testing/pkg/util" - "github.com/helm/chart-testing/pkg/config" + "github.com/helm/chart-testing/pkg/util" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" ) type fakeGit struct{} @@ -94,10 +93,18 @@ func (v fakeAccountValidator) Validate(repoDomain string, account string) error return errors.New(fmt.Sprintf("Error validating account: %s", account)) } -type fakeLinter struct{} +type fakeLinter struct { + mock.Mock +} -func (l fakeLinter) YamlLint(yamlFile, configFile string) error { return nil } -func (l fakeLinter) Yamale(yamlFile, schemaFile string) error { return nil } +func (l *fakeLinter) YamlLint(yamlFile, configFile string) error { + l.Called(yamlFile, configFile) + return nil +} +func (l *fakeLinter) Yamale(yamlFile, schemaFile string) error { + l.Called(yamlFile, schemaFile) + return nil +} type fakeHelm struct{} @@ -120,13 +127,16 @@ func init() { ExcludedCharts: []string{"excluded"}, ChartDirs: []string{"stable", "incubator"}, } + + fakeMockLinter := new(fakeLinter) + ct = Testing{ config: cfg, directoryLister: fakeDirLister{}, git: fakeGit{}, chartUtils: fakeChartUtils{}, accountValidator: fakeAccountValidator{}, - linter: fakeLinter{}, + linter: fakeMockLinter, helm: fakeHelm{}, } } @@ -200,3 +210,90 @@ func TestLintChartMaintainerValidation(t *testing.T) { runTests(true) runTests(false) } + +func TestLintChartSchemaValidation(t *testing.T) { + type testData struct { + name string + chartDir string + expected bool + } + + runTests := func(validate bool, callsYamlLint int, callsYamale int) { + fakeMockLinter := new(fakeLinter) + + fakeMockLinter.On("Yamale", mock.Anything, mock.Anything).Return(true) + fakeMockLinter.On("YamlLint", mock.Anything, mock.Anything).Return(true) + + ct.linter = fakeMockLinter + ct.config.ValidateChartSchema = validate + ct.config.ValidateMaintainers = false + ct.config.ValidateYaml = false + + var suffix string + if validate { + suffix = "with-validation" + } else { + suffix = "without-validation" + } + + testCases := []testData{ + {fmt.Sprintf("schema-%s", suffix), "testdata/test_lints", true}, + } + + for _, testData := range testCases { + t.Run(testData.name, func(t *testing.T) { + result := ct.LintChart(testData.chartDir, []string{}) + assert.Equal(t, testData.expected, result.Error == nil) + fakeMockLinter.AssertNumberOfCalls(t, "Yamale", callsYamale) + fakeMockLinter.AssertNumberOfCalls(t, "YamlLint", callsYamlLint) + }) + } + } + + runTests(true, 0, 1) + runTests(false, 0, 0) + +} + +func TestLintYamlValidation(t *testing.T) { + type testData struct { + name string + chartDir string + expected bool + } + + runTests := func(validate bool, callsYamlLint int, callsYamale int) { + fakeMockLinter := new(fakeLinter) + + fakeMockLinter.On("Yamale", mock.Anything, mock.Anything).Return(true) + fakeMockLinter.On("YamlLint", mock.Anything, mock.Anything).Return(true) + + ct.linter = fakeMockLinter + ct.config.ValidateYaml = validate + ct.config.ValidateChartSchema = false + ct.config.ValidateMaintainers = false + + var suffix string + if validate { + suffix = "with-validation" + } else { + suffix = "without-validation" + } + + testCases := []testData{ + {fmt.Sprintf("lint-%s", suffix), "testdata/test_lints", true}, + } + + for _, testData := range testCases { + t.Run(testData.name, func(t *testing.T) { + result := ct.LintChart(testData.chartDir, []string{}) + assert.Equal(t, testData.expected, result.Error == nil) + fakeMockLinter.AssertNumberOfCalls(t, "Yamale", callsYamale) + fakeMockLinter.AssertNumberOfCalls(t, "YamlLint", callsYamlLint) + }) + } + } + + runTests(true, 2, 0) + runTests(false, 0, 0) +} diff --git a/pkg/chart/testdata/test_lints/Chart.yaml b/pkg/chart/testdata/test_lints/Chart.yaml new file mode 100644 index 0000000..8fbe591 --- /dev/null +++ b/pkg/chart/testdata/test_lints/Chart.yaml @@ -0,0 +1,10 @@ +apiVersion: v1 +appVersion: xoxo +description: A Helm chart for testing +name: invalid +version: 1.2.3 +home: https://github.com/helm/chart-testing +maintainers: + - name: invalid + - email: invalid@example.com + diff --git a/pkg/chart/testdata/test_lints/values.yaml b/pkg/chart/testdata/test_lints/values.yaml new file mode 100644 index 0000000..6a226d5 --- /dev/null +++ b/pkg/chart/testdata/test_lints/values.yaml @@ -0,0 +1,3 @@ +test: abc +list: + - abc diff --git a/pkg/config/config.go b/pkg/config/config.go index 9196278..01506c5 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -45,6 +45,8 @@ type Configuration struct { LintConf string `mapstructure:"lint-conf"` ChartYamlSchema string `mapstructure:"chart-yaml-schema"` ValidateMaintainers bool `mapstructure:"validate-maintainers"` + ValidateChartSchema bool `mapstructure:"validate-chart-schema"` + ValidateYaml bool `mapstructure:"validate-yaml"` CheckVersionIncrement bool `mapstructure:"check-version-increment"` ProcessAllCharts bool `mapstructure:"all"` Charts []string `mapstructure:"charts"` diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 9516636..77d1ee8 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -38,6 +38,8 @@ func loadAndAssertConfigFromFile(t *testing.T, configFile string) { require.Equal(t, "my-lint-conf.yaml", cfg.LintConf) require.Equal(t, "my-chart-yaml-schema.yaml", cfg.ChartYamlSchema) require.Equal(t, true, cfg.ValidateMaintainers) + require.Equal(t, true, cfg.ValidateChartSchema) + require.Equal(t, true, cfg.ValidateYaml) require.Equal(t, true, cfg.CheckVersionIncrement) require.Equal(t, false, cfg.ProcessAllCharts) require.Equal(t, []string{"incubator=https://incubator"}, cfg.ChartRepos) diff --git a/pkg/config/test_config.json b/pkg/config/test_config.json index 3b3f972..fb685ed 100644 --- a/pkg/config/test_config.json +++ b/pkg/config/test_config.json @@ -6,6 +6,8 @@ "chart-yaml-schema": "my-chart-yaml-schema.yaml", "github-instance": "https://github.com", "validate-maintainers": true, + "validate-chart-schema": true, + "validate-yaml": true, "check-version-increment": true, "all": false, "chart-repos": [ diff --git a/pkg/config/test_config.yaml b/pkg/config/test_config.yaml index feaf213..5e51c53 100644 --- a/pkg/config/test_config.yaml +++ b/pkg/config/test_config.yaml @@ -5,6 +5,8 @@ lint-conf: my-lint-conf.yaml chart-yaml-schema: my-chart-yaml-schema.yaml github-instance: https://github.com validate-maintainers: true +validate-chart-schema: true +validate-yaml: true check-version-increment: true all: false chart-repos: