Skip to content

Commit 85125fd

Browse files
Ville Aikasarschles
authored andcommitted
Refactor instance spec (#1350)
* Split plan spec out of the main spec * remove conflicts * bad rebase merge fixup * fix up the generated files fiasco * fix a test case that I had missed during the merge * Rename DesiredPlan to PlanReference
1 parent 99c0644 commit 85125fd

File tree

17 files changed

+247
-91
lines changed

17 files changed

+247
-91
lines changed

pkg/apis/servicecatalog/types.go

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -410,23 +410,30 @@ type ServiceInstance struct {
410410
Status ServiceInstanceStatus
411411
}
412412

413-
// ServiceInstanceSpec represents the desired state of an Instance.
414-
type ServiceInstanceSpec struct {
415-
// ExternalClusterServiceClassName is the human-readable name of the service
416-
// as reported by the broker. Note that if the broker changes
413+
// PlanReference defines the user specification for the desired
414+
// ServicePlan and ServiceClass. Because there are multiple ways to
415+
// specify the desired Class/Plan, this structure specifies the
416+
// allowed ways to specify the intent.
417+
type PlanReference struct {
418+
// ExternalClusterServiceClassName is the human-readable name of the
419+
// service as reported by the broker. Note that if the broker changes
417420
// the name of the ClusterServiceClass, it will not be reflected here,
418421
// and to see the current name of the ClusterServiceClass, you should
419422
// follow the ClusterServiceClassRef below.
420423
//
421424
// Immutable.
422425
ExternalClusterServiceClassName string
423-
424426
// ExternalClusterServicePlanName is the human-readable name of the plan
425-
// as reported by the broker. Note that if the broker changes
426-
// the name of the ClusterServicePlan, it will not be reflected here,
427-
// and to see the current name of the ClusterServicePlan, you should
428-
// follow the ClusterServicePlanRef below.
427+
// as reported by the broker. Note that if the broker changes the name
428+
// of the ClusterServicePlan, it will not be reflected here, and to see
429+
// the current name of the ClusterServicePlan, you should follow the
430+
// ClusterServicePlanRef below.
429431
ExternalClusterServicePlanName string
432+
}
433+
434+
// ServiceInstanceSpec represents the desired state of an Instance.
435+
type ServiceInstanceSpec struct {
436+
PlanReference
430437

431438
// ClusterServiceClassRef is a reference to the ClusterServiceClass
432439
// that the user selected.

pkg/apis/servicecatalog/v1alpha1/types.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -421,22 +421,31 @@ type ServiceInstance struct {
421421
Status ServiceInstanceStatus `json:"status"`
422422
}
423423

424-
// ServiceInstanceSpec represents the desired state of an Instance.
425-
type ServiceInstanceSpec struct {
424+
// PlanReference defines the user specification for the desired
425+
// ServicePlan and ServiceClass. Because there are multiple ways to
426+
// specify the desired Class/Plan, this structure specifies the
427+
// allowed ways to specify the intent.
428+
type PlanReference struct {
426429
// ExternalClusterServiceClassName is the human-readable name of the
427430
// service as reported by the broker. Note that if the broker changes
428431
// the name of the ClusterServiceClass, it will not be reflected here,
429432
// and to see the current name of the ClusterServiceClass, you should
430433
// follow the ClusterServiceClassRef below.
431434
//
432435
// Immutable.
433-
ExternalClusterServiceClassName string `json:"externalClusterServiceClassName"`
436+
ExternalClusterServiceClassName string `json:"externalClusterServiceClassName,omitempty"`
434437
// ExternalClusterServicePlanName is the human-readable name of the plan
435438
// as reported by the broker. Note that if the broker changes the name
436439
// of the ClusterServicePlan, it will not be reflected here, and to see
437440
// the current name of the ClusterServicePlan, you should follow the
438441
// ClusterServicePlanRef below.
439442
ExternalClusterServicePlanName string `json:"externalClusterServicePlanName,omitempty"`
443+
}
444+
445+
// ServiceInstanceSpec represents the desired state of an Instance.
446+
type ServiceInstanceSpec struct {
447+
// Specification of what ServiceClass/ServicePlan is being provisioned.
448+
PlanReference `json:",inline"`
440449

441450
// ClusterServiceClassRef is a reference to the ClusterServiceClass
442451
// that the user selected.

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

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ func RegisterConversions(scheme *runtime.Scheme) error {
6161
Convert_servicecatalog_ClusterServicePlanSpec_To_v1alpha1_ClusterServicePlanSpec,
6262
Convert_v1alpha1_ParametersFromSource_To_servicecatalog_ParametersFromSource,
6363
Convert_servicecatalog_ParametersFromSource_To_v1alpha1_ParametersFromSource,
64+
Convert_v1alpha1_PlanReference_To_servicecatalog_PlanReference,
65+
Convert_servicecatalog_PlanReference_To_v1alpha1_PlanReference,
6466
Convert_v1alpha1_SecretKeyReference_To_servicecatalog_SecretKeyReference,
6567
Convert_servicecatalog_SecretKeyReference_To_v1alpha1_SecretKeyReference,
6668
Convert_v1alpha1_ServiceBinding_To_servicecatalog_ServiceBinding,
@@ -432,6 +434,28 @@ func Convert_servicecatalog_ParametersFromSource_To_v1alpha1_ParametersFromSourc
432434
return autoConvert_servicecatalog_ParametersFromSource_To_v1alpha1_ParametersFromSource(in, out, s)
433435
}
434436

437+
func autoConvert_v1alpha1_PlanReference_To_servicecatalog_PlanReference(in *PlanReference, out *servicecatalog.PlanReference, s conversion.Scope) error {
438+
out.ExternalClusterServiceClassName = in.ExternalClusterServiceClassName
439+
out.ExternalClusterServicePlanName = in.ExternalClusterServicePlanName
440+
return nil
441+
}
442+
443+
// Convert_v1alpha1_PlanReference_To_servicecatalog_PlanReference is an autogenerated conversion function.
444+
func Convert_v1alpha1_PlanReference_To_servicecatalog_PlanReference(in *PlanReference, out *servicecatalog.PlanReference, s conversion.Scope) error {
445+
return autoConvert_v1alpha1_PlanReference_To_servicecatalog_PlanReference(in, out, s)
446+
}
447+
448+
func autoConvert_servicecatalog_PlanReference_To_v1alpha1_PlanReference(in *servicecatalog.PlanReference, out *PlanReference, s conversion.Scope) error {
449+
out.ExternalClusterServiceClassName = in.ExternalClusterServiceClassName
450+
out.ExternalClusterServicePlanName = in.ExternalClusterServicePlanName
451+
return nil
452+
}
453+
454+
// Convert_servicecatalog_PlanReference_To_v1alpha1_PlanReference is an autogenerated conversion function.
455+
func Convert_servicecatalog_PlanReference_To_v1alpha1_PlanReference(in *servicecatalog.PlanReference, out *PlanReference, s conversion.Scope) error {
456+
return autoConvert_servicecatalog_PlanReference_To_v1alpha1_PlanReference(in, out, s)
457+
}
458+
435459
func autoConvert_v1alpha1_SecretKeyReference_To_servicecatalog_SecretKeyReference(in *SecretKeyReference, out *servicecatalog.SecretKeyReference, s conversion.Scope) error {
436460
out.Name = in.Name
437461
out.Key = in.Key
@@ -827,8 +851,9 @@ func Convert_servicecatalog_ServiceInstancePropertiesState_To_v1alpha1_ServiceIn
827851
}
828852

829853
func autoConvert_v1alpha1_ServiceInstanceSpec_To_servicecatalog_ServiceInstanceSpec(in *ServiceInstanceSpec, out *servicecatalog.ServiceInstanceSpec, s conversion.Scope) error {
830-
out.ExternalClusterServiceClassName = in.ExternalClusterServiceClassName
831-
out.ExternalClusterServicePlanName = in.ExternalClusterServicePlanName
854+
if err := Convert_v1alpha1_PlanReference_To_servicecatalog_PlanReference(&in.PlanReference, &out.PlanReference, s); err != nil {
855+
return err
856+
}
832857
out.ClusterServiceClassRef = (*v1.ObjectReference)(unsafe.Pointer(in.ClusterServiceClassRef))
833858
out.ClusterServicePlanRef = (*v1.ObjectReference)(unsafe.Pointer(in.ClusterServicePlanRef))
834859
out.Parameters = (*runtime.RawExtension)(unsafe.Pointer(in.Parameters))
@@ -845,8 +870,9 @@ func Convert_v1alpha1_ServiceInstanceSpec_To_servicecatalog_ServiceInstanceSpec(
845870
}
846871

847872
func autoConvert_servicecatalog_ServiceInstanceSpec_To_v1alpha1_ServiceInstanceSpec(in *servicecatalog.ServiceInstanceSpec, out *ServiceInstanceSpec, s conversion.Scope) error {
848-
out.ExternalClusterServiceClassName = in.ExternalClusterServiceClassName
849-
out.ExternalClusterServicePlanName = in.ExternalClusterServicePlanName
873+
if err := Convert_servicecatalog_PlanReference_To_v1alpha1_PlanReference(&in.PlanReference, &out.PlanReference, s); err != nil {
874+
return err
875+
}
850876
out.ClusterServiceClassRef = (*v1.ObjectReference)(unsafe.Pointer(in.ClusterServiceClassRef))
851877
out.ClusterServicePlanRef = (*v1.ObjectReference)(unsafe.Pointer(in.ClusterServicePlanRef))
852878
out.Parameters = (*runtime.RawExtension)(unsafe.Pointer(in.Parameters))

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,10 @@ func RegisterDeepCopies(scheme *runtime.Scheme) error {
8686
in.(*ParametersFromSource).DeepCopyInto(out.(*ParametersFromSource))
8787
return nil
8888
}, InType: reflect.TypeOf(&ParametersFromSource{})},
89+
conversion.GeneratedDeepCopyFunc{Fn: func(in interface{}, out interface{}, c *conversion.Cloner) error {
90+
in.(*PlanReference).DeepCopyInto(out.(*PlanReference))
91+
return nil
92+
}, InType: reflect.TypeOf(&PlanReference{})},
8993
conversion.GeneratedDeepCopyFunc{Fn: func(in interface{}, out interface{}, c *conversion.Cloner) error {
9094
in.(*SecretKeyReference).DeepCopyInto(out.(*SecretKeyReference))
9195
return nil
@@ -565,6 +569,22 @@ func (in *ParametersFromSource) DeepCopy() *ParametersFromSource {
565569
return out
566570
}
567571

572+
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
573+
func (in *PlanReference) DeepCopyInto(out *PlanReference) {
574+
*out = *in
575+
return
576+
}
577+
578+
// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PlanReference.
579+
func (in *PlanReference) DeepCopy() *PlanReference {
580+
if in == nil {
581+
return nil
582+
}
583+
out := new(PlanReference)
584+
in.DeepCopyInto(out)
585+
return out
586+
}
587+
568588
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
569589
func (in *SecretKeyReference) DeepCopyInto(out *SecretKeyReference) {
570590
*out = *in
@@ -1012,6 +1032,7 @@ func (in *ServiceInstancePropertiesState) DeepCopy() *ServiceInstancePropertiesS
10121032
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
10131033
func (in *ServiceInstanceSpec) DeepCopyInto(out *ServiceInstanceSpec) {
10141034
*out = *in
1035+
out.PlanReference = in.PlanReference
10151036
if in.ClusterServiceClassRef != nil {
10161037
in, out := &in.ClusterServiceClassRef, &out.ClusterServiceClassRef
10171038
if *in == nil {

pkg/apis/servicecatalog/validation/instance.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,15 +71,13 @@ func validateServiceInstanceSpec(spec *sc.ServiceInstanceSpec, fldPath *field.Pa
7171
if "" == spec.ExternalClusterServiceClassName {
7272
allErrs = append(allErrs, field.Required(fldPath.Child("externalClusterServiceClassName"), "externalClusterServiceClassName is required"))
7373
}
74-
7574
for _, msg := range validateServiceClassName(spec.ExternalClusterServiceClassName, false /* prefix */) {
7675
allErrs = append(allErrs, field.Invalid(fldPath.Child("externalClusterServiceClassName"), spec.ExternalClusterServiceClassName, msg))
7776
}
7877

7978
if "" == spec.ExternalClusterServicePlanName {
8079
allErrs = append(allErrs, field.Required(fldPath.Child("externalClusterServicePlanName"), "externalClusterServicePlanName is required"))
8180
}
82-
8381
for _, msg := range validateServicePlanName(spec.ExternalClusterServicePlanName, false /* prefix */) {
8482
allErrs = append(allErrs, field.Invalid(fldPath.Child("externalClusterServicePlanName"), spec.ExternalClusterServicePlanName, msg))
8583
}

pkg/apis/servicecatalog/validation/instance_test.go

Lines changed: 39 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ import (
2727
"github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog"
2828
)
2929

30+
const externalClusterServiceClassName = "test-serviceclass"
31+
const externalClusterServicePlanName = "test-plan"
32+
3033
func validServiceInstanceForCreate() *servicecatalog.ServiceInstance {
3134
return &servicecatalog.ServiceInstance{
3235
ObjectMeta: metav1.ObjectMeta{
@@ -35,8 +38,10 @@ func validServiceInstanceForCreate() *servicecatalog.ServiceInstance {
3538
Generation: 1,
3639
},
3740
Spec: servicecatalog.ServiceInstanceSpec{
38-
ExternalClusterServiceClassName: "test-serviceclass",
39-
ExternalClusterServicePlanName: "test-plan",
41+
PlanReference: servicecatalog.PlanReference{
42+
ExternalClusterServiceClassName: externalClusterServiceClassName,
43+
ExternalClusterServicePlanName: externalClusterServicePlanName,
44+
},
4045
},
4146
}
4247
}
@@ -502,8 +507,10 @@ func TestInternalValidateServiceInstanceUpdateAllowed(t *testing.T) {
502507
Namespace: "test-ns",
503508
},
504509
Spec: servicecatalog.ServiceInstanceSpec{
505-
ExternalClusterServiceClassName: "test-serviceclass",
506-
ExternalClusterServicePlanName: "test-plan",
510+
PlanReference: servicecatalog.PlanReference{
511+
ExternalClusterServiceClassName: externalClusterServiceClassName,
512+
ExternalClusterServicePlanName: externalClusterServicePlanName,
513+
},
507514
},
508515
}
509516
if tc.onGoingSpecChange {
@@ -519,8 +526,10 @@ func TestInternalValidateServiceInstanceUpdateAllowed(t *testing.T) {
519526
Namespace: "test-ns",
520527
},
521528
Spec: servicecatalog.ServiceInstanceSpec{
522-
ExternalClusterServiceClassName: "test-serviceclass",
523-
ExternalClusterServicePlanName: "test-plan",
529+
PlanReference: servicecatalog.PlanReference{
530+
ExternalClusterServiceClassName: "test-serviceclass",
531+
ExternalClusterServicePlanName: "test-plan",
532+
},
524533
},
525534
}
526535
if tc.newSpecChange {
@@ -578,10 +587,12 @@ func TestInternalValidateServiceInstanceUpdateAllowedForPlanChange(t *testing.T)
578587
Namespace: "test-ns",
579588
},
580589
Spec: servicecatalog.ServiceInstanceSpec{
581-
ExternalClusterServiceClassName: "test-serviceclass",
582-
ExternalClusterServicePlanName: tc.oldPlan,
583-
ClusterServiceClassRef: &corev1.ObjectReference{},
584-
ClusterServicePlanRef: &corev1.ObjectReference{},
590+
PlanReference: servicecatalog.PlanReference{
591+
ExternalClusterServiceClassName: "test-serviceclass",
592+
ExternalClusterServicePlanName: tc.oldPlan,
593+
},
594+
ClusterServiceClassRef: &corev1.ObjectReference{},
595+
ClusterServicePlanRef: &corev1.ObjectReference{},
585596
},
586597
}
587598

@@ -591,10 +602,12 @@ func TestInternalValidateServiceInstanceUpdateAllowedForPlanChange(t *testing.T)
591602
Namespace: "test-ns",
592603
},
593604
Spec: servicecatalog.ServiceInstanceSpec{
594-
ExternalClusterServiceClassName: "test-serviceclass",
595-
ExternalClusterServicePlanName: tc.newPlan,
596-
ClusterServiceClassRef: &corev1.ObjectReference{},
597-
ClusterServicePlanRef: tc.newPlanRef,
605+
PlanReference: servicecatalog.PlanReference{
606+
ExternalClusterServiceClassName: externalClusterServiceClassName,
607+
ExternalClusterServicePlanName: tc.newPlan,
608+
},
609+
ClusterServiceClassRef: &corev1.ObjectReference{},
610+
ClusterServicePlanRef: tc.newPlanRef,
598611
},
599612
}
600613

@@ -742,10 +755,12 @@ func TestValidateServiceInstanceStatusUpdate(t *testing.T) {
742755
Generation: 2,
743756
},
744757
Spec: servicecatalog.ServiceInstanceSpec{
745-
ExternalClusterServiceClassName: "test-serviceclass",
746-
ExternalClusterServicePlanName: "test-plan",
747-
ClusterServiceClassRef: &corev1.ObjectReference{},
748-
ClusterServicePlanRef: &corev1.ObjectReference{},
758+
PlanReference: servicecatalog.PlanReference{
759+
ExternalClusterServiceClassName: externalClusterServiceClassName,
760+
ExternalClusterServicePlanName: externalClusterServicePlanName,
761+
},
762+
ClusterServiceClassRef: &corev1.ObjectReference{},
763+
ClusterServicePlanRef: &corev1.ObjectReference{},
749764
},
750765
Status: *tc.old,
751766
}
@@ -757,10 +772,12 @@ func TestValidateServiceInstanceStatusUpdate(t *testing.T) {
757772
Generation: 2,
758773
},
759774
Spec: servicecatalog.ServiceInstanceSpec{
760-
ExternalClusterServiceClassName: "test-serviceclass",
761-
ExternalClusterServicePlanName: "test-plan",
762-
ClusterServiceClassRef: &corev1.ObjectReference{},
763-
ClusterServicePlanRef: &corev1.ObjectReference{},
775+
PlanReference: servicecatalog.PlanReference{
776+
ExternalClusterServiceClassName: externalClusterServiceClassName,
777+
ExternalClusterServicePlanName: externalClusterServicePlanName,
778+
},
779+
ClusterServiceClassRef: &corev1.ObjectReference{},
780+
ClusterServicePlanRef: &corev1.ObjectReference{},
764781
},
765782
Status: *tc.new,
766783
}

pkg/apis/servicecatalog/zz_generated.deepcopy.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,10 @@ func RegisterDeepCopies(scheme *runtime.Scheme) error {
8686
in.(*ParametersFromSource).DeepCopyInto(out.(*ParametersFromSource))
8787
return nil
8888
}, InType: reflect.TypeOf(&ParametersFromSource{})},
89+
conversion.GeneratedDeepCopyFunc{Fn: func(in interface{}, out interface{}, c *conversion.Cloner) error {
90+
in.(*PlanReference).DeepCopyInto(out.(*PlanReference))
91+
return nil
92+
}, InType: reflect.TypeOf(&PlanReference{})},
8993
conversion.GeneratedDeepCopyFunc{Fn: func(in interface{}, out interface{}, c *conversion.Cloner) error {
9094
in.(*SecretKeyReference).DeepCopyInto(out.(*SecretKeyReference))
9195
return nil
@@ -565,6 +569,22 @@ func (in *ParametersFromSource) DeepCopy() *ParametersFromSource {
565569
return out
566570
}
567571

572+
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
573+
func (in *PlanReference) DeepCopyInto(out *PlanReference) {
574+
*out = *in
575+
return
576+
}
577+
578+
// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PlanReference.
579+
func (in *PlanReference) DeepCopy() *PlanReference {
580+
if in == nil {
581+
return nil
582+
}
583+
out := new(PlanReference)
584+
in.DeepCopyInto(out)
585+
return out
586+
}
587+
568588
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
569589
func (in *SecretKeyReference) DeepCopyInto(out *SecretKeyReference) {
570590
*out = *in
@@ -1012,6 +1032,7 @@ func (in *ServiceInstancePropertiesState) DeepCopy() *ServiceInstancePropertiesS
10121032
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
10131033
func (in *ServiceInstanceSpec) DeepCopyInto(out *ServiceInstanceSpec) {
10141034
*out = *in
1035+
out.PlanReference = in.PlanReference
10151036
if in.ClusterServiceClassRef != nil {
10161037
in, out := &in.ClusterServiceClassRef, &out.ClusterServiceClassRef
10171038
if *in == nil {

0 commit comments

Comments
 (0)