Skip to content

Commit

Permalink
Allow Reconciler to change resource origin (#50896)
Browse files Browse the repository at this point in the history
By default, the resource reconciler disalows changing a resource origin
in order to enforce the segregation of resources created from different
sources.

This patch introduces an option to allow the reconciler to change a resource's
origin, bypassing the origin change check if enabled.

This is part of addressing #50654

Co-authored-by: Marek Smoliński <[email protected]>
  • Loading branch information
tcsc and smallinsky authored Jan 10, 2025
1 parent 457dc0a commit 209ea72
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 15 deletions.
32 changes: 20 additions & 12 deletions lib/services/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ type GenericReconcilerConfig[K comparable, T any] struct {
OnDelete func(context.Context, T) error
// Logger emits log messages.
Logger *slog.Logger
// AllowOriginChanges is a flag that allows the reconciler to change the
// origin value of a reconciled resource. By default, origin changes are
// disallowed to enforce segregation between of resources from different
// sources.
AllowOriginChanges bool
}

// CheckAndSetDefaults validates the reconciler configuration and sets defaults.
Expand Down Expand Up @@ -177,18 +182,21 @@ func (r *GenericReconciler[K, T]) processNewResource(ctx context.Context, curren
return nil
}

// Don't overwrite resource of a different origin (e.g., keep static resource from config and ignore dynamic resource)
registeredOrigin, err := types.GetOrigin(registered)
if err != nil {
return trace.Wrap(err)
}
newOrigin, err := types.GetOrigin(newT)
if err != nil {
return trace.Wrap(err)
}
if registeredOrigin != newOrigin {
r.logger.WarnContext(ctx, "New resource has different origin, not updating", "name", key, "new_origin", newOrigin, "existing_origin", registeredOrigin)
return nil
if !r.cfg.AllowOriginChanges {
// Don't overwrite resource of a different origin (e.g., keep static resource from config and ignore dynamic resource)
registeredOrigin, err := types.GetOrigin(registered)
if err != nil {
return trace.Wrap(err)
}
newOrigin, err := types.GetOrigin(newT)
if err != nil {
return trace.Wrap(err)
}
if registeredOrigin != newOrigin {
r.logger.WarnContext(ctx, "New resource has different origin, not updating",
"name", key, "new_origin", newOrigin, "existing_origin", registeredOrigin)
return nil
}
}

// If the resource is already registered but was updated, see if its
Expand Down
30 changes: 27 additions & 3 deletions lib/services/reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ func TestReconciler(t *testing.T) {
onCreateCalls []testResource
onUpdateCalls []updateCall
onDeleteCalls []testResource
configure func(cfg *ReconcilerConfig[testResource])
comparator func(testResource, testResource) int
}{
{
Expand Down Expand Up @@ -73,13 +74,30 @@ func TestReconciler(t *testing.T) {
},
},
{
description: "resources with different origins don't overwrite each other",
description: "resources with different origins don't overwrite each other by default",
selectors: []ResourceMatcher{{
Labels: types.Labels{"*": []string{"*"}},
}},
registeredResources: []testResource{makeStaticResource("res1", nil)},
newResources: []testResource{makeDynamicResource("res1", nil)},
},
{
description: "resources with different origins overwrite each other when allowed",
selectors: []ResourceMatcher{{
Labels: types.Labels{"*": []string{"*"}},
}},
configure: func(cfg *ReconcilerConfig[testResource]) {
cfg.AllowOriginChanges = true
},
registeredResources: []testResource{makeStaticResource("res1", nil)},
newResources: []testResource{makeDynamicResource("res1", nil)},
onUpdateCalls: []updateCall{
{
old: makeStaticResource("res1", nil),
new: makeDynamicResource("res1", nil),
},
},
},
{
description: "resource that's no longer present should be removed",
selectors: []ResourceMatcher{{
Expand Down Expand Up @@ -198,7 +216,7 @@ func TestReconciler(t *testing.T) {
var onCreateCalls, onDeleteCalls []testResource
var onUpdateCalls []updateCall

reconciler, err := NewReconciler[testResource](ReconcilerConfig[testResource]{
cfg := ReconcilerConfig[testResource]{
Matcher: func(tr testResource) bool {
return MatchResourceLabels(test.selectors, tr.GetMetadata().Labels)
},
Expand All @@ -225,7 +243,13 @@ func TestReconciler(t *testing.T) {
onDeleteCalls = append(onDeleteCalls, tr)
return nil
},
})
}

if test.configure != nil {
test.configure(&cfg)
}

reconciler, err := NewReconciler[testResource](cfg)
require.NoError(t, err)

// Reconcile and make sure we got all expected callback calls.
Expand Down

0 comments on commit 209ea72

Please sign in to comment.