From c36f33d4ca13a3a65c53af6b3fefd3d8792332b8 Mon Sep 17 00:00:00 2001 From: Thomas Hallgren Date: Fri, 28 Apr 2023 13:15:47 +0200 Subject: [PATCH] Fix unchecked cast panic in traffic-manager pod watcher. 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 --- CHANGELOG.md | 8 ++++++++ .../manager/internal/cluster/nodewatcher.go | 18 +++++++++++++++--- .../cmd/manager/internal/cluster/podwatcher.go | 18 +++++++++++++++--- 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 035f7750a7..8fbaa0a16e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/cmd/traffic/cmd/manager/internal/cluster/nodewatcher.go b/cmd/traffic/cmd/manager/internal/cluster/nodewatcher.go index ff62f81389..68b43f4e40 100644 --- a/cmd/traffic/cmd/manager/internal/cluster/nodewatcher.go +++ b/cmd/traffic/cmd/manager/internal/cluster/nodewatcher.go @@ -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 { diff --git a/cmd/traffic/cmd/manager/internal/cluster/podwatcher.go b/cmd/traffic/cmd/manager/internal/cluster/podwatcher.go index 78b3677718..4a04e474ef 100644 --- a/cmd/traffic/cmd/manager/internal/cluster/podwatcher.go +++ b/cmd/traffic/cmd/manager/internal/cluster/podwatcher.go @@ -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 {