Make it possible to bind only to bindable serviceclasses (#744)

Author: pmorie

pkg/controller/controller.go

@@ -268,6 +268,7 @@ const (
 	errorUnbindCallReason                 string = "UnbindCallFailed"
 	errorWithOngoingAsyncOperation        string = "ErrorAsyncOperationInProgress"
 	errorWithOngoingAsyncOperationMessage string = "Another operation for this service instance is in progress. "
+	errorNonbindableServiceClassReason    string = "ErrorNonbindableServiceClass"
 
 	successInjectedBindResultReason  string = "InjectedBindResult"
 	successInjectedBindResultMessage string = "Injected bind result"
@@ -1196,6 +1197,20 @@ func (c *controller) reconcileBinding(binding *v1alpha1.Binding) error {
 		return err
 	}
 
+	if !serviceClass.Bindable {
+		s := fmt.Sprintf("Binding \"%s/%s\" references a non-bindable ServiceClass %q", binding.Namespace, binding.Name, instance.Spec.ServiceClassName)
+		glog.Warning(s)
+		c.updateBindingCondition(
+			binding,
+			v1alpha1.BindingConditionReady,
+			v1alpha1.ConditionFalse,
+			errorNonbindableServiceClassReason,
+			s,
+		)
+		c.recorder.Event(binding, api.EventTypeWarning, errorNonbindableServiceClassReason, s)
+		return err
+	}
+
 	if binding.DeletionTimestamp == nil { // Add or update
 		glog.V(4).Infof("Adding/Updating Binding %v/%v", binding.Namespace, binding.Name)
 

pkg/controller/controller_test.go

@@ -46,19 +46,23 @@ import (
 )
 
 const (
-	serviceClassGUID = "SCGUID"
-	planGUID         = "PGUID"
-	instanceGUID     = "IGUID"
-	bindingGUID      = "BGUID"
-
-	testBrokerName        = "test-broker"
-	testServiceClassName  = "test-serviceclass"
-	testPlanName          = "test-plan"
-	testInstanceName      = "test-instance"
-	testBindingName       = "test-binding"
-	testNamespace         = "test-ns"
-	testBindingSecretName = "test-secret"
-	testOperation         = "test-operation"
+	serviceClassGUID            = "SCGUID"
+	planGUID                    = "PGUID"
+	nonbindableServiceClassGUID = "UNBINDABLE-SERVICE"
+	nonbindablePlanGUID         = "UNBINDABLE-PLAN"
+	instanceGUID                = "IGUID"
+	bindingGUID                 = "BGUID"
+
+	testBrokerName                  = "test-broker"
+	testServiceClassName            = "test-serviceclass"
+	testNonbindableServiceClassName = "test-unbindable-serviceclass"
+	testPlanName                    = "test-plan"
+	testNonbindablePlanName         = "test-unbindable-plan"
+	testInstanceName                = "test-instance"
+	testBindingName                 = "test-binding"
+	testNamespace                   = "test-ns"
+	testBindingSecretName           = "test-secret"
+	testOperation                   = "test-operation"
 )
 
 const testCatalog = `{
@@ -206,12 +210,13 @@ func getTestBrokerWithStatus(status v1alpha1.ConditionStatus) *v1alpha1.Broker {
 	return broker
 }
 
-// service class wired to the result of getTestBroker()
+// a bindable service class wired to the result of getTestBroker()
 func getTestServiceClass() *v1alpha1.ServiceClass {
 	return &v1alpha1.ServiceClass{
 		ObjectMeta: metav1.ObjectMeta{Name: testServiceClassName},
 		BrokerName: testBrokerName,
 		OSBGUID:    serviceClassGUID,
+		Bindable:   true,
 		Plans: []v1alpha1.ServicePlan{{
 			Name:    testPlanName,
 			OSBFree: true,
@@ -220,6 +225,21 @@ func getTestServiceClass() *v1alpha1.ServiceClass {
 	}
 }
 
+// an unbindable service class wired to the result of getTestBroker()
+func getTestNonbindableServiceClass() *v1alpha1.ServiceClass {
+	return &v1alpha1.ServiceClass{
+		ObjectMeta: metav1.ObjectMeta{Name: testNonbindableServiceClassName},
+		BrokerName: testBrokerName,
+		OSBGUID:    nonbindableServiceClassGUID,
+		Bindable:   false,
+		Plans: []v1alpha1.ServicePlan{{
+			Name:    testNonbindablePlanName,
+			OSBFree: true,
+			OSBGUID: nonbindablePlanGUID,
+		}},
+	}
+}
+
 // broker catalog that provides the service class named in of
 // getTestServiceClass()
 func getTestCatalog() *brokerapi.Catalog {
@@ -229,6 +249,7 @@ func getTestCatalog() *brokerapi.Catalog {
 				Name:        testServiceClassName,
 				ID:          serviceClassGUID,
 				Description: "a test service",
+				Bindable:    true,
 				Plans: []brokerapi.ServicePlan{
 					{
 						Name:        testPlanName,
@@ -254,6 +275,15 @@ func getTestInstance() *v1alpha1.Instance {
 	}
 }
 
+// an isntance referencing the result of getTestNonbindableServiceClass
+func getTestNonbindableInstance() *v1alpha1.Instance {
+	i := getTestInstance()
+	i.Spec.ServiceClassName = testNonbindableServiceClassName
+	i.Spec.PlanName = testNonbindablePlanName
+
+	return i
+}
+
 // getTestInstanceAsync returns an instance in async mode
 func getTestInstanceAsyncProvisioning(operation string) *v1alpha1.Instance {
 	instance := getTestInstance()
@@ -1981,6 +2011,41 @@ func TestReconcileBindingWithParameters(t *testing.T) {
 	}
 }
 
+func TestReconcileBindingNonbindableServiceClass(t *testing.T) {
+	_, fakeCatalogClient, fakeBrokerClient, testController, sharedInformers := newTestController(t)
+
+	fakeBrokerClient.CatalogClient.RetCatalog = getTestCatalog()
+
+	sharedInformers.Brokers().Informer().GetStore().Add(getTestBroker())
+	sharedInformers.ServiceClasses().Informer().GetStore().Add(getTestNonbindableServiceClass())
+	sharedInformers.Instances().Informer().GetStore().Add(getTestNonbindableInstance())
+
+	binding := &v1alpha1.Binding{
+		ObjectMeta: metav1.ObjectMeta{Name: testBindingName, Namespace: testNamespace},
+		Spec: v1alpha1.BindingSpec{
+			InstanceRef: v1.LocalObjectReference{Name: testInstanceName},
+			OSBGUID:     bindingGUID,
+		},
+	}
+
+	testController.reconcileBinding(binding)
+
+	actions := fakeCatalogClient.Actions()
+	assertNumberOfActions(t, actions, 1)
+
+	// There should only be one action that says binding was created
+	updatedBinding := assertUpdateStatus(t, actions[0], binding)
+	assertBindingReadyFalse(t, updatedBinding, errorNonbindableServiceClassReason)
+
+	events := getRecordedEvents(testController)
+	assertNumEvents(t, events, 1)
+
+	expectedEvent := api.EventTypeWarning + " " + errorNonbindableServiceClassReason + ` Binding "test-ns/test-binding" references a non-bindable ServiceClass "test-unbindable-serviceclass"`
+	if e, a := expectedEvent, events[0]; e != a {
+		t.Fatalf("Received unexpected event: %v", a)
+	}
+}
+
 func TestReconcileBindingFailsWithInstanceAsyncOngoing(t *testing.T) {
 	fakeKubeClient, fakeCatalogClient, fakeBrokerClient, testController, sharedInformers := newTestController(t)
 

test/integration/controller_test.go

@@ -80,6 +80,7 @@ func TestBasicFlows(t *testing.T) {
 				Name:        testServiceClassName,
 				ID:          "12345",
 				Description: "a test service",
+				Bindable:    true,
 				Plans: []brokerapi.ServicePlan{
 					{
 						Name:        testPlanName,