From 9cb5367a4dc2bf0492a598f0bd08195f880b09bb Mon Sep 17 00:00:00 2001 From: bhuvan-somisetty Date: Tue, 21 Jul 2026 21:08:31 +0530 Subject: [PATCH 1/2] fix: filter system labels from release labels in List/Query Secrets.List/Query and ConfigMaps.List/Query returned the raw object labels, including system labels (name, owner, status, version, createdAt, modifiedAt), unlike Get which already filters them. This let system labels ride along in lastRelease.Labels on upgrade and get merged into the new release, permanently baking stale createdAt values and system-label keys into the stored release body. Fixes #32404 Signed-off-by: bhuvan-somisetty --- 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 ++++++++++--- 4 files changed, 24 insertions(+), 10 deletions(-) diff --git a/pkg/storage/driver/cfgmaps.go b/pkg/storage/driver/cfgmaps.go index f71ce44f1..bae5fb2b9 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 = item.Labels + rls.Labels = filterSystemLabels(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 = item.Labels + rls.Labels = filterSystemLabels(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 947ebff71..397f229d8 100644 --- a/pkg/storage/driver/cfgmaps_test.go +++ b/pkg/storage/driver/cfgmaps_test.go @@ -149,11 +149,12 @@ func TestConfigMapList(t *testing.T) { if len(ssd) != 2 { t.Errorf("Expected 2 superseded, got %d", len(ssd)) } - // Check if release having both system and custom labels, this is needed to ensure that selector filtering would work. + // 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. rls := convertReleaserToV1(t, ssd[0]) _, ok := rls.Labels["name"] - if !ok { - t.Fatalf("Expected 'name' label in results, actual %v", rls.Labels) + if ok { + t.Fatalf("Expected 'name' system label to be filtered out, actual %v", rls.Labels) } _, ok = rls.Labels["key1"] if !ok { @@ -179,6 +180,12 @@ 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 a1f3e94fc..0fffcf183 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 = item.Labels + rls.Labels = filterSystemLabels(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 = item.Labels + rls.Labels = filterSystemLabels(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 a11ec4380..fe8db4b50 100644 --- a/pkg/storage/driver/secrets_test.go +++ b/pkg/storage/driver/secrets_test.go @@ -134,11 +134,12 @@ func TestSecretList(t *testing.T) { if len(ssd) != 2 { t.Errorf("Expected 2 superseded, got %d", len(ssd)) } - // Check if release having both system and custom labels, this is needed to ensure that selector filtering would work. + // 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. rls := convertReleaserToV1(t, ssd[0]) _, ok := rls.Labels["name"] - if !ok { - t.Fatalf("Expected 'name' label in results, actual %v", rls.Labels) + if ok { + t.Fatalf("Expected 'name' system label to be filtered out, actual %v", rls.Labels) } _, ok = rls.Labels["key1"] if !ok { @@ -164,6 +165,12 @@ 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) From 434d6f4d5c9b999f2946d488bc0c5a634c1f84ad Mon Sep 17 00:00:00 2001 From: bhuvan-somisetty Date: Tue, 21 Jul 2026 22:10:31 +0530 Subject: [PATCH 2/2] 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)