Skip to content

Commit

Permalink
Include broker fields when listing managed service instances
Browse files Browse the repository at this point in the history
* Allow `managed` value for `CFServiceInstance.Type.Spec`
* Introduce the `PlanGUID` field on `CFServiceInstanceSpec`
* `ServiceInstanceRecord` implements the `relationships.Resource`
  interface. User provided services have no relationships, managed ones
  have a relationship to the service plan
* The relationships repository returns empty list for resources with no
  relationships.
* The broker fields are available in the `GET /v3/service_instances`
  response only if requested and the result service instances contain
  managed ones. Requesting those fields for user provided service
  instances only does not result into `include` field in the response

fixes #3289
  • Loading branch information
danail-branekov authored and georgethebeatle committed Aug 29, 2024
1 parent 32bb50a commit 4fbe948
Show file tree
Hide file tree
Showing 15 changed files with 511 additions and 44 deletions.
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

0 comments on commit 4fbe948

Please sign in to comment.