filter all fields not managedy by topology controller for cluster object

This prevents the topology controller of taking field ownership and fixes potential race conditions
when topology controller would patch fields of the object using user input while the user changes them
in his own.

Author: chrischdi

internal/controllers/topology/cluster/reconcile_state.go

@@ -439,9 +439,7 @@ func (r *Reconciler) reconcileCluster(ctx context.Context, s *scope.Scope) error
 	ctx, log := tlog.LoggerFrom(ctx).WithObject(s.Desired.Cluster).Into(ctx)
 
 	// Check differences between current and desired state, and eventually patch the current object.
-	patchHelper, err := r.patchHelperFactory(s.Current.Cluster, s.Desired.Cluster, structuredmerge.IgnorePaths{
-		{"spec", "controlPlaneEndpoint"}, // this is a well known field that is managed by the Cluster controller, topology should not express opinions on it.
-	})
+	patchHelper, err := r.patchHelperFactory(s.Current.Cluster, s.Desired.Cluster)
 	if err != nil {
 		return errors.Wrapf(err, "failed to create patch helper for %s", tlog.KObj{Obj: s.Current.Cluster})
 	}

internal/controllers/topology/cluster/structuredmerge/options.go

@@ -17,9 +17,47 @@ limitations under the License.
 package structuredmerge
 
 import (
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/cluster-api/internal/contract"
 )
 
+var (
+	// defaultAllowedPaths are the allowed paths for all objects except Clusters.
+	defaultAllowedPaths = []contract.Path{
+		// apiVersion, kind, name and namespace are required field for a server side apply intent.
+		{"apiVersion"},
+		{"kind"},
+		{"metadata", "name"},
+		{"metadata", "namespace"},
+		// the topology controller controls/has an opinion for labels, annotation, ownerReferences and spec only.
+		{"metadata", "labels"},
+		{"metadata", "annotations"},
+		{"metadata", "ownerReferences"},
+		{"spec"},
+	}
+
+	// allowedPathsCluster are the allowed paths specific for Clusters.
+	// The cluster object is not created by the topology controller and already contains fields which are
+	// not supposed for the topology controller to have an opinion on / take (co-)ownership of.
+	// Because of that the allowedPaths are different to other objects.
+	// NOTE: This is mostly the same as defaultAllowedPaths but having more restrictions.
+	allowedPathsCluster = []contract.Path{
+		// apiVersion, kind, name and namespace are required field for a server side apply intent.
+		{"apiVersion"},
+		{"kind"},
+		{"metadata", "name"},
+		{"metadata", "namespace"},
+		// the topology controller controls/has an opinion for the labels ClusterLabelName
+		// and ClusterTopologyOwnedLabel as well as infrastructureRef and controlPlaneRef in spec.
+		{"metadata", "labels", clusterv1.ClusterLabelName},
+		{"metadata", "labels", clusterv1.ClusterTopologyOwnedLabel},
+		{"spec", "infrastructureRef"},
+		{"spec", "controlPlaneRef"},
+	}
+)
+
 // HelperOption is some configuration that modifies options for Helper.
 type HelperOption interface {
 	// ApplyToHelper applies this configuration to the given helper options.
@@ -42,6 +80,20 @@ type HelperOptions struct {
 	ignorePaths []contract.Path
 }
 
+// newHelperOptions returns initialized HelperOptions.
+func newHelperOptions(target client.Object, opts ...HelperOption) *HelperOptions {
+	helperOptions := &HelperOptions{
+		allowedPaths: defaultAllowedPaths,
+	}
+	// Overwrite the allowedPaths for Cluster objects to prevent the topology controller
+	// to take ownership of fields it is not supposed to.
+	if _, ok := target.(*clusterv1.Cluster); ok {
+		helperOptions.allowedPaths = allowedPathsCluster
+	}
+	helperOptions = helperOptions.ApplyOptions(opts)
+	return helperOptions
+}
+
 // ApplyOptions applies the given patch options on these options,
 // and then returns itself (for convenient chaining).
 func (o *HelperOptions) ApplyOptions(opts []HelperOption) *HelperOptions {

internal/controllers/topology/cluster/structuredmerge/serversidepathhelper.go

@@ -42,20 +42,8 @@ type serverSidePatchHelper struct {
 
 // NewServerSidePatchHelper returns a new PatchHelper using server side apply.
 func NewServerSidePatchHelper(original, modified client.Object, c client.Client, opts ...HelperOption) (PatchHelper, error) {
-	helperOptions := &HelperOptions{}
-	helperOptions = helperOptions.ApplyOptions(opts)
-	helperOptions.allowedPaths = []contract.Path{
-		// apiVersion, kind, name and namespace are required field for a server side apply intent.
-		{"apiVersion"},
-		{"kind"},
-		{"metadata", "name"},
-		{"metadata", "namespace"},
-		// the topology controller controls/has an opinion for labels, annotation, ownerReferences and spec only.
-		{"metadata", "labels"},
-		{"metadata", "annotations"},
-		{"metadata", "ownerReferences"},
-		{"spec"},
-	}
+	// Create helperOptions for filtering the original and modified objects to the desired intent.
+	helperOptions := newHelperOptions(modified, opts...)
 
 	// If required, convert the original and modified objects to unstructured and filter out all the information
 	// not relevant for the topology controller.