fix: skip subhelmfiles when selectors conflict with CLI selectors (#2544)

When CLI selectors are provided (e.g. -l name=b), subhelmfiles whose
explicit selectors are provably incompatible are now skipped entirely,
avoiding unnecessary YAML loading and template rendering.

Two selector sets are incompatible when every pair has a positive label
conflict: same key with different values (e.g. name=b vs name=a).
Negative labels are not compared.

Fixes #2544

Signed-off-by: yxxhero <aiopsclub@163.com>

pkg/app/app.go

@@ -1007,6 +1007,10 @@ 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(opts.Selectors, m) {
+			continue
+		}
+
 		optsForNestedState := LoadOpts{
 			CalleePath:        filepath.Join(absd, file),
 			Environment:       m.Environment,
@@ -1032,6 +1036,18 @@ func (a *App) processNestedHelmfiles(st *state.HelmState, absd, file string, def
 	return anyMatched, nil
 }
 
+// subhelmfileSelectorsConflict returns true when the subhelmfile has explicit
+// selectors that are provably incompatible with the parent's selectors,
+// meaning no release could satisfy both. In that case the subhelmfile can be
+// safely skipped without loading or evaluating it.
+func subhelmfileSelectorsConflict(parentSelectors []string, m state.SubHelmfileSpec) bool {
+	if len(parentSelectors) == 0 || len(m.Selectors) == 0 || m.SelectorsInherited {
+		return false
+	}
+	compatible, _ := state.SelectorsAreCompatible(parentSelectors, m.Selectors)
+	return !compatible
+}
+
 func (a *App) visitStatesWithContext(fileOrDir string, defOpts LoadOpts, converge func(*state.HelmState) (bool, []error), sharedCtx *Context) error {
 	noMatchInHelmfiles := true
 

pkg/app/app_test.go

@@ -722,8 +722,8 @@ releases:
 	}{
 		{label: "duplicatedOK=yes", expectedReleases: []string{"zipkin", "prometheus", "bar", "bar", "grafana", "postgresql"}, expectErr: false},
 		{label: "name=zipkin", expectedReleases: []string{"zipkin", "prometheus", "grafana", "postgresql"}, expectErr: false},
-		{label: "name=grafana", expectedReleases: []string{"zipkin", "prometheus", "grafana", "grafana", "postgresql"}, expectErr: false},
-		{label: "name=doesnotexists", expectedReleases: []string{"zipkin", "prometheus", "grafana", "postgresql"}, expectErr: false},
+		{label: "name=grafana", expectedReleases: []string{"grafana", "grafana", "postgresql"}, expectErr: false},
+		{label: "name=doesnotexists", expectedReleases: []string{"grafana", "postgresql"}, expectErr: false},
 	}
 	runFilterSubHelmFilesTests(legacyTestcases, files, t, "1st EmbeddedSelectors")
 
@@ -735,7 +735,7 @@ releases:
 		errMsg           string
 	}{
 		{label: "duplicatedOK=yes", expectedReleases: []string{"zipkin", "prometheus", "grafana", "bar", "bar", "grafana", "postgresql"}, expectErr: false},
-		{label: "name=doesnotexists", expectedReleases: []string{"zipkin", "prometheus", "grafana", "bar", "bar", "grafana", "postgresql"}, expectErr: false},
+		{label: "name=doesnotexists", expectedReleases: []string{"grafana", "bar", "bar", "grafana", "postgresql"}, expectErr: false},
 	}
 
 	t.Setenv(envvar.Experimental, ExperimentalSelectorExplicit)

pkg/state/release_filters.go

@@ -47,6 +47,64 @@ func (l LabelFilter) Match(r ReleaseSpec) bool {
 	return true
 }
 
+// SelectorsAreCompatible checks whether any pair of selectors from two sets could
+// potentially match the same release. It compares only positive labels (key=value):
+// two selectors conflict if they require the same key to have different values.
+// Returns true if at least one pair is compatible, false only if all pairs conflict.
+// On parse error, returns true (conservative: never skip on malformed input).
+func SelectorsAreCompatible(selectorsA, selectorsB []string) (bool, error) {
+	if len(selectorsA) == 0 || len(selectorsB) == 0 {
+		return true, nil
+	}
+
+	filtersA, err := parseLabelFilters(selectorsA)
+	if err != nil {
+		return true, err
+	}
+
+	filtersB, err := parseLabelFilters(selectorsB)
+	if err != nil {
+		return true, err
+	}
+
+	for _, a := range filtersA {
+		for _, b := range filtersB {
+			if a.positiveLabelsCompatibleWith(b) {
+				return true, nil
+			}
+		}
+	}
+
+	return false, nil
+}
+
+func parseLabelFilters(selectors []string) ([]LabelFilter, error) {
+	filters := make([]LabelFilter, 0, len(selectors))
+	for _, s := range selectors {
+		f, err := ParseLabels(s)
+		if err != nil {
+			return nil, err
+		}
+		filters = append(filters, f)
+	}
+	return filters, nil
+}
+
+// positiveLabelsCompatibleWith returns true if the positive labels of two filters
+// do not conflict (i.e., no shared key with a different value).
+func (l LabelFilter) positiveLabelsCompatibleWith(other LabelFilter) bool {
+	otherPos := make(map[string]string, len(other.positiveLabels))
+	for _, kv := range other.positiveLabels {
+		otherPos[kv[0]] = kv[1]
+	}
+	for _, kv := range l.positiveLabels {
+		if v, ok := otherPos[kv[0]]; ok && v != kv[1] {
+			return false
+		}
+	}
+	return true
+}
+
 // 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{}

pkg/state/release_filters_test.go

@@ -67,3 +67,93 @@ func TestParseLabelsInvalidFormat(t *testing.T) {
 	expectedErrorMsg := "malformed label: invalid_label. Expected label in form k=v or k!=v"
 	assert.EqualError(t, err, expectedErrorMsg, "unexpected error message")
 }
+
+func TestSelectorsAreCompatible(t *testing.T) {
+	tests := []struct {
+		name       string
+		selectorsA []string
+		selectorsB []string
+		compatible bool
+	}{
+		{
+			name:       "same key different value",
+			selectorsA: []string{"name=b"},
+			selectorsB: []string{"name=a"},
+			compatible: false,
+		},
+		{
+			name:       "same key same value",
+			selectorsA: []string{"name=b"},
+			selectorsB: []string{"name=b"},
+			compatible: true,
+		},
+		{
+			name:       "different keys no conflict",
+			selectorsA: []string{"name=b"},
+			selectorsB: []string{"env=prod"},
+			compatible: true,
+		},
+		{
+			name:       "one compatible pair among multiple selectors",
+			selectorsA: []string{"name=b"},
+			selectorsB: []string{"name=a", "name=b"},
+			compatible: true,
+		},
+		{
+			name:       "all pairs conflict",
+			selectorsA: []string{"name=b"},
+			selectorsB: []string{"name=a", "name=c"},
+			compatible: false,
+		},
+		{
+			name:       "one compatible pair with same key same value",
+			selectorsA: []string{"name=b", "env=prod"},
+			selectorsB: []string{"name=b"},
+			compatible: true,
+		},
+		{
+			name:       "empty selectorsA always compatible",
+			selectorsA: []string{},
+			selectorsB: []string{"name=a"},
+			compatible: true,
+		},
+		{
+			name:       "empty selectorsB always compatible",
+			selectorsA: []string{"name=a"},
+			selectorsB: []string{},
+			compatible: true,
+		},
+		{
+			name:       "negative labels not compared treated as compatible",
+			selectorsA: []string{"name!=a"},
+			selectorsB: []string{"name=a"},
+			compatible: true,
+		},
+		{
+			name:       "compound selector with conflicting key",
+			selectorsA: []string{"name=a,env=prod"},
+			selectorsB: []string{"name=a,env=staging"},
+			compatible: false,
+		},
+		{
+			name:       "compound selector with matching keys",
+			selectorsA: []string{"name=a,env=prod"},
+			selectorsB: []string{"name=a,env=prod"},
+			compatible: true,
+		},
+		{
+			name:       "compound selector partial overlap different key",
+			selectorsA: []string{"name=a,env=prod"},
+			selectorsB: []string{"name=a"},
+			compatible: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := SelectorsAreCompatible(tt.selectorsA, tt.selectorsB)
+			assert.NoError(t, err)
+			assert.Equal(t, tt.compatible, got)
+		})
+	}
+}

test/integration/run.sh

@@ -139,6 +139,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes
 . ${dir}/test-cases/issue-2418.sh
 . ${dir}/test-cases/issue-2424-sequential-values-paths.sh
 . ${dir}/test-cases/issue-2431.sh
+. ${dir}/test-cases/issue-2544.sh
 . ${dir}/test-cases/kubedog-tracking.sh
 
 # ALL DONE -----------------------------------------------------------------------------------------------------------

test/integration/test-cases/issue-2544.sh

@@ -0,0 +1,77 @@
+#!/usr/bin/env bash
+
+# Test for issue #2544: subhelmfiles should only be evaluated if their selectors match
+# When CLI selectors are provided, subhelmfiles with incompatible selectors are skipped.
+
+issue_2544_input_dir="${cases_dir}/issue-2544/input"
+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"
+
+# --- Test 1: -l name=b should only produce output from subhelmfile-b ---------------------------
+
+info "Running helmfile template -l name=b"
+${helmfile} -f "${issue_2544_input_dir}/helmfile.yaml" template -l name=b \
+    > "${issue_2544_tmp_dir}/output-b.txt" 2>&1 || {
+    cat "${issue_2544_tmp_dir}/output-b.txt"
+    fail "helmfile template -l name=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 name=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 name=a conflict with name=b)"
+
+grep -q "release-c" "${issue_2544_tmp_dir}/output-b.txt" \
+    && fail "output should NOT contain release-c (subhelmfile selectors name=c conflict with name=b)"
+
+info "PASS: -l name=b produces only release-b"
+
+# --- Test 2: no selector should produce all releases -----------------------------------------
+
+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"
+    fail "helmfile template without selector failed"
+}
+
+grep -q "release-a" "${issue_2544_tmp_dir}/output-all.txt" \
+    || fail "output should contain release-a"
+grep -q "release-b" "${issue_2544_tmp_dir}/output-all.txt" \
+    || fail "output should contain release-b"
+grep -q "release-c" "${issue_2544_tmp_dir}/output-all.txt" \
+    || fail "output should contain release-c"
+
+info "PASS: no selector produces all releases"
+
+# --- Test 3: -l name=a should skip subhelmfiles with selectors name=b and name=c ---------------
+
+info "Running helmfile template -l name=a"
+${helmfile} -f "${issue_2544_input_dir}/helmfile.yaml" template -l name=a \
+    > "${issue_2544_tmp_dir}/output-a.txt" 2>&1 || {
+    cat "${issue_2544_tmp_dir}/output-a.txt"
+    fail "helmfile template -l name=a failed"
+}
+
+grep -q "release-a" "${issue_2544_tmp_dir}/output-a.txt" \
+    || fail "output should contain release-a (matching selector name=a)"
+
+grep -q "release-b" "${issue_2544_tmp_dir}/output-a.txt" \
+    && fail "output should NOT contain release-b (subhelmfile selectors name=b conflict with name=a)"
+
+grep -q "release-c" "${issue_2544_tmp_dir}/output-a.txt" \
+    && fail "output should NOT contain release-c (subhelmfile selectors name=c conflict with name=a)"
+
+info "PASS: -l name=a produces only release-a"
+
+trap - EXIT
+test_pass "issue-2544: subhelmfiles skipped when selectors conflict with CLI"

test/integration/test-cases/issue-2544/input/helmfile.yaml

@@ -0,0 +1,10 @@
+helmfiles:
+- path: subhelmfile-a.yaml
+  selectors:
+  - name=a
+- path: subhelmfile-b.yaml
+  selectors:
+  - name=b
+- path: subhelmfile-c.yaml
+  selectors:
+  - name=c

test/integration/test-cases/issue-2544/input/subhelmfile-a.yaml

@@ -0,0 +1,15 @@
+releases:
+- name: release-a
+  chart: ../../../charts/raw
+  labels:
+    name: a
+  values:
+  - templates:
+    - |
+      apiVersion: v1
+      kind: ConfigMap
+      metadata:
+        name: release-a
+        namespace: {{ .Release.Namespace }}
+      data:
+        group: a

test/integration/test-cases/issue-2544/input/subhelmfile-b.yaml

@@ -0,0 +1,15 @@
+releases:
+- name: release-b
+  chart: ../../../charts/raw
+  labels:
+    name: b
+  values:
+  - templates:
+    - |
+      apiVersion: v1
+      kind: ConfigMap
+      metadata:
+        name: release-b
+        namespace: {{ .Release.Namespace }}
+      data:
+        group: b

test/integration/test-cases/issue-2544/input/subhelmfile-c.yaml

@@ -0,0 +1,15 @@
+releases:
+- name: release-c
+  chart: ../../../charts/raw
+  labels:
+    name: c
+  values:
+  - templates:
+    - |
+      apiVersion: v1
+      kind: ConfigMap
+      metadata:
+        name: release-c
+        namespace: {{ .Release.Namespace }}
+      data:
+        group: c