From 3f13cbc850226f9ccd69663078b6f300461b98a6 Mon Sep 17 00:00:00 2001 From: Mats Willemsen Date: Wed, 24 Dec 2025 10:25:49 +0100 Subject: [PATCH] fix(ignore): correct negation pattern handling in .helmignore The negation pattern logic in Ignore() was broken in two ways: 1. Inverted logic: when a file did NOT match a negation pattern, the function incorrectly returned true (ignore the file) 2. Early termination: the function returned immediately on the first matching rule, preventing later negation rules from overriding earlier exclusion rules This caused critical files like Chart.yaml to be incorrectly ignored when any negation pattern existed in .helmignore, breaking helm lint, helm template, and helm install with 'Chart.yaml file is missing'. The fix rewrites Ignore() to follow proper gitignore semantics: - Process all rules in order, tracking the ignore state - The last matching rule wins - Negation patterns set ignored=false when they match - Regular patterns set ignored=true when they match Example .helmignore that now works correctly: *.md !README.md Before: Chart.yaml was incorrectly ignored After: Only .md files (except README.md) are ignored Signed-off-by: Mats Willemsen --- pkg/ignore/rules.go | 29 ++++++------- pkg/ignore/rules_test.go | 89 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 98 insertions(+), 20 deletions(-) diff --git a/pkg/ignore/rules.go b/pkg/ignore/rules.go index a8160da2a..108bb6b29 100644 --- a/pkg/ignore/rules.go +++ b/pkg/ignore/rules.go @@ -85,8 +85,9 @@ func Parse(file io.Reader) (*Rules, error) { // Ignore evaluates the file at the given path, and returns true if it should be ignored. // -// Ignore evaluates path against the rules in order. Evaluation stops when a match -// is found. Matching a negative rule will stop evaluation. +// Ignore evaluates path against all rules in order. Later rules can override earlier ones, +// following gitignore semantics where negation patterns (starting with !) can re-include +// previously excluded files. func (r *Rules) Ignore(path string, fi os.FileInfo) bool { // Don't match on empty dirs. if path == "" { @@ -99,34 +100,30 @@ func (r *Rules) Ignore(path string, fi os.FileInfo) bool { if path == "." || path == "./" { return false } + + // Track whether the file should be ignored. Start with false (not ignored), + // and let each matching rule update this value. The last matching rule wins. + ignored := false + for _, p := range r.patterns { if p.match == nil { slog.Info("this will be ignored no matcher supplied", "patterns", p.raw) return false } - // For negative rules, we need to capture and return non-matches, - // and continue for matches. - if p.negate { - if p.mustDir && !fi.IsDir() { - return true - } - if !p.match(path, fi) { - return true - } - continue - } - // If the rule is looking for directories, and this is not a directory, // skip it. if p.mustDir && !fi.IsDir() { continue } + if p.match(path, fi) { - return true + // For negation rules, matching means "don't ignore this file" + // For regular rules, matching means "ignore this file" + ignored = !p.negate } } - return false + return ignored } // parseRule parses a rule string and creates a pattern, which is then stored in the Rules object. diff --git a/pkg/ignore/rules_test.go b/pkg/ignore/rules_test.go index 9581cf09f..df318b4cb 100644 --- a/pkg/ignore/rules_test.go +++ b/pkg/ignore/rules_test.go @@ -112,11 +112,11 @@ func TestIgnore(t *testing.T) { {`cargo/`, "mast/", false}, {`helm.txt/`, "helm.txt", false}, - // Negation tests + // Negation tests (single pattern - negation alone doesn't ignore non-matches) {`!helm.txt`, "helm.txt", false}, - {`!helm.txt`, "tiller.txt", true}, - {`!*.txt`, "cargo", true}, - {`!cargo/`, "mast/", true}, + {`!helm.txt`, "tiller.txt", false}, + {`!*.txt`, "cargo", false}, + {`!cargo/`, "mast/", false}, // Absolute path tests {`/a.txt`, "a.txt", true}, @@ -149,6 +149,87 @@ func TestAddDefaults(t *testing.T) { } } +func TestNegationPatterns(t *testing.T) { + // Test multi-rule scenarios with negation patterns (gitignore semantics) + tests := []struct { + name string + rules string + file string + expected bool + }{ + // Basic negation: *.txt ignored, but README.txt is un-ignored + { + name: "negation re-includes previously excluded file", + rules: "*.txt\n!helm.txt", + file: "helm.txt", + expected: false, // Should NOT be ignored (negation re-includes it) + }, + { + name: "file matching exclusion but not negation is ignored", + rules: "*.txt\n!helm.txt", + file: "tiller.txt", + expected: true, // Should be ignored (matches *.txt, doesn't match negation) + }, + { + name: "unrelated file is not affected by negation", + rules: "*.txt\n!helm.txt", + file: "cargo", + expected: false, // Should NOT be ignored (doesn't match *.txt) + }, + // Multiple negations + { + name: "multiple negations - first negated file", + rules: "*.txt\n!helm.txt\n!tiller.txt", + file: "helm.txt", + expected: false, + }, + { + name: "multiple negations - second negated file", + rules: "*.txt\n!helm.txt\n!tiller.txt", + file: "tiller.txt", + expected: false, + }, + { + name: "multiple negations - non-negated file still ignored", + rules: "*.txt\n!helm.txt\n!tiller.txt", + file: "rudder.txt", + expected: true, + }, + // Negation pattern alone should not ignore anything + { + name: "negation alone does not ignore non-matching files", + rules: "!helm.txt", + file: "tiller.txt", + expected: false, + }, + { + name: "negation alone explicitly un-ignores matching files", + rules: "!helm.txt", + file: "helm.txt", + expected: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + r, err := parseString(test.rules) + if err != nil { + t.Fatalf("Failed to parse rules: %s", err) + } + fi, err := os.Stat(filepath.Join(testdata, test.file)) + if err != nil { + t.Fatalf("Fixture missing: %s", err) + } + + result := r.Ignore(test.file, fi) + if result != test.expected { + t.Errorf("Expected Ignore(%q) to be %v for rules %q, got %v", + test.file, test.expected, test.rules, result) + } + }) + } +} + func parseString(str string) (*Rules, error) { b := bytes.NewBuffer([]byte(str)) return Parse(b)