diff --git a/pkg/app/app.go b/pkg/app/app.go index b1ba64c0..37b5b514 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(opts.Selectors, m) { + if subhelmfileSelectorsConflict(a.Selectors, m) { continue } @@ -1037,14 +1037,17 @@ func (a *App) processNestedHelmfiles(st *state.HelmState, absd, file string, def } // subhelmfileSelectorsConflict returns true when the subhelmfile has explicit -// selectors that are provably incompatible with the parent's selectors, +// selectors that are provably incompatible with the CLI 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 { +// 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 { + if len(cliSelectors) == 0 || len(m.Selectors) == 0 || m.SelectorsInherited { return false } - compatible, _ := state.SelectorsAreCompatible(parentSelectors, m.Selectors) + compatible, _ := state.SelectorsAreCompatible(cliSelectors, m.Selectors) return !compatible } diff --git a/pkg/state/release_filters.go b/pkg/state/release_filters.go index b3bf27d4..9247f3fa 100644 --- a/pkg/state/release_filters.go +++ b/pkg/state/release_filters.go @@ -51,7 +51,9 @@ func (l LabelFilter) Match(r ReleaseSpec) bool { // 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). +// On parse error, returns (true, err): the conservative true ensures subhelmfiles +// are never incorrectly skipped due to malformed input, while the non-nil error +// allows callers to log or handle the parse failure if desired. func SelectorsAreCompatible(selectorsA, selectorsB []string) (bool, error) { if len(selectorsA) == 0 || len(selectorsB) == 0 { return true, nil diff --git a/pkg/state/release_filters_test.go b/pkg/state/release_filters_test.go index 098c79b9..6d969387 100644 --- a/pkg/state/release_filters_test.go +++ b/pkg/state/release_filters_test.go @@ -74,6 +74,7 @@ func TestSelectorsAreCompatible(t *testing.T) { selectorsA []string selectorsB []string compatible bool + wantErr bool }{ { name: "same key different value", @@ -147,12 +148,23 @@ func TestSelectorsAreCompatible(t *testing.T) { selectorsB: []string{"name=a"}, compatible: true, }, + { + name: "malformed selector returns conservative true with error", + selectorsA: []string{"name=b"}, + selectorsB: []string{"invalid_label"}, + compatible: true, + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got, err := SelectorsAreCompatible(tt.selectorsA, tt.selectorsB) - assert.NoError(t, err) + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } assert.Equal(t, tt.compatible, got) }) }