diff --git a/ct/cmd/lint.go b/ct/cmd/lint.go index 73e6a834..fb4a3889 100644 --- a/ct/cmd/lint.go +++ b/ct/cmd/lint.go @@ -71,6 +71,7 @@ func addLintFlags(flags *flag.FlagSet) { Enable linting of 'Chart.yaml' and values files`)) flags.Bool("skip-helm-dependencies", false, heredoc.Doc(` Skip running 'helm dependency build' before linting`)) + flags.Bool("ignore-ci-changes", false, "If set, changes only in the chart's 'ci' directory will not trigger a chart version bump requirement") 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 diff --git a/doc/ct_lint-and-install.md b/doc/ct_lint-and-install.md index 0b341b99..97e33560 100644 --- a/doc/ct_lint-and-install.md +++ b/doc/ct_lint-and-install.md @@ -54,6 +54,7 @@ ct lint-and-install [flags] (e.g. 'myrepo=--username test --password secret'). May be specified multiple times or separate values with commas -h, --help help for lint-and-install + --ignore-ci-changes If set, changes only in the chart's 'ci' directory will not trigger a chart version bump requirement --lint-conf string The config file for YAML linting. If not specified, 'lintconf.yaml' is searched in the current directory, '$HOME/.ct', and '/etc/ct', in that order diff --git a/doc/ct_lint.md b/doc/ct_lint.md index 738d8001..b2b1628a 100644 --- a/doc/ct_lint.md +++ b/doc/ct_lint.md @@ -62,6 +62,7 @@ ct lint [flags] (e.g. 'myrepo=--username test --password secret'). May be specified multiple times or separate values with commas -h, --help help for lint + --ignore-ci-changes If set, changes only in the chart's 'ci' directory will not trigger a chart version bump requirement --lint-conf string The config file for YAML linting. If not specified, 'lintconf.yaml' is searched in the current directory, '$HOME/.ct', and '/etc/ct', in that order diff --git a/pkg/chart/chart.go b/pkg/chart/chart.go index d409e0a2..57accb08 100644 --- a/pkg/chart/chart.go +++ b/pkg/chart/chart.go @@ -307,9 +307,9 @@ func (t *Testing) computePreviousRevisionPath(fileOrDirPath string) string { return filepath.Join(t.previousRevisionWorktree, fileOrDirPath) } -func (t *Testing) processCharts(action func(chart *Chart) TestResult) ([]TestResult, error) { +func (t *Testing) processCharts(action func(chart *Chart, ciOnly bool) TestResult) ([]TestResult, error) { var results []TestResult // nolint: prealloc - chartDirs, err := t.FindChartDirsToBeProcessed() + chartDirs, ciOnlyCharts, err := t.findChartDirsToBeProcessed() if err != nil { return nil, fmt.Errorf("failed identifying charts to process: %w", err) } else if len(chartDirs) == 0 { @@ -408,7 +408,8 @@ func (t *Testing) processCharts(action func(chart *Chart) TestResult) ([]TestRes } } - result := action(chart) + _, ciOnly := ciOnlyCharts[chart.Path()] + result := action(chart, ciOnly) if result.Error != nil { testResults.OverallSuccess = false } @@ -423,17 +424,19 @@ func (t *Testing) processCharts(action func(chart *Chart) TestResult) ([]TestRes // LintCharts lints charts (changed, all, specific) depending on the configuration. func (t *Testing) LintCharts() ([]TestResult, error) { - return t.processCharts(t.LintChart) + return t.processCharts(t.lintChart) } // InstallCharts install charts (changed, all, specific) depending on the configuration. func (t *Testing) InstallCharts() ([]TestResult, error) { - return t.processCharts(t.InstallChart) + return t.processCharts(func(chart *Chart, _ bool) TestResult { + return t.InstallChart(chart) + }) } // LintAndInstallCharts first lints and then installs charts (changed, all, specific) depending on the configuration. func (t *Testing) LintAndInstallCharts() ([]TestResult, error) { - return t.processCharts(t.LintAndInstallChart) + return t.processCharts(t.lintAndInstallChart) } // PrintResults writes test results to stdout. @@ -465,12 +468,18 @@ func (t *Testing) PrintResults(results []TestResult) { // LintChart lints the specified chart. func (t *Testing) LintChart(chart *Chart) TestResult { + return t.lintChart(chart, false) +} + +func (t *Testing) lintChart(chart *Chart, ciOnly bool) TestResult { fmt.Printf("Linting chart %q\n", chart) result := TestResult{Chart: chart} if t.config.CheckVersionIncrement { - if err := t.CheckVersionIncrement(chart); err != nil { + if ciOnly { + fmt.Printf("Skipping version increment check for %q because only 'ci' directory files changed\n", chart) + } else if err := t.CheckVersionIncrement(chart); err != nil { result.Error = err return result } @@ -713,7 +722,11 @@ func (t *Testing) generateInstallConfig(chart *Chart) (namespace, release, relea // LintAndInstallChart first lints and then installs the specified chart. func (t *Testing) LintAndInstallChart(chart *Chart) TestResult { - result := t.LintChart(chart) + return t.lintAndInstallChart(chart, false) +} + +func (t *Testing) lintAndInstallChart(chart *Chart, ciOnly bool) TestResult { + result := t.lintChart(chart, ciOnly) if result.Error != nil { return result } @@ -723,13 +736,19 @@ func (t *Testing) LintAndInstallChart(chart *Chart) TestResult { // FindChartDirsToBeProcessed identifies charts to be processed depending on the configuration // (changed charts, all charts, or specific charts). func (t *Testing) FindChartDirsToBeProcessed() ([]string, error) { + dirs, _, err := t.findChartDirsToBeProcessed() + return dirs, err +} + +func (t *Testing) findChartDirsToBeProcessed() ([]string, map[string]struct{}, error) { cfg := t.config if cfg.ProcessAllCharts { - return t.ReadAllChartDirectories() + dirs, err := t.ReadAllChartDirectories() + return dirs, nil, err } else if len(cfg.Charts) > 0 { - return t.config.Charts, nil + return cfg.Charts, nil, nil } - return t.ComputeChangedChartDirectories() + return t.computeChangedChartDirectories() } func (t *Testing) computeMergeBase() (string, error) { @@ -749,16 +768,23 @@ func (t *Testing) computeMergeBase() (string, error) { // ComputeChangedChartDirectories takes the merge base of HEAD and the configured remote and target branch and computes a // slice of changed charts from that in the configured chart directories excluding those configured to be excluded. func (t *Testing) ComputeChangedChartDirectories() ([]string, error) { + dirs, _, err := t.computeChangedChartDirectories() + return dirs, err +} + +// computeChangedChartDirectories is the internal implementation that also returns +// a set of chart directories where only files in the 'ci' directory changed. +func (t *Testing) computeChangedChartDirectories() ([]string, map[string]struct{}, error) { cfg := t.config mergeBase, err := t.computeMergeBase() if err != nil { - return nil, err + return nil, nil, err } allChangedChartFiles, err := t.git.ListChangedFilesInDirs(mergeBase, cfg.ChartDirs...) if err != nil { - return nil, fmt.Errorf("failed creating diff: %w", err) + return nil, nil, fmt.Errorf("failed creating diff: %w", err) } changedChartFiles := map[string][]string{} @@ -784,28 +810,49 @@ func (t *Testing) ComputeChangedChartDirectories() ([]string, error) { } } - changedChartDirs := []string{} + // Apply helmignore filtering if t.config.UseHelmignore { - for chartDir, changedChartFiles := range changedChartFiles { + for chartDir, files := range changedChartFiles { rules, err := t.loadRules(chartDir) if err != nil { - return nil, err + return nil, nil, err } - filteredChartFiles, err := ignore.FilterFiles(changedChartFiles, rules) + filteredFiles, err := ignore.FilterFiles(files, rules) if err != nil { - return nil, err + return nil, nil, err } - if len(filteredChartFiles) > 0 { - changedChartDirs = append(changedChartDirs, chartDir) + if len(filteredFiles) > 0 { + changedChartFiles[chartDir] = filteredFiles + } else { + delete(changedChartFiles, chartDir) } } - } else { - for chartDir := range changedChartFiles { - changedChartDirs = append(changedChartDirs, chartDir) + } + + // Detect charts where only 'ci' directory files changed (after helmignore filtering) + var ciOnlyCharts map[string]struct{} + if cfg.IgnoreCIChanges { + ciOnlyCharts = make(map[string]struct{}) + for chartDir, files := range changedChartFiles { + allCI := true + for _, f := range files { + if !strings.HasPrefix(filepath.ToSlash(f), "ci/") { + allCI = false + break + } + } + if allCI { + ciOnlyCharts[chartDir] = struct{}{} + } } } - return changedChartDirs, nil + changedChartDirs := make([]string, 0, len(changedChartFiles)) + for chartDir := range changedChartFiles { + changedChartDirs = append(changedChartDirs, chartDir) + } + + return changedChartDirs, ciOnlyCharts, nil } // ReadAllChartDirectories returns a slice of all charts in the configured chart directories except those diff --git a/pkg/chart/chart_test.go b/pkg/chart/chart_test.go index b95dd9ee..93821cc5 100644 --- a/pkg/chart/chart_test.go +++ b/pkg/chart/chart_test.go @@ -75,6 +75,19 @@ func (g fakeGit) BranchExists(_ string) bool { return true } +type fakeGitCIOnlyChanges struct{ fakeGit } + +func (g fakeGitCIOnlyChanges) Show(_ string, _ string, _ string) (string, error) { + return "name: test\nversion: 1.0.0\n", nil +} + +func (g fakeGitCIOnlyChanges) ListChangedFilesInDirs(_ string, _ ...string) ([]string, error) { + return []string{ + "test_charts/foo/ci/test-values.yaml", + "test_charts/bar/Chart.yaml", + }, nil +} + type fakeAccountValidator struct{} func (v fakeAccountValidator) Validate(_ string, account string) error { @@ -227,6 +240,78 @@ func TestComputeChangedChartDirectoriesWithMultiLevelChartWithHelmIgnore(t *test assert.ElementsMatch(t, expected, actual) } +func TestComputeChangedChartDirectoriesIgnoreCIChanges(t *testing.T) { + cfg := config.Configuration{ + ExcludedCharts: []string{"excluded"}, + ChartDirs: []string{"test_charts", "."}, + IgnoreCIChanges: true, + } + ct := newTestingMock(cfg) + ct.git = fakeGitCIOnlyChanges{} + actual, err := ct.ComputeChangedChartDirectories() + assert.Nil(t, err) + // Both charts are still returned — CI-only charts are not excluded from change detection + assert.ElementsMatch(t, []string{"test_charts/foo", "test_charts/bar"}, actual) +} + +func TestLintChartsIgnoreCIChanges(t *testing.T) { + cfg := config.Configuration{ + ExcludedCharts: []string{"excluded"}, + ChartDirs: []string{"test_charts"}, + IgnoreCIChanges: true, + CheckVersionIncrement: true, + SkipHelmDependencies: true, + } + ct := newTestingMock(cfg) + ct.git = fakeGitCIOnlyChanges{} + + results, err := ct.LintCharts() + // bar fails version check, so overall error is returned + assert.NotNil(t, err) + assert.Len(t, results, 2) + + for _, result := range results { + switch result.Chart.Path() { + case "test_charts/foo": + // CI-only: version check skipped, lint passes + assert.Nil(t, result.Error) + case "test_charts/bar": + // Non-CI-only: version check runs and fails (fakeGit returns old version 1.0.0) + assert.NotNil(t, result.Error) + default: + t.Fatalf("unexpected chart: %s", result.Chart.Path()) + } + } +} + +func TestLintChartsVersionCheckWithoutIgnoreCIChanges(t *testing.T) { + cfg := config.Configuration{ + ExcludedCharts: []string{"excluded"}, + ChartDirs: []string{"test_charts"}, + IgnoreCIChanges: false, + CheckVersionIncrement: true, + SkipHelmDependencies: true, + } + ct := newTestingMock(cfg) + ct.git = fakeGitCIOnlyChanges{} + + results, err := ct.LintCharts() + assert.NotNil(t, err) + assert.Len(t, results, 2) + + for _, result := range results { + switch result.Chart.Path() { + case "test_charts/foo": + // Without --ignore-ci-changes, CI-only chart still gets version-checked and fails + assert.NotNil(t, result.Error) + case "test_charts/bar": + assert.NotNil(t, result.Error) + default: + t.Fatalf("unexpected chart: %s", result.Chart.Path()) + } + } +} + func TestReadAllChartDirectories(t *testing.T) { actual, err := ct.ReadAllChartDirectories() expected := []string{ diff --git a/pkg/config/config.go b/pkg/config/config.go index 981b2762..d00f34fd 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -77,6 +77,7 @@ type Configuration struct { PrintLogs bool `mapstructure:"print-logs"` GithubGroups bool `mapstructure:"github-groups"` UseHelmignore bool `mapstructure:"use-helmignore"` + IgnoreCIChanges bool `mapstructure:"ignore-ci-changes"` } func LoadConfiguration(cfgFile string, cmd *cobra.Command, printConfig bool) (*Configuration, error) { diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index f07d912e..e0564549 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -62,6 +62,7 @@ func loadAndAssertConfigFromFile(t *testing.T, configFile string) { require.Equal(t, 120*time.Second, cfg.KubectlTimeout) require.Equal(t, true, cfg.SkipCleanUp) require.Equal(t, true, cfg.UseHelmignore) + require.Equal(t, true, cfg.IgnoreCIChanges) } func Test_findConfigFile(t *testing.T) { diff --git a/pkg/config/test_config.json b/pkg/config/test_config.json index 73cd4574..d5e6c0a0 100644 --- a/pkg/config/test_config.json +++ b/pkg/config/test_config.json @@ -33,5 +33,6 @@ "exclude-deprecated": true, "kubectl-timeout": "120s", "skip-clean-up": true, - "use-helmignore": true + "use-helmignore": true, + "ignore-ci-changes": true } diff --git a/pkg/config/test_config.yaml b/pkg/config/test_config.yaml index 0d1c7c02..2cb006bb 100644 --- a/pkg/config/test_config.yaml +++ b/pkg/config/test_config.yaml @@ -29,3 +29,4 @@ exclude-deprecated: true kubectl-timeout: 120s skip-clean-up: true use-helmignore: true +ignore-ci-changes: true