Add patch helper to runtime

This commit adds a `patch` helper to the `runtime` package, making it
easier to e.g. securely patch status conditions while taking condition
type ownership into account. This ensures that if properly setup, merge
conflicts can be solved by the controller without it accidentally
overwriting state.

The work has been derived from
https://github.com/kubernetes-sigs/cluster-api/tree/d2faf482116114c4075da1390d905742e524ff89/util/patch,
but adapted to work with our `conditions` package, and
`metav1.Condition` types.

End-to-end / Ginkgo tests have not been included, as there is no proper
framework for these types of tests yet.

Signed-off-by: Hidde Beydals <hello@hidde.co>

runtime/patch/options.go

@@ -0,0 +1,68 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+Copyright 2021 The Flux authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package patch
+
+// Option is some configuration that modifies options for a patch request.
+type Option interface {
+	// ApplyToHelper applies this configuration to the given Helper options.
+	ApplyToHelper(*HelperOptions)
+}
+
+// HelperOptions contains options for patch options.
+type HelperOptions struct {
+	// IncludeStatusObservedGeneration sets the status.observedGeneration field
+	// 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.
+	OwnedConditions []string
+}
+
+// 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{}
+
+// ApplyToHelper applies this configuration to the given HelperOptions.
+func (w WithStatusObservedGeneration) ApplyToHelper(in *HelperOptions) {
+	in.IncludeStatusObservedGeneration = true
+}
+
+// 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.
+type WithOwnedConditions struct {
+	Conditions []string
+}
+
+// ApplyToHelper applies this configuration to the given HelperOptions.
+func (w WithOwnedConditions) ApplyToHelper(in *HelperOptions) {
+	in.OwnedConditions = w.Conditions
+}

runtime/patch/patch.go

@@ -0,0 +1,289 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+Copyright 2021 The Flux authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package patch
+
+import (
+	"context"
+	"encoding/json"
+	"time"
+
+	"github.com/pkg/errors"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	kerrors "k8s.io/apimachinery/pkg/util/errors"
+	"k8s.io/apimachinery/pkg/util/wait"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
+
+	"github.com/fluxcd/pkg/runtime/conditions"
+)
+
+// Helper is a utility for ensuring the proper patching of objects.
+type Helper struct {
+	client       client.Client
+	gvk          schema.GroupVersionKind
+	beforeObject client.Object
+	before       *unstructured.Unstructured
+	after        *unstructured.Unstructured
+	changes      map[string]bool
+
+	isConditionsSetter bool
+}
+
+// NewHelper returns an initialized Helper.
+func NewHelper(obj client.Object, crClient client.Client) (*Helper, error) {
+	// Get the GroupVersionKind of the object,
+	// used to validate against later on.
+	gvk, err := apiutil.GVKForObject(obj, crClient.Scheme())
+	if err != nil {
+		return nil, err
+	}
+
+	// Convert the object to unstructured to compare against our before copy.
+	unstructuredObj, err := toUnstructured(obj)
+	if err != nil {
+		return nil, err
+	}
+
+	// Check if the object satisfies the GitOps Toolkit API conditions contract.
+	_, canInterfaceConditions := obj.(conditions.Setter)
+
+	return &Helper{
+		client:             crClient,
+		gvk:                gvk,
+		before:             unstructuredObj,
+		beforeObject:       obj.DeepCopyObject().(client.Object),
+		isConditionsSetter: canInterfaceConditions,
+	}, nil
+}
+
+// Patch will attempt to patch the given object, including its status.
+func (h *Helper) Patch(ctx context.Context, obj client.Object, opts ...Option) error {
+	// Get the GroupVersionKind of the object that we want to patch.
+	gvk, err := apiutil.GVKForObject(obj, h.client.Scheme())
+	if err != nil {
+		return err
+	}
+	if gvk != h.gvk {
+		return errors.Errorf("unmatched GroupVersionKind, expected %q got %q", h.gvk, gvk)
+	}
+
+	// Calculate the options.
+	options := &HelperOptions{}
+	for _, opt := range opts {
+		opt.ApplyToHelper(options)
+	}
+
+	// Convert the object to unstructured to compare against our before copy.
+	h.after, err = toUnstructured(obj)
+	if err != nil {
+		return err
+	}
+
+	// Determine if the object has status.
+	if unstructuredHasStatus(h.after) {
+		if options.IncludeStatusObservedGeneration {
+			// Set status.observedGeneration if we're asked to do so.
+			if err := unstructured.SetNestedField(h.after.Object, h.after.GetGeneration(), "status", "observedGeneration"); err != nil {
+				return err
+			}
+
+			// Restore the changes back to the original object.
+			if err := runtime.DefaultUnstructuredConverter.FromUnstructured(h.after.Object, obj); err != nil {
+				return err
+			}
+		}
+	}
+
+	// Calculate and store the top-level field changes (e.g. "metadata", "spec", "status") we have before/after.
+	h.changes, err = h.calculateChanges(obj)
+	if err != nil {
+		return err
+	}
+
+	// Issue patches and return errors in an aggregate.
+	return kerrors.NewAggregate([]error{
+		// Patch the conditions first.
+		//
+		// Given that we pass in metadata.resourceVersion to perform a 3-way-merge conflict resolution,
+		// patching conditions first avoids an extra loop if spec or status patch succeeds first
+		// given that causes the resourceVersion to mutate.
+		h.patchStatusConditions(ctx, obj, options.ForceOverwriteConditions, options.OwnedConditions),
+
+		// Then proceed to patch the rest of the object.
+		h.patch(ctx, obj),
+		h.patchStatus(ctx, obj),
+	})
+}
+
+// patch issues a patch for metadata and spec.
+func (h *Helper) patch(ctx context.Context, obj client.Object) error {
+	if !h.shouldPatch("metadata") && !h.shouldPatch("spec") {
+		return nil
+	}
+	beforeObject, afterObject, err := h.calculatePatch(obj, specPatch)
+	if err != nil {
+		return err
+	}
+	return h.client.Patch(ctx, afterObject, client.MergeFrom(beforeObject))
+}
+
+// patchStatus issues a patch if the status has changed.
+func (h *Helper) patchStatus(ctx context.Context, obj client.Object) error {
+	if !h.shouldPatch("status") {
+		return nil
+	}
+	beforeObject, afterObject, err := h.calculatePatch(obj, statusPatch)
+	if err != nil {
+		return err
+	}
+	return h.client.Status().Patch(ctx, afterObject, client.MergeFrom(beforeObject))
+}
+
+// patchStatusConditions issues a patch if there are any changes to the conditions slice under
+// the status subresource. This is a special case and it's handled separately given that
+// we allow different controllers to act on conditions of the same object.
+//
+// This method has an internal backoff loop. When a conflict is detected, the method
+// asks the Client for the a new version of the object we're trying to patch.
+//
+// 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 client.Object, forceOverwrite bool, ownedConditions []string) error {
+	// Nothing to do if the object isn't a condition patcher.
+	if !h.isConditionsSetter {
+		return nil
+	}
+
+	// Make sure our before/after objects satisfy the proper interface before continuing.
+	//
+	// NOTE: The checks and error below are done so that we don't panic if any of the objects don't satisfy the
+	// interface any longer, although this shouldn't happen because we already check when creating the patcher.
+	before, ok := h.beforeObject.(conditions.Getter)
+	if !ok {
+		return errors.Errorf("object %s doesn't satisfy conditions.Getter, cannot patch", before.GetObjectKind())
+	}
+	after, ok := obj.(conditions.Getter)
+	if !ok {
+		return errors.Errorf("object %s doesn't satisfy conditions.Getter, cannot patch", after.GetObjectKind())
+	}
+
+	// Store the diff from the before/after object, and return early if there are no changes.
+	diff := conditions.NewPatch(
+		before,
+		after,
+	)
+	if diff.IsZero() {
+		return nil
+	}
+
+	// Make a copy of the object and store the key used if we have conflicts.
+	key := client.ObjectKeyFromObject(after)
+
+	// Define and start a backoff loop to handle conflicts
+	// between controllers working on the same object.
+	//
+	// This has been copied from https://github.com/kubernetes/kubernetes/blob/release-1.16/pkg/controller/controller_utils.go#L86-L88.
+	backoff := wait.Backoff{
+		Steps:    5,
+		Duration: 100 * time.Millisecond,
+		Jitter:   1.0,
+	}
+
+	// Start the backoff loop and return errors if any.
+	return wait.ExponentialBackoff(backoff, func() (bool, error) {
+		latest, ok := before.DeepCopyObject().(conditions.Setter)
+		if !ok {
+			return false, errors.Errorf("object %s doesn't satisfy conditions.Setter, cannot patch", latest.GetObjectKind())
+		}
+
+		// Get a new copy of the object.
+		if err := h.client.Get(ctx, key, latest); err != nil {
+			return false, err
+		}
+
+		// Create the condition patch before merging conditions.
+		conditionsPatch := client.MergeFromWithOptions(latest.DeepCopyObject().(conditions.Setter), client.MergeFromWithOptimisticLock{})
+
+		// Set the condition patch previously created on the new object.
+		if err := diff.Apply(latest, conditions.WithForceOverwrite(forceOverwrite), conditions.WithOwnedConditions(ownedConditions...)); err != nil {
+			return false, err
+		}
+
+		// Issue the patch.
+		err := h.client.Status().Patch(ctx, latest, conditionsPatch)
+		switch {
+		case apierrors.IsConflict(err):
+			// Requeue.
+			return false, nil
+		case err != nil:
+			return false, err
+		default:
+			return true, nil
+		}
+	})
+}
+
+// calculatePatch returns the before/after objects to be given in a controller-runtime patch, scoped down to the absolute necessary.
+func (h *Helper) calculatePatch(afterObj client.Object, focus patchType) (client.Object, client.Object, error) {
+	// Get a shallow unsafe copy of the before/after object in unstructured form.
+	before := unsafeUnstructuredCopy(h.before, focus, h.isConditionsSetter)
+	after := unsafeUnstructuredCopy(h.after, focus, h.isConditionsSetter)
+
+	// We've now applied all modifications to local unstructured objects,
+	// make copies of the original objects and convert them back.
+	beforeObj := h.beforeObject.DeepCopyObject().(client.Object)
+	if err := runtime.DefaultUnstructuredConverter.FromUnstructured(before.Object, beforeObj); err != nil {
+		return nil, nil, err
+	}
+	afterObj = afterObj.DeepCopyObject().(client.Object)
+	if err := runtime.DefaultUnstructuredConverter.FromUnstructured(after.Object, afterObj); err != nil {
+		return nil, nil, err
+	}
+	return beforeObj, afterObj, nil
+}
+
+func (h *Helper) shouldPatch(in string) bool {
+	return h.changes[in]
+}
+
+// calculate changes tries to build a patch from the before/after objects we have
+// and store in a map which top-level fields (e.g. `metadata`, `spec`, `status`, etc.) have changed.
+func (h *Helper) calculateChanges(after client.Object) (map[string]bool, error) {
+	// Calculate patch data.
+	patch := client.MergeFrom(h.beforeObject)
+	diff, err := patch.Data(after)
+	if err != nil {
+		return nil, errors.Wrapf(err, "failed to calculate patch data")
+	}
+
+	// Unmarshal patch data into a local map.
+	patchDiff := map[string]interface{}{}
+	if err := json.Unmarshal(diff, &patchDiff); err != nil {
+		return nil, errors.Wrapf(err, "failed to unmarshal patch data into a map")
+	}
+
+	// Return the map.
+	res := make(map[string]bool, len(patchDiff))
+	for key := range patchDiff {
+		res[key] = true
+	}
+	return res, nil
+}