Skip to content

Commit

Permalink
Fix unchecked cast panic in traffic-manager pod watcher.
Browse files Browse the repository at this point in the history
A watcher isn't guaranteed to always receive an object of the watched
type. The `DeleteFunc` in particular, can receive an instance of
`cache.DeleteFinalStateUnknown`. This commit ensures that the traffic-
manager's node and pod watcher uses checked casts only and that the
`cache.DeleteFinalStateUnknown` is handled properly.

Closed #3149

Signed-off-by: Thomas Hallgren <[email protected]>
  • Loading branch information
thallgren committed Apr 28, 2023
1 parent 732114c commit c36f33d
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 6 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
# Changelog

### 2.13.2 (TBD)

- Bugfix: The traffic-manager would sometimes panic and exit after some time due to a type cast panic.

### 2.13.1 (April 20, 2023)

- Change: Update ambassador-agent to version 1.13.13

### 2.13.0 (April 18, 2023)

- Feature: The Docker network used by a Kind or Minikube (using the "docker" driver) installation, is automatically
Expand Down
18 changes: 15 additions & 3 deletions cmd/traffic/cmd/manager/internal/cluster/nodewatcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,25 @@ func newNodeWatcher(ctx context.Context, lister licorev1.NodeLister, informer ca
}
_, err := informer.AddEventHandler(cache.ResourceEventHandlerFuncs{
AddFunc: func(obj any) {
w.onNodeAdded(ctx, obj.(*corev1.Node))
if node, ok := obj.(*corev1.Node); ok {
w.onNodeAdded(ctx, node)
}
},
DeleteFunc: func(obj any) {
w.onNodeDeleted(ctx, obj.(*corev1.Node))
if node, ok := obj.(*corev1.Node); ok {
w.onNodeDeleted(ctx, node)
} else if dfsu, ok := obj.(*cache.DeletedFinalStateUnknown); ok {
if node, ok := dfsu.Obj.(*corev1.Node); ok {
w.onNodeDeleted(ctx, node)
}
}
},
UpdateFunc: func(oldObj, newObj any) {
w.onNodeUpdated(ctx, oldObj.(*corev1.Node), newObj.(*corev1.Node))
if oldNode, ok := oldObj.(*corev1.Node); ok {
if newNode, ok := newObj.(*corev1.Node); ok {
w.onNodeUpdated(ctx, oldNode, newNode)
}
}
},
})
if err != nil {
Expand Down
18 changes: 15 additions & 3 deletions cmd/traffic/cmd/manager/internal/cluster/podwatcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,25 @@ func newPodWatcher(ctx context.Context, listers []PodLister, informers []cache.S
for _, informer := range informers {
_, err := informer.AddEventHandler(cache.ResourceEventHandlerFuncs{
AddFunc: func(obj any) {
w.onPodAdded(ctx, obj.(*corev1.Pod))
if pod, ok := obj.(*corev1.Pod); ok {
w.onPodAdded(ctx, pod)
}
},
DeleteFunc: func(obj any) {
w.onPodDeleted(ctx, obj.(*corev1.Pod))
if pod, ok := obj.(*corev1.Pod); ok {
w.onPodDeleted(ctx, pod)
} else if dfsu, ok := obj.(*cache.DeletedFinalStateUnknown); ok {
if pod, ok := dfsu.Obj.(*corev1.Pod); ok {
w.onPodDeleted(ctx, pod)
}
}
},
UpdateFunc: func(oldObj, newObj any) {
w.onPodUpdated(ctx, oldObj.(*corev1.Pod), newObj.(*corev1.Pod))
if oldPod, ok := oldObj.(*corev1.Pod); ok {
if newPod, ok := newObj.(*corev1.Pod); ok {
w.onPodUpdated(ctx, oldPod, newPod)
}
}
},
})
if err != nil {
Expand Down

0 comments on commit c36f33d

Please sign in to comment.