Skip to content

Commit

Permalink
Prevent panic in proxy diffing
Browse files Browse the repository at this point in the history
#47561 incorrectly translated the logic on when to call the diff
function for resources. The previous logic only called the differ
_if_ the resource had already been seen before, which guaranteed
that both the old and new resource provided to the diff function
were not nil. However, after the conversion to the generic watcher
the diff function was called for new resources, resulting in a nil
value being provided for the old resource and caused a panic.
The previous behavior has been restored, and the ProxyWatcher test
has been updated to set a diff function to prevent regressions.
  • Loading branch information
rosstimothy committed Oct 31, 2024
1 parent b998b92 commit bacbfff
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 2 deletions.
4 changes: 2 additions & 2 deletions lib/services/watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -839,9 +839,9 @@ func (g *genericCollector[T, R]) processEventsAndUpdateCurrent(ctx context.Conte
}

key := g.ResourceKey(resource)
current := g.current[key]
current, exists := g.current[key]
g.current[key] = resource
updated = g.ResourceDiffer(current, resource)
updated = !exists || g.ResourceDiffer(current, resource)
default:
g.Logger.WarnContext(ctx, "Skipping unsupported event type", "event_type", event.Type)
}
Expand Down
3 changes: 3 additions & 0 deletions lib/services/watcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,9 @@ func TestProxyWatcher(t *testing.T) {
},
ProxyGetter: presence,
ProxiesC: make(chan []types.Server, 10),
ProxyDiffer: func(old, new types.Server) bool {
return old.GetPeerAddr() != new.GetPeerAddr()
},
})
require.NoError(t, err)
t.Cleanup(w.Close)
Expand Down

0 comments on commit bacbfff

Please sign in to comment.