diff --git a/pkg/app/app.go b/pkg/app/app.go index f4eede7d..1ee1cb48 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -1007,7 +1007,7 @@ func (a *App) processStateFileParallel(relPath string, defOpts LoadOpts, converg func (a *App) processNestedHelmfiles(st *state.HelmState, absd, file string, defOpts, opts LoadOpts, converge func(*state.HelmState) (bool, []error), sharedCtx *Context) (bool, error) { anyMatched := false for i, m := range st.Helmfiles { - if subhelmfileSelectorsConflict(a.Selectors, m) { + if subhelmfileSelectorsConflict(a.Selectors, m, a.Logger) { a.Logger.Debugf("skipping subhelmfile %q: CLI selectors %v conflict with subhelmfile selectors %v", m.Path, a.Selectors, m.Selectors) continue } @@ -1044,11 +1044,14 @@ func (a *App) processNestedHelmfiles(st *state.HelmState, absd, file string, def // Only CLI selectors (not inherited parent selectors) are used for comparison, // so this optimization is restricted to cases where the user explicitly // provided selectors via the command line (e.g. -l name=b). -func subhelmfileSelectorsConflict(cliSelectors []string, m state.SubHelmfileSpec) bool { +func subhelmfileSelectorsConflict(cliSelectors []string, m state.SubHelmfileSpec, logger *zap.SugaredLogger) bool { if len(cliSelectors) == 0 || len(m.Selectors) == 0 || m.SelectorsInherited { return false } - compatible, _ := state.SelectorsAreCompatible(cliSelectors, m.Selectors) + compatible, err := state.SelectorsAreCompatible(cliSelectors, m.Selectors) + if err != nil { + logger.Debugf("selector compatibility check failed for subhelmfile %q: %v", m.Path, err) + } return !compatible } diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index efcd8ab4..aa792b33 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -928,7 +928,7 @@ func TestSubhelmfileSelectorsConflict(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := subhelmfileSelectorsConflict(tt.cliSelectors, tt.spec) + got := subhelmfileSelectorsConflict(tt.cliSelectors, tt.spec, newAppTestLogger()) assert.Equal(t, tt.conflict, got) }) } diff --git a/pkg/state/release_filters.go b/pkg/state/release_filters.go index dfc482e7..4371ac6e 100644 --- a/pkg/state/release_filters.go +++ b/pkg/state/release_filters.go @@ -105,6 +105,11 @@ func (l LabelFilter) positiveLabelsCompatibleWith(other LabelFilter) bool { return true } +var ( + reLabelMismatch = regexp.MustCompile(`^[a-zA-Z0-9_\.\/\+-]+!=[a-zA-Z0-9_\.\/\+-]+$`) + reLabelMatch = regexp.MustCompile(`^[a-zA-Z0-9_\.\/\+-]+=[a-zA-Z0-9_\.\/\+-]+$`) +) + // ParseLabels takes a label in the form foo=bar,baz!=bat and returns a LabelFilter that will match the labels func ParseLabels(l string) (LabelFilter, error) { lf := LabelFilter{} @@ -112,16 +117,14 @@ func ParseLabels(l string) (LabelFilter, error) { lf.negativeLabels = [][]string{} var err error labels := strings.Split(l, ",") - reMissmatch := regexp.MustCompile(`^[a-zA-Z0-9_\.\/\+-]+!=[a-zA-Z0-9_\.\/\+-]+$`) - reMatch := regexp.MustCompile(`^[a-zA-Z0-9_\.\/\+-]+=[a-zA-Z0-9_\.\/\+-]+$`) for _, label := range labels { - if match := reMissmatch.MatchString(label); match { // k!=v case + if match := reLabelMismatch.MatchString(label); match { kv := strings.Split(label, "!=") lf.negativeLabels = append(lf.negativeLabels, kv) - } else if match := reMatch.MatchString(label); match { // k=v case + } else if match := reLabelMatch.MatchString(label); match { kv := strings.Split(label, "=") lf.positiveLabels = append(lf.positiveLabels, kv) - } else { // malformed case + } else { return lf, fmt.Errorf("malformed label: %s. Expected label in form k=v or k!=v", label) } } diff --git a/test/integration/test-cases/issue-2544.sh b/test/integration/test-cases/issue-2544.sh index e42a82eb..978808e9 100644 --- a/test/integration/test-cases/issue-2544.sh +++ b/test/integration/test-cases/issue-2544.sh @@ -9,7 +9,6 @@ issue_2544_tmp_dir=$(mktemp -d) cleanup_issue_2544() { rm -rf "${issue_2544_tmp_dir}" } -trap cleanup_issue_2544 EXIT test_start "issue-2544: subhelmfiles skipped when selectors conflict with CLI" @@ -19,19 +18,20 @@ info "Running helmfile template -l app=b" ${helmfile} -f "${issue_2544_input_dir}/helmfile.yaml" template -l app=b \ > "${issue_2544_tmp_dir}/output-b.txt" 2>&1 || { cat "${issue_2544_tmp_dir}/output-b.txt" + cleanup_issue_2544 fail "helmfile template -l app=b failed" } # Should contain release-b grep -q "release-b" "${issue_2544_tmp_dir}/output-b.txt" \ - || fail "output should contain release-b (matching selector app=b)" + || { cleanup_issue_2544; fail "output should contain release-b (matching selector app=b)"; } # Should NOT contain release-a or release-c (their subhelmfiles have incompatible selectors) grep -q "release-a" "${issue_2544_tmp_dir}/output-b.txt" \ - && fail "output should NOT contain release-a (subhelmfile selectors app=a conflict with app=b)" + && { cleanup_issue_2544; fail "output should NOT contain release-a (subhelmfile selectors app=a conflict with app=b)"; } grep -q "release-c" "${issue_2544_tmp_dir}/output-b.txt" \ - && fail "output should NOT contain release-c (subhelmfile selectors app=c conflict with app=b)" + && { cleanup_issue_2544; fail "output should NOT contain release-c (subhelmfile selectors app=c conflict with app=b)"; } info "PASS: -l app=b produces only release-b" @@ -41,15 +41,16 @@ info "Running helmfile template without selector" ${helmfile} -f "${issue_2544_input_dir}/helmfile.yaml" template \ > "${issue_2544_tmp_dir}/output-all.txt" 2>&1 || { cat "${issue_2544_tmp_dir}/output-all.txt" + cleanup_issue_2544 fail "helmfile template without selector failed" } grep -q "release-a" "${issue_2544_tmp_dir}/output-all.txt" \ - || fail "output should contain release-a" + || { cleanup_issue_2544; fail "output should contain release-a"; } grep -q "release-b" "${issue_2544_tmp_dir}/output-all.txt" \ - || fail "output should contain release-b" + || { cleanup_issue_2544; fail "output should contain release-b"; } grep -q "release-c" "${issue_2544_tmp_dir}/output-all.txt" \ - || fail "output should contain release-c" + || { cleanup_issue_2544; fail "output should contain release-c"; } info "PASS: no selector produces all releases" @@ -59,19 +60,20 @@ info "Running helmfile template -l app=a" ${helmfile} -f "${issue_2544_input_dir}/helmfile.yaml" template -l app=a \ > "${issue_2544_tmp_dir}/output-a.txt" 2>&1 || { cat "${issue_2544_tmp_dir}/output-a.txt" + cleanup_issue_2544 fail "helmfile template -l app=a failed" } grep -q "release-a" "${issue_2544_tmp_dir}/output-a.txt" \ - || fail "output should contain release-a (matching selector app=a)" + || { cleanup_issue_2544; fail "output should contain release-a (matching selector app=a)"; } grep -q "release-b" "${issue_2544_tmp_dir}/output-a.txt" \ - && fail "output should NOT contain release-b (subhelmfile selectors app=b conflict with app=a)" + && { cleanup_issue_2544; fail "output should NOT contain release-b (subhelmfile selectors app=b conflict with app=a)"; } grep -q "release-c" "${issue_2544_tmp_dir}/output-a.txt" \ - && fail "output should NOT contain release-c (subhelmfile selectors app=c conflict with app=a)" + && { cleanup_issue_2544; fail "output should NOT contain release-c (subhelmfile selectors app=c conflict with app=a)"; } info "PASS: -l app=a produces only release-a" -trap - EXIT +cleanup_issue_2544 test_pass "issue-2544: subhelmfiles skipped when selectors conflict with CLI"