Skip to content

Commit 6cf2e16

Browse files
committed
Embed ssa.FilterObjectInput into HelperOption
HelperOption defines the same fields as ssa.FilterObjectInput, which is redundant. This commit removes its duplication and embed ssa.FilterObjectInput into HelperOption instead.
1 parent 6955116 commit 6cf2e16

File tree

4 files changed

+17
-27
lines changed

4 files changed

+17
-27
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ func dryRunSSAPatch(ctx context.Context, dryRunCtx *dryRunSSAPatchInput) (bool,
6868
// For dry run we use the same options as for the intent but with adding metadata.managedFields
6969
// to ensure that changes to ownership are detected.
7070
filterObjectInput := &ssa.FilterObjectInput{
71-
AllowedPaths: append(dryRunCtx.helperOptions.allowedPaths, []string{"metadata", "managedFields"}),
72-
IgnorePaths: dryRunCtx.helperOptions.ignorePaths,
71+
AllowedPaths: append(dryRunCtx.helperOptions.AllowedPaths, []string{"metadata", "managedFields"}),
72+
IgnorePaths: dryRunCtx.helperOptions.IgnorePaths,
7373
}
7474

7575
// Add TopologyDryRunAnnotation to notify validation webhooks to skip immutability checks.

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

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121

2222
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
2323
"sigs.k8s.io/cluster-api/internal/contract"
24+
"sigs.k8s.io/cluster-api/internal/util/ssa"
2425
)
2526

2627
var (
@@ -70,29 +71,21 @@ type HelperOption interface {
7071

7172
// HelperOptions contains options for Helper.
7273
type HelperOptions struct {
73-
// internally managed options.
74-
75-
// allowedPaths instruct the Helper to ignore everything except given paths when computing a patch.
76-
allowedPaths []contract.Path
77-
78-
// user defined options.
79-
80-
// IgnorePaths instruct the Helper to ignore given paths when computing a patch.
81-
// NOTE: ignorePaths are used to filter out fields nested inside allowedPaths, e.g.
82-
// spec.ControlPlaneEndpoint.
83-
// NOTE: ignore paths which point to an array are not supported by the current implementation.
84-
ignorePaths []contract.Path
74+
ssa.FilterObjectInput
8575
}
8676

8777
// newHelperOptions returns initialized HelperOptions.
8878
func newHelperOptions(target client.Object, opts ...HelperOption) *HelperOptions {
8979
helperOptions := &HelperOptions{
90-
allowedPaths: defaultAllowedPaths,
80+
FilterObjectInput: ssa.FilterObjectInput{
81+
AllowedPaths: defaultAllowedPaths,
82+
IgnorePaths: []contract.Path{},
83+
},
9184
}
9285
// Overwrite the allowedPaths for Cluster objects to prevent the topology controller
9386
// to take ownership of fields it is not supposed to.
9487
if _, ok := target.(*clusterv1.Cluster); ok {
95-
helperOptions.allowedPaths = allowedPathsCluster
88+
helperOptions.AllowedPaths = allowedPathsCluster
9689
}
9790
helperOptions = helperOptions.ApplyOptions(opts)
9891
return helperOptions
@@ -114,5 +107,5 @@ type IgnorePaths []contract.Path
114107

115108
// ApplyToHelper applies this configuration to the given helper options.
116109
func (i IgnorePaths) ApplyToHelper(opts *HelperOptions) {
117-
opts.ignorePaths = i
110+
opts.IgnorePaths = i
118111
}

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,7 @@ func NewServerSidePatchHelper(ctx context.Context, original, modified client.Obj
7171

7272
// Filter the modifiedUnstructured object to only contain changes intendet to be done.
7373
// The originalUnstructured object will be filtered in dryRunSSAPatch using other options.
74-
ssa.FilterObject(modifiedUnstructured, &ssa.FilterObjectInput{
75-
AllowedPaths: helperOptions.allowedPaths,
76-
IgnorePaths: helperOptions.ignorePaths,
77-
})
74+
ssa.FilterObject(modifiedUnstructured, &helperOptions.FilterObjectInput)
7875

7976
// Carry over uid to match the intent to:
8077
// * create (uid==""):

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ type TwoWaysPatchHelper struct {
6767
func NewTwoWaysPatchHelper(original, modified client.Object, c client.Client, opts ...HelperOption) (*TwoWaysPatchHelper, error) {
6868
helperOptions := &HelperOptions{}
6969
helperOptions = helperOptions.ApplyOptions(opts)
70-
helperOptions.allowedPaths = []contract.Path{
70+
helperOptions.AllowedPaths = []contract.Path{
7171
{"metadata", "labels"},
7272
{"metadata", "annotations"},
7373
{"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
7676
// metadata.name, metadata.namespace (who are required by the API server) and metadata.ownerReferences
7777
// that gets set to avoid orphaned objects.
7878
if util.IsNil(original) {
79-
helperOptions.allowedPaths = append(helperOptions.allowedPaths,
79+
helperOptions.AllowedPaths = append(helperOptions.AllowedPaths,
8080
contract.Path{"apiVersion"},
8181
contract.Path{"kind"},
8282
contract.Path{"metadata", "name"},
@@ -166,24 +166,24 @@ func applyOptions(in *applyOptionsInput) ([]byte, error) {
166166

167167
// drop changes for exclude paths (fields to not consider, e.g. status);
168168
// Note: for everything not allowed it sets modified equal to original, so the generated patch doesn't include this change
169-
if len(in.options.allowedPaths) > 0 {
169+
if len(in.options.AllowedPaths) > 0 {
170170
dropDiff(&dropDiffInput{
171171
path: contract.Path{},
172172
original: originalMap,
173173
modified: modifiedMap,
174-
shouldDropDiffFunc: ssa.IsPathNotAllowed(in.options.allowedPaths),
174+
shouldDropDiffFunc: ssa.IsPathNotAllowed(in.options.AllowedPaths),
175175
})
176176
}
177177

178178
// drop changes for ignore paths (well known fields owned by something else, e.g.
179179
// spec.controlPlaneEndpoint in the InfrastructureCluster object);
180180
// Note: for everything ignored it sets modified equal to original, so the generated patch doesn't include this change
181-
if len(in.options.ignorePaths) > 0 {
181+
if len(in.options.IgnorePaths) > 0 {
182182
dropDiff(&dropDiffInput{
183183
path: contract.Path{},
184184
original: originalMap,
185185
modified: modifiedMap,
186-
shouldDropDiffFunc: ssa.IsPathIgnored(in.options.ignorePaths),
186+
shouldDropDiffFunc: ssa.IsPathIgnored(in.options.IgnorePaths),
187187
})
188188
}
189189

0 commit comments

Comments
 (0)