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

Include broker fields when listing managed service instances #3448

Merged
merged 1 commit into from
Aug 29, 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
22 changes: 17 additions & 5 deletions api/handlers/service_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"sort"

apierrors "code.cloudfoundry.org/korifi/api/errors"
"code.cloudfoundry.org/korifi/api/handlers/include"
"code.cloudfoundry.org/korifi/api/payloads"
"code.cloudfoundry.org/korifi/api/routing"

Expand Down Expand Up @@ -38,19 +39,25 @@ type ServiceInstance struct {
serviceInstanceRepo CFServiceInstanceRepository
spaceRepo CFSpaceRepository
requestValidator RequestValidator
includeResolver *include.IncludeResolver[
[]repositories.ServiceInstanceRecord,
repositories.ServiceInstanceRecord,
]
}

func NewServiceInstance(
serverURL url.URL,
serviceInstanceRepo CFServiceInstanceRepository,
spaceRepo CFSpaceRepository,
requestValidator RequestValidator,
relationshipRepo include.ResourceRelationshipRepository,
) *ServiceInstance {
return &ServiceInstance{
serverURL: serverURL,
serviceInstanceRepo: serviceInstanceRepo,
spaceRepo: spaceRepo,
requestValidator: requestValidator,
includeResolver: include.NewIncludeResolver[[]repositories.ServiceInstanceRecord](relationshipRepo, presenter.NewResource(serverURL)),
}
}

Expand Down Expand Up @@ -112,20 +119,25 @@ func (h *ServiceInstance) list(r *http.Request) (*routing.Response, error) {
authInfo, _ := authorization.InfoFromContext(r.Context())
logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.service-instance.list")

listFilter := new(payloads.ServiceInstanceList)
err := h.requestValidator.DecodeAndValidateURLValues(r, listFilter)
payload := new(payloads.ServiceInstanceList)
err := h.requestValidator.DecodeAndValidateURLValues(r, payload)
if err != nil {
return nil, apierrors.LogAndReturn(logger, err, "Unable to decode request query parameters")
}

serviceInstanceList, err := h.serviceInstanceRepo.ListServiceInstances(r.Context(), authInfo, listFilter.ToMessage())
serviceInstances, err := h.serviceInstanceRepo.ListServiceInstances(r.Context(), authInfo, payload.ToMessage())
if err != nil {
return nil, apierrors.LogAndReturn(logger, err, "Failed to list service instance")
}

h.sortList(serviceInstanceList, listFilter.OrderBy)
h.sortList(serviceInstances, payload.OrderBy)

return routing.NewResponse(http.StatusOK).WithBody(presenter.ForList(presenter.ForServiceInstance, serviceInstanceList, h.serverURL, *r.URL)), nil
includedResources, err := h.includeResolver.ResolveIncludes(r.Context(), authInfo, serviceInstances, payload.IncludeResourceRules)
if err != nil {
return nil, apierrors.LogAndReturn(logger, err, "failed to build included resources")
}

return routing.NewResponse(http.StatusOK).WithBody(presenter.ForList(presenter.ForServiceInstance, serviceInstances, h.serverURL, *r.URL, includedResources...)), nil
}

// nolint:dupl
Expand Down
85 changes: 85 additions & 0 deletions api/handlers/service_instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ import (
. "code.cloudfoundry.org/korifi/api/handlers"
"code.cloudfoundry.org/korifi/api/handlers/fake"
"code.cloudfoundry.org/korifi/api/payloads"
"code.cloudfoundry.org/korifi/api/payloads/params"
"code.cloudfoundry.org/korifi/api/repositories"
"code.cloudfoundry.org/korifi/api/repositories/relationships"
korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
"code.cloudfoundry.org/korifi/model"
"code.cloudfoundry.org/korifi/model/services"
. "code.cloudfoundry.org/korifi/tests/matchers"
"code.cloudfoundry.org/korifi/tools"

Expand All @@ -23,6 +28,9 @@ var _ = Describe("ServiceInstance", func() {
var (
serviceInstanceRepo *fake.CFServiceInstanceRepository
spaceRepo *fake.CFSpaceRepository
serviceOfferingRepo *fake.CFServiceOfferingRepository
servicePlanRepo *fake.CFServicePlanRepository
serviceBrokerRepo *fake.CFServiceBrokerRepository
requestValidator *fake.RequestValidator

reqMethod string
Expand All @@ -37,6 +45,9 @@ var _ = Describe("ServiceInstance", func() {
}, nil)

spaceRepo = new(fake.CFSpaceRepository)
serviceBrokerRepo = new(fake.CFServiceBrokerRepository)
serviceOfferingRepo = new(fake.CFServiceOfferingRepository)
servicePlanRepo = new(fake.CFServicePlanRepository)

requestValidator = new(fake.RequestValidator)

Expand All @@ -45,6 +56,11 @@ var _ = Describe("ServiceInstance", func() {
serviceInstanceRepo,
spaceRepo,
requestValidator,
relationships.NewResourseRelationshipsRepo(
serviceOfferingRepo,
serviceBrokerRepo,
servicePlanRepo,
),
)
routerBuilder.LoadRoutes(apiHandler)

Expand Down Expand Up @@ -207,6 +223,75 @@ var _ = Describe("ServiceInstance", func() {
})
})

When("params to inlude fields[service_plan.service_offering.service_broker]", func() {
BeforeEach(func() {
requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&payloads.ServiceInstanceList{
IncludeResourceRules: []params.IncludeResourceRule{{
RelationshipPath: []string{"service_plan", "service_offering", "service_broker"},
Fields: []string{"name", "guid"},
}},
})
})

It("does not include resources", func() {
Expect(rr).Should(HaveHTTPStatus(http.StatusOK))
Expect(rr).To(HaveHTTPBody(Not(ContainSubstring("included"))))
})

When("the service instance is managed", func() {
BeforeEach(func() {
serviceInstanceRepo.ListServiceInstancesReturns([]repositories.ServiceInstanceRecord{
{GUID: "service-inst-guid-1", Type: korifiv1alpha1.ManagedType, PlanGUID: "service-plan-guid"},
{GUID: "service-inst-guid-2", Type: korifiv1alpha1.ManagedType, PlanGUID: "service-plan-guid"},
}, nil)

serviceBrokerRepo.ListServiceBrokersReturns([]repositories.ServiceBrokerRecord{{
ServiceBroker: services.ServiceBroker{
Name: "service-broker-name",
},
CFResource: model.CFResource{
GUID: "service-broker-guid",
},
}}, nil)

serviceOfferingRepo.ListOfferingsReturns([]repositories.ServiceOfferingRecord{{
ServiceOffering: services.ServiceOffering{
Name: "service-offering-name",
},
CFResource: model.CFResource{
GUID: "service-offering-guid",
},
ServiceBrokerGUID: "service-broker-guid",
}}, nil)

servicePlanRepo.ListPlansReturns([]repositories.ServicePlanRecord{{
ServicePlan: services.ServicePlan{
Name: "service-plan-name",
},
CFResource: model.CFResource{
GUID: "service-plan-guid",
},
ServiceOfferingGUID: "service-offering-guid",
}}, nil)

requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&payloads.ServiceInstanceList{
IncludeResourceRules: []params.IncludeResourceRule{{
RelationshipPath: []string{"service_plan", "service_offering", "service_broker"},
Fields: []string{"name", "guid"},
}},
})
})

It("includes broker fields in the response", func() {
Expect(rr).Should(HaveHTTPStatus(http.StatusOK))
Expect(rr).To(HaveHTTPBody(SatisfyAll(
MatchJSONPath("$.included.service_brokers[0].guid", "service-broker-guid"),
MatchJSONPath("$.included.service_brokers[0].name", "service-broker-name"),
)))
})
})
})

Describe("Order results", func() {
BeforeEach(func() {
serviceInstanceRepo.ListServiceInstancesReturns([]repositories.ServiceInstanceRecord{
Expand Down
1 change: 1 addition & 0 deletions api/handlers/service_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ var _ = Describe("ServicePlan", func() {
relationships.NewResourseRelationshipsRepo(
serviceOfferingRepo,
serviceBrokerRepo,
servicePlanRepo,
),
)
routerBuilder.LoadRoutes(apiHandler)
Expand Down
2 changes: 2 additions & 0 deletions api/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ func main() {
relationshipsRepo := relationships.NewResourseRelationshipsRepo(
serviceOfferingRepo,
serviceBrokerRepo,
servicePlanRepo,
)

apiHandlers := []routing.Routable{
Expand Down Expand Up @@ -408,6 +409,7 @@ func main() {
serviceInstanceRepo,
spaceRepo,
requestValidator,
relationshipsRepo,
),
handlers.NewServiceBinding(
*serverURL,
Expand Down
33 changes: 26 additions & 7 deletions api/payloads/service_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import (
"fmt"
"net/url"
"regexp"
"strings"

"code.cloudfoundry.org/korifi/api/payloads/params"
"code.cloudfoundry.org/korifi/api/payloads/parse"
"code.cloudfoundry.org/korifi/api/payloads/validation"
"code.cloudfoundry.org/korifi/api/repositories"
Expand Down Expand Up @@ -129,16 +131,29 @@ func (p *ServiceInstancePatch) UnmarshalJSON(data []byte) error {
}

type ServiceInstanceList struct {
Names string
GUIDs string
SpaceGUIDs string
OrderBy string
LabelSelector string
Names string
GUIDs string
SpaceGUIDs string
OrderBy string
LabelSelector string
IncludeResourceRules []params.IncludeResourceRule
}

func (l ServiceInstanceList) Validate() error {
return jellidation.ValidateStruct(&l,
jellidation.Field(&l.OrderBy, validation.OneOfOrderBy("created_at", "name", "updated_at")),
jellidation.Field(&l.IncludeResourceRules, jellidation.Each(jellidation.By(func(value any) error {
rule, ok := value.(params.IncludeResourceRule)
if !ok {
return fmt.Errorf("%T is not supported, IncludeResourceRule is expected", value)
}

if strings.Join(rule.RelationshipPath, ".") != "service_plan.service_offering.service_broker" {
return jellidation.NewError("invalid_fields_param", "must be fields[service_plan.service_offering.service_broker]")
}

return jellidation.Each(validation.OneOf("guid", "name")).Validate(rule.Fields)
}))),
)
}

Expand All @@ -152,11 +167,14 @@ func (l *ServiceInstanceList) ToMessage() repositories.ListServiceInstanceMessag
}

func (l *ServiceInstanceList) SupportedKeys() []string {
return []string{"names", "space_guids", "guids", "order_by", "per_page", "page", "label_selector"}
return []string{"names", "space_guids", "guids", "order_by", "label_selector", "fields[service_plan.service_offering.service_broker]"}
}

func (l *ServiceInstanceList) IgnoredKeys() []*regexp.Regexp {
return []*regexp.Regexp{regexp.MustCompile(`fields\[.+\]`)}
return []*regexp.Regexp{
regexp.MustCompile("page"),
regexp.MustCompile("per_page"),
}
}

func (l *ServiceInstanceList) DecodeFromURLValues(values url.Values) error {
Expand All @@ -165,5 +183,6 @@ func (l *ServiceInstanceList) DecodeFromURLValues(values url.Values) error {
l.GUIDs = values.Get("guids")
l.OrderBy = values.Get("order_by")
l.LabelSelector = values.Get("label_selector")
l.IncludeResourceRules = append(l.IncludeResourceRules, params.ParseFields(values)...)
return nil
}
10 changes: 9 additions & 1 deletion api/payloads/service_instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"strings"

"code.cloudfoundry.org/korifi/api/payloads"
"code.cloudfoundry.org/korifi/api/payloads/params"
"code.cloudfoundry.org/korifi/api/repositories"
"code.cloudfoundry.org/korifi/tools"
. "github.com/onsi/ginkgo/v2"
Expand All @@ -29,7 +30,12 @@ var _ = Describe("ServiceInstanceList", func() {
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("fields[service_plan.service_offering.service_broker]",
"fields[service_plan.service_offering.service_broker]=guid,name",
payloads.ServiceInstanceList{IncludeResourceRules: []params.IncludeResourceRule{{
RelationshipPath: []string{"service_plan", "service_offering", "service_broker"},
Fields: []string{"guid", "name"},
}}}),
Entry("label_selector=foo", "label_selector=foo", payloads.ServiceInstanceList{LabelSelector: "foo"}),
)

Expand All @@ -39,6 +45,8 @@ var _ = Describe("ServiceInstanceList", func() {
Expect(decodeErr).To(MatchError(ContainSubstring(expectedErrMsg)))
},
Entry("invalid order_by", "order_by=foo", "value must be one of"),
Entry("invalid fields", "fields[foo]=bar", "unsupported query parameter: fields[foo]"),
Entry("invalid service broker fields", "fields[service_plan.service_offering.service_broker]=foo", "value must be one of"),
)

Describe("ToMessage", func() {
Expand Down
Loading
Loading