Make service/plan reference fields on instance spec selectable (#1422)

pkg/apis/servicecatalog/v1beta1/conversion.go

@@ -49,3 +49,15 @@ func ClusterServiceClassFieldLabelConversionFunc(label, value string) (string, s
 		return "", "", fmt.Errorf("field label not supported: %s", label)
 	}
 }
+
+// ServiceInstanceFieldLabelConversionFunc does not convert anything, just returns
+// what it's given for the supported fields, and errors for unsupported.
+func ServiceInstanceFieldLabelConversionFunc(label, value string) (string, string, error) {
+	switch label {
+	case "spec.clusterServiceClassRef.name",
+		"spec.clusterServicePlanRef.name":
+		return label, value, nil
+	default:
+		return "", "", fmt.Errorf("field label not supported: %s", label)
+	}
+}

pkg/apis/servicecatalog/v1beta1/conversion_test.go

@@ -130,6 +130,38 @@ func TestClusterServiceClassFieldLabelConversionFunc(t *testing.T) {
 
 }
 
+func TestServiceInstanceFieldLabelConversionFunc(t *testing.T) {
+	cases := []testcase{
+		{
+			name:     "spec.clusterServiceClassRef.name works",
+			inLabel:  "spec.clusterServiceClassRef.name",
+			inValue:  "someref",
+			outLabel: "spec.clusterServiceClassRef.name",
+			outValue: "someref",
+			success:  true,
+		},
+		{
+			name:     "spec.clusterServicePlanRef.name works",
+			inLabel:  "spec.clusterServicePlanRef.name",
+			inValue:  "someref",
+			outLabel: "spec.clusterServicePlanRef.name",
+			outValue: "someref",
+			success:  true,
+		},
+		{
+			name:          "random fails",
+			inLabel:       "spec.random",
+			inValue:       "randomvalue",
+			outLabel:      "",
+			outValue:      "",
+			success:       false,
+			expectedError: "field label not supported: spec.random",
+		},
+	}
+	runTestCases(t, cases, "ServiceInstanceFieldLabelConversionFunc", ServiceInstanceFieldLabelConversionFunc)
+
+}
+
 func runTestCases(t *testing.T, cases []testcase, testFuncName string, testFunc conversionFunc) {
 	for _, tc := range cases {
 		outLabel, outValue, err := testFunc(tc.inLabel, tc.inValue)

pkg/apis/servicecatalog/v1beta1/register.go

@@ -65,5 +65,7 @@ func addKnownTypes(scheme *runtime.Scheme) error {
 	scheme.AddKnownTypes(schema.GroupVersion{Version: "v1"}, &metav1.Status{})
 	scheme.AddFieldLabelConversionFunc("servicecatalog.k8s.io/v1beta1", "ClusterServiceClass", ClusterServiceClassFieldLabelConversionFunc)
 	scheme.AddFieldLabelConversionFunc("servicecatalog.k8s.io/v1beta1", "ClusterServicePlan", ClusterServicePlanFieldLabelConversionFunc)
+	scheme.AddFieldLabelConversionFunc("servicecatalog.k8s.io/v1beta1", "ServiceInstance", ServiceInstanceFieldLabelConversionFunc)
+
 	return nil
 }

pkg/openapi/openapi_generated.go

@@ -656,7 +656,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA
 		"github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1.ObjectReference": {
 			Schema: spec.Schema{
 				SchemaProps: spec.SchemaProps{
-					Description: "ObjectReference contains enough information to let you inspect or modify the referred object.",
+					Description: "ObjectReference contains enough information to let you locate the referenced object.",
 					Properties: map[string]spec.Schema{
 						"namespace": {
 							SchemaProps: spec.SchemaProps{

pkg/registry/servicecatalog/instance/storage.go

@@ -88,8 +88,21 @@ func Match(label labels.Selector, field fields.Selector) storage.SelectionPredic
 
 // toSelectableFields returns a field set that represents the object for matching purposes.
 func toSelectableFields(instance *servicecatalog.ServiceInstance) fields.Set {
+	// If you add a new selectable field, you also need to modify
+	// pkg/apis/servicecatalog/v1beta1/conversion[_test].go
 	objectMetaFieldsSet := generic.ObjectMetaFieldsSet(&instance.ObjectMeta, true)
-	return generic.MergeFieldsSets(objectMetaFieldsSet, nil)
+
+	specFieldSet := make(fields.Set, 2)
+
+	if instance.Spec.ClusterServiceClassRef != nil {
+		specFieldSet["spec.clusterServiceClassRef.name"] = instance.Spec.ClusterServiceClassRef.Name
+	}
+
+	if instance.Spec.ClusterServicePlanRef != nil {
+		specFieldSet["spec.clusterServicePlanRef.name"] = instance.Spec.ClusterServicePlanRef.Name
+	}
+
+	return generic.MergeFieldsSets(objectMetaFieldsSet, specFieldSet)
 }
 
 // GetAttrs returns labels and fields of a given object for filtering purposes.

test/integration/clientset_test.go

@@ -827,6 +827,12 @@ func testInstanceClient(sType server.StorageType, client servicecatalogclient.In
 				ExternalClusterServiceClassName: "service-class-name",
 				ExternalClusterServicePlanName:  "plan-name",
 			},
+			ClusterServiceClassRef: &v1beta1.ClusterObjectReference{
+				Name: "test-serviceclass",
+			},
+			ClusterServicePlanRef: &v1beta1.ClusterObjectReference{
+				Name: "test-serviceplan",
+			},
 			Parameters: &runtime.RawExtension{Raw: []byte(instanceParameter)},
 			ExternalID: osbGUID,
 		},
@@ -967,6 +973,34 @@ func testInstanceClient(sType server.StorageType, client servicecatalogclient.In
 		return fmt.Errorf("Generation was changed, expected: %q got: %q", oldGeneration, instanceServer.Generation)
 	}
 
+	// field selector tests
+	instances, err = instanceClient.List(metav1.ListOptions{FieldSelector: "spec.clusterServiceClassRef.name=should-return-zero"})
+	if err != nil {
+		return fmt.Errorf("error listing instances: %v", err)
+	}
+
+	if 0 != len(instances.Items) {
+		return fmt.Errorf("should have exactly zero instances, had %v instances", len(instances.Items))
+	}
+
+	instances, err = instanceClient.List(metav1.ListOptions{FieldSelector: "spec.clusterServiceClassRef.name=service-class-ref"})
+	if err != nil {
+		return fmt.Errorf("error listing instances: %v", err)
+	}
+
+	if 1 != len(instances.Items) {
+		return fmt.Errorf("should have exactly one instance, had %v instances", len(instances.Items))
+	}
+
+	instances, err = instanceClient.List(metav1.ListOptions{FieldSelector: "spec.clusterServicePlanRef.name=service-plan-ref"})
+	if err != nil {
+		return fmt.Errorf("error listing instances: %v", err)
+	}
+
+	if 1 != len(instances.Items) {
+		return fmt.Errorf("should have exactly one instance, had %v instances", len(instances.Items))
+	}
+
 	// update the instance's spec
 	updateRequests := instanceServer.Spec.UpdateRequests + 1
 	expectedGeneration := instanceServer.Generation + 1