From 66b7b001be5b03dde561b03e6db3dbd5872b8589 Mon Sep 17 00:00:00 2001
From: kubevirt-bot <rmohr+kubebot@redhat.com>
Date: Mon, 16 Nov 2020 16:31:57 +0100
Subject: [PATCH] Validate updates to the whole .spec stanza (#913)

HCO is supposed to validate updates to its CR
also delegating to components operators the validation
of the changes it has to propagate (trying to apply
them in dry run mode first and committing only if all the
involved components accepted it).

This was done only for the .spec.worloads stanza,
applying the same strategy to the whole .spec stanza.

Fixes: https://bugzilla.redhat.com/1893646

Signed-off-by: Simone Tiraboschi <stirabos@redhat.com>

Co-authored-by: Simone Tiraboschi <stirabos@redhat.com>
---
 .../hco/v1beta1/hyperconverged_webhook.go     |  5 ++--
 .../v1beta1/hyperconverged_webhook_test.go    | 25 -------------------
 2 files changed, 3 insertions(+), 27 deletions(-)

diff --git a/pkg/apis/hco/v1beta1/hyperconverged_webhook.go b/pkg/apis/hco/v1beta1/hyperconverged_webhook.go
index 21e7d9ac60..715184216e 100644
--- a/pkg/apis/hco/v1beta1/hyperconverged_webhook.go
+++ b/pkg/apis/hco/v1beta1/hyperconverged_webhook.go
@@ -115,13 +115,14 @@ func (r *HyperConverged) ValidateUpdate(old runtime.Object) error {
 	}
 
 	if !reflect.DeepEqual(
-		oldR.Spec.Workloads,
-		r.Spec.Workloads) {
+		oldR.Spec,
+		r.Spec) {
 
 		opts := &client.UpdateOptions{DryRun: []string{metav1.DryRunAll}}
 		for _, obj := range []runtime.Object{
 			r.NewKubeVirt(),
 			r.NewCDI(),
+			// TODO: try to validate with all the components
 		} {
 			if err := r.UpdateOperatorCr(ctx, obj, opts); err != nil {
 				return err
diff --git a/pkg/apis/hco/v1beta1/hyperconverged_webhook_test.go b/pkg/apis/hco/v1beta1/hyperconverged_webhook_test.go
index db571e8e80..bbae0cb4b5 100644
--- a/pkg/apis/hco/v1beta1/hyperconverged_webhook_test.go
+++ b/pkg/apis/hco/v1beta1/hyperconverged_webhook_test.go
@@ -155,31 +155,6 @@ var _ = Describe("Hyperconverged Webhooks", func() {
 			Expect(err).Should(Equal(ErrFakeCdiError))
 		})
 
-		It("should not return error if no different in Spec.Workloads", func() {
-			hco := &HyperConverged{
-				Spec: HyperConvergedSpec{
-					Infra: HyperConvergedConfig{
-						NodePlacement: newHyperConvergedConfig(),
-					},
-					Workloads: HyperConvergedConfig{
-						NodePlacement: newHyperConvergedConfig(),
-					},
-				},
-			}
-
-			// replace the real client with a mock
-			cli = fake.NewFakeClientWithScheme(s, hco)
-
-			newHco := &HyperConverged{}
-			hco.DeepCopyInto(newHco)
-			// Change only infra, but leave workloads as is
-			newHco.Spec.Infra.NodePlacement.NodeSelector["a change"] = "Something else"
-
-			// should new return error, even when there are no CDI and KV
-			err := newHco.ValidateUpdate(hco)
-			Expect(err).To(BeNil())
-		})
-
 		It("should not return error if dry-run update of CDI CR passes", func() {
 			hco := &HyperConverged{
 				Spec: HyperConvergedSpec{