Add warning for setting enableCommonBootImageImport

setting or changing the enableCommonBootImageImport FG triggers warning
from the validation webhook.

Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>

Author: nunnatsa

pkg/webhooks/validator/validator.go

@@ -154,7 +154,7 @@ func (wh *WebhookHandler) ValidateCreate(_ context.Context, dryrun bool, hc *v1b
 		return err
 	}
 
-	if err := wh.validateFeatureGates(hc); err != nil {
+	if err := wh.validateFeatureGatesOnCreate(hc); err != nil {
 		return err
 	}
 
@@ -221,7 +221,7 @@ func (wh *WebhookHandler) ValidateUpdate(ctx context.Context, dryrun bool, reque
 		return err
 	}
 
-	if err := wh.validateFeatureGates(requested); err != nil {
+	if err := wh.validateFeatureGatesOnUpdate(requested, exists); err != nil {
 		return err
 	}
 
@@ -433,7 +433,29 @@ const (
 	fgDeprecationWarning = "spec.featureGates.%s is deprecated and ignored. It will be removed in a future version;"
 )
 
-func (wh *WebhookHandler) validateFeatureGates(hc *v1beta1.HyperConverged) error {
+func (wh *WebhookHandler) validateFeatureGatesOnCreate(hc *v1beta1.HyperConverged) error {
+	warnings := wh.validateDeprecatedFeatureGates(hc)
+	warnings = validateOldFGOnCreate(warnings, hc)
+
+	if len(warnings) > 0 {
+		return newValidationWarning(warnings)
+	}
+
+	return nil
+}
+
+func (wh *WebhookHandler) validateFeatureGatesOnUpdate(requested, exists *v1beta1.HyperConverged) error {
+	warnings := wh.validateDeprecatedFeatureGates(requested)
+	warnings = validateOldFGOnUpdate(warnings, requested, exists)
+
+	if len(warnings) > 0 {
+		return newValidationWarning(warnings)
+	}
+
+	return nil
+}
+
+func (wh *WebhookHandler) validateDeprecatedFeatureGates(hc *v1beta1.HyperConverged) []string {
 	var warnings []string
 
 	//nolint:staticcheck
@@ -456,11 +478,29 @@ func (wh *WebhookHandler) validateFeatureGates(hc *v1beta1.HyperConverged) error
 		warnings = append(warnings, fmt.Sprintf(fgDeprecationWarning, "enableManagedTenantQuota"))
 	}
 
-	if len(warnings) > 0 {
-		return newValidationWarning(warnings)
+	return warnings
+}
+
+func validateOldFGOnCreate(warnings []string, hc *v1beta1.HyperConverged) []string {
+	//nolint:staticcheck
+	if hc.Spec.FeatureGates.EnableCommonBootImageImport != nil {
+		warnings = append(warnings, fmt.Sprintf(fgMovedWarning, "enableCommonBootImageImport"))
 	}
 
-	return nil
+	return warnings
+}
+
+func validateOldFGOnUpdate(warnings []string, hc, prevHC *v1beta1.HyperConverged) []string {
+	//nolint:staticcheck
+	if oldFGChanged(hc.Spec.FeatureGates.EnableCommonBootImageImport, prevHC.Spec.FeatureGates.EnableCommonBootImageImport) {
+		warnings = append(warnings, fmt.Sprintf(fgMovedWarning, "enableCommonBootImageImport"))
+	}
+
+	return warnings
+}
+
+func oldFGChanged(newFG, prevFG *bool) bool {
+	return newFG != nil && (prevFG == nil || *newFG != *prevFG)
 }
 
 func hasRequiredHTTP2Ciphers(ciphers []string) bool {

pkg/webhooks/validator/validator_test.go

@@ -514,7 +514,7 @@ var _ = Describe("webhooks validator", func() {
 						NonRoot:                     ptr.To(false),
 						EnableCommonBootImageImport: ptr.To(true),
 						EnableManagedTenantQuota:    ptr.To(false),
-					}, "enableManagedTenantQuota", "nonRoot"),
+					}, "enableManagedTenantQuota", "nonRoot", "enableCommonBootImageImport"),
 			)
 		})
 	})
@@ -1181,9 +1181,47 @@ var _ = Describe("webhooks validator", func() {
 						NonRoot:                     ptr.To(false),
 						EnableCommonBootImageImport: ptr.To(true),
 						EnableManagedTenantQuota:    ptr.To(false),
-					}, "enableManagedTenantQuota", "nonRoot"),
+					}, "enableManagedTenantQuota", "nonRoot", "enableCommonBootImageImport"),
+			)
+		})
+
+		Context("validate moved FG on update", func() {
+			DescribeTable("should return warning for enableCommonBootImageImport on update", func(newFG, oldFG *bool) {
+				newHCO := hco.DeepCopy()
+				hco.Spec.FeatureGates.EnableCommonBootImageImport = newFG
+				newHCO.Spec.FeatureGates.EnableCommonBootImageImport = oldFG
+
+				err := wh.ValidateUpdate(ctx, dryRun, newHCO, hco)
+
+				Expect(err).To(HaveOccurred())
+				expected := &ValidationWarning{}
+				Expect(errors.As(err, &expected)).To(BeTrue())
+
+				Expect(expected.warnings).To(HaveLen(1))
+				Expect(expected.warnings).To(ContainElements(ContainSubstring("enableCommonBootImageImport")))
+			},
+				Entry("should trigger warning if enableCommonBootImageImport appeared as true", nil, ptr.To(true)),
+				Entry("should trigger warning if enableCommonBootImageImport appeared as false", nil, ptr.To(false)),
+				Entry("should trigger warning if enableCommonBootImageImport has changed from true to false", ptr.To(true), ptr.To(false)),
+				Entry("should trigger warning if enableCommonBootImageImport has changed from false to true", ptr.To(false), ptr.To(true)),
+			)
+
+			DescribeTable("should not return warning for enableCommonBootImageImport if not change", func(newFG, oldFG *bool) {
+				cli := getFakeClient(hco)
+				wh := NewWebhookHandler(logger, cli, decoder, HcoValidNamespace, true, nil)
+				newHCO := hco.DeepCopy()
+				hco.Spec.FeatureGates.EnableCommonBootImageImport = newFG
+				newHCO.Spec.FeatureGates.EnableCommonBootImageImport = oldFG
+
+				Expect(wh.ValidateUpdate(ctx, dryRun, newHCO, hco)).To(Succeed())
+			},
+				Entry("should not trigger warning if enableCommonBootImageImport (true) disappeared", ptr.To(true), nil),
+				Entry("should not trigger warning if enableCommonBootImageImport (false) disappeared", ptr.To(false), nil),
+				Entry("should not trigger warning if enableCommonBootImageImport (true) wasn't changed", ptr.To(true), ptr.To(true)),
+				Entry("should not trigger warning if enableCommonBootImageImport (false) wasn't changed", ptr.To(false), ptr.To(false)),
 			)
 		})
+
 	})
 
 	Context("validate delete validation webhook", func() {