Introduce spec.free to catalog restrictions for Plans (#2211)

* Adds ability to filter by [Cluster]ServicePlan.spec.free

Closes: #2209

* Adding catalog restriction docs

* Fix html formatting

* Review comments

* Trying relative paths

* Review comments from Jay

* linking to namespaced docs

docs/README.md

@@ -32,6 +32,7 @@ Afterward, see the topics below.
 ## Topics for operators:
 
 - [Using Namespaced Broker Resources](./namespaced-broker-resources.md)
+- [Filtering Broker Catalogs](./catalog-restrictions.md)
 
 ## Request for Comments
 

docs/catalog-restrictions.md

@@ -0,0 +1,194 @@
+---
+title: Filtering Broker Catalogs
+layout: docwithnav
+---
+
+# Catalog Restrictions
+
+Services provided by service brokers are represented in Kubernetes by two 
+different [resources](resources.md), service classes and service plans. When a 
+`ClusterServiceBroker` or `ServiceBroker` resource is created, the Service 
+Catalog will query the Service Broker for the list of available Services. 
+Service Catalog will then create `ClusterServiceClass` or `ServiceClass` 
+resources to represent the service classes and `ClusterServicePlan` or 
+`ServicePlan` resources to represent service plans. By default, Service Catalog
+will create a `ClusterServiceClass` or `ServiceClass` for each service class 
+and a `ClusterServicePlan` or `ServicePlan` for each service plan 
+provided by the service broker. When creating a `ClusterServiceBroker` or 
+`ServiceBroker` resource, you can change this behavior by specifying one or 
+more catalog restrictions. Catalog restrictions act in a manner similar to 
+Kubernetes label selectors to enable you to control how service classes and 
+service plans should be exposed from the service brokers.
+
+## Using Catalog Restrictions
+
+Catalog restrictions are specified in `ClusterServiceBroker` or `ServiceBroker`
+ resources. A sample YAML might look like:
+
+```yaml
+apiVersion: servicecatalog.k8s.io/v1beta1
+kind: ClusterServiceBroker
+metadata:
+  name: sample-broker
+spec:
+  authInfo:
+    basic:
+      secretRef:
+        name: sample-broker-auth
+        namespace: brokers
+  catalogRestrictions:
+    servicePlan:
+    - "spec.externalName==basic"
+  url: http://sample-broker.brokers.svc.cluster.local
+```
+
+In this example, a catalog restriction has been defined that specifies that 
+only service plans that have an external name of basic should be selected. 
+Catalog restrictions are defined as a set of one or more rules that target 
+ service classes and/or service plans. These rules have a special format
+ similar to Kubernetes label selectors. 
+
+The rule format is expected to be `<property><conditional><requirement>`
+
+* `<property>` is one of the supported properties of a service class or service plan resource, described below
+* `<conditional>` is allowed to be one of the following: `==`, `!=`, `in`, `notin`
+* `<requirement>` will be a string value if `==` or `!=` are used, otherwise it will be a set of string values if `in` or `notin` are used
+* `<requirement>` is case sensitive
+
+Catalog restrictions, while similar to label selectors, only operate on a 
+subset of properties on service class and service plan resources. The following
+ sections detail what properties can be used to define catalog restrictions for
+  each resource type.  
+
+`ClusterServiceClass` allowed property names:
+
+| Property Key    | Description    |
+| name |  This key will match the ClusterServiceClass.Name property |
+| spec.externalName | This key will match the ClusterServiceClass.Spec.ExternalName property |
+| spec.externalID | This key will match the ClusterServiceClass.Spec.ExternalID property |
+
+`ServiceClass` allowed property names:
+
+| Property Key    | Description    |
+| name |  This key will match the ServiceClass.Name |
+| spec.externalName | This key will match the ServiceClass.Spec.ExternalName property |
+| spec.externalID | This key will match the ServiceClass.Spec.ExternalID property |
+
+`ClusterServicePlan` allowed property names:
+
+| Property Key    | Description    |
+| name | This key will match the ClusterServicePlan.Name |
+| spec.externalName | This key will match the ClusterServicePlan.Spec.ExternalName property |
+| spec.externalID | This key will match the ClusterServicePlan.Spec.ExternalID property |
+| spec.free | This key will match the ClusterServicePlan.Spec.Free property |
+| spec.clusterServiceClass.name | This key will match the ClusterServicePlan.Spec.ClusterServiceClassRef.Name property |
+
+`ServicePlan` allowed property names:
+
+| Property Key    | Description    |
+| name | This key will match the ServicePlan.Name property |
+| spec.externalName | This key will match the ServicePlan.Spec.ExternalName property |
+| spec.externalID | This key will match the ServicePlan.Spec.ExternalID property |
+| spec.free | This key will match the ServicePlan.Spec.Free property |
+| spec.serviceClass.name | This key will match the ServicePlan.Spec.ServiceClassRef.Name property |
+
+## Examples
+
+The following examples show some possible ways to apply catalog restrictions.
+
+### Allow Only Service Class Resources with Specific External Name
+
+This example creates a Service Class restriction on spec.externalName using the
+ `in` operator. In this case, only services that have the externalName 
+ `FooService` or `BarService` will have Service Catalog resources created. 
+ The YAML for this would look like:
+
+```yaml
+apiVersion: servicecatalog.k8s.io/v1beta1
+kind: ClusterServiceBroker
+metadata:
+  name: sample-broker
+spec:
+  authInfo:
+    basic:
+      secretRef:
+        name: sample-broker-auth
+        namespace: brokers
+  catalogRestrictions:
+    serviceClass:
+    - "spec.externalName in (FooService, BarService)"
+  url: http://sample-broker.brokers.svc.cluster.local
+```
+
+### Allow All Service Class Resources Except Those with Specific External Name
+
+ To allow all services, except those named `FooService` or `BarService`, 
+ the `notin` operator can be used. The YAML for this would look like:
+ above.
+
+```yaml
+apiVersion: servicecatalog.k8s.io/v1beta1
+kind: ClusterServiceBroker
+metadata:
+  name: sample-broker
+spec:
+  authInfo:
+    basic:
+      secretRef:
+        name: sample-broker-auth
+        namespace: brokers
+  catalogRestrictions:
+    serviceClass:
+    - "spec.externalName notin (FooService, BarService)"
+  url: http://sample-broker.brokers.svc.cluster.local
+```
+
+### Using Multiple Predicates
+
+As mentioned above, you can chain rules together. For example,
+to restrict service plans to only those free plans with an externalName of 
+`Demo`, the YAML would look like:
+
+```yaml
+apiVersion: servicecatalog.k8s.io/v1beta1
+kind: ClusterServiceBroker
+metadata:
+  name: sample-broker
+spec:
+  authInfo:
+    basic:
+      secretRef:
+        name: sample-broker-auth
+        namespace: brokers
+  catalogRestrictions:
+    servicePlan:
+    - "spec.externalName in (Demo)"
+    - "spec.free=true"
+  url: http://sample-broker.brokers.svc.cluster.local
+```
+
+### Combining Service Class and Service Plan Catalog Restrictions
+
+You can also combine restrictions on classes and plans. An example that 
+allow all free plans with the externalName `Demo`, and not a specific service
+ named `AABBB-CCDD-EEGG-HIJK`, you would create a YAML like:
+
+```yaml
+apiVersion: servicecatalog.k8s.io/v1beta1
+kind: ClusterServiceBroker
+metadata:
+  name: sample-broker
+spec:
+  authInfo:
+    basic:
+      secretRef:
+        name: sample-broker-auth
+        namespace: brokers
+  catalogRestrictions:
+    serviceClass:
+    - "name!=AABBB-CCDD-EEGG-HIJK"
+    servicePlan:
+    - "spec.externalName in (Demo)"
+    - "spec.free=true"
+  url: http://sample-broker.brokers.svc.cluster.local
+```

docs/namespaced-broker-resources.md

@@ -68,7 +68,7 @@ Service Catalog's cluster-scoped resources for brokers, services, and plans are
 be used to prevent a user from creating a `ServiceInstance` using that 
 `ClusterServiceClass` and `ClusterServicePlan`. Namespace-scoped brokers, 
 services and plans, however, can be effectively combined with Kubernetes RBAC
-and Service Catalog Catalog Restrictions in order to provide more granular 
+and Service Catalog [Catalog Restrictions](catalog-restrictions.md) in order to provide more granular 
 control over service instance provisioning.
 
 ## Enabling Namespace Scoped Broker Resources
@@ -212,7 +212,7 @@ The use of namespace-scoped resources enables you to register brokers within a
  given namespace and leverage RBAC in order to control who can 
 provision services in that namespace. By default, all service classes and plans 
 from that broker will be available to users of the namespace. When registering 
-a broker, catalog restrictions can be specified in order to restrict what plans
+a broker, [catalog restrictions](catalog-restrictions.md) can be specified in order to restrict what plans
  are available within a given namespace. This allows you to specify that in 
  the `developer` namespace, only plans named `basic` can be created. The YAML 
  to accomplish this might look like:

docsite/_config.yml

@@ -58,4 +58,5 @@ tocs:
   - concepts
   - resources
   - namespaced-broker-resources
+  - catalog-restrictions
   - docs-home # fallthrough, leave this last

docsite/_data/catalog-restrictions.yml

@@ -0,0 +1,11 @@
+bigheader: "Catalog Restrictions"
+abstract: "Using Catalog Restrictions to control Service Class and Plan access."
+landing_page: /docs/catalog-restrictions/
+toc:
+- docs/catalog-restrictions.md
+- title: Catalog Restrictions
+  path: "#catalog-restrictions"
+- title: Using Catalog Restrictions
+  path: "#using-catalog-restrictions"
+- title: Examples
+  path: "#examples"

pkg/apis/servicecatalog/types.go

@@ -150,6 +150,7 @@ type CommonServiceBrokerSpec struct {
 //   name - the value set to [Cluster]ServicePlan.Name
 //   spec.externalName - the value set to [Cluster]ServicePlan.Spec.ExternalName
 //   spec.externalID - the value set to [Cluster]ServicePlan.Spec.ExternalID
+//   spec.free - the value set to [Cluster]ServicePlan.Spec.Free
 //   spec.serviceClassName - the value set to ServicePlan.Spec.ServiceClassRef.Name
 //   spec.clusterServiceClass.name - the value set to ClusterServicePlan.Spec.ClusterServiceClassRef.Name
 type CatalogRestrictions struct {

pkg/apis/servicecatalog/v1beta1/filter.go

@@ -17,6 +17,8 @@ limitations under the License.
 package v1beta1
 
 import (
+	"strconv"
+
 	"github.com/kubernetes-incubator/service-catalog/pkg/filter"
 	"k8s.io/apimachinery/pkg/labels"
 )
@@ -50,6 +52,7 @@ func ConvertServicePlanToProperties(servicePlan *ServicePlan) filter.Properties
 		FilterSpecExternalName:     servicePlan.Spec.ExternalName,
 		FilterSpecExternalID:       servicePlan.Spec.ExternalID,
 		FilterSpecServiceClassName: servicePlan.Spec.ServiceClassRef.Name,
+		FilterSpecFree:             strconv.FormatBool(servicePlan.Spec.Free),
 	}
 }
 
@@ -79,5 +82,6 @@ func ConvertClusterServicePlanToProperties(servicePlan *ClusterServicePlan) filt
 		FilterSpecExternalName:            servicePlan.Spec.ExternalName,
 		FilterSpecExternalID:              servicePlan.Spec.ExternalID,
 		FilterSpecClusterServiceClassName: servicePlan.Spec.ClusterServiceClassRef.Name,
+		FilterSpecFree:                    strconv.FormatBool(servicePlan.Spec.Free),
 	}
 }

pkg/apis/servicecatalog/v1beta1/filter_test.go

@@ -83,13 +83,14 @@ func TestConvertServicePlanToProperties(t *testing.T) {
 					CommonServicePlanSpec: CommonServicePlanSpec{
 						ExternalName: "external-plan-name",
 						ExternalID:   "external-id",
+						Free:         true,
 					},
 					ServiceClassRef: LocalObjectReference{
 						Name: "service-class-name",
 					},
 				},
 			},
-			json: `{"name":"service-plan","spec.externalID":"external-id","spec.externalName":"external-plan-name","spec.serviceClass.name":"service-class-name"}`,
+			json: `{"name":"service-plan","spec.externalID":"external-id","spec.externalName":"external-plan-name","spec.free":"true","spec.serviceClass.name":"service-class-name"}`,
 		},
 	}
 	for _, tc := range cases {
@@ -170,13 +171,14 @@ func TestConvertClusterServicePlanToProperties(t *testing.T) {
 					CommonServicePlanSpec: CommonServicePlanSpec{
 						ExternalName: "external-plan-name",
 						ExternalID:   "external-id",
+						Free:         true,
 					},
 					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"}`,
+			json: `{"name":"service-plan","spec.clusterServiceClass.name":"cluster-service-class-name","spec.externalID":"external-id","spec.externalName":"external-plan-name","spec.free":"true"}`,
 		},
 	}
 	for _, tc := range cases {

pkg/apis/servicecatalog/v1beta1/types.go

@@ -136,21 +136,21 @@ type CommonServiceBrokerSpec struct {
 // 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)"]
+// 		ServiceClass: ["spec.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)"]
+// 		ServiceClass: ["spec.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"]
+// 		ServicePlan: ["spec.externalName in (Demo)", "name!=AABBCC"]
 // }
 //
 // CatalogRestrictions strings have a special format similar to Label Selectors,
@@ -171,8 +171,9 @@ type CommonServiceBrokerSpec struct {
 //   name - the value set to [Cluster]ServicePlan.Name
 //   spec.externalName - the value set to [Cluster]ServicePlan.Spec.ExternalName
 //   spec.externalID - the value set to [Cluster]ServicePlan.Spec.ExternalID
+//   spec.free - the value set to [Cluster]ServicePlan.Spec.Free
 //   spec.serviceClass.name - the value set to ServicePlan.Spec.ServiceClassRef.Name
-//   spec.clusterServiceClass.name - the vlaue set to ClusterServicePlan.Spec.ClusterServiceClassRef.Name
+//   spec.clusterServiceClass.name - the value set to ClusterServicePlan.Spec.ClusterServiceClassRef.Name
 type CatalogRestrictions struct {
 	// ServiceClass represents a selector for plans, used to filter catalog re-lists.
 	ServiceClass []string `json:"serviceClass,omitempty"`
@@ -1372,6 +1373,8 @@ const (
 	FilterSpecClusterServiceClassName = "spec.clusterServiceClass.name"
 	// SpecServiceClassName is only used for plans, the parent service class name.
 	FilterSpecServiceClassName = "spec.serviceClass.name"
+	// FilterSpecFree is only used for plans, determines if the plan is free.
+	FilterSpecFree = "spec.free"
 )
 
 // SecretTransform is a single transformation that is applied to the

pkg/controller/controller_test.go

@@ -1588,6 +1588,15 @@ func TestConvertAndFilterCatalog(t *testing.T) {
 			plans:   []string{"Goldengrove", "Queensgate"},
 			catalog: largeTestCatalog,
 		},
+		{
+			name: "filter free plans",
+			restrictions: &v1beta1.CatalogRestrictions{
+				ServicePlan: []string{"spec.free==true"},
+			},
+			classes: []string{"Arrax", "Balerion"},
+			plans:   []string{"Eastwatch-by-the-Sea", "OldOak", "Queensgate"},
+			catalog: largeTestCatalog,
+		},
 	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {