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 <somisettybhuvan5@gmail.com>
pull/32421/head
bhuvan-somisetty 2 months ago
parent 9cb5367a4d
commit 434d6f4d5c
No known key found for this signature in database
GPG Key ID: 18A2EBE1548EEEE3

@ -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
}

@ -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) {

@ -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

@ -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)

@ -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

@ -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)

Loading…
Cancel
Save