Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow Reconciler to change resource origin #50851

Merged
merged 1 commit into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading