Skip to content

Commit

Permalink
Support broker name filter when listing service plans
Browse files Browse the repository at this point in the history
* The broker controller sets the broker name under the
  `korifiv1alpha1.RelServiceBrokerNameLabel` label when reconciling
  broker catalog. This is safe as upon a broker rename the controller
  would reconcile its catalog again and would ammend plans/offering
  labels
* The plan repository filters plans by broker name whenever the broker
  names filter is specified
* While being here:
  - The constant `RelServiceBrokerLabel` has been renamed to
    `RelServiceBrokerGUIDLabel` to make it explicit that this is the
    broker guid
  - The constant `RelServiceOfferingLabel has been renamed to
    `RelServiceOfferingGUIDLabel` to make it explicit that this is the
    offering guid
  - The value of the `RelServiceBrokerGUIDLabel` and
    `RelServiceOfferingGUIDLabel` constants has the `-guid` suffix added
    in order to be more explicit

fixes #3420
  • Loading branch information
danail-branekov authored and georgethebeatle committed Aug 15, 2024
1 parent 79c4845 commit abc683f
Show file tree
Hide file tree
Showing 12 changed files with 63 additions and 31 deletions.
7 changes: 5 additions & 2 deletions api/payloads/service_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (

type ServicePlanList struct {
ServiceOfferingGUIDs string
BrokerNames string
Names string
Available *bool
IncludeResourceRules []params.IncludeResourceRule
Expand All @@ -34,21 +35,23 @@ func (l *ServicePlanList) ToMessage() repositories.ListServicePlanMessage {
return repositories.ListServicePlanMessage{
ServiceOfferingGUIDs: parse.ArrayParam(l.ServiceOfferingGUIDs),
Names: parse.ArrayParam(l.Names),
BrokerNames: parse.ArrayParam(l.BrokerNames),
Available: l.Available,
}
}

func (l *ServicePlanList) SupportedKeys() []string {
return []string{"service_offering_guids", "names", "available", "fields[service_offering.service_broker]", "page", "per_page", "include"}
return []string{"service_offering_guids", "names", "available", "fields[service_offering.service_broker]", "service_broker_names", "page", "per_page", "include"}
}

func (l *ServicePlanList) IgnoredKeys() []*regexp.Regexp {
return nil
return []*regexp.Regexp{regexp.MustCompile("space_guids")}
}

func (l *ServicePlanList) DecodeFromURLValues(values url.Values) error {
l.ServiceOfferingGUIDs = values.Get("service_offering_guids")
l.Names = values.Get("names")
l.BrokerNames = values.Get("service_broker_names")

available, err := parseBool(values.Get("available"))
if err != nil {
Expand Down
5 changes: 4 additions & 1 deletion api/payloads/service_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ var _ = Describe("ServicePlan", func() {
Entry("names", "names=b1,b2", payloads.ServicePlanList{Names: "b1,b2"}),
Entry("available", "available=true", payloads.ServicePlanList{Available: tools.PtrTo(true)}),
Entry("not available", "available=false", payloads.ServicePlanList{Available: tools.PtrTo(false)}),
Entry("include", "include=service_offering", payloads.ServicePlanList{
Entry("broker names", "service_broker_names=b1,b2", payloads.ServicePlanList{BrokerNames: "b1,b2"}),
Entry("include service offering", "include=service_offering", payloads.ServicePlanList{
IncludeResourceRules: []params.IncludeResourceRule{{
RelationshipPath: []string{"service_offering"},
Fields: []string{},
Expand Down Expand Up @@ -53,11 +54,13 @@ var _ = Describe("ServicePlan", func() {
It("converts payload to repository message", func() {
payload := payloads.ServicePlanList{
ServiceOfferingGUIDs: "b1,b2",
BrokerNames: "br1,br2",
Names: "n1,n2",
Available: tools.PtrTo(true),
}
Expect(payload.ToMessage()).To(Equal(repositories.ListServicePlanMessage{
ServiceOfferingGUIDs: []string{"b1", "b2"},
BrokerNames: []string{"br1", "br2"},
Names: []string{"n1", "n2"},
Available: tools.PtrTo(true),
}))
Expand Down
2 changes: 1 addition & 1 deletion api/repositories/service_offering_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,6 @@ func offeringToRecord(offering korifiv1alpha1.CFServiceOffering) ServiceOffering
Annotations: offering.Annotations,
},
},
ServiceBrokerGUID: offering.Labels[korifiv1alpha1.RelServiceBrokerLabel],
ServiceBrokerGUID: offering.Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel],
}
}
6 changes: 3 additions & 3 deletions api/repositories/service_offering_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ var _ = Describe("ServiceOfferingRepo", func() {
Namespace: rootNamespace,
Name: offeringGUID,
Labels: map[string]string{
korifiv1alpha1.RelServiceBrokerLabel: broker.Name,
korifiv1alpha1.RelServiceBrokerGUIDLabel: broker.Name,
},
Annotations: map[string]string{
"annotation": "annotation-value",
Expand Down Expand Up @@ -95,7 +95,7 @@ var _ = Describe("ServiceOfferingRepo", func() {
Namespace: rootNamespace,
Name: anotherOfferingGUID,
Labels: map[string]string{
korifiv1alpha1.RelServiceBrokerLabel: "another-broker",
korifiv1alpha1.RelServiceBrokerGUIDLabel: "another-broker",
},
},
Spec: korifiv1alpha1.CFServiceOfferingSpec{
Expand Down Expand Up @@ -142,7 +142,7 @@ var _ = Describe("ServiceOfferingRepo", func() {
"CreatedAt": Not(BeZero()),
"UpdatedAt": BeNil(),
"Metadata": MatchAllFields(Fields{
"Labels": HaveKeyWithValue(korifiv1alpha1.RelServiceBrokerLabel, broker.Name),
"Labels": HaveKeyWithValue(korifiv1alpha1.RelServiceBrokerGUIDLabel, broker.Name),
"Annotations": HaveKeyWithValue("annotation", "annotation-value"),
}),
}),
Expand Down
6 changes: 4 additions & 2 deletions api/repositories/service_plan_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,14 @@ type ServicePlanRepo struct {
type ListServicePlanMessage struct {
ServiceOfferingGUIDs []string
Names []string
BrokerNames []string
Available *bool
}

func (m *ListServicePlanMessage) matches(cfServicePlan korifiv1alpha1.CFServicePlan) bool {
return tools.EmptyOrContains(m.ServiceOfferingGUIDs, cfServicePlan.Labels[korifiv1alpha1.RelServiceOfferingLabel]) &&
return tools.EmptyOrContains(m.ServiceOfferingGUIDs, cfServicePlan.Labels[korifiv1alpha1.RelServiceOfferingGUIDLabel]) &&
tools.EmptyOrContains(m.Names, cfServicePlan.Spec.Name) &&
tools.EmptyOrContains(m.BrokerNames, cfServicePlan.Labels[korifiv1alpha1.RelServiceBrokerNameLabel]) &&
tools.NilOrEquals(m.Available, isAvailable(cfServicePlan))
}

Expand Down Expand Up @@ -209,7 +211,7 @@ func (r *ServicePlanRepo) planToRecord(ctx context.Context, authInfo authorizati
Type: plan.Spec.Visibility.Type,
Organizations: organizations,
},
ServiceOfferingGUID: plan.Labels[korifiv1alpha1.RelServiceOfferingLabel],
ServiceOfferingGUID: plan.Labels[korifiv1alpha1.RelServiceOfferingGUIDLabel],
Available: isAvailable(plan),
}, nil
}
Expand Down
23 changes: 20 additions & 3 deletions api/repositories/service_plan_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ var _ = Describe("ServicePlanRepo", func() {
Namespace: rootNamespace,
Name: planGUID,
Labels: map[string]string{
korifiv1alpha1.RelServiceOfferingLabel: "offering-guid",
korifiv1alpha1.RelServiceOfferingGUIDLabel: "offering-guid",
korifiv1alpha1.RelServiceBrokerNameLabel: "broker-name",
},
Annotations: map[string]string{
"annotation": "annotation-value",
Expand Down Expand Up @@ -142,7 +143,7 @@ var _ = Describe("ServicePlanRepo", func() {
"CreatedAt": Not(BeZero()),
"UpdatedAt": BeNil(),
"Metadata": MatchAllFields(Fields{
"Labels": HaveKeyWithValue(korifiv1alpha1.RelServiceOfferingLabel, "offering-guid"),
"Labels": HaveKeyWithValue(korifiv1alpha1.RelServiceOfferingGUIDLabel, "offering-guid"),
"Annotations": HaveKeyWithValue("annotation", "annotation-value"),
}),
}),
Expand Down Expand Up @@ -189,7 +190,8 @@ var _ = Describe("ServicePlanRepo", func() {
Namespace: rootNamespace,
Name: otherPlanGUID,
Labels: map[string]string{
korifiv1alpha1.RelServiceOfferingLabel: "other-offering-guid",
korifiv1alpha1.RelServiceOfferingGUIDLabel: "other-offering-guid",
korifiv1alpha1.RelServiceBrokerNameLabel: "other-broker-name",
},
},
Spec: korifiv1alpha1.CFServicePlanSpec{
Expand Down Expand Up @@ -265,6 +267,21 @@ var _ = Describe("ServicePlanRepo", func() {
})))
})
})

When("filtering by broker name", func() {
BeforeEach(func() {
message.BrokerNames = []string{"other-broker-name"}
})

It("returns matching service plans", func() {
Expect(listErr).NotTo(HaveOccurred())
Expect(listedPlans).To(ConsistOf(MatchFields(IgnoreExtras, Fields{
"CFResource": MatchFields(IgnoreExtras, Fields{
"GUID": Equal(otherPlanGUID),
}),
})))
})
})
})

Describe("PlanVisibility", func() {
Expand Down
7 changes: 4 additions & 3 deletions controllers/api/v1alpha1/shared_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ const (
PropagateDeletionAnnotation = "cloudfoundry.org/propagate-deletion"
PropagatedFromLabel = "cloudfoundry.org/propagated-from"

RelationshipsLabelPrefix = "korifi.cloudfoundry.org/rel-"
RelServiceBrokerLabel = RelationshipsLabelPrefix + "service_broker"
RelServiceOfferingLabel = RelationshipsLabelPrefix + "service_offering"
RelationshipsLabelPrefix = "korifi.cloudfoundry.org/rel-"
RelServiceBrokerGUIDLabel = RelationshipsLabelPrefix + "service-broker-guid"
RelServiceBrokerNameLabel = RelationshipsLabelPrefix + "service-broker-name"
RelServiceOfferingGUIDLabel = RelationshipsLabelPrefix + "service-offering-guid"
)

type Lifecycle struct {
Expand Down
10 changes: 6 additions & 4 deletions controllers/controllers/services/brokers/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,8 @@ func (r *Reconciler) reconcileCatalogService(ctx context.Context, cfServiceBroke
if serviceOffering.Labels == nil {
serviceOffering.Labels = map[string]string{}
}
serviceOffering.Labels[korifiv1alpha1.RelServiceBrokerLabel] = cfServiceBroker.Name
serviceOffering.Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel] = cfServiceBroker.Name
serviceOffering.Labels[korifiv1alpha1.RelServiceBrokerNameLabel] = cfServiceBroker.Spec.Name

var err error
serviceOffering.Spec.ServiceOffering, err = toSpecServiceOffering(catalogService)
Expand All @@ -216,7 +217,7 @@ func (r *Reconciler) reconcileCatalogService(ctx context.Context, cfServiceBroke
func (r *Reconciler) reconcileCatalogPlan(ctx context.Context, serviceOffering *korifiv1alpha1.CFServiceOffering, catalogPlan osbapi.Plan) error {
servicePlan := &korifiv1alpha1.CFServicePlan{
ObjectMeta: metav1.ObjectMeta{
Name: tools.NamespacedUUID(serviceOffering.Labels[korifiv1alpha1.RelServiceBrokerLabel], catalogPlan.ID),
Name: tools.NamespacedUUID(serviceOffering.Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel], catalogPlan.ID),
Namespace: serviceOffering.Namespace,
},
}
Expand All @@ -225,8 +226,9 @@ func (r *Reconciler) reconcileCatalogPlan(ctx context.Context, serviceOffering *
if servicePlan.Labels == nil {
servicePlan.Labels = map[string]string{}
}
servicePlan.Labels[korifiv1alpha1.RelServiceBrokerLabel] = serviceOffering.Labels[korifiv1alpha1.RelServiceBrokerLabel]
servicePlan.Labels[korifiv1alpha1.RelServiceOfferingLabel] = serviceOffering.Name
servicePlan.Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel] = serviceOffering.Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel]
servicePlan.Labels[korifiv1alpha1.RelServiceBrokerNameLabel] = serviceOffering.Labels[korifiv1alpha1.RelServiceBrokerNameLabel]
servicePlan.Labels[korifiv1alpha1.RelServiceOfferingGUIDLabel] = serviceOffering.Name

rawMetadata, err := json.Marshal(catalogPlan.Metadata)
if err != nil {
Expand Down
18 changes: 11 additions & 7 deletions controllers/controllers/services/brokers/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,10 @@ var _ = Describe("CFServiceBroker", func() {
g.Expect(offerings.Items).To(HaveLen(1))

offering := offerings.Items[0]
g.Expect(offering.Labels).To(HaveKeyWithValue(korifiv1alpha1.RelServiceBrokerLabel, serviceBroker.Name))
g.Expect(offering.Labels).To(SatisfyAll(
HaveKeyWithValue(korifiv1alpha1.RelServiceBrokerGUIDLabel, serviceBroker.Name),
HaveKeyWithValue(korifiv1alpha1.RelServiceBrokerNameLabel, serviceBroker.Spec.Name),
))
g.Expect(offering.Spec).To(MatchAllFields(Fields{
"ServiceOffering": MatchAllFields(Fields{
"Name": Equal("service-name"),
Expand Down Expand Up @@ -187,8 +190,9 @@ var _ = Describe("CFServiceBroker", func() {
plan := plans.Items[0]

g.Expect(plan.Labels).To(SatisfyAll(
HaveKeyWithValue(korifiv1alpha1.RelServiceBrokerLabel, serviceBroker.Name),
HaveKeyWithValue(korifiv1alpha1.RelServiceOfferingLabel, offerings.Items[0].Name),
HaveKeyWithValue(korifiv1alpha1.RelServiceBrokerGUIDLabel, serviceBroker.Name),
HaveKeyWithValue(korifiv1alpha1.RelServiceBrokerNameLabel, serviceBroker.Spec.Name),
HaveKeyWithValue(korifiv1alpha1.RelServiceOfferingGUIDLabel, offerings.Items[0].Name),
))
g.Expect(plan.Spec).To(MatchAllFields(Fields{
"ServicePlan": MatchAllFields(Fields{
Expand Down Expand Up @@ -332,8 +336,8 @@ var _ = Describe("CFServiceBroker", func() {
g.Expect(offerings.Items).To(HaveLen(2))

brokerGUIDs := []string{
offerings.Items[0].Labels[korifiv1alpha1.RelServiceBrokerLabel],
offerings.Items[1].Labels[korifiv1alpha1.RelServiceBrokerLabel],
offerings.Items[0].Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel],
offerings.Items[1].Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel],
}
g.Expect(brokerGUIDs).To(ConsistOf(serviceBroker.Name, anotherServiceBroker.Name))

Expand All @@ -349,8 +353,8 @@ var _ = Describe("CFServiceBroker", func() {
g.Expect(plans.Items).To(HaveLen(2))

brokerGUIDs := []string{
plans.Items[0].Labels[korifiv1alpha1.RelServiceBrokerLabel],
plans.Items[1].Labels[korifiv1alpha1.RelServiceBrokerLabel],
plans.Items[0].Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel],
plans.Items[1].Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel],
}
g.Expect(brokerGUIDs).To(ConsistOf(serviceBroker.Name, anotherServiceBroker.Name))

Expand Down
4 changes: 2 additions & 2 deletions tests/e2e/e2e_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1226,7 +1226,7 @@ func cleanupBroker(brokerGUID string) {
&korifiv1alpha1.CFServiceOffering{},
client.InNamespace(rootNamespace),
client.MatchingLabels{
korifiv1alpha1.RelServiceBrokerLabel: brokerGUID,
korifiv1alpha1.RelServiceBrokerGUIDLabel: brokerGUID,
},
)).To(Succeed())

Expand All @@ -1235,7 +1235,7 @@ func cleanupBroker(brokerGUID string) {
&korifiv1alpha1.CFServicePlan{},
client.InNamespace(rootNamespace),
client.MatchingLabels{
korifiv1alpha1.RelServiceBrokerLabel: brokerGUID,
korifiv1alpha1.RelServiceBrokerGUIDLabel: brokerGUID,
},
)).To(Succeed())
}
2 changes: 1 addition & 1 deletion tests/e2e/service_brokers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ var _ = Describe("Service Brokers", func() {
Expect(resp).To(HaveRestyStatusCode(http.StatusOK))
Expect(servicePlans.Resources).To(ContainElement(MatchFields(IgnoreExtras, Fields{
"Metadata": PointTo(MatchFields(IgnoreExtras, Fields{
"Labels": HaveKeyWithValue(korifiv1alpha1.RelServiceBrokerLabel, brokerGUID),
"Labels": HaveKeyWithValue(korifiv1alpha1.RelServiceBrokerGUIDLabel, brokerGUID),
})),
})))
})
Expand Down
4 changes: 2 additions & 2 deletions tests/e2e/service_plans_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ var _ = Describe("Service Plans", func() {
Expect(resp).To(HaveRestyStatusCode(http.StatusOK))
Expect(result.Resources).To(ContainElement(MatchFields(IgnoreExtras, Fields{
"Metadata": PointTo(MatchFields(IgnoreExtras, Fields{
"Labels": HaveKeyWithValue(korifiv1alpha1.RelServiceBrokerLabel, brokerGUID),
"Labels": HaveKeyWithValue(korifiv1alpha1.RelServiceBrokerGUIDLabel, brokerGUID),
})),
})))
})
Expand All @@ -58,7 +58,7 @@ var _ = Describe("Service Plans", func() {
Expect(listResp).To(HaveRestyStatusCode(http.StatusOK))

brokerPlans := iter.Lift(plans.Resources).Filter(func(r resource) bool {
return r.Metadata.Labels[korifiv1alpha1.RelServiceBrokerLabel] == brokerGUID
return r.Metadata.Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel] == brokerGUID
}).Collect()

Expect(brokerPlans).NotTo(BeEmpty())
Expand Down

0 comments on commit abc683f

Please sign in to comment.