Allow the same user to edit an instance

pkg/apis/servicecatalog/validation/instance.go

@@ -23,6 +23,8 @@ import (
 
 	sc "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog"
 	"github.com/kubernetes-incubator/service-catalog/pkg/controller"
+	scfeatures "github.com/kubernetes-incubator/service-catalog/pkg/features"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
 )
 
 // validateServiceInstanceName is the validation function for Instance names.
@@ -254,13 +256,31 @@ 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.
+// 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(new *sc.ServiceInstance, old *sc.ServiceInstance) field.ErrorList {
 	errors := field.ErrorList{}
-	if old.Generation != new.Generation && old.Status.CurrentOperation != "" {
-		errors = append(errors, field.Forbidden(field.NewPath("spec"), "Another update for this service instance is in progress"))
+
+	// If the OriginatingIdentityLocking feature is not set then only allow the
+	// same user to edit the Instance.
+	if utilfeature.DefaultFeatureGate.Enabled(scfeatures.OriginatingIdentityLocking) {
+		oldUID, newUID := "", ""
+
+		if old.Spec.UserInfo != nil {
+			oldUID = old.Spec.UserInfo.UID
+		}
+
+		if new.Spec.UserInfo != nil {
+			newUID = new.Spec.UserInfo.UID
+		}
+
+		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 {
 		errors = append(errors, field.Forbidden(field.NewPath("spec").Child("clusterServicePlanRef"), "clusterServicePlanRef must not be present when clusterServicePlanExternalName is being changed"))
 	}

pkg/apis/servicecatalog/validation/instance_test.go

@@ -744,30 +744,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,6 +820,13 @@ func TestInternalValidateServiceInstanceUpdateAllowed(t *testing.T) {
 			newInstance.Generation = oldInstance.Generation
 		}
 
+		if tc.setUser {
+			newInstance.Spec.UserInfo = &servicecatalog.UserInfo{
+				Username: "JohnDoe",
+				UID:      "123",
+			}
+		}
+
 		errs := internalValidateServiceInstanceUpdateAllowed(newInstance, oldInstance)
 		if len(errs) != 0 && tc.valid {
 			t.Errorf("%v: unexpected error: %v", tc.name, errs)

pkg/features/features.go

@@ -58,6 +58,17 @@ const (
 	// owner: @luksa
 	// alpha: v0.1.12
 	ResponseSchema utilfeature.Feature = "ResponseSchema"
+
+	// OriginatingIdentityLocking controls whether we lock OSB API resources
+	// that are being updated while we are still processing the update request.
+	// Meaning, we're still talking to the Broker to see if the requested
+	// change will be accepted or not. If locked, then no *other* user is
+	// allowed to update the resource.
+	// This lock was added in an attempt to fulfill the requirements
+	// of the OSBAPI OriginatingIdentity header.
+	// owner: @duglin
+	// alpha: v0.1.12
+	OriginatingIdentityLocking utilfeature.Feature = "OriginatingIdentityLocking"
 )
 
 func init() {
@@ -68,9 +79,10 @@ func init() {
 // To add a new feature, define a key for it above and add it here. The features will be
 // available throughout service catalog binaries.
 var defaultServiceCatalogFeatureGates = map[utilfeature.Feature]utilfeature.FeatureSpec{
-	PodPreset:               {Default: false, PreRelease: utilfeature.Alpha},
-	OriginatingIdentity:     {Default: false, PreRelease: utilfeature.Alpha},
-	AsyncBindingOperations:  {Default: false, PreRelease: utilfeature.Alpha},
-	NamespacedServiceBroker: {Default: false, PreRelease: utilfeature.Alpha},
-	ResponseSchema:          {Default: false, PreRelease: utilfeature.Alpha},
+	PodPreset:                  {Default: false, PreRelease: utilfeature.Alpha},
+	OriginatingIdentity:        {Default: false, PreRelease: utilfeature.Alpha},
+	AsyncBindingOperations:     {Default: false, PreRelease: utilfeature.Alpha},
+	NamespacedServiceBroker:    {Default: false, PreRelease: utilfeature.Alpha},
+	ResponseSchema:             {Default: false, PreRelease: utilfeature.Alpha},
+	OriginatingIdentityLocking: {Default: true, PreRelease: utilfeature.Alpha},
 }