use a release wrapper to serialize custom labels into the release blob

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

Author: joelanford

pkg/storage/chunked.go

@@ -10,7 +10,6 @@ import (
 	"hash"
 	"hash/fnv"
 	"io"
-	"maps"
 	"strconv"
 	"sync"
 	"time"
@@ -95,6 +94,18 @@ type chunk struct {
 	data []byte
 }
 
+type releaseWrapper struct {
+	release.Release
+	Labels map[string]string `json:"labels"`
+}
+
+func wrapRelease(rls *release.Release) *releaseWrapper {
+	return &releaseWrapper{
+		Release: *rls,
+		Labels:  rls.Labels,
+	}
+}
+
 // encodeRelease encodes a release returning a base64 encoded
 // gzipped string representation, or error.
 func (c *chunkedSecrets) encodeReleaseAsChunks(key string, rls *release.Release) ([]chunk, error) {
@@ -106,7 +117,7 @@ func (c *chunkedSecrets) encodeReleaseAsChunks(key string, rls *release.Release)
 			return err
 		}
 		defer gzw.Close()
-		return json.NewEncoder(gzw).Encode(rls)
+		return json.NewEncoder(gzw).Encode(wrapRelease(rls))
 	}(); err != nil {
 		return nil, err
 	}
@@ -148,13 +159,12 @@ func (c *chunkedSecrets) indexSecretFromChunks(key string, rls *release.Release,
 		panic(err)
 	}
 
-	indexLabels, indexAnnotations := newIndexLabelsAndAnnotations(c.owner, key, rls)
+	indexLabels := newIndexLabels(c.owner, key, rls)
 	indexSecret := &corev1.Secret{
 		Type: SecretTypeChunkedIndex,
 		ObjectMeta: metav1.ObjectMeta{
-			Name:        key,
-			Labels:      indexLabels,
-			Annotations: indexAnnotations,
+			Name:   key,
+			Labels: indexLabels,
 		},
 		Immutable: ptr.To(false),
 		Data: map[string][]byte{
@@ -320,29 +330,51 @@ func (c *chunkedSecrets) Query(queryLabels map[string]string) ([]*release.Releas
 	c.Log("query: labels=%v", queryLabels)
 	defer c.Log("queried: labels=%v", queryLabels)
 
-	selector := newListIndicesLabelSelector(c.owner)
-	if queryRequirements, selectable := labels.Set(queryLabels).AsSelector().Requirements(); selectable {
-		selector = selector.Add(queryRequirements...)
+	// The only labels that get stored on the index secret are system labels, so we'll do a two-pass
+	// query. First, we'll request index secrets from the API server that match the query labels that
+	// are system labels. From there, we decode the releases that match, and then further filter those
+	// based on the rest of the query labels that are not system labels.
+	serverSelectorSet := labels.Set{}
+	clientSelectorSet := labels.Set{}
+	for k, v := range queryLabels {
+		if isSystemLabel(k) {
+			serverSelectorSet[k] = v
+		} else {
+			clientSelectorSet[k] = v
+		}
 	}
 
-	indexSecrets, err := c.client.List(context.Background(), metav1.ListOptions{LabelSelector: selector.String()})
-	if err != nil {
-		return nil, fmt.Errorf("query: %w", err)
+	// Pass 1: build the server selector and query for index secrets
+	serverSelector := newListIndicesLabelSelector(c.owner)
+	if queryRequirements, selectable := serverSelectorSet.AsSelector().Requirements(); selectable {
+		serverSelector = serverSelector.Add(queryRequirements...)
 	}
 
-	if len(indexSecrets.Items) == 0 {
-		return nil, driver.ErrReleaseNotFound
+	indexSecrets, err := c.client.List(context.Background(), metav1.ListOptions{LabelSelector: serverSelector.String()})
+	if err != nil {
+		return nil, fmt.Errorf("query: %w", err)
 	}
 
+	// Pass 2: decode the releases that matched the server selector and filter based on the client selector
 	results := make([]*release.Release, 0, len(indexSecrets.Items))
+	clientSelector := clientSelectorSet.AsSelector()
 	for _, indexSecret := range indexSecrets.Items {
 		indexSecret := indexSecret
 		rls, err := c.decodeRelease(context.Background(), &indexSecret)
 		if err != nil {
 			return nil, fmt.Errorf("query: failed to decode release: %w", err)
 		}
+
+		if !clientSelector.Matches(labels.Set(rls.Labels)) {
+			continue
+		}
 		results = append(results, rls)
 	}
+
+	if len(results) == 0 {
+		return nil, driver.ErrReleaseNotFound
+	}
+
 	return results, nil
 }
 
@@ -400,12 +432,13 @@ func (c *chunkedSecrets) decodeRelease(ctx context.Context, indexSecret *corev1.
 		return nil, fmt.Errorf("failed to create gzip reader: %w", err)
 	}
 	releaseDecoder := json.NewDecoder(gzr)
-	var r release.Release
-	if err := releaseDecoder.Decode(&r); err != nil {
+	var wrappedRelease releaseWrapper
+	if err := releaseDecoder.Decode(&wrappedRelease); err != nil {
 		return nil, fmt.Errorf("failed to decode release: %w", err)
 	}
-	r.Labels = filterSystemLabels(indexSecret.Labels)
-	maps.Copy(r.Labels, indexSecret.Annotations)
+
+	r := wrappedRelease.Release
+	r.Labels = filterSystemLabels(wrappedRelease.Labels)
 	return &r, nil
 }
 

pkg/storage/chunked_test.go

@@ -5,7 +5,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"maps"
-	"strings"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -96,28 +95,6 @@ var _ = Describe("chunkedSecrets", func() {
 			_, err := maxReadDriver.Get(releaseKey(rel))
 			Expect(err).To(MatchError(ContainSubstring("release too large")))
 		})
-		It("should handle release labels that are invalid metadata labels", func() {
-			invalidMetadataLabelKey := "stored-as-annotation"
-			invalidMetadataLabelValue := strings.Repeat("a", 64)
-
-			expected := genRelease("test-release", 1, release.StatusPendingInstall, map[string]string{invalidMetadataLabelKey: invalidMetadataLabelValue}, chunkSize/2)
-			Expect(expected.Labels).To(HaveKey(invalidMetadataLabelKey))
-			Expect(chunkedDriver.Create(releaseKey(expected), expected)).To(Succeed())
-
-			// Make sure the release-only label is in the release metadata
-			actual, err := chunkedDriver.Get(releaseKey(expected))
-			Expect(err).ToNot(HaveOccurred())
-			Expect(actual).To(Equal(expected))
-			Expect(actual.Labels).To(HaveKey(invalidMetadataLabelKey))
-			Expect(actual.Labels[invalidMetadataLabelKey]).To(Equal(invalidMetadataLabelValue))
-
-			// Make sure the invalid label is stored in the secret annotations, not labels
-			items, err := secretInterface.List(context.Background(), metav1.ListOptions{})
-			Expect(err).ToNot(HaveOccurred())
-			Expect(items.Items).To(HaveLen(1))
-			Expect(items.Items[0].Labels).ToNot(HaveKey(invalidMetadataLabelKey))
-			Expect(items.Items[0].Annotations).To(HaveKey(invalidMetadataLabelKey))
-		})
 	})
 
 	var _ = Describe("Update", func() {

pkg/storage/labels.go

@@ -6,32 +6,17 @@ import (
 	"helm.sh/helm/v3/pkg/release"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/sets"
-	"k8s.io/apimachinery/pkg/util/validation"
 )
 
-func newIndexLabelsAndAnnotations(owner, key string, rls *release.Release) (map[string]string, map[string]string) {
+func newIndexLabels(owner, key string, rls *release.Release) map[string]string {
 	labels := map[string]string{}
-	annotations := map[string]string{}
-	for k, v := range rls.Labels {
-		if promoteToAnnotation(k, v) {
-			annotations[k] = v
-		} else {
-			labels[k] = v
-		}
-	}
-
 	labels["name"] = rls.Name
 	labels["owner"] = owner
 	labels["status"] = rls.Info.Status.String()
 	labels["version"] = strconv.Itoa(rls.Version)
 	labels["key"] = key
 	labels["type"] = "index"
-	return labels, annotations
-}
-
-func promoteToAnnotation(k, v string) bool {
-	isValidLabel := len(validation.IsQualifiedName(k)) == 0 && len(validation.IsValidLabelValue(v)) == 0
-	return !isValidLabel
+	return labels
 }
 
 func newChunkLabels(owner, key string) map[string]string {