Extracting common plan spec into embeddable struct (openshift#1833)

* Extracting common plan spec elements into embeddable struct

As part of the Prosopal for Namespaced Resources (openshift#1826), this extracts
the common elements from the ClusterServicePlanSpec into a SharedServicePlanSpec
that can be embedded in both the ClusterServicePlanSpec and a new namesapced plan

* Cleaned up comments on shared fields to remove "Cluster" from description.

* Cleaned up comments on shared fields to remove "Cluster" from description.

* Refactored ClusterPlanStatus to match proposal

* Revert "Refactored ClusterPlanStatus to match proposal"

This reverts commit 6b36cf773c20178e47e5d0930a1f81b62b960440.

pkg/apis/servicecatalog/types.go

@@ -323,12 +323,8 @@ type ClusterServicePlan struct {
 	Status ClusterServicePlanStatus
 }
 
-// ClusterServicePlanSpec represents details about the ClusterServicePlan
-type ClusterServicePlanSpec struct {
-	// ClusterServiceBrokerName is the name of the ClusterServiceBroker that offers this
-	// ClusterServicePlan.
-	ClusterServiceBrokerName string
-
+// SharedServicePlanSpec represents details about the ServicePlan
+type SharedServicePlanSpec struct {
 	// ExternalName is the name of this object that the Service Broker
 	// exposed this Service Plan as. Mutable.
 	ExternalName string
@@ -338,15 +334,15 @@ type ClusterServicePlanSpec struct {
 	// Immutable.
 	ExternalID string
 
-	// Description is a short description of this ClusterServicePlan.
+	// Description is a short description of this ServicePlan.
 	Description string
 
 	// Bindable indicates whether a user can create bindings to an ServiceInstance
-	// using this ClusterServicePlan.  If set, overrides the value of the
-	// ClusterServiceClass.Bindable field.
+	// using this ServicePlan.  If set, overrides the value of the
+	// corresponding ServiceClassSpec Bindable field.
 	Bindable *bool
 
-	// Free indicates whether this ClusterServicePlan is available at no cost.
+	// Free indicates whether this ServicePlan is available at no cost.
 	Free bool
 
 	// ExternalMetadata is a blob of information about the plan, meant to be
@@ -366,7 +362,7 @@ type ClusterServicePlanSpec struct {
 	//
 	// ServiceInstanceUpdateParameterSchema is the schema for the parameters
 	// that may be updated once an ServiceInstance has been provisioned on this plan.
-	// This field only has meaning if the ClusterServiceClass is PlanUpdatable.
+	// This field only has meaning if the corresponding ServiceClassSpec is PlanUpdatable.
 	ServiceInstanceUpdateParameterSchema *runtime.RawExtension
 
 	// Currently, this field is ALPHA: it may change or disappear at any time
@@ -375,6 +371,15 @@ type ClusterServicePlanSpec struct {
 	// ServiceBindingCreateParameterSchema is the schema for the parameters that
 	// may be supplied binding to an ServiceInstance on this plan.
 	ServiceBindingCreateParameterSchema *runtime.RawExtension
+}
+
+// ClusterServicePlanSpec represents details about the ClusterServicePlan
+type ClusterServicePlanSpec struct {
+	SharedServicePlanSpec
+
+	// ClusterServiceBrokerName is the name of the ClusterServiceBroker that offers this
+	// ClusterServicePlan.
+	ClusterServiceBrokerName string
 
 	// ClusterServiceClassRef is a reference to the service class that
 	// owns this plan.

pkg/apis/servicecatalog/v1beta1/types.go

@@ -358,12 +358,9 @@ type ClusterServicePlan struct {
 	Status ClusterServicePlanStatus `json:"status,omitempty"`
 }
 
-// ClusterServicePlanSpec represents details about a ClusterServicePlan.
-type ClusterServicePlanSpec struct {
-	// ClusterServiceBrokerName is the name of the ClusterServiceBroker
-	// that offers this ClusterServicePlan.
-	ClusterServiceBrokerName string `json:"clusterServiceBrokerName"`
-
+// SharedServicePlanSpec represents details that are shared by both
+// a ClusterServicePlan and a namespaced ServicePlan
+type SharedServicePlanSpec struct {
 	// ExternalName is the name of this object that the Service Broker
 	// exposed this Service Plan as. Mutable.
 	ExternalName string `json:"externalName"`
@@ -373,12 +370,12 @@ type ClusterServicePlanSpec struct {
 	// Immutable.
 	ExternalID string `json:"externalID"`
 
-	// Description is a short description of this ClusterServicePlan.
+	// Description is a short description of this ServicePlan.
 	Description string `json:"description"`
 
 	// Bindable indicates whether a user can create bindings to an
-	// ServiceInstance using this ClusterServicePlan.  If set, overrides
-	// the value of the ClusterServiceClass.Bindable field.
+	// ServiceInstance using this ServicePlan.  If set, overrides
+	// the value of the corresponding ServiceClassSpec Bindable field.
 	Bindable *bool `json:"bindable,omitempty"`
 
 	// Free indicates whether this plan is available at no cost.
@@ -401,7 +398,7 @@ type ClusterServicePlanSpec struct {
 	//
 	// ServiceInstanceUpdateParameterSchema is the schema for the parameters
 	// that may be updated once an ServiceInstance has been provisioned on
-	// this plan. This field only has meaning if the ClusterServiceClass is
+	// this plan. This field only has meaning if the corresponding ServiceClassSpec is
 	// PlanUpdatable.
 	ServiceInstanceUpdateParameterSchema *runtime.RawExtension `json:"instanceUpdateParameterSchema,omitempty"`
 
@@ -411,6 +408,16 @@ type ClusterServicePlanSpec struct {
 	// ServiceBindingCreateParameterSchema is the schema for the parameters that
 	// may be supplied binding to an ServiceInstance on this plan.
 	ServiceBindingCreateParameterSchema *runtime.RawExtension `json:"serviceBindingCreateParameterSchema,omitempty"`
+}
+
+// ClusterServicePlanSpec represents details about a ClusterServicePlan.
+type ClusterServicePlanSpec struct {
+	// SharedServicePlanSpec contains the common details of this ClusterServicePlan
+	SharedServicePlanSpec `json:",inline"`
+
+	// ClusterServiceBrokerName is the name of the ClusterServiceBroker
+	// that offers this ClusterServicePlan.
+	ClusterServiceBrokerName string `json:"clusterServiceBrokerName"`
 
 	// ClusterServiceClassRef is a reference to the service class that
 	// owns this plan.

pkg/apis/servicecatalog/v1beta1/zz_generated.conversion.go

@@ -104,6 +104,8 @@ func RegisterConversions(scheme *runtime.Scheme) error {
 		Convert_servicecatalog_ServiceInstanceSpec_To_v1beta1_ServiceInstanceSpec,
 		Convert_v1beta1_ServiceInstanceStatus_To_servicecatalog_ServiceInstanceStatus,
 		Convert_servicecatalog_ServiceInstanceStatus_To_v1beta1_ServiceInstanceStatus,
+		Convert_v1beta1_SharedServicePlanSpec_To_servicecatalog_SharedServicePlanSpec,
+		Convert_servicecatalog_SharedServicePlanSpec_To_v1beta1_SharedServicePlanSpec,
 		Convert_v1beta1_UserInfo_To_servicecatalog_UserInfo,
 		Convert_servicecatalog_UserInfo_To_v1beta1_UserInfo,
 	)
@@ -448,16 +450,10 @@ func Convert_servicecatalog_ClusterServicePlanList_To_v1beta1_ClusterServicePlan
 }
 
 func autoConvert_v1beta1_ClusterServicePlanSpec_To_servicecatalog_ClusterServicePlanSpec(in *ClusterServicePlanSpec, out *servicecatalog.ClusterServicePlanSpec, s conversion.Scope) error {
+	if err := Convert_v1beta1_SharedServicePlanSpec_To_servicecatalog_SharedServicePlanSpec(&in.SharedServicePlanSpec, &out.SharedServicePlanSpec, s); err != nil {
+		return err
+	}
 	out.ClusterServiceBrokerName = in.ClusterServiceBrokerName
-	out.ExternalName = in.ExternalName
-	out.ExternalID = in.ExternalID
-	out.Description = in.Description
-	out.Bindable = (*bool)(unsafe.Pointer(in.Bindable))
-	out.Free = in.Free
-	out.ExternalMetadata = (*runtime.RawExtension)(unsafe.Pointer(in.ExternalMetadata))
-	out.ServiceInstanceCreateParameterSchema = (*runtime.RawExtension)(unsafe.Pointer(in.ServiceInstanceCreateParameterSchema))
-	out.ServiceInstanceUpdateParameterSchema = (*runtime.RawExtension)(unsafe.Pointer(in.ServiceInstanceUpdateParameterSchema))
-	out.ServiceBindingCreateParameterSchema = (*runtime.RawExtension)(unsafe.Pointer(in.ServiceBindingCreateParameterSchema))
 	if err := Convert_v1beta1_ClusterObjectReference_To_servicecatalog_ClusterObjectReference(&in.ClusterServiceClassRef, &out.ClusterServiceClassRef, s); err != nil {
 		return err
 	}
@@ -470,16 +466,10 @@ func Convert_v1beta1_ClusterServicePlanSpec_To_servicecatalog_ClusterServicePlan
 }
 
 func autoConvert_servicecatalog_ClusterServicePlanSpec_To_v1beta1_ClusterServicePlanSpec(in *servicecatalog.ClusterServicePlanSpec, out *ClusterServicePlanSpec, s conversion.Scope) error {
+	if err := Convert_servicecatalog_SharedServicePlanSpec_To_v1beta1_SharedServicePlanSpec(&in.SharedServicePlanSpec, &out.SharedServicePlanSpec, s); err != nil {
+		return err
+	}
 	out.ClusterServiceBrokerName = in.ClusterServiceBrokerName
-	out.ExternalName = in.ExternalName
-	out.ExternalID = in.ExternalID
-	out.Description = in.Description
-	out.Bindable = (*bool)(unsafe.Pointer(in.Bindable))
-	out.Free = in.Free
-	out.ExternalMetadata = (*runtime.RawExtension)(unsafe.Pointer(in.ExternalMetadata))
-	out.ServiceInstanceCreateParameterSchema = (*runtime.RawExtension)(unsafe.Pointer(in.ServiceInstanceCreateParameterSchema))
-	out.ServiceInstanceUpdateParameterSchema = (*runtime.RawExtension)(unsafe.Pointer(in.ServiceInstanceUpdateParameterSchema))
-	out.ServiceBindingCreateParameterSchema = (*runtime.RawExtension)(unsafe.Pointer(in.ServiceBindingCreateParameterSchema))
 	if err := Convert_servicecatalog_ClusterObjectReference_To_v1beta1_ClusterObjectReference(&in.ClusterServiceClassRef, &out.ClusterServiceClassRef, s); err != nil {
 		return err
 	}
@@ -1041,6 +1031,42 @@ func Convert_servicecatalog_ServiceInstanceStatus_To_v1beta1_ServiceInstanceStat
 	return autoConvert_servicecatalog_ServiceInstanceStatus_To_v1beta1_ServiceInstanceStatus(in, out, s)
 }
 
+func autoConvert_v1beta1_SharedServicePlanSpec_To_servicecatalog_SharedServicePlanSpec(in *SharedServicePlanSpec, out *servicecatalog.SharedServicePlanSpec, s conversion.Scope) error {
+	out.ExternalName = in.ExternalName
+	out.ExternalID = in.ExternalID
+	out.Description = in.Description
+	out.Bindable = (*bool)(unsafe.Pointer(in.Bindable))
+	out.Free = in.Free
+	out.ExternalMetadata = (*runtime.RawExtension)(unsafe.Pointer(in.ExternalMetadata))
+	out.ServiceInstanceCreateParameterSchema = (*runtime.RawExtension)(unsafe.Pointer(in.ServiceInstanceCreateParameterSchema))
+	out.ServiceInstanceUpdateParameterSchema = (*runtime.RawExtension)(unsafe.Pointer(in.ServiceInstanceUpdateParameterSchema))
+	out.ServiceBindingCreateParameterSchema = (*runtime.RawExtension)(unsafe.Pointer(in.ServiceBindingCreateParameterSchema))
+	return nil
+}
+
+// Convert_v1beta1_SharedServicePlanSpec_To_servicecatalog_SharedServicePlanSpec is an autogenerated conversion function.
+func Convert_v1beta1_SharedServicePlanSpec_To_servicecatalog_SharedServicePlanSpec(in *SharedServicePlanSpec, out *servicecatalog.SharedServicePlanSpec, s conversion.Scope) error {
+	return autoConvert_v1beta1_SharedServicePlanSpec_To_servicecatalog_SharedServicePlanSpec(in, out, s)
+}
+
+func autoConvert_servicecatalog_SharedServicePlanSpec_To_v1beta1_SharedServicePlanSpec(in *servicecatalog.SharedServicePlanSpec, out *SharedServicePlanSpec, s conversion.Scope) error {
+	out.ExternalName = in.ExternalName
+	out.ExternalID = in.ExternalID
+	out.Description = in.Description
+	out.Bindable = (*bool)(unsafe.Pointer(in.Bindable))
+	out.Free = in.Free
+	out.ExternalMetadata = (*runtime.RawExtension)(unsafe.Pointer(in.ExternalMetadata))
+	out.ServiceInstanceCreateParameterSchema = (*runtime.RawExtension)(unsafe.Pointer(in.ServiceInstanceCreateParameterSchema))
+	out.ServiceInstanceUpdateParameterSchema = (*runtime.RawExtension)(unsafe.Pointer(in.ServiceInstanceUpdateParameterSchema))
+	out.ServiceBindingCreateParameterSchema = (*runtime.RawExtension)(unsafe.Pointer(in.ServiceBindingCreateParameterSchema))
+	return nil
+}
+
+// Convert_servicecatalog_SharedServicePlanSpec_To_v1beta1_SharedServicePlanSpec is an autogenerated conversion function.
+func Convert_servicecatalog_SharedServicePlanSpec_To_v1beta1_SharedServicePlanSpec(in *servicecatalog.SharedServicePlanSpec, out *SharedServicePlanSpec, s conversion.Scope) error {
+	return autoConvert_servicecatalog_SharedServicePlanSpec_To_v1beta1_SharedServicePlanSpec(in, out, s)
+}
+
 func autoConvert_v1beta1_UserInfo_To_servicecatalog_UserInfo(in *UserInfo, out *servicecatalog.UserInfo, s conversion.Scope) error {
 	out.Username = in.Username
 	out.UID = in.UID

pkg/apis/servicecatalog/v1beta1/zz_generated.deepcopy.go

@@ -414,51 +414,7 @@ func (in *ClusterServicePlanList) DeepCopyObject() runtime.Object {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *ClusterServicePlanSpec) DeepCopyInto(out *ClusterServicePlanSpec) {
 	*out = *in
-	if in.Bindable != nil {
-		in, out := &in.Bindable, &out.Bindable
-		if *in == nil {
-			*out = nil
-		} else {
-			*out = new(bool)
-			**out = **in
-		}
-	}
-	if in.ExternalMetadata != nil {
-		in, out := &in.ExternalMetadata, &out.ExternalMetadata
-		if *in == nil {
-			*out = nil
-		} else {
-			*out = new(runtime.RawExtension)
-			(*in).DeepCopyInto(*out)
-		}
-	}
-	if in.ServiceInstanceCreateParameterSchema != nil {
-		in, out := &in.ServiceInstanceCreateParameterSchema, &out.ServiceInstanceCreateParameterSchema
-		if *in == nil {
-			*out = nil
-		} else {
-			*out = new(runtime.RawExtension)
-			(*in).DeepCopyInto(*out)
-		}
-	}
-	if in.ServiceInstanceUpdateParameterSchema != nil {
-		in, out := &in.ServiceInstanceUpdateParameterSchema, &out.ServiceInstanceUpdateParameterSchema
-		if *in == nil {
-			*out = nil
-		} else {
-			*out = new(runtime.RawExtension)
-			(*in).DeepCopyInto(*out)
-		}
-	}
-	if in.ServiceBindingCreateParameterSchema != nil {
-		in, out := &in.ServiceBindingCreateParameterSchema, &out.ServiceBindingCreateParameterSchema
-		if *in == nil {
-			*out = nil
-		} else {
-			*out = new(runtime.RawExtension)
-			(*in).DeepCopyInto(*out)
-		}
-	}
+	in.SharedServicePlanSpec.DeepCopyInto(&out.SharedServicePlanSpec)
 	out.ClusterServiceClassRef = in.ClusterServiceClassRef
 	return
 }
@@ -1086,6 +1042,67 @@ func (in *ServiceInstanceStatus) DeepCopy() *ServiceInstanceStatus {
 	return out
 }
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *SharedServicePlanSpec) DeepCopyInto(out *SharedServicePlanSpec) {
+	*out = *in
+	if in.Bindable != nil {
+		in, out := &in.Bindable, &out.Bindable
+		if *in == nil {
+			*out = nil
+		} else {
+			*out = new(bool)
+			**out = **in
+		}
+	}
+	if in.ExternalMetadata != nil {
+		in, out := &in.ExternalMetadata, &out.ExternalMetadata
+		if *in == nil {
+			*out = nil
+		} else {
+			*out = new(runtime.RawExtension)
+			(*in).DeepCopyInto(*out)
+		}
+	}
+	if in.ServiceInstanceCreateParameterSchema != nil {
+		in, out := &in.ServiceInstanceCreateParameterSchema, &out.ServiceInstanceCreateParameterSchema
+		if *in == nil {
+			*out = nil
+		} else {
+			*out = new(runtime.RawExtension)
+			(*in).DeepCopyInto(*out)
+		}
+	}
+	if in.ServiceInstanceUpdateParameterSchema != nil {
+		in, out := &in.ServiceInstanceUpdateParameterSchema, &out.ServiceInstanceUpdateParameterSchema
+		if *in == nil {
+			*out = nil
+		} else {
+			*out = new(runtime.RawExtension)
+			(*in).DeepCopyInto(*out)
+		}
+	}
+	if in.ServiceBindingCreateParameterSchema != nil {
+		in, out := &in.ServiceBindingCreateParameterSchema, &out.ServiceBindingCreateParameterSchema
+		if *in == nil {
+			*out = nil
+		} else {
+			*out = new(runtime.RawExtension)
+			(*in).DeepCopyInto(*out)
+		}
+	}
+	return
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SharedServicePlanSpec.
+func (in *SharedServicePlanSpec) DeepCopy() *SharedServicePlanSpec {
+	if in == nil {
+		return nil
+	}
+	out := new(SharedServicePlanSpec)
+	in.DeepCopyInto(out)
+	return out
+}
+
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *UserInfo) DeepCopyInto(out *UserInfo) {
 	*out = *in

pkg/apis/servicecatalog/validation/serviceplan_test.go

@@ -30,10 +30,12 @@ func validClusterServicePlan() *servicecatalog.ClusterServicePlan {
 			Name: "test-plan",
 		},
 		Spec: servicecatalog.ClusterServicePlanSpec{
+			SharedServicePlanSpec: servicecatalog.SharedServicePlanSpec{
+				ExternalName: "test-plan",
+				ExternalID:   "40d-0983-1b89",
+				Description:  "plan description",
+			},
 			ClusterServiceBrokerName: "test-broker",
-			ExternalName:             "test-plan",
-			ExternalID:               "40d-0983-1b89",
-			Description:              "plan description",
 			ClusterServiceClassRef: servicecatalog.ClusterObjectReference{
 				Name: "test-service-class",
 			},