From 434d6f4d5c9b999f2946d488bc0c5a634c1f84ad Mon Sep 17 00:00:00 2001 From: bhuvan-somisetty Date: Tue, 21 Jul 2026 22:10:31 +0530 Subject: [PATCH] fix: strip system labels in mergeCustomLabels instead of driver List/Query Filtering system labels in the k8s drivers' List/Query (as done in the previous commit) also strips them from helm list --selector results, since filterSelector matches against rls.Labels. That silently breaks `helm list -l owner=helm`, `-l status=deployed`, etc., which currently rely on List/Query returning system labels alongside custom ones. Move the fix to where the leak actually turns into a stored value: mergeCustomLabels in the upgrade path, which is what folds the previous release's labels into the new one. Stripping system labels there keeps List/Query/selector behavior unchanged while still preventing stale createdAt/modifiedAt and system-label keys from being carried into the new release's persisted Labels. Fixes #32404 Signed-off-by: bhuvan-somisetty --- pkg/action/upgrade.go | 7 +++++++ pkg/action/upgrade_test.go | 4 ++++ pkg/storage/driver/cfgmaps.go | 4 ++-- pkg/storage/driver/cfgmaps_test.go | 13 +++---------- pkg/storage/driver/secrets.go | 4 ++-- pkg/storage/driver/secrets_test.go | 13 +++---------- 6 files changed, 21 insertions(+), 24 deletions(-) diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 7f66ceefb..b9e4a016b 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -661,6 +661,13 @@ func mergeCustomLabels(current, desired map[string]string) map[string]string { delete(labels, k) } } + // current comes from the previously stored release, which the k8s + // drivers (unlike Get) return with system labels still attached; strip + // them here so they don't ride along into the new release's Labels and + // clobber the fresh createdAt/modifiedAt set when it's persisted. + for _, k := range driver.GetSystemLabels() { + delete(labels, k) + } return labels } diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index 7a73c7179..30c7d545d 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -489,6 +489,10 @@ func TestMergeCustomLabels(t *testing.T) { {map[string]string{"k1": "v1", "k2": "v2"}, nil, map[string]string{"k1": "v1", "k2": "v2"}}, {nil, map[string]string{"k1": "v1", "k2": "v2"}, map[string]string{"k1": "v1", "k2": "v2"}}, {map[string]string{"k1": "v1", "k2": "v2"}, map[string]string{"k1": "null", "k2": "v3"}, map[string]string{"k2": "v3"}}, + // current can carry stale system labels forward from a previous + // revision (the k8s drivers' List/Query don't filter them like Get + // does); they must never end up in the merged result. + {map[string]string{"k1": "v1", "createdAt": "111", "owner": "helm"}, map[string]string{"k2": "v2"}, map[string]string{"k1": "v1", "k2": "v2"}}, } for _, test := range tests { if output := mergeCustomLabels(test[0], test[1]); !reflect.DeepEqual(test[2], output) { diff --git a/pkg/storage/driver/cfgmaps.go b/pkg/storage/driver/cfgmaps.go index bae5fb2b9..f71ce44f1 100644 --- a/pkg/storage/driver/cfgmaps.go +++ b/pkg/storage/driver/cfgmaps.go @@ -113,7 +113,7 @@ func (cfgmaps *ConfigMaps) List(filter func(release.Releaser) bool) ([]release.R continue } - rls.Labels = filterSystemLabels(item.Labels) + rls.Labels = item.Labels if filter(rls) { results = append(results, rls) @@ -152,7 +152,7 @@ func (cfgmaps *ConfigMaps) Query(labels map[string]string) ([]release.Releaser, cfgmaps.Logger().Debug("failed to decode release", slog.Any("error", err)) continue } - rls.Labels = filterSystemLabels(item.Labels) + rls.Labels = item.Labels results = append(results, rls) } return results, nil diff --git a/pkg/storage/driver/cfgmaps_test.go b/pkg/storage/driver/cfgmaps_test.go index 397f229d8..947ebff71 100644 --- a/pkg/storage/driver/cfgmaps_test.go +++ b/pkg/storage/driver/cfgmaps_test.go @@ -149,12 +149,11 @@ func TestConfigMapList(t *testing.T) { if len(ssd) != 2 { t.Errorf("Expected 2 superseded, got %d", len(ssd)) } - // List should return custom labels only, system labels (name, owner, status, etc.) - // must not leak into rls.Labels or they'll be carried into the next revision on upgrade. + // Check if release having both system and custom labels, this is needed to ensure that selector filtering would work. rls := convertReleaserToV1(t, ssd[0]) _, ok := rls.Labels["name"] - if ok { - t.Fatalf("Expected 'name' system label to be filtered out, actual %v", rls.Labels) + if !ok { + t.Fatalf("Expected 'name' label in results, actual %v", rls.Labels) } _, ok = rls.Labels["key1"] if !ok { @@ -180,12 +179,6 @@ func TestConfigMapQuery(t *testing.T) { t.Errorf("Expected 2 results, got %d", len(rls)) } - // Query should return custom labels only, same as Get and List. - queried := convertReleaserToV1(t, rls[0]) - if _, ok := queried.Labels["name"]; ok { - t.Fatalf("Expected 'name' system label to be filtered out, actual %v", queried.Labels) - } - _, err = cfgmaps.Query(map[string]string{"name": "notExist"}) if !errors.Is(err, ErrReleaseNotFound) { t.Errorf("Expected {%v}, got {%v}", ErrReleaseNotFound, err) diff --git a/pkg/storage/driver/secrets.go b/pkg/storage/driver/secrets.go index 0fffcf183..a1f3e94fc 100644 --- a/pkg/storage/driver/secrets.go +++ b/pkg/storage/driver/secrets.go @@ -110,7 +110,7 @@ func (secrets *Secrets) List(filter func(release.Releaser) bool) ([]release.Rele continue } - rls.Labels = filterSystemLabels(item.Labels) + rls.Labels = item.Labels if filter(rls) { results = append(results, rls) @@ -152,7 +152,7 @@ func (secrets *Secrets) Query(labels map[string]string) ([]release.Releaser, err ) continue } - rls.Labels = filterSystemLabels(item.Labels) + rls.Labels = item.Labels results = append(results, rls) } return results, nil diff --git a/pkg/storage/driver/secrets_test.go b/pkg/storage/driver/secrets_test.go index fe8db4b50..a11ec4380 100644 --- a/pkg/storage/driver/secrets_test.go +++ b/pkg/storage/driver/secrets_test.go @@ -134,12 +134,11 @@ func TestSecretList(t *testing.T) { if len(ssd) != 2 { t.Errorf("Expected 2 superseded, got %d", len(ssd)) } - // List should return custom labels only, system labels (name, owner, status, etc.) - // must not leak into rls.Labels or they'll be carried into the next revision on upgrade. + // Check if release having both system and custom labels, this is needed to ensure that selector filtering would work. rls := convertReleaserToV1(t, ssd[0]) _, ok := rls.Labels["name"] - if ok { - t.Fatalf("Expected 'name' system label to be filtered out, actual %v", rls.Labels) + if !ok { + t.Fatalf("Expected 'name' label in results, actual %v", rls.Labels) } _, ok = rls.Labels["key1"] if !ok { @@ -165,12 +164,6 @@ func TestSecretQuery(t *testing.T) { t.Fatalf("Expected 2 results, actual %d", len(rls)) } - // Query should return custom labels only, same as Get and List. - queried := convertReleaserToV1(t, rls[0]) - if _, ok := queried.Labels["name"]; ok { - t.Fatalf("Expected 'name' system label to be filtered out, actual %v", queried.Labels) - } - _, err = secrets.Query(map[string]string{"name": "notExist"}) if !errors.Is(err, ErrReleaseNotFound) { t.Errorf("Expected {%v}, got {%v}", ErrReleaseNotFound, err)