🌱 Support patch ForceOverwriteConditions option

Signed-off-by: Vince Prignano <vincepri@vmware.com>

util/conditions/patch.go

@@ -82,6 +82,7 @@ func NewPatch(before Getter, after Getter) Patch {
 // applyOptions allows to set strategies for patch apply.
 type applyOptions struct {
 	ownedConditions []clusterv1.ConditionType
+	forceOverwrite  bool
 }
 
 func (o *applyOptions) isOwnedCondition(t clusterv1.ConditionType) bool {
@@ -104,6 +105,13 @@ func WithOwnedConditions(t ...clusterv1.ConditionType) ApplyOption {
 	}
 }
 
+// WithForceOverwrite In case of conflicts for the owned conditions, the patch helper will always use the value provided by the controller.
+func WithForceOverwrite(v bool) ApplyOption {
+	return func(c *applyOptions) {
+		c.forceOverwrite = v
+	}
+}
+
 // Apply executes a three-way merge of a list of Patch.
 // When merge conflicts are detected (latest deviated from before in an incompatible way), an error is returned.
 func (p Patch) Apply(latest Setter, options ...ApplyOption) error {
@@ -120,7 +128,7 @@ func (p Patch) Apply(latest Setter, options ...ApplyOption) error {
 		switch conditionPatch.Op {
 		case AddConditionPatch:
 			// If the conditions is owned, always keep the after value.
-			if applyOpt.isOwnedCondition(conditionPatch.After.Type) {
+			if applyOpt.forceOverwrite || applyOpt.isOwnedCondition(conditionPatch.After.Type) {
 				Set(latest, conditionPatch.After)
 				continue
 			}
@@ -140,7 +148,7 @@ func (p Patch) Apply(latest Setter, options ...ApplyOption) error {
 
 		case ChangeConditionPatch:
 			// If the conditions is owned, always keep the after value.
-			if applyOpt.isOwnedCondition(conditionPatch.After.Type) {
+			if applyOpt.forceOverwrite || applyOpt.isOwnedCondition(conditionPatch.After.Type) {
 				Set(latest, conditionPatch.After)
 				continue
 			}
@@ -167,7 +175,7 @@ func (p Patch) Apply(latest Setter, options ...ApplyOption) error {
 
 		case RemoveConditionPatch:
 			// If the conditions is owned, always keep the after value (condition should be deleted).
-			if applyOpt.isOwnedCondition(conditionPatch.Before.Type) {
+			if applyOpt.forceOverwrite || applyOpt.isOwnedCondition(conditionPatch.Before.Type) {
 				Delete(latest, conditionPatch.Before.Type)
 				continue
 			}

util/patch/options.go

@@ -30,11 +30,26 @@ type HelperOptions struct {
 	// on the incoming object to match metadata.generation, only if there is a change.
 	IncludeStatusObservedGeneration bool
 
+	// ForceOverwriteConditions allows the patch helper to overwrite conditions in case of conflicts.
+	// This option should only ever be set in controller managing the object being patched.
+	ForceOverwriteConditions bool
+
 	// OwnedConditions defines condition types owned by the controller.
 	// In case of conflicts for the owned conditions, the patch helper will always use the value provided by the controller.
+	//
+	// DEPRECATED: Use ForceOverwriteConditions.
 	OwnedConditions []clusterv1.ConditionType
 }
 
+// WithForceOverwriteConditions allows the patch helper to overwrite conditions in case of conflicts.
+// This option should only ever be set in controller managing the object being patched.
+type WithForceOverwriteConditions struct{}
+
+// ApplyToHelper applies this configuration to the given HelperOptions.
+func (w WithForceOverwriteConditions) ApplyToHelper(in *HelperOptions) {
+	in.ForceOverwriteConditions = true
+}
+
 // WithStatusObservedGeneration sets the status.observedGeneration field
 // on the incoming object to match metadata.generation, only if there is a change.
 type WithStatusObservedGeneration struct{}
@@ -46,6 +61,8 @@ func (w WithStatusObservedGeneration) ApplyToHelper(in *HelperOptions) {
 
 // WithOwnedConditions allows to define condition types owned by the controller.
 // In case of conflicts for the owned conditions, the patch helper will always use the value provided by the controller.
+//
+// DEPRECATED: Use WithForceOverwriteConditions.
 type WithOwnedConditions struct {
 	Conditions []clusterv1.ConditionType
 }

util/patch/patch.go

@@ -114,7 +114,7 @@ func (h *Helper) Patch(ctx context.Context, obj runtime.Object, opts ...Option)
 	return kerrors.NewAggregate([]error{
 		h.patch(ctx, obj),
 		h.patchStatus(ctx, obj),
-		h.patchStatusConditions(ctx, obj, options.OwnedConditions),
+		h.patchStatusConditions(ctx, obj, options.ForceOverwriteConditions, options.OwnedConditions),
 	})
 }
 
@@ -151,7 +151,7 @@ func (h *Helper) patchStatus(ctx context.Context, obj runtime.Object) error {
 //
 // Condition changes are then applied to the latest version of the object, and if there are
 // no unresolvable conflicts, the patch is sent again.
-func (h *Helper) patchStatusConditions(ctx context.Context, obj runtime.Object, ownedConditions []clusterv1.ConditionType) error {
+func (h *Helper) patchStatusConditions(ctx context.Context, obj runtime.Object, forceOverwrite bool, ownedConditions []clusterv1.ConditionType) error {
 	// Nothing to do if the object isn't a condition patcher.
 	if !h.isConditionsSetter {
 		return nil
@@ -211,7 +211,7 @@ func (h *Helper) patchStatusConditions(ctx context.Context, obj runtime.Object,
 		conditionsPatch := client.MergeFromWithOptions(latest.DeepCopyObject(), client.MergeFromWithOptimisticLock{})
 
 		// Set the condition patch previously created on the new object.
-		if err := diff.Apply(latest, conditions.WithOwnedConditions(ownedConditions...)); err != nil {
+		if err := diff.Apply(latest, conditions.WithForceOverwrite(forceOverwrite), conditions.WithOwnedConditions(ownedConditions...)); err != nil {
 			return false, err
 		}
 

util/patch/patch_test.go

@@ -321,7 +321,7 @@ var _ = Describe("Patch Helper", func() {
 				}, timeout).Should(BeTrue())
 			})
 
-			Specify("should return not an error if there is an unresolvable conflict but the conditions is owned by the controller", func() {
+			Specify("should not return an error if there is an unresolvable conflict but the conditions is owned by the controller", func() {
 				obj := obj.DeepCopy()
 
 				By("Creating the object")
@@ -363,6 +363,48 @@ var _ = Describe("Patch Helper", func() {
 				}, timeout).Should(BeTrue())
 			})
 
+			Specify("should not return an error if there is an unresolvable conflict when force overwrite is enabled", func() {
+				obj := obj.DeepCopy()
+
+				By("Creating the object")
+				Expect(testEnv.Create(ctx, obj)).ToNot(HaveOccurred())
+				key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace}
+				defer func() {
+					Expect(testEnv.Delete(ctx, obj)).To(Succeed())
+				}()
+				objCopy := obj.DeepCopy()
+
+				By("Marking a custom condition to be false")
+				conditions.MarkFalse(objCopy, clusterv1.ReadyCondition, "reason", clusterv1.ConditionSeverityInfo, "message")
+				Expect(testEnv.Status().Update(ctx, objCopy)).To(Succeed())
+
+				By("Validating that the local object's resource version is behind")
+				Expect(obj.ResourceVersion).ToNot(Equal(objCopy.ResourceVersion))
+
+				By("Creating a new patch helper")
+				patcher, err := NewHelper(obj, testEnv)
+				Expect(err).NotTo(HaveOccurred())
+
+				By("Marking Ready=True")
+				conditions.MarkTrue(obj, clusterv1.ReadyCondition)
+
+				By("Patching the object")
+				Expect(patcher.Patch(ctx, obj, WithForceOverwriteConditions{})).To(Succeed())
+
+				By("Validating the object has been updated")
+				Eventually(func() bool {
+					objAfter := obj.DeepCopy()
+					if err := testEnv.Get(ctx, key, objAfter); err != nil {
+						return false
+					}
+
+					readyBefore := conditions.Get(obj, clusterv1.ReadyCondition)
+					readyAfter := conditions.Get(objAfter, clusterv1.ReadyCondition)
+
+					return cmp.Equal(readyBefore, readyAfter)
+				}, timeout).Should(BeTrue())
+			})
+
 		})
 	})