Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support listing broker by names #3379

Merged
merged 2 commits into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions api/handlers/fake/cfservice_broker_repository.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 7 additions & 2 deletions api/handlers/service_broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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")
}
Expand Down
31 changes: 30 additions & 1 deletion api/handlers/service_broker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"))
Expand All @@ -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"))
Expand Down
22 changes: 22 additions & 0 deletions api/payloads/service_broker.go
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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),
}
}
31 changes: 31 additions & 0 deletions api/payloads/service_broker_test.go
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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"},
}))
})
})
})
29 changes: 20 additions & 9 deletions api/repositories/service_broker_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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)
Expand All @@ -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
}
35 changes: 33 additions & 2 deletions api/repositories/service_broker_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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())
})

Expand Down Expand Up @@ -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())
})
})
})
})
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down