Disallow only spec updates for now (#1146)

What this PR does / why we need it:
For now, we disallowed all updates except status updates. This narrows it down and just disallows updates to the spec that means:

- no change in params
- no change in OV
This potentially narrows down issues with the webhook as controller etc. can update annotation or finalizers, which I think is fine even when plan is running.

Addition to #883

pkg/apis/kudo/v1beta1/instance_validator.go

@@ -2,6 +2,7 @@ package v1beta1
 
 import (
 	"fmt"
+	"reflect"
 
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
@@ -12,25 +13,30 @@ import (
 // this forces the instance type to implement Validator interface, we'll get compile time error if it's not true anymore
 var _ kudo.Validator = &Instance{}
 
-// ValidateCreate implements webhookutil.validator (from controller-runtime)
+// ValidateCreate implements kudo.Validator (slightly tweaked interface originally from controller-runtime)
 // we do not enforce any rules upon creation right now
 func (i *Instance) ValidateCreate(req admission.Request) error {
 	return nil
 }
 
 // ValidateUpdate hook called when UPDATE operation is triggered and our admission webhook is triggered
-// ValidateUpdate implements webhookutil.validator (from controller-runtime)
+// ValidateUpdate implements kudo.Validator (slightly tweaked interface originally from controller-runtime)
 func (i *Instance) ValidateUpdate(old runtime.Object, req admission.Request) error {
-	if i.Status.AggregatedStatus.Status.IsRunning() && req.RequestSubResource != "status" {
+	oldInstance := old.(*Instance)
+	if i.Status.AggregatedStatus.Status.IsRunning() && specChanged(i.Spec, oldInstance.Spec) {
 		// when updating anything else than status, there shouldn't be a running plan
 		return fmt.Errorf("cannot update Instance %s/%s right now, there's plan %s in progress", i.Namespace, i.Name, i.Status.AggregatedStatus.ActivePlanName)
 	}
 	return nil
 }
 
+func specChanged(old InstanceSpec, new InstanceSpec) bool {
+	return !reflect.DeepEqual(old, new)
+}
+
 // ValidateDelete hook called when DELETE operation is triggered and our admission webhook is triggered
 // we don't enforce any validation on DELETE right now
-// ValidateDelete implements webhookutil.validator (from controller-runtime)
+// ValidateDelete implements kudo.Validator (slightly tweaked interface originally from controller-runtime)
 func (i *Instance) ValidateDelete(req admission.Request) error {
 	return nil
 }

pkg/apis/kudo/v1beta1/instance_validator_test.go

@@ -0,0 +1,62 @@
+package v1beta1
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestValidateUpdate(t *testing.T) {
+	runningInstance := Instance{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: "kudo.dev/v1beta1",
+			Kind:       "Instance",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test",
+			Namespace: "test",
+		},
+		Spec: InstanceSpec{
+			OperatorVersion: v1.ObjectReference{
+				Name: "test-1.0",
+			},
+		},
+		Status: InstanceStatus{
+			AggregatedStatus: AggregatedStatus{
+				Status:         ExecutionInProgress,
+				ActivePlanName: "deploy",
+			},
+		},
+	}
+
+	req := admission.Request{}
+
+	tests := []struct {
+		name          string
+		i             Instance
+		oldInstance   Instance
+		expectedError error
+	}{
+		{"no change", runningInstance, runningInstance, nil},
+		{"change in labels is allowed on running instance", runningInstance, func() Instance {
+			updatedInstance := runningInstance.DeepCopy()
+			updatedInstance.ObjectMeta.Labels = map[string]string{"label": "label2"}
+			return *updatedInstance
+		}(), nil},
+		{"change in spec is not allowed on running instance", runningInstance, func() Instance {
+			updatedInstance := runningInstance.DeepCopy()
+			updatedInstance.Spec.Parameters = map[string]string{"newparam": "newvalue"}
+			return *updatedInstance
+		}(), errors.New("cannot update Instance test/test right now, there's plan deploy in progress")},
+	}
+
+	for _, tt := range tests {
+		err := tt.i.ValidateUpdate(&tt.oldInstance, req)
+		assert.Equal(t, tt.expectedError, err)
+	}
+}