SecretsService: Filter secure values by status phase (#101780)

* filter by status phase field

* comment and error message

* missed error to handle

* fix test after merge

pkg/apis/secret/v0alpha1/register.go

@@ -4,8 +4,10 @@ import (
 	"fmt"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apiserver/pkg/registry/generic"
 
 	"github.com/grafana/grafana/pkg/apimachinery/utils"
 )
@@ -90,8 +92,15 @@ var (
 	AddToScheme        = localSchemeBuilder.AddToScheme
 )
 
+// Adds the status phase to the selectable fields, besides the generic metadata name and namespace.
+func SelectableSecureValueFields(obj *SecureValue) fields.Set {
+	return generic.MergeFieldsSets(generic.ObjectMetaFieldsSet(&obj.ObjectMeta, false), fields.Set{
+		"status.phase": string(obj.Status.Phase),
+	})
+}
+
 // Adds the list of known types to the given scheme.
-func AddKnownTypes(scheme *runtime.Scheme, version string) {
+func AddKnownTypes(scheme *runtime.Scheme, version string) error {
 	// TODO: do we need a type for the secure value decrypt?
 	// Since it is a subresource, it could be interesting to not use `SecureValue`, but rather something distinct like `DecryptedSecureValue`?
 	scheme.AddKnownTypes(
@@ -102,4 +111,21 @@ func AddKnownTypes(scheme *runtime.Scheme, version string) {
 		&KeeperList{},
 		// &secretV0.SecureValueActivityList{},
 	)
+
+	err := scheme.AddFieldLabelConversionFunc(
+		SecureValuesResourceInfo.GroupVersionKind(),
+		func(label, value string) (string, string, error) {
+			fieldSet := SelectableSecureValueFields(&SecureValue{})
+			for key := range fieldSet {
+				if label == key {
+					return label, value, nil
+				}
+			}
+			return "", "", fmt.Errorf("field label not supported for %s: %s", SecureValuesResourceInfo.GroupVersionKind(), label)
+		},
+	)
+	if err != nil {
+		return err
+	}
+	return nil
 }

pkg/registry/apis/secret/register.go

@@ -110,12 +110,18 @@ func (b *SecretAPIBuilder) GetGroupVersion() schema.GroupVersion {
 
 // InstallSchema is called by the `apiserver` which exposes the defined kinds.
 func (b *SecretAPIBuilder) InstallSchema(scheme *runtime.Scheme) error {
-	secretv0alpha1.AddKnownTypes(scheme, secretv0alpha1.VERSION)
+	err := secretv0alpha1.AddKnownTypes(scheme, secretv0alpha1.VERSION)
+	if err != nil {
+		return err
+	}
 
 	// Link this version to the internal representation.
 	// This is used for server-side-apply (PATCH), and avoids the error:
 	// "no kind is registered for the type"
-	secretv0alpha1.AddKnownTypes(scheme, runtime.APIVersionInternal)
+	err = secretv0alpha1.AddKnownTypes(scheme, runtime.APIVersionInternal)
+	if err != nil {
+		return err
+	}
 
 	// Internal Kubernetes metadata API. Presumably to display the available APIs?
 	// e.g. http://localhost:3000/apis/secret.grafana.app/v0alpha1

pkg/storage/secret/metadata/secure_value_store.go

@@ -16,6 +16,7 @@ import (
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/storage/secret/migrator"
 	"k8s.io/apimachinery/pkg/apis/meta/internalversion"
+	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/labels"
 )
 
@@ -263,6 +264,10 @@ func (s *secureValueMetadataStorage) List(ctx context.Context, namespace xkube.N
 	if labelSelector == nil {
 		labelSelector = labels.Everything()
 	}
+	fieldSelector := options.FieldSelector
+	if fieldSelector == nil {
+		fieldSelector = fields.Everything()
+	}
 
 	secureValueRows := make([]*secureValueDB, 0)
 
@@ -293,7 +298,11 @@ func (s *secureValueMetadataStorage) List(ctx context.Context, namespace xkube.N
 		}
 
 		if labelSelector.Matches(labels.Set(secureValue.Labels)) {
-			secureValues = append(secureValues, *secureValue)
+			if fieldSelector.Matches(fields.Set{
+				"status.phase": string(secureValue.Status.Phase),
+			}) {
+				secureValues = append(secureValues, *secureValue)
+			}
 		}
 	}
 

pkg/tests/apis/secret/secure_value_test.go

@@ -144,6 +144,43 @@ func TestIntegrationSecureValue(t *testing.T) {
 			require.Error(t, err)
 			require.Nil(t, updatedRaw)
 		})
+
+		t.Run("and listing securevalues with status.phase Succeeded returns the created secure value", func(t *testing.T) {
+			rawList, err := client.Resource.List(ctx, metav1.ListOptions{
+				FieldSelector: "status.phase=Succeeded",
+			})
+			require.NoError(t, err)
+			require.NotNil(t, rawList)
+			require.GreaterOrEqual(t, len(rawList.Items), 1)
+			require.Equal(t, secureValue.Name, rawList.Items[0].GetName())
+		})
+
+		t.Run("and listing securevalues with label bb returns the created secure value", func(t *testing.T) {
+			rawList, err := client.Resource.List(ctx, metav1.ListOptions{
+				LabelSelector: "bb=BBB",
+			})
+			require.NoError(t, err)
+			require.NotNil(t, rawList)
+			require.GreaterOrEqual(t, len(rawList.Items), 1)
+			require.Equal(t, secureValue.Name, rawList.Items[0].GetName())
+		})
+
+		t.Run("and listing securevalues with status.phase Failed returns empty", func(t *testing.T) {
+			rawList, err := client.Resource.List(ctx, metav1.ListOptions{
+				FieldSelector: "status.phase=Failed",
+			})
+			require.NoError(t, err)
+			require.NotNil(t, rawList)
+			require.Equal(t, len(rawList.Items), 0)
+		})
+
+		t.Run("and listing securevalues with a not allowed filter returns error", func(t *testing.T) {
+			rawList, err := client.Resource.List(ctx, metav1.ListOptions{
+				FieldSelector: "some.label=something",
+			})
+			require.Error(t, err)
+			require.Nil(t, rawList)
+		})
 	})
 
 	t.Run("creating a secure value with a `value` then updating it to a `ref` returns an error", func(t *testing.T) {