Add option to skip upgrade testing of deleted/renamed values files (#132)

* feat(upgrade): allow skipping missing values files

This allows fixing previous chart versions in a single pull request
without applying a major version bump to the chart version.

Signed-off-by: Jacob LeGrone <[email protected]>

* test(skip-missing-values): validate config and HasCIValuesFile method

Signed-off-by: Jacob LeGrone <[email protected]>

* docs(skip-missing-values): generate documentation

Signed-off-by: Jacob LeGrone <[email protected]>

* fix(chart): switch to filepath from path

Signed-off-by: Jacob LeGrone <[email protected]>

Author: jlegrone

ct/cmd/install.go

@@ -68,6 +68,10 @@ func addInstallFlags(flags *flag.FlagSet) {
 	flags.Bool("upgrade", false, heredoc.Doc(`
 		Whether to test an in-place upgrade of each chart from its previous revision if the
 		current version should not introduce a breaking change according to the SemVer spec`))
+	flags.Bool("skip-missing-values", false, heredoc.Doc(`
+		When --upgrade has been passed, this flag will skip testing CI values files from the
+		previous chart revision if they have been deleted or renamed at the current chart
+		revision`))
 	flags.String("namespace", "", heredoc.Doc(`
 		Namespace to install the release(s) into. If not specified, each release will be
 		installed in its own randomly generated namespace`))

doc/ct.md

@@ -26,4 +26,4 @@ in given chart directories.
 * [ct list-changed](ct_list-changed.md)	 - List changed charts
 * [ct version](ct_version.md)	 - Print version information
 
-###### Auto generated by spf13/cobra on 26-Feb-2019
+###### Auto generated by spf13/cobra on 21-Mar-2019

doc/ct_install.md

@@ -59,6 +59,9 @@ ct install [flags]
       --release-label string           The label to be used as a selector when inspecting resources created by charts.
                                        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")
+      --skip-missing-values            When --upgrade has been passed, this flag will skip testing CI values files from the
+                                       previous chart revision if they have been deleted or renamed at the current chart
+                                       revision
       --target-branch string           The name of the target branch used to identify changed charts (default "master")
       --upgrade                        Whether to test an in-place upgrade of each chart from its previous revision if the
                                        current version should not introduce a breaking change according to the SemVer spec
@@ -68,4 +71,4 @@ ct install [flags]
 
 * [ct](ct.md)	 - The Helm chart testing tool
 
-###### Auto generated by spf13/cobra on 26-Feb-2019
+###### Auto generated by spf13/cobra on 21-Mar-2019

doc/ct_lint-and-install.md

@@ -50,6 +50,9 @@ ct lint-and-install [flags]
       --release-label string           The label to be used as a selector when inspecting resources created by charts.
                                        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")
+      --skip-missing-values            When --upgrade has been passed, this flag will skip testing CI values files from the
+                                       previous chart revision if they have been deleted or renamed at the current chart
+                                       revision
       --target-branch string           The name of the target branch used to identify changed charts (default "master")
       --upgrade                        Whether to test an in-place upgrade of each chart from its previous revision if the
                                        current version should not introduce a breaking change according to the SemVer spec
@@ -63,4 +66,4 @@ ct lint-and-install [flags]
 
 * [ct](ct.md)	 - The Helm chart testing tool
 
-###### Auto generated by spf13/cobra on 26-Feb-2019
+###### Auto generated by spf13/cobra on 21-Mar-2019

doc/ct_lint.md

@@ -65,4 +65,4 @@ ct lint [flags]
 
 * [ct](ct.md)	 - The Helm chart testing tool
 
-###### Auto generated by spf13/cobra on 26-Feb-2019
+###### Auto generated by spf13/cobra on 21-Mar-2019

doc/ct_list-changed.md

@@ -28,4 +28,4 @@ ct list-changed [flags]
 
 * [ct](ct.md)	 - The Helm chart testing tool
 
-###### Auto generated by spf13/cobra on 26-Feb-2019
+###### Auto generated by spf13/cobra on 21-Mar-2019

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 26-Feb-2019
+###### Auto generated by spf13/cobra on 21-Mar-2019

pkg/chart/chart.go

@@ -16,7 +16,6 @@ package chart
 
 import (
 	"fmt"
-	"path"
 	"path/filepath"
 	"strings"
 
@@ -175,10 +174,21 @@ func (c *Chart) ValuesFilePathsForCI() []string {
 	return c.ciValuesPaths
 }
 
+// HasCIValuesFile checks whether a given CI values file is present.
+func (c *Chart) HasCIValuesFile(path string) bool {
+	fileName := filepath.Base(path)
+	for _, file := range c.ValuesFilePathsForCI() {
+		if fileName == filepath.Base(file) {
+			return true
+		}
+	}
+	return false
+}
+
 // CreateInstallParams generates a randomized release name and namespace based on the chart path
 // and optional buildID. If a buildID is specified, it will be part of the generated namespace.
 func (c *Chart) CreateInstallParams(buildID string) (release string, namespace string) {
-	release = path.Base(c.Path())
+	release = filepath.Base(c.Path())
 	if release == "." || release == "/" {
 		yaml := c.Yaml()
 		release = yaml.Name
@@ -200,7 +210,7 @@ func NewChart(chartPath string) (*Chart, error) {
 	if err != nil {
 		return nil, err
 	}
-	matches, _ := filepath.Glob(path.Join(chartPath, "ci/*-values.yaml"))
+	matches, _ := filepath.Glob(filepath.Join(chartPath, "ci", "*-values.yaml"))
 	return &Chart{chartPath, yaml, matches}, nil
 }
 
@@ -248,7 +258,7 @@ const ctPreviousRevisionTree = "ct_previous_revision"
 // computePreviousRevisionPath converts any file or directory path to the same path in the
 // previous revision's working tree.
 func computePreviousRevisionPath(fileOrDirPath string) string {
-	return path.Join(ctPreviousRevisionTree, fileOrDirPath)
+	return filepath.Join(ctPreviousRevisionTree, fileOrDirPath)
 }
 
 func (t *Testing) processCharts(action func(chart *Chart) TestResult) ([]TestResult, error) {
@@ -389,8 +399,8 @@ func (t *Testing) LintChart(chart *Chart) TestResult {
 		}
 	}
 
-	chartYaml := path.Join(chart.Path(), "Chart.yaml")
-	valuesYaml := path.Join(chart.Path(), "values.yaml")
+	chartYaml := filepath.Join(chart.Path(), "Chart.yaml")
+	valuesYaml := filepath.Join(chart.Path(), "values.yaml")
 	valuesFiles := chart.ValuesFilePathsForCI()
 
 	if t.config.ValidateChartSchema {
@@ -530,7 +540,11 @@ func (t *Testing) doUpgrade(oldChart, newChart *Chart, oldChartMustPass bool) er
 	}
 	for _, valuesFile := range valuesFiles {
 		if valuesFile != "" {
-			fmt.Printf("\nInstalling chart '%s' with values file '%s'...\n\n", oldChart.Path(), valuesFile)
+			if t.config.SkipMissingValues && !newChart.HasCIValuesFile(valuesFile) {
+				fmt.Printf("Upgrade testing for values file '%s' skipped because a corresponding values file was not found in %s/ci", valuesFile, newChart.Path())
+				continue
+			}
+			fmt.Printf("\nInstalling chart '%s' with values file '%s'...\n\n", oldChart, valuesFile)
 		}
 
 		// Use anonymous function. Otherwise deferred calls would pile up
@@ -677,7 +691,7 @@ func (t *Testing) ReadAllChartDirectories() ([]string, error) {
 		dirs, err := t.directoryLister.ListChildDirs(chartParentDir,
 			func(dir string) bool {
 				_, err := t.chartUtils.LookupChartDir(cfg.ChartDirs, dir)
-				return err == nil && !util.StringSliceContains(cfg.ExcludedCharts, path.Base(dir))
+				return err == nil && !util.StringSliceContains(cfg.ExcludedCharts, filepath.Base(dir))
 			})
 		if err != nil {
 			return nil, errors.Wrap(err, "Error reading chart directories")
@@ -739,7 +753,7 @@ func (t *Testing) checkBreakingChangeAllowed(chart *Chart) (allowed bool, err er
 func (t *Testing) GetOldChartVersion(chartPath string) (string, error) {
 	cfg := t.config
 
-	chartYamlFile := path.Join(chartPath, "Chart.yaml")
+	chartYamlFile := filepath.Join(chartPath, "Chart.yaml")
 	if !t.git.FileExistsOnBranch(chartYamlFile, cfg.Remote, cfg.TargetBranch) {
 		fmt.Printf("Unable to find chart on %s. New chart detected.\n", cfg.TargetBranch)
 		return "", nil

pkg/chart/chart_test.go

@@ -379,3 +379,46 @@ func TestGenerateInstallConfig(t *testing.T) {
 		})
 	}
 }
+
+func TestChart_HasCIValuesFile(t *testing.T) {
+	type testData struct {
+		name     string
+		chart    *Chart
+		file     string
+		expected bool
+	}
+
+	testCases := []testData{
+		{
+			name: "has file",
+			chart: &Chart{
+				ciValuesPaths: []string{"foo-values.yaml"},
+			},
+			file:     "foo-values.yaml",
+			expected: true,
+		},
+		{
+			name: "different paths",
+			chart: &Chart{
+				ciValuesPaths: []string{"ci/foo-values.yaml"},
+			},
+			file:     "foo/bar/foo-values.yaml",
+			expected: true,
+		},
+		{
+			name: "does not have file",
+			chart: &Chart{
+				ciValuesPaths: []string{"foo-values.yaml"},
+			},
+			file:     "bar-values.yaml",
+			expected: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			actual := tc.chart.HasCIValuesFile(tc.file)
+			assert.Equal(t, tc.expected, actual)
+		})
+	}
+}

pkg/config/config.go

@@ -57,6 +57,7 @@ type Configuration struct {
 	HelmRepoExtraArgs     []string `mapstructure:"helm-repo-extra-args"`
 	Debug                 bool     `mapstructure:"debug"`
 	Upgrade               bool     `mapstructure:"upgrade"`
+	SkipMissingValues     bool     `mapstructure:"skip-missing-values"`
 	Namespace             string   `mapstructure:"namespace"`
 	ReleaseLabel          string   `mapstructure:"release-label"`
 }

pkg/config/config_test.go

@@ -50,6 +50,7 @@ func loadAndAssertConfigFromFile(t *testing.T, configFile string) {
 	require.Equal(t, []string{"common"}, cfg.ExcludedCharts)
 	require.Equal(t, "--timeout 300", cfg.HelmExtraArgs)
 	require.Equal(t, true, cfg.Upgrade)
+	require.Equal(t, true, cfg.SkipMissingValues)
 	require.Equal(t, "default", cfg.Namespace)
 	require.Equal(t, "release", cfg.ReleaseLabel)
 }

pkg/config/test_config.json

@@ -25,6 +25,7 @@
     ],
     "helm-extra-args": "--timeout 300",
     "upgrade": true,
+    "skip-missing-values": true,
     "namespace": "default",
     "release-label": "release"
 }

pkg/config/test_config.yaml

@@ -20,5 +20,6 @@ excluded-charts:
   - common
 helm-extra-args: --timeout 300
 upgrade: true
+skip-missing-values: true
 namespace: default
 release-label: release