Repository: helm/helm
Pull request #7927 (2020-04-15 00:29:49)

test: forward-port regression test from Helm 2
Add a regression test from #7926 (Helm v2) that makes sure `helm create` produces charts that will pass `helm lint --strict`.

Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>


Patch:
diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go
index 48eac0b0f..c263d32e7 100644
--- a/cmd/helm/upgrade.go
+++ b/cmd/helm/upgrade.go
@@ -75,21 +75,8 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 		RunE: func(cmd *cobra.Command, args []string) error {
 			client.Namespace = settings.Namespace()
 
-			if client.Version == "" && client.Devel {
-				debug("setting version to >0.0.0-0")
-				client.Version = ">0.0.0-0"
-			}
-
-			vals, err := valueOpts.MergeValues(getter.All(settings))
-			if err != nil {
-				return err
-			}
-
-			chartPath, err := client.ChartPathOptions.LocateChart(args[1], settings)
-			if err != nil {
-				return err
-			}
-
+			// Fixes #7002 - Support reading values from STDIN for `upgrade` command
+			// Must load values AFTER determining if we have to call install so that values loaded from stdin are are not read twice
 			if client.Install {
 				// If a release does not exist, install it.
 				histClient := action.NewHistory(cfg)
@@ -124,6 +111,21 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 				}
 			}
 
+			if client.Version == "" && client.Devel {
+				debug("setting version to >0.0.0-0")
+				client.Version = ">0.0.0-0"
+			}
+
+			chartPath, err := client.ChartPathOptions.LocateChart(args[1], settings)
+			if err != nil {
+				return err
+			}
+
+			vals, err := valueOpts.MergeValues(getter.All(settings))
+			if err != nil {
+				return err
+			}
+
 			// Check chart dependencies to make sure all are present in /charts
 			ch, err := loader.Load(chartPath)
 			if err != nil {
diff --git a/pkg/action/rollback.go b/pkg/action/rollback.go
index 942c9d8af..81812983f 100644
--- a/pkg/action/rollback.go
+++ b/pkg/action/rollback.go
@@ -211,7 +211,7 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas
 	}
 
 	deployed, err := r.cfg.Releases.DeployedAll(currentRelease.Name)
-	if err != nil {
+	if err != nil && !strings.Contains(err.Error(), "has no deployed releases") {
 		return nil, err
 	}
 	// Supersede all previous deployments, see issue #2941.
diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go
index b73e2ac8b..9d05217e9 100644
--- a/pkg/action/upgrade.go
+++ b/pkg/action/upgrade.go
@@ -43,27 +43,57 @@ type Upgrade struct {
 
 	ChartPathOptions
 
-	Install   bool
-	Devel     bool
+	// Install is a purely informative flag that indicates whether this upgrade was done in "install" mode.
+	//
+	// Applications may use this to determine whether this Upgrade operation was done as part of a
+	// pure upgrade (Upgrade.Install == false) or as part of an install-or-upgrade operation
+	// (Upgrade.Install == true).
+	//
+	// Setting this to `true` will NOT cause `Upgrade` to perform an install if the release does not exist.
+	// That process must be handled by creating an Install action directly. See cmd/upgrade.go for an
+	// example of how this flag is used.
+	Install bool
+	// Devel indicates that the operation is done in devel mode.
+	Devel bool
+	// Namespace is the namespace in which this operation should be performed.
 	Namespace string
-	// SkipCRDs skip installing CRDs when install flag is enabled during upgrade
-	SkipCRDs     bool
-	Timeout      time.Duration
-	Wait         bool
+	// SkipCRDs skips installing CRDs when install flag is enabled during upgrade
+	SkipCRDs bool
+	// Timeout is the timeout for this operation
+	Timeout time.Duration
+	// Wait determines whether the wait operation should be performed after the upgrade is requested.
+	Wait bool
+	// DisableHooks disables hook processing if set to true.
 	DisableHooks bool
-	DryRun       bool
-	Force        bool
-	ResetValues  bool
-	ReuseValues  bool
+	// DryRun controls whether the operation is prepared, but not executed.
+	// If `true`, the upgrade is prepared but not performed.
+	DryRun bool
+	// Force will, if set to `true`, ignore certain warnings and perform the upgrade anyway.
+	//
+	// This should be used with caution.
+	Force bool
+	// ResetValues will reset the values to the chart's built-ins rather than merging with existing.
+	ResetValues bool
+	// ReuseValues will re-use the user's last supplied values.
+	ReuseValues bool
 	// Recreate will (if true) recreate pods after a rollback.
 	Recreate bool
 	// MaxHistory limits the maximum number of revisions saved per release
-	MaxHistory               int
-	Atomic                   bool
-	CleanupOnFail            bool
-	SubNotes                 bool
-	Description              string
-	PostRenderer             postrender.PostRenderer
+	MaxHistory int
+	// Atomic, if true, will roll back on failure.
+	Atomic bool
+	// CleanupOnFail will, if true, cause the upgrade to delete newly-created resources on a failed update.
+	CleanupOnFail bool
+	// SubNotes determines whether sub-notes are rendered in the chart.
+	SubNotes bool
+	// Description is the description of this operation
+	Description string
+	// PostRender is an optional post-renderer
+	//
+	// If this is non-nil, then after templates are rendered, they will be sent to the
+	// post renderer before sending to the Kuberntes API server.
+	PostRenderer postrender.PostRenderer
+	// DisableOpenAPIValidation controls whether OpenAPI validation is enforced.
 	DisableOpenAPIValidation bool
 }
 
diff --git a/pkg/chartutil/create.go b/pkg/chartutil/create.go
index daaaa897c..a02cf4b98 100644
--- a/pkg/chartutil/create.go
+++ b/pkg/chartutil/create.go
@@ -214,29 +214,29 @@ metadata:
     {{- toYaml . | nindent 4 }}
   {{- end }}
 spec:
-{{- if .Values.ingress.tls }}
+  {{- if .Values.ingress.tls }}
   tls:
-  {{- range .Values.ingress.tls }}
+    {{- range .Values.ingress.tls }}
     - hosts:
-      {{- range .hosts }}
+        {{- range .hosts }}
         - {{ . | quote }}
-      {{- end }}
+        {{- end }}
       secretName: {{ .secretName }}
+    {{- end }}
   {{- end }}
-{{- end }}
   rules:
-  {{- range .Values.ingress.hosts }}
+    {{- range .Values.ingress.hosts }}
     - host: {{ .host | quote }}
       http:
         paths:
-        {{- range .paths }}
+          {{- range .paths }}
           - path: {{ . }}
             backend:
               serviceName: {{ $fullName }}
               servicePort: {{ $svcPort }}
-        {{- end }}
+          {{- end }}
+    {{- end }}
   {{- end }}
-{{- end }}
 `
 
 const defaultDeployment = `apiVersion: apps/v1
@@ -261,10 +261,10 @@ spec:
       labels:
         {{- include "<CHARTNAME>.selectorLabels" . | nindent 8 }}
     spec:
-    {{- with .Values.imagePullSecrets }}
+      {{- with .Values.imagePullSecrets }}
       imagePullSecrets:
         {{- toYaml . | nindent 8 }}
-    {{- end }}
+      {{- end }}
       serviceAccountName: {{ include "<CHARTNAME>.serviceAccountName" . }}
       securityContext:
         {{- toYaml .Values.podSecurityContext | nindent 8 }}
@@ -292,14 +292,14 @@ spec:
       nodeSelector:
         {{- toYaml . | nindent 8 }}
       {{- end }}
-    {{- with .Values.affinity }}
+      {{- with .Values.affinity }}
       affinity:
         {{- toYaml . | nindent 8 }}
-    {{- end }}
-    {{- with .Values.tolerations }}
+      {{- end }}
+      {{- with .Values.tolerations }}
       tolerations:
         {{- toYaml . | nindent 8 }}
-    {{- end }}
+      {{- end }}
 `
 
 const defaultService = `apiVersion: v1
@@ -330,7 +330,7 @@ metadata:
   annotations:
     {{- toYaml . | nindent 4 }}
   {{- end }}
-{{- end -}}
+{{- end }}
 `
 
 const defaultHorizontalPodAutoscaler = `{{- if .Values.autoscaling.enabled }}
@@ -391,8 +391,8 @@ const defaultHelpers = `{{/* vim: set filetype=mustache: */}}
 Expand the name of the chart.
 */}}
 {{- define "<CHARTNAME>.name" -}}
-{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" -}}
-{{- end -}}
+{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" }}
+{{- end }}
 
 {{/*
 Create a default fully qualified app name.
@@ -400,24 +400,24 @@ We truncate at 63 chars because some Kubernetes name fields are limited to this
 If release name contains chart name it will be used as a full name.
 */}}
 {{- define "<CHARTNAME>.fullname" -}}
-{{- if .Values.fullnameOverride -}}
-{{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" -}}
-{{- else -}}
-{{- $name := default .Chart.Name .Values.nameOverride -}}
-{{- if contains $name .Release.Name -}}
-{{- .Release.Name | trunc 63 | trimSuffix "-" -}}
-{{- else -}}
-{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" -}}
-{{- end -}}
-{{- end -}}
-{{- end -}}
+{{- if .Values.fullnameOverride }}
+{{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" }}
+{{- else }}
+{{- $name := default .Chart.Name .Values.nameOverride }}
+{{- if contains $name .Release.Name }}
+{{- .Release.Name | trunc 63 | trimSuffix "-" }}
+{{- else }}
+{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" }}
+{{- end }}
+{{- end }}
+{{- end }}
 
 {{/*
 Create chart name and version as used by the chart label.
 */}}
 {{- define "<CHARTNAME>.chart" -}}
-{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimSuffix "-" -}}
-{{- end -}}
+{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimSuffix "-" }}
+{{- end }}
 
 {{/*
 Common labels
@@ -429,7 +429,7 @@ helm.sh/chart: {{ include "<CHARTNAME>.chart" . }}
 app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}
 {{- end }}
 app.kubernetes.io/managed-by: {{ .Release.Service }}
-{{- end -}}
+{{- end }}
 
 {{/*
 Selector labels
@@ -437,18 +437,18 @@ Selector labels
 {{- define "<CHARTNAME>.selectorLabels" -}}
 app.kubernetes.io/name: {{ include "<CHARTNAME>.name" . }}
 app.kubernetes.io/instance: {{ .Release.Name }}
-{{- end -}}
+{{- end }}
 
 {{/*
 Create the name of the service account to use
 */}}
 {{- define "<CHARTNAME>.serviceAccountName" -}}
-{{- if .Values.serviceAccount.create -}}
-    {{ default (include "<CHARTNAME>.fullname" .) .Values.serviceAccount.name }}
-{{- else -}}
-    {{ default "default" .Values.serviceAccount.name }}
-{{- end -}}
-{{- end -}}
+{{- if .Values.serviceAccount.create }}
+{{- default (include "<CHARTNAME>.fullname" .) .Values.serviceAccount.name }}
+{{- else }}
+{{- default "default" .Values.serviceAccount.name }}
+{{- end }}
+{{- end }}
 `
 
 const defaultTestConnection = `apiVersion: v1
diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go
index ff451a6e8..00198de0c 100644
--- a/pkg/downloader/manager.go
+++ b/pkg/downloader/manager.go
@@ -239,6 +239,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error {
 
 	fmt.Fprintf(m.Out, "Saving %d charts\n", len(deps))
 	var saveError error
+	churls := make(map[string]struct{})
 	for _, dep := range deps {
 		// No repository means the chart is in charts directory
 		if dep.Repository == "" {
@@ -278,8 +279,6 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error {
 			continue
 		}
 
-		fmt.Fprintf(m.Out, "Downloading %s from repo %s\n", dep.Name, dep.Repository)
-
 		// Any failure to resolve/download a chart should fail:
 		// https://github.com/helm/helm/issues/1439
 		churl, username, password, err := m.findChartURL(dep.Name, dep.Version, dep.Repository, repos)
@@ -288,6 +287,13 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error {
 			break
 		}
 
+		if _, ok := churls[churl]; ok {
+			fmt.Fprintf(m.Out, "Already downloaded %s from repo %s\n", dep.Name, dep.Repository)
+			continue
+		}
+
+		fmt.Fprintf(m.Out, "Downloading %s from repo %s\n", dep.Name, dep.Repository)
+
 		dl := ChartDownloader{
 			Out:              m.Out,
 			Verify:           m.Verify,
@@ -304,6 +310,8 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error {
 			saveError = errors.Wrapf(err, "could not download %s", churl)
 			break
 		}
+
+		churls[churl] = struct{}{}
 	}
 
 	if saveError == nil {
diff --git a/pkg/storage/driver/memory.go b/pkg/storage/driver/memory.go
index a99b36ef0..91378f588 100644
--- a/pkg/storage/driver/memory.go
+++ b/pkg/storage/driver/memory.go
@@ -141,6 +141,11 @@ func (mem *Memory) Query(keyvals map[string]string) ([]*rspb.Release, error) {
 			break
 		}
 	}
+
+	if len(ls) == 0 {
+		return nil, ErrReleaseNotFound
+	}
+
 	return ls, nil
 }
 


Test patch:
diff --git a/cmd/helm/helm_test.go b/cmd/helm/helm_test.go
index 94646a5a3..9ba9d78fb 100644
--- a/cmd/helm/helm_test.go
+++ b/cmd/helm/helm_test.go
@@ -97,10 +97,15 @@ func storageFixture() *storage.Storage {
 }
 
 func executeActionCommandC(store *storage.Storage, cmd string) (*cobra.Command, string, error) {
+	return executeActionCommandStdinC(store, nil, cmd)
+}
+
+func executeActionCommandStdinC(store *storage.Storage, in *os.File, cmd string) (*cobra.Command, string, error) {
 	args, err := shellwords.Parse(cmd)
 	if err != nil {
 		return nil, "", err
 	}
+
 	buf := new(bytes.Buffer)
 
 	actionConfig := &action.Configuration{
@@ -111,15 +116,26 @@ func executeActionCommandC(store *storage.Storage, cmd string) (*cobra.Command,
 	}
 
 	root := newRootCmd(actionConfig, buf, args)
-	root.SetOutput(buf)
+	root.SetOut(buf)
+	root.SetErr(buf)
 	root.SetArgs(args)
 
+	oldStdin := os.Stdin
+	if in != nil {
+		root.SetIn(in)
+		os.Stdin = in
+	}
+
 	if mem, ok := store.Driver.(*driver.Memory); ok {
 		mem.SetNamespace(settings.Namespace())
 	}
 	c, err := root.ExecuteC()
 
-	return c, buf.String(), err
+	result := buf.String()
+
+	os.Stdin = oldStdin
+
+	return c, result, err
 }
 
 // cmdTestCase describes a test case that works with releases.
diff --git a/cmd/helm/upgrade_test.go b/cmd/helm/upgrade_test.go
index 4956c0247..3a6d75adc 100644
--- a/cmd/helm/upgrade_test.go
+++ b/cmd/helm/upgrade_test.go
@@ -19,6 +19,7 @@ package main
 import (
 	"fmt"
 	"io/ioutil"
+	"os"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -256,6 +257,69 @@ func TestUpgradeWithValuesFile(t *testing.T) {
 
 }
 
+func TestUpgradeWithValuesFromStdin(t *testing.T) {
+
+	releaseName := "funny-bunny-v5"
+	relMock, ch, chartPath := prepareMockRelease(releaseName, t)
+
+	defer resetEnv()()
+
+	store := storageFixture()
+
+	store.Create(relMock(releaseName, 3, ch))
+
+	in, err := os.Open("testdata/testcharts/upgradetest/values.yaml")
+	if err != nil {
+		t.Errorf("unexpected error, got '%v'", err)
+	}
+
+	cmd := fmt.Sprintf("upgrade %s --values - '%s'", releaseName, chartPath)
+	_, _, err = executeActionCommandStdinC(store, in, cmd)
+	if err != nil {
+		t.Errorf("unexpected error, got '%v'", err)
+	}
+
+	updatedRel, err := store.Get(releaseName, 4)
+	if err != nil {
+		t.Errorf("unexpected error, got '%v'", err)
+	}
+
+	if !strings.Contains(updatedRel.Manifest, "drink: beer") {
+		t.Errorf("The value is not set correctly. manifest: %s", updatedRel.Manifest)
+	}
+}
+
+func TestUpgradeInstallWithValuesFromStdin(t *testing.T) {
+
+	releaseName := "funny-bunny-v6"
+	_, _, chartPath := prepareMockRelease(releaseName, t)
+
+	defer resetEnv()()
+
+	store := storageFixture()
+
+	in, err := os.Open("testdata/testcharts/upgradetest/values.yaml")
+	if err != nil {
+		t.Errorf("unexpected error, got '%v'", err)
+	}
+
+	cmd := fmt.Sprintf("upgrade %s -f - --install '%s'", releaseName, chartPath)
+	_, _, err = executeActionCommandStdinC(store, in, cmd)
+	if err != nil {
+		t.Errorf("unexpected error, got '%v'", err)
+	}
+
+	updatedRel, err := store.Get(releaseName, 1)
+	if err != nil {
+		t.Errorf("unexpected error, got '%v'", err)
+	}
+
+	if !strings.Contains(updatedRel.Manifest, "drink: beer") {
+		t.Errorf("The value is not set correctly. manifest: %s", updatedRel.Manifest)
+	}
+
+}
+
 func prepareMockRelease(releaseName string, t *testing.T) (func(n string, v int, ch *chart.Chart) *release.Release, *chart.Chart, string) {
 	tmpChart := ensure.TempDir(t)
 	configmapData, err := ioutil.ReadFile("testdata/testcharts/upgradetest/templates/configmap.yaml")
diff --git a/pkg/lint/lint_test.go b/pkg/lint/lint_test.go
index b51939d76..2c110009d 100644
--- a/pkg/lint/lint_test.go
+++ b/pkg/lint/lint_test.go
@@ -17,9 +17,12 @@ limitations under the License.
 package lint
 
 import (
+	"io/ioutil"
+	"os"
 	"strings"
 	"testing"
 
+	"helm.sh/helm/v3/pkg/chartutil"
 	"helm.sh/helm/v3/pkg/lint/support"
 )
 
@@ -104,3 +107,30 @@ func TestGoodChart(t *testing.T) {
 		t.Errorf("All failed but shouldn't have: %#v", m)
 	}
 }
+
+// TestHelmCreateChart tests that a `helm create` always passes a `helm lint` test.
+//
+// See https://github.com/helm/helm/issues/7923
+func TestHelmCreateChart(t *testing.T) {
+	dir, err := ioutil.TempDir("", "-helm-test")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+
+	createdChart, err := chartutil.Create("testhelmcreatepasseslint", dir)
+	if err != nil {
+		t.Error(err)
+		// Fatal is bad because of the defer.
+		return
+	}
+
+	// Note: we test with strict=true here, even though others have
+	// strict = false.
+	m := All(createdChart, values, namespace, true).Messages
+	if ll := len(m); ll != 1 {
+		t.Errorf("All should have had exactly 1 error. Got %d", ll)
+	} else if msg := m[0].Err.Error(); !strings.Contains(msg, "icon is recommended") {
+		t.Errorf("Unexpected lint error: %s", msg)
+	}
+}
diff --git a/pkg/storage/driver/cfgmaps_test.go b/pkg/storage/driver/cfgmaps_test.go
index e40247d3c..626c36cb9 100644
--- a/pkg/storage/driver/cfgmaps_test.go
+++ b/pkg/storage/driver/cfgmaps_test.go
@@ -130,6 +130,30 @@ func TestConfigMapList(t *testing.T) {
 	}
 }
 
+func TestConfigMapQuery(t *testing.T) {
+	cfgmaps := newTestFixtureCfgMaps(t, []*rspb.Release{
+		releaseStub("key-1", 1, "default", rspb.StatusUninstalled),
+		releaseStub("key-2", 1, "default", rspb.StatusUninstalled),
+		releaseStub("key-3", 1, "default", rspb.StatusDeployed),
+		releaseStub("key-4", 1, "default", rspb.StatusDeployed),
+		releaseStub("key-5", 1, "default", rspb.StatusSuperseded),
+		releaseStub("key-6", 1, "default", rspb.StatusSuperseded),
+	}...)
+
+	rls, err := cfgmaps.Query(map[string]string{"status": "deployed"})
+	if err != nil {
+		t.Errorf("Failed to query: %s", err)
+	}
+	if len(rls) != 2 {
+		t.Errorf("Expected 2 results, got %d", len(rls))
+	}
+
+	_, err = cfgmaps.Query(map[string]string{"name": "notExist"})
+	if err != ErrReleaseNotFound {
+		t.Errorf("Expected {%v}, got {%v}", ErrReleaseNotFound, err)
+	}
+}
+
 func TestConfigMapCreate(t *testing.T) {
 	cfgmaps := newTestFixtureCfgMaps(t)
 
diff --git a/pkg/storage/driver/mock_test.go b/pkg/storage/driver/mock_test.go
index 22e6454a1..77ddca43d 100644
--- a/pkg/storage/driver/mock_test.go
+++ b/pkg/storage/driver/mock_test.go
@@ -24,6 +24,7 @@ import (
 	v1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	kblabels "k8s.io/apimachinery/pkg/labels"
 	corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
 
 	rspb "helm.sh/helm/v3/pkg/release"
@@ -112,10 +113,18 @@ func (mock *MockConfigMapsInterface) Get(_ context.Context, name string, _ metav
 }
 
 // List returns the a of ConfigMaps.
-func (mock *MockConfigMapsInterface) List(_ context.Context, _ metav1.ListOptions) (*v1.ConfigMapList, error) {
+func (mock *MockConfigMapsInterface) List(_ context.Context, opts metav1.ListOptions) (*v1.ConfigMapList, error) {
 	var list v1.ConfigMapList
+
+	labelSelector, err := kblabels.Parse(opts.LabelSelector)
+	if err != nil {
+		return nil, err
+	}
+
 	for _, cfgmap := range mock.objects {
-		list.Items = append(list.Items, *cfgmap)
+		if labelSelector.Matches(kblabels.Set(cfgmap.ObjectMeta.Labels)) {
+			list.Items = append(list.Items, *cfgmap)
+		}
 	}
 	return &list, nil
 }
@@ -190,10 +199,18 @@ func (mock *MockSecretsInterface) Get(_ context.Context, name string, _ metav1.G
 }
 
 // List returns the a of Secret.
-func (mock *MockSecretsInterface) List(_ context.Context, _ metav1.ListOptions) (*v1.SecretList, error) {
+func (mock *MockSecretsInterface) List(_ context.Context, opts metav1.ListOptions) (*v1.SecretList, error) {
 	var list v1.SecretList
+
+	labelSelector, err := kblabels.Parse(opts.LabelSelector)
+	if err != nil {
+		return nil, err
+	}
+
 	for _, secret := range mock.objects {
-		list.Items = append(list.Items, *secret)
+		if labelSelector.Matches(kblabels.Set(secret.ObjectMeta.Labels)) {
+			list.Items = append(list.Items, *secret)
+		}
 	}
 	return &list, nil
 }
diff --git a/pkg/storage/driver/secrets_test.go b/pkg/storage/driver/secrets_test.go
index 5f0ecc8bb..d509c7b3a 100644
--- a/pkg/storage/driver/secrets_test.go
+++ b/pkg/storage/driver/secrets_test.go
@@ -130,6 +130,30 @@ func TestSecretList(t *testing.T) {
 	}
 }
 
+func TestSecretQuery(t *testing.T) {
+	secrets := newTestFixtureSecrets(t, []*rspb.Release{
+		releaseStub("key-1", 1, "default", rspb.StatusUninstalled),
+		releaseStub("key-2", 1, "default", rspb.StatusUninstalled),
+		releaseStub("key-3", 1, "default", rspb.StatusDeployed),
+		releaseStub("key-4", 1, "default", rspb.StatusDeployed),
+		releaseStub("key-5", 1, "default", rspb.StatusSuperseded),
+		releaseStub("key-6", 1, "default", rspb.StatusSuperseded),
+	}...)
+
+	rls, err := secrets.Query(map[string]string{"status": "deployed"})
+	if err != nil {
+		t.Fatalf("Failed to query: %s", err)
+	}
+	if len(rls) != 2 {
+		t.Fatalf("Expected 2 results, actual %d", len(rls))
+	}
+
+	_, err = secrets.Query(map[string]string{"name": "notExist"})
+	if err != ErrReleaseNotFound {
+		t.Errorf("Expected {%v}, got {%v}", ErrReleaseNotFound, err)
+	}
+}
+
 func TestSecretCreate(t *testing.T) {
 	secrets := newTestFixtureSecrets(t)