Allow the same user to edit an instance

Signed-off-by: Doug Davis <[email protected]>

pkg/apis/servicecatalog/validation/instance.go

@@ -23,6 +23,7 @@ import (
 
 	sc "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog"
 	"github.com/kubernetes-incubator/service-catalog/pkg/controller"
+	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 )
 
 // validateServiceInstanceName is the validation function for Instance names.
@@ -254,11 +255,26 @@ func validateServiceInstanceUpdate(instance *sc.ServiceInstance) field.ErrorList
 }
 
 // internalValidateServiceInstanceUpdateAllowed ensures there is not a
-// pending update on-going with the spec of the instance before allowing an update
-// to the spec to go through.
-func internalValidateServiceInstanceUpdateAllowed(new *sc.ServiceInstance, old *sc.ServiceInstance) field.ErrorList {
+// pending update on-going with the spec of the instance before allowing an
+// update to the spec to go through unless its the same user who made the
+// original update.
+func internalValidateServiceInstanceUpdateAllowed(ctx genericapirequest.Context, new *sc.ServiceInstance, old *sc.ServiceInstance) field.ErrorList {
 	errors := field.ErrorList{}
-	if old.Generation != new.Generation && old.Status.CurrentOperation != "" {
+
+	oldUID := ""
+	newUID := ""
+
+	if old.Spec.UserInfo != nil {
+		oldUID = old.Spec.UserInfo.UID
+	}
+
+	if ctx != nil {
+		if user, _ := genericapirequest.UserFrom(ctx); user != nil {
+			newUID = user.GetUID()
+		}
+	}
+
+	if old.Generation != new.Generation && old.Status.CurrentOperation != "" && oldUID != newUID {
 		errors = append(errors, field.Forbidden(field.NewPath("spec"), "Another update for this service instance is in progress"))
 	}
 	if old.Spec.ClusterServicePlanExternalName != new.Spec.ClusterServicePlanExternalName && new.Spec.ClusterServicePlanRef != nil {
@@ -268,13 +284,13 @@ func internalValidateServiceInstanceUpdateAllowed(new *sc.ServiceInstance, old *
 }
 
 // ValidateServiceInstanceUpdate validates a change to the Instance's spec.
-func ValidateServiceInstanceUpdate(new *sc.ServiceInstance, old *sc.ServiceInstance) field.ErrorList {
+func ValidateServiceInstanceUpdate(ctx genericapirequest.Context, new *sc.ServiceInstance, old *sc.ServiceInstance) field.ErrorList {
 	allErrs := field.ErrorList{}
 
 	specFieldPath := field.NewPath("spec")
 
 	allErrs = append(allErrs, validatePlanReferenceUpdate(&new.Spec.PlanReference, &old.Spec.PlanReference, specFieldPath)...)
-	allErrs = append(allErrs, internalValidateServiceInstanceUpdateAllowed(new, old)...)
+	allErrs = append(allErrs, internalValidateServiceInstanceUpdateAllowed(ctx, new, old)...)
 	allErrs = append(allErrs, internalValidateServiceInstance(new, false)...)
 
 	allErrs = append(allErrs, apivalidation.ValidateImmutableField(new.Spec.ClusterServiceClassExternalName, old.Spec.ClusterServiceClassExternalName, specFieldPath.Child("clusterServiceClassExternalName"))...)

pkg/apis/servicecatalog/validation/instance_test.go

@@ -25,6 +25,8 @@ import (
 	"k8s.io/apimachinery/pkg/util/validation/field"
 
 	"github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog"
+	"k8s.io/apiserver/pkg/authentication/user"
+	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 )
 
 const (
@@ -744,30 +746,42 @@ func TestInternalValidateServiceInstanceUpdateAllowed(t *testing.T) {
 		name             string
 		specChange       bool
 		onGoingOperation bool
+		setUser          bool
 		valid            bool
 	}{
 		{
 			name:             "spec change when no on-going operation",
 			specChange:       true,
 			onGoingOperation: false,
+			setUser:          false,
 			valid:            true,
 		},
 		{
-			name:             "spec change when on-going operation",
+			name:             "spec change when on-going operation, same user",
 			specChange:       true,
 			onGoingOperation: true,
+			setUser:          false,
+			valid:            true,
+		},
+		{
+			name:             "spec change when on-going operation, diff user",
+			specChange:       true,
+			onGoingOperation: true,
+			setUser:          true,
 			valid:            false,
 		},
 		{
 			name:             "meta change when no on-going operation",
 			specChange:       false,
 			onGoingOperation: false,
+			setUser:          false,
 			valid:            true,
 		},
 		{
 			name:             "meta change when on-going operation",
 			specChange:       false,
 			onGoingOperation: true,
+			setUser:          false,
 			valid:            true,
 		},
 	}
@@ -808,7 +822,14 @@ func TestInternalValidateServiceInstanceUpdateAllowed(t *testing.T) {
 			newInstance.Generation = oldInstance.Generation
 		}
 
-		errs := internalValidateServiceInstanceUpdateAllowed(newInstance, oldInstance)
+		user := user.DefaultInfo{}
+		if tc.setUser {
+			user.Name = "JohnDoe"
+			user.UID = "123"
+		}
+		ctx := genericapirequest.WithUser(genericapirequest.NewContext(), &user)
+
+		errs := internalValidateServiceInstanceUpdateAllowed(ctx, newInstance, oldInstance)
 		if len(errs) != 0 && tc.valid {
 			t.Errorf("%v: unexpected error: %v", tc.name, errs)
 			continue
@@ -880,7 +901,7 @@ func TestInternalValidateServiceInstanceUpdateAllowedForPlanChange(t *testing.T)
 			},
 		}
 
-		errs := internalValidateServiceInstanceUpdateAllowed(newInstance, oldInstance)
+		errs := internalValidateServiceInstanceUpdateAllowed(nil, newInstance, oldInstance)
 		if len(errs) != 0 && tc.valid {
 			t.Errorf("%v: unexpected error: %v", tc.name, errs)
 			continue

pkg/registry/servicecatalog/instance/strategy.go

@@ -186,7 +186,7 @@ func (instanceRESTStrategy) ValidateUpdate(ctx genericapirequest.Context, new, o
 		glog.Fatal("received a non-instance object to validate from")
 	}
 
-	return scv.ValidateServiceInstanceUpdate(newServiceInstance, oldServiceInstance)
+	return scv.ValidateServiceInstanceUpdate(ctx, newServiceInstance, oldServiceInstance)
 }
 
 // CheckGracefulDelete sets the UserInfo on the resource to that of the user that