diff --git a/api/payloads/app.go b/api/payloads/app.go index 9b8f49f80..1fa3dd9b1 100644 --- a/api/payloads/app.go +++ b/api/payloads/app.go @@ -81,10 +81,11 @@ func (d AppSetCurrentDroplet) Validate() error { } type AppList struct { - Names string - GUIDs string - SpaceGuids string - OrderBy string + Names string + GUIDs string + SpaceGuids string + OrderBy string + LabelSelector string } func (a AppList) Validate() error { @@ -95,14 +96,15 @@ func (a AppList) Validate() error { func (a *AppList) ToMessage() repositories.ListAppsMessage { return repositories.ListAppsMessage{ - Names: parse.ArrayParam(a.Names), - Guids: parse.ArrayParam(a.GUIDs), - SpaceGuids: parse.ArrayParam(a.SpaceGuids), + Names: parse.ArrayParam(a.Names), + Guids: parse.ArrayParam(a.GUIDs), + SpaceGuids: parse.ArrayParam(a.SpaceGuids), + LabelSelector: a.LabelSelector, } } func (a *AppList) SupportedKeys() []string { - return []string{"names", "guids", "space_guids", "order_by", "per_page", "page"} + return []string{"names", "guids", "space_guids", "order_by", "per_page", "page", "label_selector"} } func (a *AppList) DecodeFromURLValues(values url.Values) error { @@ -110,6 +112,7 @@ func (a *AppList) DecodeFromURLValues(values url.Values) error { a.GUIDs = values.Get("guids") a.SpaceGuids = values.Get("space_guids") a.OrderBy = values.Get("order_by") + a.LabelSelector = values.Get("label_selector") return nil } diff --git a/api/payloads/app_test.go b/api/payloads/app_test.go index f91a20cc8..317994871 100644 --- a/api/payloads/app_test.go +++ b/api/payloads/app_test.go @@ -30,6 +30,7 @@ var _ = Describe("AppList", func() { Entry("order_by -name", "order_by=-name", payloads.AppList{OrderBy: "-name"}), Entry("order_by state", "order_by=state", payloads.AppList{OrderBy: "state"}), Entry("order_by -state", "order_by=-state", payloads.AppList{OrderBy: "-state"}), + Entry("label_selector=foo", "label_selector=foo", payloads.AppList{LabelSelector: "foo"}), ) DescribeTable("invalid query", @@ -44,15 +45,17 @@ var _ = Describe("AppList", func() { Describe("ToMessage", func() { It("translates to repository message", func() { appList := payloads.AppList{ - Names: "n1,n2", - GUIDs: "g1,g2", - SpaceGuids: "s1,s2", - OrderBy: "created_at", + Names: "n1,n2", + GUIDs: "g1,g2", + SpaceGuids: "s1,s2", + OrderBy: "created_at", + LabelSelector: "foo=bar", } Expect(appList.ToMessage()).To(Equal(repositories.ListAppsMessage{ - Names: []string{"n1", "n2"}, - Guids: []string{"g1", "g2"}, - SpaceGuids: []string{"s1", "s2"}, + Names: []string{"n1", "n2"}, + Guids: []string{"g1", "g2"}, + SpaceGuids: []string{"s1", "s2"}, + LabelSelector: "foo=bar", })) }) }) diff --git a/api/payloads/payloads_suite_test.go b/api/payloads/payloads_suite_test.go index 134ced420..070ae8383 100644 --- a/api/payloads/payloads_suite_test.go +++ b/api/payloads/payloads_suite_test.go @@ -9,7 +9,6 @@ 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" @@ -72,9 +71,3 @@ 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...) -} diff --git a/api/payloads/service_binding.go b/api/payloads/service_binding.go index eb1057229..371c8168b 100644 --- a/api/payloads/service_binding.go +++ b/api/payloads/service_binding.go @@ -7,7 +7,6 @@ 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 ServiceBindingCreate struct { @@ -48,7 +47,7 @@ type ServiceBindingList struct { AppGUIDs string ServiceInstanceGUIDs string Include string - LabelSelector labels.Selector + LabelSelector string } func (l *ServiceBindingList) ToMessage() repositories.ListServiceBindingsMessage { @@ -67,14 +66,7 @@ func (l *ServiceBindingList) DecodeFromURLValues(values url.Values) error { l.AppGUIDs = values.Get("app_guids") l.ServiceInstanceGUIDs = values.Get("service_instance_guids") l.Include = values.Get("include") - - labelSelectorRequirements, err := labels.ParseToRequirements(values.Get("label_selector")) - if err != nil { - return err - } - - l.LabelSelector = labels.NewSelector().Add(labelSelectorRequirements...) - + l.LabelSelector = values.Get("label_selector") return nil } diff --git a/api/payloads/service_binding_test.go b/api/payloads/service_binding_test.go index 293d0f76e..b542101ce 100644 --- a/api/payloads/service_binding_test.go +++ b/api/payloads/service_binding_test.go @@ -8,7 +8,6 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/onsi/gomega/gstruct" - "k8s.io/apimachinery/pkg/labels" ) var _ = Describe("ServiceBindingList", func() { @@ -19,41 +18,24 @@ var _ = Describe("ServiceBindingList", func() { Expect(decodeErr).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)")}), - ) - - DescribeTable("invalid query", - func(query string, expectedErrMsg string) { - _, decodeErr := decodeQuery[payloads.ServiceBindingList](query) - Expect(decodeErr).To(MatchError(ContainSubstring(expectedErrMsg))) - }, - Entry("invalid label_selector", "label_selector=~~~", "Invalid value"), + Entry("app_guids", "app_guids=app_guid", payloads.ServiceBindingList{AppGUIDs: "app_guid"}), + Entry("service_instance_guids", "service_instance_guids=si_guid", payloads.ServiceBindingList{ServiceInstanceGUIDs: "si_guid"}), + Entry("include", "include=include", payloads.ServiceBindingList{Include: "include"}), + Entry("label_selector=foo", "label_selector=foo", payloads.ServiceBindingList{LabelSelector: "foo"}), ) Describe("ToMessage", func() { var ( - payload payloads.ServiceBindingList - message repositories.ListServiceBindingsMessage - labelSelector labels.Selector + payload payloads.ServiceBindingList + message repositories.ListServiceBindingsMessage ) BeforeEach(func() { - labelSelector = parseLabelSelector("foo=bar") - payload = payloads.ServiceBindingList{ AppGUIDs: "app1,app2", ServiceInstanceGUIDs: "s1,s2", Include: "include", - LabelSelector: labelSelector, + LabelSelector: "foo=bar", } }) @@ -65,7 +47,7 @@ var _ = Describe("ServiceBindingList", func() { Expect(message).To(Equal(repositories.ListServiceBindingsMessage{ AppGUIDs: []string{"app1", "app2"}, ServiceInstanceGUIDs: []string{"s1", "s2"}, - LabelSelector: labelSelector, + LabelSelector: "foo=bar", })) }) }) diff --git a/api/payloads/service_instance.go b/api/payloads/service_instance.go index b5d120722..6cac9fba3 100644 --- a/api/payloads/service_instance.go +++ b/api/payloads/service_instance.go @@ -11,7 +11,6 @@ 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 { @@ -134,7 +133,7 @@ type ServiceInstanceList struct { GUIDs string SpaceGUIDs string OrderBy string - LabelSelector labels.Selector + LabelSelector string } func (l ServiceInstanceList) Validate() error { @@ -165,12 +164,6 @@ 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...) + l.LabelSelector = values.Get("label_selector") return nil } diff --git a/api/payloads/service_instance_test.go b/api/payloads/service_instance_test.go index 22f9536c1..dc474625e 100644 --- a/api/payloads/service_instance_test.go +++ b/api/payloads/service_instance_test.go @@ -10,7 +10,6 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" . "github.com/onsi/gomega/gstruct" - "k8s.io/apimachinery/pkg/labels" ) var _ = Describe("ServiceInstanceList", func() { @@ -21,23 +20,17 @@ var _ = Describe("ServiceInstanceList", func() { Expect(decodeErr).NotTo(HaveOccurred()) Expect(*actualServiceInstanceList).To(Equal(expectedServiceInstanceList)) }, - 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)")}), + 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("label_selector=foo", "label_selector=foo", payloads.ServiceInstanceList{LabelSelector: "foo"}), ) DescribeTable("invalid query", @@ -46,25 +39,21 @@ 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 + payload payloads.ServiceInstanceList + message repositories.ListServiceInstanceMessage ) BeforeEach(func() { - labelSelector = parseLabelSelector("foo=bar") - payload = payloads.ServiceInstanceList{ Names: "n1,n2", GUIDs: "g1,g2", SpaceGUIDs: "sg1,sg2", OrderBy: "order", - LabelSelector: labelSelector, + LabelSelector: "foo=bar", } }) @@ -77,7 +66,7 @@ var _ = Describe("ServiceInstanceList", func() { Names: []string{"n1", "n2"}, SpaceGUIDs: []string{"sg1", "sg2"}, GUIDs: []string{"g1", "g2"}, - LabelSelector: labelSelector, + LabelSelector: "foo=bar", })) }) }) diff --git a/api/repositories/app_repository.go b/api/repositories/app_repository.go index 97037a4e4..626422ebf 100644 --- a/api/repositories/app_repository.go +++ b/api/repositories/app_repository.go @@ -21,6 +21,7 @@ import ( k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -168,9 +169,10 @@ type SetAppDesiredStateMessage struct { } type ListAppsMessage struct { - Names []string - Guids []string - SpaceGuids []string + Names []string + Guids []string + SpaceGuids []string + LabelSelector string } type byName []AppRecord @@ -315,6 +317,11 @@ func (f *AppRepo) ListApps(ctx context.Context, authInfo authorization.Info, mes SetPredicate(message.Guids, func(s korifiv1alpha1.CFApp) string { return s.Name }), } + labelSelector, err := labels.Parse(message.LabelSelector) + if err != nil { + return []AppRecord{}, apierrors.NewUnprocessableEntityError(err, "invalid label selector") + } + var filteredApps []korifiv1alpha1.CFApp spaceGUIDSet := NewSet(message.SpaceGuids...) for ns := range nsList { @@ -323,7 +330,7 @@ func (f *AppRepo) ListApps(ctx context.Context, authInfo authorization.Info, mes } appList := &korifiv1alpha1.CFAppList{} - err := userClient.List(ctx, appList, client.InNamespace(ns)) + err := userClient.List(ctx, appList, client.InNamespace(ns), &client.ListOptions{LabelSelector: labelSelector}) if k8serrors.IsForbidden(err) { continue diff --git a/api/repositories/app_repository_test.go b/api/repositories/app_repository_test.go index 068b322df..8ae794d7e 100644 --- a/api/repositories/app_repository_test.go +++ b/api/repositories/app_repository_test.go @@ -14,6 +14,7 @@ import ( korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" "code.cloudfoundry.org/korifi/controllers/controllers/shared" "code.cloudfoundry.org/korifi/controllers/controllers/workloads/env" + "code.cloudfoundry.org/korifi/controllers/controllers/workloads/testutils" "code.cloudfoundry.org/korifi/tests/matchers" "code.cloudfoundry.org/korifi/tools" "code.cloudfoundry.org/korifi/tools/k8s" @@ -50,7 +51,7 @@ var _ = Describe("AppRepository", func() { cfOrg = createOrgWithCleanup(testCtx, prefixedGUID("org")) cfSpace = createSpaceWithCleanup(testCtx, cfOrg.Name, prefixedGUID("space1")) - cfApp = createApp(cfSpace.Name) + cfApp = createAppWithGUID(cfSpace.Name, testutils.PrefixedGUID("cfapp1-")) }) Describe("GetApp", func() { @@ -228,6 +229,7 @@ var _ = Describe("AppRepository", func() { message ListAppsMessage appList []AppRecord cfApp2 *korifiv1alpha1.CFApp + listErr error ) BeforeEach(func() { @@ -238,17 +240,16 @@ var _ = Describe("AppRepository", func() { createRoleBinding(testCtx, userName, spaceDeveloperRole.Name, cfSpace.Name) createRoleBinding(testCtx, userName, spaceDeveloperRole.Name, space2.Name) - cfApp2 = createApp(space2.Name) + cfApp2 = createAppWithGUID(space2.Name, testutils.PrefixedGUID("cfapp2-")) createApp(space3.Name) }) JustBeforeEach(func() { - var err error - appList, err = appRepo.ListApps(testCtx, authInfo, message) - Expect(err).NotTo(HaveOccurred()) + appList, listErr = appRepo.ListApps(testCtx, authInfo, message) }) It("returns all the AppRecord CRs where client has permission", func() { + Expect(listErr).NotTo(HaveOccurred()) Expect(appList).To(ConsistOf( MatchFields(IgnoreExtras, Fields{"GUID": Equal(cfApp.Name)}), MatchFields(IgnoreExtras, Fields{"GUID": Equal(cfApp2.Name)}), @@ -276,6 +277,7 @@ var _ = Describe("AppRepository", func() { }) It("does not list them", func() { + Expect(listErr).NotTo(HaveOccurred()) Expect(appList).NotTo(ContainElement( MatchFields(IgnoreExtras, Fields{"GUID": Equal(nonCFApp.Name)}), )) @@ -286,7 +288,7 @@ var _ = Describe("AppRepository", func() { var cfApp12 *korifiv1alpha1.CFApp BeforeEach(func() { - cfApp12 = createApp(cfSpace.Name) + cfApp12 = createAppWithGUID(cfSpace.Name, testutils.PrefixedGUID("cfapp12-")) }) Describe("filtering by name", func() { @@ -306,6 +308,7 @@ var _ = Describe("AppRepository", func() { }) It("returns the matching apps", func() { + Expect(listErr).NotTo(HaveOccurred()) Expect(appList).To(ConsistOf( MatchFields(IgnoreExtras, Fields{"GUID": Equal(cfApp2.Name)}), MatchFields(IgnoreExtras, Fields{"GUID": Equal(cfApp12.Name)}), @@ -321,6 +324,7 @@ var _ = Describe("AppRepository", func() { }) It("returns an empty list of apps", func() { + Expect(listErr).NotTo(HaveOccurred()) Expect(appList).To(BeEmpty()) }) }) @@ -331,6 +335,7 @@ var _ = Describe("AppRepository", func() { }) It("returns the matching apps", func() { + Expect(listErr).NotTo(HaveOccurred()) Expect(appList).To(ConsistOf( MatchFields(IgnoreExtras, Fields{"GUID": Equal(cfApp.Name)}), MatchFields(IgnoreExtras, Fields{"GUID": Equal(cfApp2.Name)}), @@ -346,6 +351,7 @@ var _ = Describe("AppRepository", func() { }) It("returns an empty list of apps", func() { + Expect(listErr).NotTo(HaveOccurred()) Expect(appList).To(BeEmpty()) }) }) @@ -356,6 +362,7 @@ var _ = Describe("AppRepository", func() { }) It("returns the matching apps", func() { + Expect(listErr).NotTo(HaveOccurred()) Expect(appList).To(ConsistOf( MatchFields(IgnoreExtras, Fields{"GUID": Equal(cfApp.Name)}), MatchFields(IgnoreExtras, Fields{"GUID": Equal(cfApp12.Name)}), @@ -372,6 +379,7 @@ var _ = Describe("AppRepository", func() { When("an App matches by Name but not by Space", func() { It("returns an empty list of apps", func() { + Expect(listErr).NotTo(HaveOccurred()) Expect(appList).To(BeEmpty()) }) }) @@ -382,6 +390,7 @@ var _ = Describe("AppRepository", func() { }) It("returns an empty list of apps", func() { + Expect(listErr).NotTo(HaveOccurred()) Expect(appList).To(BeEmpty()) }) }) @@ -393,11 +402,59 @@ var _ = Describe("AppRepository", func() { }) It("returns the matching apps", func() { + Expect(listErr).NotTo(HaveOccurred()) Expect(appList).To(HaveLen(1)) Expect(appList[0].GUID).To(Equal(cfApp12.Name)) }) }) }) + + Describe("filtering by label selector", func() { + BeforeEach(func() { + Expect(k8s.PatchResource(ctx, k8sClient, cfApp, func() { + cfApp.Labels = map[string]string{"foo": "FOO1"} + })).To(Succeed()) + Expect(k8s.PatchResource(ctx, k8sClient, cfApp2, func() { + cfApp2.Labels = map[string]string{"foo": "FOO2"} + })).To(Succeed()) + Expect(k8s.PatchResource(ctx, k8sClient, cfApp12, func() { + cfApp12.Labels = map[string]string{"not_foo": "NOT_FOO"} + })).To(Succeed()) + }) + + DescribeTable("valid label selectors", + func(selector string, appGUIDPrefixes ...string) { + serviceBindings, err := appRepo.ListApps(context.Background(), authInfo, ListAppsMessage{ + LabelSelector: selector, + }) + Expect(err).NotTo(HaveOccurred()) + + matchers := []any{} + for _, prefix := range appGUIDPrefixes { + matchers = append(matchers, MatchFields(IgnoreExtras, Fields{"GUID": HavePrefix(prefix)})) + } + + Expect(serviceBindings).To(ConsistOf(matchers...)) + }, + Entry("key", "foo", "cfapp1-", "cfapp2-"), + Entry("!key", "!foo", "cfapp12-"), + Entry("key=value", "foo=FOO1", "cfapp1-"), + Entry("key==value", "foo==FOO2", "cfapp2-"), + Entry("key!=value", "foo!=FOO1", "cfapp2-", "cfapp12-"), + Entry("key in (value1,value2)", "foo in (FOO1,FOO2)", "cfapp1-", "cfapp2-"), + Entry("key notin (value1,value2)", "foo notin (FOO2)", "cfapp1-", "cfapp12-"), + ) + + When("the label selector is invalid", func() { + BeforeEach(func() { + message = ListAppsMessage{LabelSelector: "~"} + }) + + It("returns an error", func() { + Expect(listErr).To(matchers.WrapErrorAssignableToTypeOf(apierrors.UnprocessableEntityError{})) + }) + }) + }) }) }) diff --git a/api/repositories/service_binding_repository.go b/api/repositories/service_binding_repository.go index c0aec8c08..e364edca2 100644 --- a/api/repositories/service_binding_repository.go +++ b/api/repositories/service_binding_repository.go @@ -85,7 +85,7 @@ type DeleteServiceBindingMessage struct { type ListServiceBindingsMessage struct { AppGUIDs []string ServiceInstanceGUIDs []string - LabelSelector labels.Selector + LabelSelector string } func (m CreateServiceBindingMessage) toCFServiceBinding() *korifiv1alpha1.CFServiceBinding { @@ -269,10 +269,15 @@ func (r *ServiceBindingRepo) ListServiceBindings(ctx context.Context, authInfo a SetPredicate(message.AppGUIDs, func(s korifiv1alpha1.CFServiceBinding) string { return s.Spec.AppRef.Name }), } + labelSelector, err := labels.Parse(message.LabelSelector) + if err != nil { + return []ServiceBindingRecord{}, apierrors.NewUnprocessableEntityError(err, "invalid label selector") + } + var filteredServiceBindings []korifiv1alpha1.CFServiceBinding for ns := range nsList { serviceBindingList := new(korifiv1alpha1.CFServiceBindingList) - err = userClient.List(ctx, serviceBindingList, client.InNamespace(ns), &client.ListOptions{LabelSelector: message.LabelSelector}) + err = userClient.List(ctx, serviceBindingList, client.InNamespace(ns), &client.ListOptions{LabelSelector: labelSelector}) if k8serrors.IsForbidden(err) { continue } diff --git a/api/repositories/service_binding_repository_test.go b/api/repositories/service_binding_repository_test.go index d75e28e0b..7a359272d 100644 --- a/api/repositories/service_binding_repository_test.go +++ b/api/repositories/service_binding_repository_test.go @@ -7,7 +7,6 @@ import ( "time" "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/labels" "code.cloudfoundry.org/korifi/api/authorization" apierrors "code.cloudfoundry.org/korifi/api/errors" @@ -474,7 +473,7 @@ var _ = Describe("ServiceBindingRepo", func() { DescribeTable("valid label selectors", func(selector string, serviceBindingGUIDPrefixes ...string) { serviceBindings, err := repo.ListServiceBindings(context.Background(), authInfo, repositories.ListServiceBindingsMessage{ - LabelSelector: labelSelector(selector), + LabelSelector: selector, }) Expect(err).NotTo(HaveOccurred()) @@ -493,6 +492,16 @@ var _ = Describe("ServiceBindingRepo", func() { Entry("key in (value1,value2)", "foo in (FOO1,FOO2)", "binding-1", "binding-2"), Entry("key notin (value1,value2)", "foo notin (FOO2)", "binding-1", "binding-3"), ) + + When("the label selector is invalid", func() { + BeforeEach(func() { + requestMessage = repositories.ListServiceBindingsMessage{LabelSelector: "~"} + }) + + It("returns an error", func() { + Expect(listErr).To(matchers.WrapErrorAssignableToTypeOf(apierrors.UnprocessableEntityError{})) + }) + }) }) When("filtered by multiple params", func() { @@ -683,10 +692,3 @@ var _ = Describe("ServiceBindingRepo", func() { }) }) }) - -func labelSelector(s string) labels.Selector { - requirements, err := labels.ParseToRequirements(s) - Expect(err).NotTo(HaveOccurred()) - - return labels.NewSelector().Add(requirements...) -} diff --git a/api/repositories/service_instance_repository.go b/api/repositories/service_instance_repository.go index cdf6f6d5c..1f6e4b19f 100644 --- a/api/repositories/service_instance_repository.go +++ b/api/repositories/service_instance_repository.go @@ -80,7 +80,7 @@ type ListServiceInstanceMessage struct { Names []string SpaceGUIDs []string GUIDs []string - LabelSelector labels.Selector + LabelSelector string } type DeleteServiceInstanceMessage struct { @@ -204,6 +204,11 @@ func (r *ServiceInstanceRepo) ListServiceInstances(ctx context.Context, authInfo SetPredicate(message.GUIDs, func(s korifiv1alpha1.CFServiceInstance) string { return s.Name }), } + labelSelector, err := labels.Parse(message.LabelSelector) + if err != nil { + return []ServiceInstanceRecord{}, apierrors.NewUnprocessableEntityError(err, "invalid label selector") + } + spaceGUIDSet := NewSet(message.SpaceGUIDs...) var filteredServiceInstances []korifiv1alpha1.CFServiceInstance for ns := range nsList { @@ -212,7 +217,7 @@ func (r *ServiceInstanceRepo) ListServiceInstances(ctx context.Context, authInfo } serviceInstanceList := new(korifiv1alpha1.CFServiceInstanceList) - err = userClient.List(ctx, serviceInstanceList, client.InNamespace(ns), &client.ListOptions{LabelSelector: message.LabelSelector}) + err = userClient.List(ctx, serviceInstanceList, client.InNamespace(ns), &client.ListOptions{LabelSelector: labelSelector}) if k8serrors.IsForbidden(err) { continue } diff --git a/api/repositories/service_instance_repository_test.go b/api/repositories/service_instance_repository_test.go index 2e291a6fe..ef6e97590 100644 --- a/api/repositories/service_instance_repository_test.go +++ b/api/repositories/service_instance_repository_test.go @@ -9,6 +9,7 @@ import ( apierrors "code.cloudfoundry.org/korifi/api/errors" "code.cloudfoundry.org/korifi/api/repositories" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/tests/matchers" "code.cloudfoundry.org/korifi/tools" "code.cloudfoundry.org/korifi/tools/k8s" "sigs.k8s.io/controller-runtime/pkg/client" @@ -359,6 +360,7 @@ var _ = Describe("ServiceInstanceRepository", func() { cfServiceInstance1, cfServiceInstance2, cfServiceInstance3 *korifiv1alpha1.CFServiceInstance nonCFNamespace string filters repositories.ListServiceInstanceMessage + listErr error serviceInstanceList []repositories.ServiceInstanceRecord ) @@ -382,13 +384,12 @@ var _ = Describe("ServiceInstanceRepository", func() { }) JustBeforeEach(func() { - var err error - serviceInstanceList, err = serviceInstanceRepo.ListServiceInstances(testCtx, authInfo, filters) - Expect(err).NotTo(HaveOccurred()) + serviceInstanceList, listErr = serviceInstanceRepo.ListServiceInstances(testCtx, authInfo, filters) }) When("no service instances exist in spaces where the user has permission", func() { It("returns an empty list of ServiceInstanceRecord", func() { + Expect(listErr).NotTo(HaveOccurred()) Expect(serviceInstanceList).To(BeEmpty()) }) }) @@ -400,6 +401,7 @@ var _ = Describe("ServiceInstanceRepository", func() { }) It("returns ServiceInstance records from only the spaces where the user has permission", func() { + Expect(listErr).NotTo(HaveOccurred()) Expect(serviceInstanceList).To(ConsistOf( MatchFields(IgnoreExtras, Fields{"GUID": Equal(cfServiceInstance1.Name)}), MatchFields(IgnoreExtras, Fields{"GUID": Equal(cfServiceInstance2.Name)}), @@ -425,6 +427,7 @@ var _ = Describe("ServiceInstanceRepository", func() { }) It("returns only records for the ServiceInstances with matching spec.name fields", func() { + Expect(listErr).NotTo(HaveOccurred()) Expect(serviceInstanceList).To(ConsistOf( MatchFields(IgnoreExtras, Fields{"GUID": Equal(cfServiceInstance1.Name)}), MatchFields(IgnoreExtras, Fields{"GUID": Equal(cfServiceInstance3.Name)}), @@ -441,7 +444,9 @@ var _ = Describe("ServiceInstanceRepository", func() { }, } }) + It("returns only records for the ServiceInstances within the matching spaces", func() { + Expect(listErr).NotTo(HaveOccurred()) Expect(serviceInstanceList).To(ConsistOf( MatchFields(IgnoreExtras, Fields{"GUID": Equal(cfServiceInstance2.Name)}), MatchFields(IgnoreExtras, Fields{"GUID": Equal(cfServiceInstance3.Name)}), @@ -456,6 +461,7 @@ var _ = Describe("ServiceInstanceRepository", func() { } }) It("returns only records for the ServiceInstances within the matching spaces", func() { + Expect(listErr).NotTo(HaveOccurred()) Expect(serviceInstanceList).To(ConsistOf( MatchFields(IgnoreExtras, Fields{"GUID": Equal(cfServiceInstance1.Name)}), MatchFields(IgnoreExtras, Fields{"GUID": Equal(cfServiceInstance3.Name)}), @@ -479,7 +485,7 @@ var _ = Describe("ServiceInstanceRepository", func() { DescribeTable("valid label selectors", func(selector string, serviceBindingGUIDPrefixes ...string) { serviceInstances, err := serviceInstanceRepo.ListServiceInstances(context.Background(), authInfo, repositories.ListServiceInstanceMessage{ - LabelSelector: labelSelector(selector), + LabelSelector: selector, }) Expect(err).NotTo(HaveOccurred()) @@ -498,6 +504,16 @@ var _ = Describe("ServiceInstanceRepository", func() { Entry("key in (value1,value2)", "foo in (FOO1,FOO2)", "service-instance-1", "service-instance-2"), Entry("key notin (value1,value2)", "foo notin (FOO2)", "service-instance-1", "service-instance-3"), ) + + When("the label selector is invalid", func() { + BeforeEach(func() { + filters = repositories.ListServiceInstanceMessage{LabelSelector: "~"} + }) + + It("returns an error", func() { + Expect(listErr).To(matchers.WrapErrorAssignableToTypeOf(apierrors.UnprocessableEntityError{})) + }) + }) }) }) }) diff --git a/tests/e2e/apps_test.go b/tests/e2e/apps_test.go index 77a8c75ad..0ca4d862a 100644 --- a/tests/e2e/apps_test.go +++ b/tests/e2e/apps_test.go @@ -7,6 +7,7 @@ import ( "code.cloudfoundry.org/korifi/tools" "github.com/go-resty/resty/v2" + "github.com/google/uuid" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" . "github.com/onsi/gomega/gstruct" @@ -35,9 +36,12 @@ var _ = Describe("Apps", func() { app1GUID, app2GUID, app3GUID string app4GUID, app5GUID, app6GUID string result resourceList[resource] + query string ) BeforeEach(func() { + query = "" + space2GUID = createSpace(generateGUID("space2"), commonTestOrgGUID) space3GUID = createSpace(generateGUID("space3"), commonTestOrgGUID) @@ -59,7 +63,7 @@ var _ = Describe("Apps", func() { JustBeforeEach(func() { var err error - resp, err = certClient.R().SetResult(&result).Get("/v3/apps") + resp, err = certClient.R().SetResult(&result).Get("/v3/apps" + query) Expect(err).NotTo(HaveOccurred()) }) @@ -78,6 +82,23 @@ var _ = Describe("Apps", func() { MatchFields(IgnoreExtras, Fields{"GUID": Equal(app4GUID)}), )) }) + + When("filtering by label selector", func() { + BeforeEach(func() { + label := uuid.NewString() + addAppLabels(app1GUID, map[string]string{label: ""}) + + query = "?label_selector=" + label + }) + + It("lists apps with matching labels", func() { + Expect(resp).To(HaveRestyStatusCode(http.StatusOK)) + + Expect(result.Resources).To(ConsistOf( + MatchFields(IgnoreExtras, Fields{"GUID": Equal(app1GUID)}), + )) + }) + }) }) Describe("Create an app as a user", func() { diff --git a/tests/e2e/e2e_suite_test.go b/tests/e2e/e2e_suite_test.go index 96e3723e0..c9a256227 100644 --- a/tests/e2e/e2e_suite_test.go +++ b/tests/e2e/e2e_suite_test.go @@ -734,6 +734,12 @@ func addServiceInstanceLabels(serviceInstanceGUID string, labels map[string]stri addLabels("/v3/service_instances/"+serviceInstanceGUID, labels) } +func addAppLabels(appGUID string, labels map[string]string) { + GinkgoHelper() + + addLabels("/v3/apps/"+appGUID, labels) +} + func addLabels(resourcePath string, labels map[string]string) { GinkgoHelper()