Skip to content
This repository has been archived by the owner on May 6, 2022. It is now read-only.

Add List/Relist Catalog Restrictions #1773

Merged
merged 53 commits into from
Apr 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
c0de801
Pause, going to try to use the label selector again.
Feb 28, 2018
4060ca1
A working example using label selectors backing code for class.
Feb 28, 2018
3eac4e2
Working simple example of classes and plans filtering.
Feb 28, 2018
c1fd237
Cleaning up the types.
Feb 28, 2018
9dd7bc2
Add comments for current changes.
Feb 28, 2018
7ecd694
Adding feature flag for Catalog Restrictions.
Feb 28, 2018
72f7539
A better way to filter the broker response for while/blacklists.
Mar 5, 2018
8cb4639
Adding documentation for how to use the label selectors.
Mar 6, 2018
815dba4
Adding whitespace and reordering comments to avoid confusion and delays.
Mar 9, 2018
4bc662a
Changing to use list, adding tests for helper.
Mar 12, 2018
e6c99d8
Call the catalog restriction type ServiceCatalogRestrictions
Mar 14, 2018
bd96336
Call the catalog restriction type ClusterServiceCatalogRestrictions
Mar 14, 2018
25cf545
Document the consts for filter.
Mar 14, 2018
6cdeb3b
document comments are strict.
Mar 15, 2018
48f4444
No comma. I blame Doug.
Mar 15, 2018
84f52a9
Revert "Pass correct plan ID in deprovision request (for both deletin…
Mar 16, 2018
94c89b0
Merge remote-tracking branch 'upstream/master'
Mar 16, 2018
4b7bc89
Merge remote-tracking branch 'upstream/master'
Mar 23, 2018
2e08402
Merge with master.
Mar 23, 2018
f959fd2
Changing types to just be lists of strings. moving some conversion to…
Mar 23, 2018
1590bd8
Getting Can't handle <nil> from the fuzzer.
Mar 23, 2018
ad38b27
just use []string
Mar 26, 2018
ebff045
Merge with master.
Apr 2, 2018
2fe49b8
Fixing up feature gate.
Apr 2, 2018
5785b69
Make the flag generic in the unit test.
Apr 3, 2018
0ce636c
Fix ConvertToSelector comment.
Apr 3, 2018
f0a23ad
Moving convert functions out of conversion.go.
Apr 3, 2018
0eb1529
make it clear we will use for list/relist.
Apr 3, 2018
9dcfa79
Fix comment for NewPredicate
Apr 3, 2018
76b5450
Fix CatalogRestrictions comment.
Apr 3, 2018
9039cf0
fix openapi.
Apr 3, 2018
5de5989
fix ConvertCluster function comments
Apr 3, 2018
d33b2a0
Update comment in type.
Apr 4, 2018
4285b72
Remove the old test no longer needed.
Apr 4, 2018
7144232
move filter test code to the new file location.
Apr 4, 2018
b6507f8
Forgot the C header.
Apr 4, 2018
71a7d85
copy godoc for unversioned type.
Apr 4, 2018
d6f8448
Reword commend from feedback.
Apr 4, 2018
88022e6
Open api changed after generation. update.
Apr 4, 2018
114dcb1
upate comments.
Apr 4, 2018
1b105b4
adding integration test, super simple.
Apr 5, 2018
ce24688
Merge branch 'master' into whitelist_from_labels
n3wscott Apr 16, 2018
5e9e946
fmt'ed the features.
Apr 16, 2018
b7276aa
yolo, no flag.
Apr 16, 2018
60650e8
Rework integration test to not assume the flag.
Apr 16, 2018
e3aed41
Remove more cruft from the flag.
Apr 17, 2018
63f5691
Merge branch 'master' into whitelist_from_labels
Apr 19, 2018
14c39d1
godocs match 100% now.
Apr 19, 2018
6cbedc4
Generator generated.
Apr 20, 2018
0b99c5d
Merge remote-tracking branch 'upstream/master' into whitelist_from_la…
Apr 20, 2018
a75f8ee
ran deps ensure, got this
Apr 20, 2018
362278f
Merge with master.
Apr 20, 2018
dfdabaa
Revert sort of... back to k8s 1.10 gen.
Apr 20, 2018
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
5 changes: 3 additions & 2 deletions Gopkg.lock

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

4 changes: 4 additions & 0 deletions charts/catalog/templates/controller-manager-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ spec:
- --feature-gates
- AsyncBindingOperations=true
{{- end }}
{{- if .Values.catalogRestrictionsEnabled }}
- --feature-gates
- CatalogRestrictions=true
{{- end }}
{{- if .Values.namespacedServiceBrokerEnabled }}
- --feature-gates
- NamespacedServiceBroker=true
Expand Down
53 changes: 53 additions & 0 deletions pkg/apis/servicecatalog/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,59 @@ type CommonServiceBrokerSpec struct {
// RelistRequests is a strictly increasing, non-negative integer counter that
// can be manually incremented by a user to manually trigger a relist.
RelistRequests int64

// CatalogRestrictions is a set of restrictions on which of a broker's services
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to put the same godoc as v1beta1 here.

Copy link
Contributor Author

@n3wscott n3wscott Apr 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok updated. too much codez... (will push soon)

// and plans have resources created for them.
CatalogRestrictions *CatalogRestrictions
}

// CatalogRestrictions is a set of restrictions on which of a broker's services
// and plans have resources created for them.
//
// Some examples of this object are as follows:
//
// This is an example of a whitelist on service externalName.
// Goal: Only list Services with the externalName of FooService and BarService,
// Solution: restrictions := ServiceCatalogRestrictions{
// ServiceClass: ["externalName in (FooService, BarService)"]
// }
//
// This is an example of a blacklist on service externalName.
// Goal: Allow all services except the ones with the externalName of FooService and BarService,
// Solution: restrictions := ServiceCatalogRestrictions{
// ServiceClass: ["externalName notin (FooService, BarService)"]
// }
//
// This whitelists plans called "Demo", and blacklists (but only a single element in
// the list) a service and a plan.
// Goal: Allow all plans with the externalName demo, but not AABBCC, and not a specific service by name,
// Solution: restrictions := ServiceCatalogRestrictions{
// ServiceClass: ["name!=AABBB-CCDD-EEGG-HIJK"]
// ServicePlan: ["externalName in (Demo)", "name!=AABBCC"]
// }
//
// CatalogRestrictions strings have a special format similar to Label Selectors,
// except the catalog supports only a very specific property set.
//
// The predicate format is expected to be `<property><conditional><requirement>`
// Check the *Requirements type definition for which <property> strings will be allowed.
// <conditional> is allowed to be one of the following: ==, !=, in, notin
// <requirement> will be a string value if `==` or `!=` are used.
// <requirement> will be a set of string values if `in` or `notin` are used.
// Multiple predicates are allowed to be chained with a comma (,)
//
// ServiceClass allowed property names:
// name - the value set to [Cluster]ServiceClass.Name
// externalName - the value set to [Cluster]ServiceClass.Spec.ExternalName
//
// ServicePlan allowed property names:
// name - the value set to [Cluster]ServiceClass.Name
// externalName - the value set to [Cluster]ServiceClass.Spec.ExternalName
type CatalogRestrictions struct {
// ServiceClass represents a selector for plans, used to filter catalog re-lists.
ServicePlan []string
// ServicePlan represents a selector for classes, used to filter catalog re-lists.
ServiceClass []string
}

// ClusterServiceBrokerSpec represents a description of a Broker.
Expand Down
4 changes: 3 additions & 1 deletion pkg/apis/servicecatalog/v1beta1/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ limitations under the License.

package v1beta1

import "fmt"
import (
"fmt"
)

// These functions are used for field selectors. They are only needed if
// field selection is made available for types, hence we only have them for
Expand Down
54 changes: 54 additions & 0 deletions pkg/apis/servicecatalog/v1beta1/filter.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
Copyright 2018 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package v1beta1

import (
"github.com/kubernetes-incubator/service-catalog/pkg/filter"
"k8s.io/apimachinery/pkg/labels"
)

// These are functions to support filtering and are class specific for the ClusterServiceClass and ClusterServicePlan
// This is where we can add more fields to the labels.Set to support other kinds of catalog filtering.

// ConvertClusterServiceClassToProperties takes a Service Class and pulls out the
// properties we support for filtering, converting them into a map in the
// expected format.
func ConvertClusterServiceClassToProperties(serviceClass *ClusterServiceClass) filter.Properties {
if serviceClass == nil {
return labels.Set{}
}
return labels.Set{
FilterName: serviceClass.Name,
FilterSpecExternalName: serviceClass.Spec.ExternalName,
FilterSpecExternalID: serviceClass.Spec.ExternalID,
}
}

// ConvertClusterServicePlanToProperties takes a Service Plan and pulls out the
// properties we support for filtering, converting them into a map in the
// expected format.
func ConvertClusterServicePlanToProperties(servicePlan *ClusterServicePlan) filter.Properties {
if servicePlan == nil {
return labels.Set{}
}
return labels.Set{
FilterName: servicePlan.Name,
FilterSpecExternalName: servicePlan.Spec.ExternalName,
FilterSpecExternalID: servicePlan.Spec.ExternalID,
FilterSpecClusterServiceClassName: servicePlan.Spec.ClusterServiceClassRef.Name,
}
}
111 changes: 111 additions & 0 deletions pkg/apis/servicecatalog/v1beta1/filter_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/*
Copyright 2018 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package v1beta1

import (
"encoding/json"
"testing"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func TestConvertClusterServiceClassToProperties(t *testing.T) {
cases := []struct {
name string
sc *ClusterServiceClass
json string
}{
{
name: "nil object",
json: "{}",
},
{
name: "normal object",
sc: &ClusterServiceClass{
ObjectMeta: metav1.ObjectMeta{Name: "service-class"},
Spec: ClusterServiceClassSpec{
CommonServiceClassSpec: CommonServiceClassSpec{
ExternalName: "external-class-name",
ExternalID: "external-id",
},
},
},
json: `{"name":"service-class","spec.externalID":"external-id","spec.externalName":"external-class-name"}`,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
p := ConvertClusterServiceClassToProperties(tc.sc)
if p == nil {
t.Fatalf("Failed to create Properties object from %+v", tc.sc)
}
b, err := json.Marshal(p)
if err != nil {
t.Fatalf("Unexpected error with json marchal, %v", err)
}
js := string(b)
if js != tc.json {
t.Fatalf("Failed to create expected Properties object,\n\texpected: \t%q,\n \tgot: \t\t%q", tc.json, js)
}
})
}
}

func TestConvertClusterServicePlanToProperties(t *testing.T) {
cases := []struct {
name string
sp *ClusterServicePlan
json string
}{
{
name: "nil object",
json: "{}",
},
{
name: "normal object",
sp: &ClusterServicePlan{
ObjectMeta: metav1.ObjectMeta{Name: "service-plan"},
Spec: ClusterServicePlanSpec{
CommonServicePlanSpec: CommonServicePlanSpec{
ExternalName: "external-plan-name",
ExternalID: "external-id",
},
ClusterServiceClassRef: ClusterObjectReference{
Name: "cluster-service-class-name",
},
},
},
json: `{"name":"service-plan","spec.clusterServiceClass.name":"cluster-service-class-name","spec.externalID":"external-id","spec.externalName":"external-plan-name"}`,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
p := ConvertClusterServicePlanToProperties(tc.sp)
if p == nil {
t.Fatalf("Failed to create Properties object from %+v", tc.sp)
}
b, err := json.Marshal(p)
if err != nil {
t.Fatalf("Unexpected error with json marchal, %v", err)
}
js := string(b)
if js != tc.json {
t.Fatalf("Failed to create expected Properties object,\n\texpected: \t%q,\n \tgot: \t\t%q", tc.json, js)
}
})
}
}
66 changes: 66 additions & 0 deletions pkg/apis/servicecatalog/v1beta1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,60 @@ type CommonServiceBrokerSpec struct {
// can be manually incremented by a user to manually trigger a relist.
// +optional
RelistRequests int64 `json:"relistRequests"`

// CatalogRestrictions is a set of restrictions on which of a broker's services
// and plans have resources created for them.
// +optional
CatalogRestrictions *CatalogRestrictions `json:"catalogRestrictions,omitempty"`
}

// CatalogRestrictions is a set of restrictions on which of a broker's services
// and plans have resources created for them.
//
// Some examples of this object are as follows:
//
// This is an example of a whitelist on service externalName.
// Goal: Only list Services with the externalName of FooService and BarService,
// Solution: restrictions := ServiceCatalogRestrictions{
// ServiceClass: ["externalName in (FooService, BarService)"]
// }
//
// This is an example of a blacklist on service externalName.
// Goal: Allow all services except the ones with the externalName of FooService and BarService,
// Solution: restrictions := ServiceCatalogRestrictions{
// ServiceClass: ["externalName notin (FooService, BarService)"]
// }
//
// This whitelists plans called "Demo", and blacklists (but only a single element in
// the list) a service and a plan.
// Goal: Allow all plans with the externalName demo, but not AABBCC, and not a specific service by name,
// Solution: restrictions := ServiceCatalogRestrictions{
// ServiceClass: ["name!=AABBB-CCDD-EEGG-HIJK"]
// ServicePlan: ["externalName in (Demo)", "name!=AABBCC"]
// }
//
// CatalogRestrictions strings have a special format similar to Label Selectors,
// except the catalog supports only a very specific property set.
//
// The predicate format is expected to be `<property><conditional><requirement>`
// Check the *Requirements type definition for which <property> strings will be allowed.
// <conditional> is allowed to be one of the following: ==, !=, in, notin
// <requirement> will be a string value if `==` or `!=` are used.
// <requirement> will be a set of string values if `in` or `notin` are used.
// Multiple predicates are allowed to be chained with a comma (,)
//
// ServiceClass allowed property names:
// name - the value set to [Cluster]ServiceClass.Name
// externalName - the value set to [Cluster]ServiceClass.Spec.ExternalName
//
// ServicePlan allowed property names:
// name - the value set to [Cluster]ServiceClass.Name
// externalName - the value set to [Cluster]ServiceClass.Spec.ExternalName
type CatalogRestrictions struct {
// ServiceClass represents a selector for plans, used to filter catalog re-lists.
ServiceClass []string `json:"serviceClass,omitempty"`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note I removed custom typed strings because it was not adding value to the code.

// ServicePlan represents a selector for classes, used to filter catalog re-lists.
ServicePlan []string `json:"servicePlan,omitempty"`
}

// ClusterServiceBrokerSpec represents a description of a Broker.
Expand Down Expand Up @@ -1250,6 +1304,18 @@ type ClusterObjectReference struct {
Name string `json:"name,omitempty"`
}

// Filter path for Properties
const (
// Name field.
FilterName = "name"
// SpecExternalName is the external name of the object.
FilterSpecExternalName = "spec.externalName"
// SpecExternalID is the external id of the object.
FilterSpecExternalID = "spec.externalID"
// SpecClusterServiceClassName is only used for plans, the parent service class name.
FilterSpecClusterServiceClassName = "spec.clusterServiceClass.name"
)

// SecretTransform is a single transformation that is applied to the
// credentials returned from the broker before they are inserted into
// the Secret associated with the ServiceBinding.
Expand Down
Loading