Merge pull request kubernetes-sigs#9512 from musaprg/8395-replace-helperoptions-with-filterobjectinput

🌱 Embed ssa.FilterObjectInput into HelperOption to remove duplication

Author: k8s-ci-robot

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

@@ -68,8 +68,8 @@ func dryRunSSAPatch(ctx context.Context, dryRunCtx *dryRunSSAPatchInput) (bool,
 	// For dry run we use the same options as for the intent but with adding metadata.managedFields
 	// to ensure that changes to ownership are detected.
 	filterObjectInput := &ssa.FilterObjectInput{
-		AllowedPaths: append(dryRunCtx.helperOptions.allowedPaths, []string{"metadata", "managedFields"}),
-		IgnorePaths:  dryRunCtx.helperOptions.ignorePaths,
+		AllowedPaths: append(dryRunCtx.helperOptions.AllowedPaths, []string{"metadata", "managedFields"}),
+		IgnorePaths:  dryRunCtx.helperOptions.IgnorePaths,
 	}
 
 	// Add TopologyDryRunAnnotation to notify validation webhooks to skip immutability checks.

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

@@ -21,6 +21,7 @@ import (
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/cluster-api/internal/contract"
+	"sigs.k8s.io/cluster-api/internal/util/ssa"
 )
 
 var (
@@ -70,29 +71,21 @@ type HelperOption interface {
 
 // HelperOptions contains options for Helper.
 type HelperOptions struct {
-	// internally managed options.
-
-	// allowedPaths instruct the Helper to ignore everything except given paths when computing a patch.
-	allowedPaths []contract.Path
-
-	// user defined options.
-
-	// IgnorePaths instruct the Helper to ignore given paths when computing a patch.
-	// NOTE: ignorePaths are used to filter out fields nested inside allowedPaths, e.g.
-	// spec.ControlPlaneEndpoint.
-	// NOTE: ignore paths which point to an array are not supported by the current implementation.
-	ignorePaths []contract.Path
+	ssa.FilterObjectInput
 }
 
 // newHelperOptions returns initialized HelperOptions.
 func newHelperOptions(target client.Object, opts ...HelperOption) *HelperOptions {
 	helperOptions := &HelperOptions{
-		allowedPaths: defaultAllowedPaths,
+		FilterObjectInput: ssa.FilterObjectInput{
+			AllowedPaths: defaultAllowedPaths,
+			IgnorePaths:  []contract.Path{},
+		},
 	}
 	// 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.AllowedPaths = allowedPathsCluster
 	}
 	helperOptions = helperOptions.ApplyOptions(opts)
 	return helperOptions
@@ -114,5 +107,5 @@ type IgnorePaths []contract.Path
 
 // ApplyToHelper applies this configuration to the given helper options.
 func (i IgnorePaths) ApplyToHelper(opts *HelperOptions) {
-	opts.ignorePaths = i
+	opts.IgnorePaths = i
 }

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

@@ -71,10 +71,7 @@ func NewServerSidePatchHelper(ctx context.Context, original, modified client.Obj
 
 	// Filter the modifiedUnstructured object to only contain changes intendet to be done.
 	// The originalUnstructured object will be filtered in dryRunSSAPatch using other options.
-	ssa.FilterObject(modifiedUnstructured, &ssa.FilterObjectInput{
-		AllowedPaths: helperOptions.allowedPaths,
-		IgnorePaths:  helperOptions.ignorePaths,
-	})
+	ssa.FilterObject(modifiedUnstructured, &helperOptions.FilterObjectInput)
 
 	// Carry over uid to match the intent to:
 	// * create (uid==""):

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

@@ -67,7 +67,7 @@ type TwoWaysPatchHelper struct {
 func NewTwoWaysPatchHelper(original, modified client.Object, c client.Client, opts ...HelperOption) (*TwoWaysPatchHelper, error) {
 	helperOptions := &HelperOptions{}
 	helperOptions = helperOptions.ApplyOptions(opts)
-	helperOptions.allowedPaths = []contract.Path{
+	helperOptions.AllowedPaths = []contract.Path{
 		{"metadata", "labels"},
 		{"metadata", "annotations"},
 		{"spec"}, // NOTE: The handling of managed path requires/assumes spec to be within allowed path.
@@ -76,7 +76,7 @@ func NewTwoWaysPatchHelper(original, modified client.Object, c client.Client, op
 	// metadata.name, metadata.namespace (who are required by the API server) and metadata.ownerReferences
 	// that gets set to avoid orphaned objects.
 	if util.IsNil(original) {
-		helperOptions.allowedPaths = append(helperOptions.allowedPaths,
+		helperOptions.AllowedPaths = append(helperOptions.AllowedPaths,
 			contract.Path{"apiVersion"},
 			contract.Path{"kind"},
 			contract.Path{"metadata", "name"},
@@ -166,24 +166,24 @@ func applyOptions(in *applyOptionsInput) ([]byte, error) {
 
 	// drop changes for exclude paths (fields to not consider, e.g. status);
 	// Note: for everything not allowed it sets modified equal to original, so the generated patch doesn't include this change
-	if len(in.options.allowedPaths) > 0 {
+	if len(in.options.AllowedPaths) > 0 {
 		dropDiff(&dropDiffInput{
 			path:               contract.Path{},
 			original:           originalMap,
 			modified:           modifiedMap,
-			shouldDropDiffFunc: ssa.IsPathNotAllowed(in.options.allowedPaths),
+			shouldDropDiffFunc: ssa.IsPathNotAllowed(in.options.AllowedPaths),
 		})
 	}
 
 	// drop changes for ignore paths (well known fields owned by something else, e.g.
 	//   spec.controlPlaneEndpoint in the InfrastructureCluster object);
 	// Note: for everything ignored it sets  modified equal to original, so the generated patch doesn't include this change
-	if len(in.options.ignorePaths) > 0 {
+	if len(in.options.IgnorePaths) > 0 {
 		dropDiff(&dropDiffInput{
 			path:               contract.Path{},
 			original:           originalMap,
 			modified:           modifiedMap,
-			shouldDropDiffFunc: ssa.IsPathIgnored(in.options.ignorePaths),
+			shouldDropDiffFunc: ssa.IsPathIgnored(in.options.IgnorePaths),
 		})
 	}