Merge pull request #3379 from /issues/3267-list-brokers-by-name

Support listing broker by names

api/handlers/service_broker.go

@@ -21,7 +21,7 @@ const (
 //counterfeiter:generate -o fake -fake-name CFServiceBrokerRepository . CFServiceBrokerRepository
 type CFServiceBrokerRepository interface {
 	CreateServiceBroker(context.Context, authorization.Info, repositories.CreateServiceBrokerMessage) (repositories.ServiceBrokerResource, error)
-	ListServiceBrokers(context.Context, authorization.Info) ([]repositories.ServiceBrokerResource, error)
+	ListServiceBrokers(context.Context, authorization.Info, repositories.ListServiceBrokerMessage) ([]repositories.ServiceBrokerResource, error)
 }
 
 type ServiceBroker struct {
@@ -64,7 +64,12 @@ func (h *ServiceBroker) list(r *http.Request) (*routing.Response, error) {
 	authInfo, _ := authorization.InfoFromContext(r.Context())
 	logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.service-broker.list")
 
-	brokers, err := h.serviceBrokerRepo.ListServiceBrokers(r.Context(), authInfo)
+	var serviceBrokerListFilter payloads.ServiceBrokerList
+	if err := h.requestValidator.DecodeAndValidateURLValues(r, &serviceBrokerListFilter); err != nil {
+		return nil, apierrors.LogAndReturn(logger, err, "failed to decode request values")
+	}
+
+	brokers, err := h.serviceBrokerRepo.ListServiceBrokers(r.Context(), authInfo, serviceBrokerListFilter.ToMessage())
 	if err != nil {
 		return nil, apierrors.LogAndReturn(logger, err, "failed to list service brokers")
 	}

api/handlers/service_broker_test.go

@@ -12,6 +12,7 @@ import (
 	"code.cloudfoundry.org/korifi/model"
 	"code.cloudfoundry.org/korifi/model/services"
 	. "code.cloudfoundry.org/korifi/tests/matchers"
+	. "github.com/onsi/gomega/gstruct"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -115,8 +116,11 @@ var _ = Describe("ServiceBroker", func() {
 
 		It("lists the service brokers", func() {
 			Expect(serviceBrokerRepo.ListServiceBrokersCallCount()).To(Equal(1))
-			_, actualAuthInfo := serviceBrokerRepo.ListServiceBrokersArgsForCall(0)
+			_, actualAuthInfo, actualListMsg := serviceBrokerRepo.ListServiceBrokersArgsForCall(0)
 			Expect(actualAuthInfo).To(Equal(authInfo))
+			Expect(actualListMsg).To(MatchAllFields(Fields{
+				"Names": BeEmpty(),
+			}))
 
 			Expect(rr).Should(HaveHTTPStatus(http.StatusOK))
 			Expect(rr).To(HaveHTTPHeaderWithValue("Content-Type", "application/json"))
@@ -128,6 +132,31 @@ var _ = Describe("ServiceBroker", func() {
 			)))
 		})
 
+		When("filtering query params are provided", func() {
+			BeforeEach(func() {
+				requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&payloads.ServiceBrokerList{
+					Names: "b1,b2",
+				})
+			})
+
+			It("passes them to the repository", func() {
+				Expect(serviceBrokerRepo.ListServiceBrokersCallCount()).To(Equal(1))
+				_, _, message := serviceBrokerRepo.ListServiceBrokersArgsForCall(0)
+
+				Expect(message.Names).To(ConsistOf("b1", "b2"))
+			})
+		})
+
+		When("decoding query parameters fails", func() {
+			BeforeEach(func() {
+				requestValidator.DecodeAndValidateURLValuesReturns(errors.New("decode-err"))
+			})
+
+			It("returns an error", func() {
+				expectUnknownError()
+			})
+		})
+
 		When("listing service brokers fails", func() {
 			BeforeEach(func() {
 				serviceBrokerRepo.ListServiceBrokersReturns(nil, errors.New("list-brokers-error"))

api/payloads/service_broker.go

@@ -1,6 +1,9 @@
 package payloads
 
 import (
+	"net/url"
+
+	"code.cloudfoundry.org/korifi/api/payloads/parse"
 	"code.cloudfoundry.org/korifi/api/payloads/validation"
 	"code.cloudfoundry.org/korifi/api/repositories"
 	"code.cloudfoundry.org/korifi/model"
@@ -40,3 +43,22 @@ func (c ServiceBrokerCreate) ToCreateServiceBrokerMessage() repositories.CreateS
 		Credentials: c.Authentication.Credentials,
 	}
 }
+
+type ServiceBrokerList struct {
+	Names string
+}
+
+func (b *ServiceBrokerList) DecodeFromURLValues(values url.Values) error {
+	b.Names = values.Get("names")
+	return nil
+}
+
+func (b *ServiceBrokerList) SupportedKeys() []string {
+	return []string{"names"}
+}
+
+func (b *ServiceBrokerList) ToMessage() repositories.ListServiceBrokerMessage {
+	return repositories.ListServiceBrokerMessage{
+		Names: parse.ArrayParam(b.Names),
+	}
+}

api/payloads/service_broker_test.go

@@ -1,6 +1,8 @@
 package payloads_test
 
 import (
+	"net/http"
+
 	"code.cloudfoundry.org/korifi/api/payloads"
 	"code.cloudfoundry.org/korifi/api/repositories"
 	"code.cloudfoundry.org/korifi/model"
@@ -111,3 +113,32 @@ var _ = Describe("ServiceBrokerCreate", func() {
 		})
 	})
 })
+
+var _ = Describe("ServiceBrokerList", func() {
+	var serviceBrokerList payloads.ServiceBrokerList
+
+	BeforeEach(func() {
+		serviceBrokerList = payloads.ServiceBrokerList{
+			Names: "b1, b2",
+		}
+	})
+
+	Describe("decodes from url values", func() {
+		It("succeeds", func() {
+			req, err := http.NewRequest("GET", "http://foo.com/bar?names=foo,bar", nil)
+			Expect(err).NotTo(HaveOccurred())
+			err = validator.DecodeAndValidateURLValues(req, &serviceBrokerList)
+
+			Expect(err).NotTo(HaveOccurred())
+			Expect(serviceBrokerList.Names).To(Equal("foo,bar"))
+		})
+	})
+
+	Describe("ToMessage", func() {
+		It("converts to repo message correctly", func() {
+			Expect(serviceBrokerList.ToMessage()).To(Equal(repositories.ListServiceBrokerMessage{
+				Names: []string{"b1", "b2"},
+			}))
+		})
+	})
+})

api/repositories/service_broker_repository.go

@@ -3,13 +3,15 @@ package repositories
 import (
 	"context"
 	"fmt"
+	"slices"
 
 	"code.cloudfoundry.org/korifi/api/authorization"
 	apierrors "code.cloudfoundry.org/korifi/api/errors"
 	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
 	"code.cloudfoundry.org/korifi/controllers/controllers/services/credentials"
 	"code.cloudfoundry.org/korifi/model"
 	"code.cloudfoundry.org/korifi/model/services"
+	"github.com/BooleanCat/go-functional/iter"
 	"github.com/google/uuid"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -27,6 +29,18 @@ type CreateServiceBrokerMessage struct {
 	Credentials services.BrokerCredentials
 }
 
+type ListServiceBrokerMessage struct {
+	Names []string
+}
+
+func (l ListServiceBrokerMessage) matches(b korifiv1alpha1.CFServiceBroker) bool {
+	if len(l.Names) == 0 {
+		return true
+	}
+
+	return slices.Contains(l.Names, b.Spec.Name)
+}
+
 type ServiceBrokerRepo struct {
 	userClientFactory authorization.UserK8sClientFactory
 	rootNamespace     string
@@ -93,10 +107,10 @@ func (r *ServiceBrokerRepo) CreateServiceBroker(ctx context.Context, authInfo au
 		return ServiceBrokerResource{}, apierrors.FromK8sError(err, ServiceBrokerResourceType)
 	}
 
-	return toServiceBrokerResource(cfServiceBroker), nil
+	return toServiceBrokerResource(*cfServiceBroker), nil
 }
 
-func toServiceBrokerResource(cfServiceBroker *korifiv1alpha1.CFServiceBroker) ServiceBrokerResource {
+func toServiceBrokerResource(cfServiceBroker korifiv1alpha1.CFServiceBroker) ServiceBrokerResource {
 	return ServiceBrokerResource{
 		ServiceBroker: cfServiceBroker.Spec.ServiceBroker,
 		CFResource: model.CFResource{
@@ -136,7 +150,7 @@ func (r *ServiceBrokerRepo) GetState(ctx context.Context, authInfo authorization
 	return model.CFResourceState{}, nil
 }
 
-func (r *ServiceBrokerRepo) ListServiceBrokers(ctx context.Context, authInfo authorization.Info) ([]ServiceBrokerResource, error) {
+func (r *ServiceBrokerRepo) ListServiceBrokers(ctx context.Context, authInfo authorization.Info, message ListServiceBrokerMessage) ([]ServiceBrokerResource, error) {
 	userClient, err := r.userClientFactory.BuildClient(authInfo)
 	if err != nil {
 		return nil, fmt.Errorf("failed to build user client: %w", err)
@@ -148,13 +162,10 @@ func (r *ServiceBrokerRepo) ListServiceBrokers(ctx context.Context, authInfo aut
 		// All authenticated users are allowed to list brokers. Therefore, the
 		// usual pattern of checking for forbidden error and return an empty
 		// list does not make sense here
-		return nil, apierrors.FromK8sError(err, ServiceBrokerResourceType)
+		return nil, fmt.Errorf("failed to list brokers: %w", apierrors.FromK8sError(err, ServiceBrokerResourceType))
 	}
 
-	result := []ServiceBrokerResource{}
-	for _, broker := range brokersList.Items {
-		result = append(result, toServiceBrokerResource(&broker))
-	}
+	brokers := iter.Filter(iter.Lift(brokersList.Items), message.matches)
 
-	return result, nil
+	return iter.Collect(iter.Map(brokers, toServiceBrokerResource)), nil
 }

api/repositories/service_broker_repository_test.go

@@ -228,9 +228,14 @@ var _ = Describe("ServiceBrokerRepo", func() {
 	})
 
 	Describe("ListServiceBrokers", func() {
-		var brokers []repositories.ServiceBrokerResource
+		var (
+			brokers []repositories.ServiceBrokerResource
+			message repositories.ListServiceBrokerMessage
+		)
 
 		BeforeEach(func() {
+			message = repositories.ListServiceBrokerMessage{}
+
 			Expect(k8sClient.Create(ctx, &korifiv1alpha1.CFServiceBroker{
 				ObjectMeta: metav1.ObjectMeta{
 					Namespace: rootNamespace,
@@ -266,7 +271,7 @@ var _ = Describe("ServiceBrokerRepo", func() {
 
 		JustBeforeEach(func() {
 			var err error
-			brokers, err = repo.ListServiceBrokers(ctx, authInfo)
+			brokers, err = repo.ListServiceBrokers(ctx, authInfo, message)
 			Expect(err).NotTo(HaveOccurred())
 		})
 
@@ -302,5 +307,31 @@ var _ = Describe("ServiceBrokerRepo", func() {
 				}),
 			))
 		})
+
+		When("a name filter is applied", func() {
+			BeforeEach(func() {
+				message.Names = []string{"second-broker"}
+			})
+
+			It("only returns the matching brokers", func() {
+				Expect(brokers).To(ConsistOf(
+					MatchFields(IgnoreExtras, Fields{
+						"ServiceBroker": MatchFields(IgnoreExtras, Fields{
+							"Name": Equal("second-broker"),
+						}),
+					}),
+				))
+			})
+		})
+
+		When("a nonexistent name filter is applied", func() {
+			BeforeEach(func() {
+				message.Names = []string{"no-such-broker"}
+			})
+
+			It("returns an empty list", func() {
+				Expect(brokers).To(BeEmpty())
+			})
+		})
 	})
 })

go.mod

@@ -7,6 +7,7 @@ toolchain go1.22.5
 require (
 	code.cloudfoundry.org/bytefmt v0.0.0-20211005130812-5bb3c17173e5
 	code.cloudfoundry.org/go-loggregator/v8 v8.0.5
+	github.com/BooleanCat/go-functional v1.1.0
 	github.com/Masterminds/semver v1.5.0
 	github.com/PaesslerAG/jsonpath v0.1.1
 	github.com/SermoDigital/jose v0.9.2-0.20161205224733-f6df55f235c2

go.sum

@@ -36,6 +36,8 @@ github.com/Azure/go-autorest/logger v0.2.1 h1:IG7i4p/mDa2Ce4TRyAO8IHnVhAVF3RFU+Z
 github.com/Azure/go-autorest/logger v0.2.1/go.mod h1:T9E3cAhj2VqvPOtCYAvby9aBXkZmbF5NWuPV8+WeEW8=
 github.com/Azure/go-autorest/tracing v0.6.0 h1:TYi4+3m5t6K48TGI9AUdb+IzbnSxvnvUMfuitfgcfuo=
 github.com/Azure/go-autorest/tracing v0.6.0/go.mod h1:+vhtPC754Xsa23ID7GlGsrdKBpUA79WCAKPPZVC2DeU=
+github.com/BooleanCat/go-functional v1.1.0 h1:ZU8ejc2u/71I5LZbI5qiJI75Ttw1FueLNmoccPt8nDI=
+github.com/BooleanCat/go-functional v1.1.0/go.mod h1:Zd1xLrGFohrDdjojLUCrzSex40yf/PVP2KB86ha9Qqg=
 github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
 github.com/BurntSushi/toml v1.3.2 h1:o7IhLm0Msx3BaB+n3Ag7L8EVlByGnpq14C4YWiu/gL8=
 github.com/BurntSushi/toml v1.3.2/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ=