Filter by label selector when GET /v3/service_instances

Co-authored-by: Danail Branekov <danailster@gmail.com>
Co-authored-by: Giuseppe Capizzi <gcapizzi@vmware.com>

README.helm.md

@@ -114,4 +114,4 @@ Here are all the values that can be set for the chart:
       - `memory` (_String_): Memory limit.
     - `requests`: Resource requests.
       - `cpu` (_String_): CPU request.
-      - `memory` (_String_): Memory request.
+      - `memory` (_String_): Memory request.

api/payloads/payloads_suite_test.go

@@ -9,6 +9,7 @@ import (
 	apierrors "code.cloudfoundry.org/korifi/api/errors"
 	"code.cloudfoundry.org/korifi/api/payloads/validation"
 	"gopkg.in/yaml.v2"
+	"k8s.io/apimachinery/pkg/labels"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -71,3 +72,9 @@ func decodeQuery[T any, PT keyedPayload[T]](query string) (PT, error) {
 
 	return actual, decodeErr
 }
+
+func parseLabelSelector(s string) labels.Selector {
+	reqs, err := labels.ParseToRequirements(s)
+	Expect(err).NotTo(HaveOccurred())
+	return labels.NewSelector().Add(reqs...)
+}

api/payloads/service_binding_test.go

@@ -1,8 +1,6 @@
 package payloads_test
 
 import (
-	"net/http"
-
 	"code.cloudfoundry.org/korifi/api/errors"
 	"code.cloudfoundry.org/korifi/api/payloads"
 	"code.cloudfoundry.org/korifi/api/repositories"
@@ -11,109 +9,34 @@ import (
 	. "github.com/onsi/gomega"
 	"github.com/onsi/gomega/gstruct"
 	"k8s.io/apimachinery/pkg/labels"
-	"k8s.io/apimachinery/pkg/selection"
 )
 
 var _ = Describe("ServiceBindingList", func() {
-	decode := func(queryString string) (payloads.ServiceBindingList, error) {
-		serviceBindingList := payloads.ServiceBindingList{}
-		req, err := http.NewRequest("GET", "http://foo.com/bar"+queryString, nil)
-		Expect(err).NotTo(HaveOccurred())
-
-		err = validator.DecodeAndValidateURLValues(req, &serviceBindingList)
-		return serviceBindingList, err
-	}
-
-	Describe("decode from url values", func() {
-		var (
-			queryString        string
-			serviceBindingList payloads.ServiceBindingList
-			decodeErr          error
-		)
-
-		BeforeEach(func() {
-			queryString = "?app_guids=app_guid&service_instance_guids=service_instance_guid&include=include"
-		})
-
-		JustBeforeEach(func() {
-			serviceBindingList, decodeErr = decode(queryString)
-		})
+	DescribeTable("valid query",
+		func(query string, expectedServiceBindingList payloads.ServiceBindingList) {
+			actualServiceBindingList, decodeErr := decodeQuery[payloads.ServiceBindingList](query)
 
-		It("succeeds", func() {
 			Expect(decodeErr).NotTo(HaveOccurred())
-			Expect(serviceBindingList).To(Equal(payloads.ServiceBindingList{
-				AppGUIDs:             "app_guid",
-				ServiceInstanceGUIDs: "service_instance_guid",
-				Include:              "include",
-				LabelSelector:        labels.Everything(),
-			}))
-		})
-
-		When("the query string contains an invalid label selector", func() {
-			BeforeEach(func() {
-				queryString = "?label_selector=~~~"
-			})
-
-			It("returns an error", func() {
-				Expect(decodeErr).To(MatchError(ContainSubstring("Invalid value")))
-			})
-		})
-	})
-
-	DescribeTable("valid label selectors",
-		func(labelSelector string, verifyRequirements func(labels.Requirements)) {
-			serviceBindingList := payloads.ServiceBindingList{}
-			req, err := http.NewRequest("GET", "http://foo.com/bar?label_selector="+labelSelector, nil)
-			Expect(err).NotTo(HaveOccurred())
-			err = validator.DecodeAndValidateURLValues(req, &serviceBindingList)
-			Expect(err).NotTo(HaveOccurred())
+			Expect(*actualServiceBindingList).To(Equal(expectedServiceBindingList))
+		},
+		Entry("app_guids", "app_guids=app_guid", payloads.ServiceBindingList{AppGUIDs: "app_guid", LabelSelector: labels.Everything()}),
+		Entry("service_instance_guids", "service_instance_guids=si_guid", payloads.ServiceBindingList{ServiceInstanceGUIDs: "si_guid", LabelSelector: labels.Everything()}),
+		Entry("include", "include=include", payloads.ServiceBindingList{Include: "include", LabelSelector: labels.Everything()}),
+		Entry("label_selector=foo", "label_selector=foo", payloads.ServiceBindingList{LabelSelector: parseLabelSelector("foo")}),
+		Entry("label_selector=!foo", "label_selector=!foo", payloads.ServiceBindingList{LabelSelector: parseLabelSelector("!foo")}),
+		Entry("label_selector=foo=bar", "label_selector=foo=bar", payloads.ServiceBindingList{LabelSelector: parseLabelSelector("foo=bar")}),
+		Entry("label_selector=foo==bar", "label_selector=foo==bar", payloads.ServiceBindingList{LabelSelector: parseLabelSelector("foo==bar")}),
+		Entry("label_selector=foo!=bar", "label_selector=foo!=bar", payloads.ServiceBindingList{LabelSelector: parseLabelSelector("foo!=bar")}),
+		Entry("label_selector=foo in (bar1,bar2)", "label_selector=foo in (bar1,bar2)", payloads.ServiceBindingList{LabelSelector: parseLabelSelector("foo in (bar1, bar2)")}),
+		Entry("label_selector=foo notin (bar1,bar2)", "label_selector=foo notin (bar1,bar2)", payloads.ServiceBindingList{LabelSelector: parseLabelSelector("foo notin (bar1, bar2)")}),
+	)
 
-			requirements, selectable := serviceBindingList.LabelSelector.Requirements()
-			Expect(selectable).To(BeTrue())
-			verifyRequirements(requirements)
+	DescribeTable("invalid query",
+		func(query string, expectedErrMsg string) {
+			_, decodeErr := decodeQuery[payloads.ServiceBindingList](query)
+			Expect(decodeErr).To(MatchError(ContainSubstring(expectedErrMsg)))
 		},
-		Entry("foo exists", "foo", func(r labels.Requirements) {
-			Expect(r).To(HaveLen(1))
-			Expect(r[0].Key()).To(Equal("foo"))
-			Expect(r[0].Operator()).To(Equal(selection.Exists))
-			Expect(r[0].Values()).To(BeEmpty())
-		}),
-		Entry("foo does not exist", "!foo", func(r labels.Requirements) {
-			Expect(r).To(HaveLen(1))
-			Expect(r[0].Key()).To(Equal("foo"))
-			Expect(r[0].Operator()).To(Equal(selection.DoesNotExist))
-			Expect(r[0].Values()).To(BeEmpty())
-		}),
-		Entry("foo=bar", "foo=bar", func(r labels.Requirements) {
-			Expect(r).To(HaveLen(1))
-			Expect(r[0].Key()).To(Equal("foo"))
-			Expect(r[0].Operator()).To(Equal(selection.Equals))
-			Expect(r[0].Values()).To(SatisfyAll(HaveLen(1), HaveKey("bar")))
-		}),
-		Entry("foo==bar", "foo==bar", func(r labels.Requirements) {
-			Expect(r).To(HaveLen(1))
-			Expect(r[0].Key()).To(Equal("foo"))
-			Expect(r[0].Operator()).To(Equal(selection.DoubleEquals))
-			Expect(r[0].Values()).To(SatisfyAll(HaveLen(1), HaveKey("bar")))
-		}),
-		Entry("foo!=bar", "foo!=bar", func(r labels.Requirements) {
-			Expect(r).To(HaveLen(1))
-			Expect(r[0].Key()).To(Equal("foo"))
-			Expect(r[0].Operator()).To(Equal(selection.NotEquals))
-			Expect(r[0].Values()).To(SatisfyAll(HaveLen(1), HaveKey("bar")))
-		}),
-		Entry("foo in (bar1,bar2)", "foo in (bar1,bar2)", func(r labels.Requirements) {
-			Expect(r).To(HaveLen(1))
-			Expect(r[0].Key()).To(Equal("foo"))
-			Expect(r[0].Operator()).To(Equal(selection.In))
-			Expect(r[0].Values()).To(SatisfyAll(HaveLen(2), HaveKey("bar1"), HaveKey("bar2")))
-		}),
-		Entry("foo notin (bar1,bar2)", "foo notin (bar1,bar2)", func(r labels.Requirements) {
-			Expect(r).To(HaveLen(1))
-			Expect(r[0].Key()).To(Equal("foo"))
-			Expect(r[0].Operator()).To(Equal(selection.NotIn))
-			Expect(r[0].Values()).To(SatisfyAll(HaveLen(2), HaveKey("bar1"), HaveKey("bar2")))
-		}),
+		Entry("invalid label_selector", "label_selector=~~~", "Invalid value"),
 	)
 
 	Describe("ToMessage", func() {
@@ -124,9 +47,7 @@ var _ = Describe("ServiceBindingList", func() {
 		)
 
 		BeforeEach(func() {
-			fooBarRequirement, err := labels.NewRequirement("foo", selection.Equals, []string{"bar"})
-			Expect(err).NotTo(HaveOccurred())
-			labelSelector = labels.NewSelector().Add(*fooBarRequirement)
+			labelSelector = parseLabelSelector("foo=bar")
 
 			payload = payloads.ServiceBindingList{
 				AppGUIDs:             "app1,app2",

api/payloads/service_instance.go

@@ -11,6 +11,7 @@ import (
 	"code.cloudfoundry.org/korifi/api/payloads/validation"
 	"code.cloudfoundry.org/korifi/api/repositories"
 	jellidation "github.com/jellydator/validation"
+	"k8s.io/apimachinery/pkg/labels"
 )
 
 type ServiceInstanceCreate struct {
@@ -129,10 +130,11 @@ func (p *ServiceInstancePatch) UnmarshalJSON(data []byte) error {
 }
 
 type ServiceInstanceList struct {
-	Names      string
-	GUIDs      string
-	SpaceGUIDs string
-	OrderBy    string
+	Names         string
+	GUIDs         string
+	SpaceGUIDs    string
+	OrderBy       string
+	LabelSelector labels.Selector
 }
 
 func (l ServiceInstanceList) Validate() error {
@@ -143,14 +145,15 @@ func (l ServiceInstanceList) Validate() error {
 
 func (l *ServiceInstanceList) ToMessage() repositories.ListServiceInstanceMessage {
 	return repositories.ListServiceInstanceMessage{
-		Names:      parse.ArrayParam(l.Names),
-		SpaceGUIDs: parse.ArrayParam(l.SpaceGUIDs),
-		GUIDs:      parse.ArrayParam(l.GUIDs),
+		Names:         parse.ArrayParam(l.Names),
+		SpaceGUIDs:    parse.ArrayParam(l.SpaceGUIDs),
+		GUIDs:         parse.ArrayParam(l.GUIDs),
+		LabelSelector: l.LabelSelector,
 	}
 }
 
 func (l *ServiceInstanceList) SupportedKeys() []string {
-	return []string{"names", "space_guids", "guids", "order_by", "per_page", "page"}
+	return []string{"names", "space_guids", "guids", "order_by", "per_page", "page", "label_selector"}
 }
 
 func (l *ServiceInstanceList) IgnoredKeys() []*regexp.Regexp {
@@ -162,5 +165,12 @@ func (l *ServiceInstanceList) DecodeFromURLValues(values url.Values) error {
 	l.SpaceGUIDs = values.Get("space_guids")
 	l.GUIDs = values.Get("guids")
 	l.OrderBy = values.Get("order_by")
+
+	labelSelectorRequirements, err := labels.ParseToRequirements(values.Get("label_selector"))
+	if err != nil {
+		return err
+	}
+
+	l.LabelSelector = labels.NewSelector().Add(labelSelectorRequirements...)
 	return nil
 }

api/payloads/service_instance_test.go

@@ -5,10 +5,12 @@ import (
 	"strings"
 
 	"code.cloudfoundry.org/korifi/api/payloads"
+	"code.cloudfoundry.org/korifi/api/repositories"
 	"code.cloudfoundry.org/korifi/tools"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	. "github.com/onsi/gomega/gstruct"
+	"k8s.io/apimachinery/pkg/labels"
 )
 
 var _ = Describe("ServiceInstanceList", func() {
@@ -19,16 +21,23 @@ var _ = Describe("ServiceInstanceList", func() {
 			Expect(decodeErr).NotTo(HaveOccurred())
 			Expect(*actualServiceInstanceList).To(Equal(expectedServiceInstanceList))
 		},
-		Entry("names", "names=name", payloads.ServiceInstanceList{Names: "name"}),
-		Entry("space_guids", "space_guids=space_guid", payloads.ServiceInstanceList{SpaceGUIDs: "space_guid"}),
-		Entry("guids", "guids=guid", payloads.ServiceInstanceList{GUIDs: "guid"}),
-		Entry("created_at", "order_by=created_at", payloads.ServiceInstanceList{OrderBy: "created_at"}),
-		Entry("-created_at", "order_by=-created_at", payloads.ServiceInstanceList{OrderBy: "-created_at"}),
-		Entry("updated_at", "order_by=updated_at", payloads.ServiceInstanceList{OrderBy: "updated_at"}),
-		Entry("-updated_at", "order_by=-updated_at", payloads.ServiceInstanceList{OrderBy: "-updated_at"}),
-		Entry("name", "order_by=name", payloads.ServiceInstanceList{OrderBy: "name"}),
-		Entry("-name", "order_by=-name", payloads.ServiceInstanceList{OrderBy: "-name"}),
-		Entry("fields[xxx]", "fields[abc.d]=e", payloads.ServiceInstanceList{}),
+		Entry("names", "names=name", payloads.ServiceInstanceList{Names: "name", LabelSelector: labels.Everything()}),
+		Entry("space_guids", "space_guids=space_guid", payloads.ServiceInstanceList{SpaceGUIDs: "space_guid", LabelSelector: labels.Everything()}),
+		Entry("guids", "guids=guid", payloads.ServiceInstanceList{GUIDs: "guid", LabelSelector: labels.Everything()}),
+		Entry("created_at", "order_by=created_at", payloads.ServiceInstanceList{OrderBy: "created_at", LabelSelector: labels.Everything()}),
+		Entry("-created_at", "order_by=-created_at", payloads.ServiceInstanceList{OrderBy: "-created_at", LabelSelector: labels.Everything()}),
+		Entry("updated_at", "order_by=updated_at", payloads.ServiceInstanceList{OrderBy: "updated_at", LabelSelector: labels.Everything()}),
+		Entry("-updated_at", "order_by=-updated_at", payloads.ServiceInstanceList{OrderBy: "-updated_at", LabelSelector: labels.Everything()}),
+		Entry("name", "order_by=name", payloads.ServiceInstanceList{OrderBy: "name", LabelSelector: labels.Everything()}),
+		Entry("-name", "order_by=-name", payloads.ServiceInstanceList{OrderBy: "-name", LabelSelector: labels.Everything()}),
+		Entry("fields[xxx]", "fields[abc.d]=e", payloads.ServiceInstanceList{LabelSelector: labels.Everything()}),
+		Entry("label_selector=foo", "label_selector=foo", payloads.ServiceInstanceList{LabelSelector: parseLabelSelector("foo")}),
+		Entry("label_selector=!foo", "label_selector=!foo", payloads.ServiceInstanceList{LabelSelector: parseLabelSelector("!foo")}),
+		Entry("label_selector=foo=bar", "label_selector=foo=bar", payloads.ServiceInstanceList{LabelSelector: parseLabelSelector("foo=bar")}),
+		Entry("label_selector=foo==bar", "label_selector=foo==bar", payloads.ServiceInstanceList{LabelSelector: parseLabelSelector("foo==bar")}),
+		Entry("label_selector=foo!=bar", "label_selector=foo!=bar", payloads.ServiceInstanceList{LabelSelector: parseLabelSelector("foo!=bar")}),
+		Entry("label_selector=foo in (bar1,bar2)", "label_selector=foo in (bar1,bar2)", payloads.ServiceInstanceList{LabelSelector: parseLabelSelector("foo in (bar1, bar2)")}),
+		Entry("label_selector=foo notin (bar1,bar2)", "label_selector=foo notin (bar1,bar2)", payloads.ServiceInstanceList{LabelSelector: parseLabelSelector("foo notin (bar1, bar2)")}),
 	)
 
 	DescribeTable("invalid query",
@@ -37,7 +46,41 @@ var _ = Describe("ServiceInstanceList", func() {
 			Expect(decodeErr).To(MatchError(ContainSubstring(expectedErrMsg)))
 		},
 		Entry("invalid order_by", "order_by=foo", "value must be one of"),
+		Entry("invalid label_selector", "label_selector=~~~", "Invalid value"),
 	)
+
+	Describe("ToMessage", func() {
+		var (
+			payload       payloads.ServiceInstanceList
+			message       repositories.ListServiceInstanceMessage
+			labelSelector labels.Selector
+		)
+
+		BeforeEach(func() {
+			labelSelector = parseLabelSelector("foo=bar")
+
+			payload = payloads.ServiceInstanceList{
+				Names:         "n1,n2",
+				GUIDs:         "g1,g2",
+				SpaceGUIDs:    "sg1,sg2",
+				OrderBy:       "order",
+				LabelSelector: labelSelector,
+			}
+		})
+
+		JustBeforeEach(func() {
+			message = payload.ToMessage()
+		})
+
+		It("returns a list service instances message", func() {
+			Expect(message).To(Equal(repositories.ListServiceInstanceMessage{
+				Names:         []string{"n1", "n2"},
+				SpaceGUIDs:    []string{"sg1", "sg2"},
+				GUIDs:         []string{"g1", "g2"},
+				LabelSelector: labelSelector,
+			}))
+		})
+	})
 })
 
 var _ = Describe("ServiceInstanceCreate", func() {

api/repositories/service_instance_repository.go

@@ -14,6 +14,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
@@ -76,9 +77,10 @@ func (p PatchServiceInstanceMessage) Apply(cfServiceInstance *korifiv1alpha1.CFS
 }
 
 type ListServiceInstanceMessage struct {
-	Names      []string
-	SpaceGUIDs []string
-	GUIDs      []string
+	Names         []string
+	SpaceGUIDs    []string
+	GUIDs         []string
+	LabelSelector labels.Selector
 }
 
 type DeleteServiceInstanceMessage struct {
@@ -210,7 +212,7 @@ func (r *ServiceInstanceRepo) ListServiceInstances(ctx context.Context, authInfo
 		}
 
 		serviceInstanceList := new(korifiv1alpha1.CFServiceInstanceList)
-		err = userClient.List(ctx, serviceInstanceList, client.InNamespace(ns))
+		err = userClient.List(ctx, serviceInstanceList, client.InNamespace(ns), &client.ListOptions{LabelSelector: message.LabelSelector})
 		if k8serrors.IsForbidden(err) {
 			continue
 		}