From f95d5340e39c5f4af3b020e10d5c459938cc84ea Mon Sep 17 00:00:00 2001 From: anjalii-28 Date: Tue, 16 Dec 2025 19:36:25 +0530 Subject: [PATCH 1/3] Fix panic on DeletedFinalStateUnknown in ConfigMap informer --- configmap/informer/informed_watcher.go | 16 ++- configmap/informer/informed_watcher_test.go | 137 ++++++++++++++++++++ 2 files changed, 152 insertions(+), 1 deletion(-) diff --git a/configmap/informer/informed_watcher.go b/configmap/informer/informed_watcher.go index 1ae7ef8d40..c9f33bf34d 100644 --- a/configmap/informer/informed_watcher.go +++ b/configmap/informer/informed_watcher.go @@ -231,7 +231,21 @@ func (i *InformedWatcher) updateConfigMapEvent(o, n interface{}) { } func (i *InformedWatcher) deleteConfigMapEvent(obj interface{}) { - configMap := obj.(*corev1.ConfigMap) + // Handle DeletedFinalStateUnknown which can occur when the final state + // of the deleted object is not known. + tombstone, ok := obj.(cache.DeletedFinalStateUnknown) + if ok { + obj = tombstone.Obj + } + + // Safely extract the ConfigMap from the object. + configMap, ok := obj.(*corev1.ConfigMap) + if !ok { + // If the object is not a ConfigMap, gracefully return. + // This can happen if the tombstone contains an invalid object. + return + } + if def, ok := i.defaults[configMap.Name]; ok { i.OnChange(def) } diff --git a/configmap/informer/informed_watcher_test.go b/configmap/informer/informed_watcher_test.go index 5a812bb69c..9f02c22bf0 100644 --- a/configmap/informer/informed_watcher_test.go +++ b/configmap/informer/informed_watcher_test.go @@ -28,6 +28,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/selection" fakekubeclientset "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/tools/cache" ) type counter struct { @@ -487,3 +488,139 @@ func TestWatchWithDefaultAfterStart(t *testing.T) { t.Fatalf("foo1.count = %v, want %d", got, want) } } + +func TestDeleteConfigMapEventWithDeletedFinalStateUnknown(t *testing.T) { + defaultFooCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "foo", + }, + Data: map[string]string{ + "default": "from code", + }, + } + fooCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "foo", + }, + Data: map[string]string{ + "from": "k8s", + }, + } + + kc := fakekubeclientset.NewSimpleClientset(fooCM) + cmw := NewInformedWatcher(kc, "default") + + foo1 := &counter{name: "foo1"} + cmw.WatchWithDefault(*defaultFooCM, foo1.callback) + + stopCh := make(chan struct{}) + defer close(stopCh) + if err := cmw.Start(stopCh); err != nil { + t.Fatal("cm.Start() =", err) + } + + // Test that deleteConfigMapEvent does NOT panic when given cache.DeletedFinalStateUnknown with a ConfigMap + // This simulates the scenario where the informer receives a DeletedFinalStateUnknown tombstone. + tombstone := cache.DeletedFinalStateUnknown{ + Key: "default/foo", + Obj: fooCM, + } + + // This should not panic and should trigger the default ConfigMap + cmw.deleteConfigMapEvent(tombstone) + + // We expect: + // 1. The default to be seen once during startup. + // 2. The real K8s version during the initial pass. + // 3. The default again, when the real K8s version is deleted via tombstone. + expected := []*corev1.ConfigMap{defaultFooCM, fooCM, defaultFooCM} + if got, want := foo1.count(), len(expected); got != want { + t.Fatalf("foo1.count = %v, want %d", got, want) + } + for i, cfg := range expected { + if got, want := foo1.cfg[i].Data, cfg.Data; !equality.Semantic.DeepEqual(want, got) { + t.Errorf("%d config seen should have been '%v', actually '%v'", i, want, got) + } + } +} + +func TestDeleteConfigMapEventWithDeletedFinalStateUnknownInvalidObject(t *testing.T) { + defaultFooCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "foo", + }, + Data: map[string]string{ + "default": "from code", + }, + } + + kc := fakekubeclientset.NewSimpleClientset() + cmw := NewInformedWatcher(kc, "default") + + foo1 := &counter{name: "foo1"} + cmw.WatchWithDefault(*defaultFooCM, foo1.callback) + + stopCh := make(chan struct{}) + defer close(stopCh) + if err := cmw.Start(stopCh); err != nil { + t.Fatal("cm.Start() =", err) + } + + initialCount := foo1.count() + + // Test that deleteConfigMapEvent does NOT panic when given cache.DeletedFinalStateUnknown with an invalid object + // This simulates the scenario where the tombstone contains a non-ConfigMap object. + tombstone := cache.DeletedFinalStateUnknown{ + Key: "default/foo", + Obj: "not-a-configmap", // Invalid object type + } + + // This should not panic and should gracefully return without changing the count + cmw.deleteConfigMapEvent(tombstone) + + // The count should remain unchanged since the object is invalid + if got, want := foo1.count(), initialCount; got != want { + t.Fatalf("foo1.count = %v, want %d (should not change)", got, want) + } +} + +func TestDeleteConfigMapEventWithInvalidObject(t *testing.T) { + defaultFooCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "foo", + }, + Data: map[string]string{ + "default": "from code", + }, + } + + kc := fakekubeclientset.NewSimpleClientset() + cmw := NewInformedWatcher(kc, "default") + + foo1 := &counter{name: "foo1"} + cmw.WatchWithDefault(*defaultFooCM, foo1.callback) + + stopCh := make(chan struct{}) + defer close(stopCh) + if err := cmw.Start(stopCh); err != nil { + t.Fatal("cm.Start() =", err) + } + + initialCount := foo1.count() + + // Test that deleteConfigMapEvent does NOT panic when given a completely invalid object + // This simulates the scenario where the informer receives an unexpected object type. + invalidObj := "not-a-configmap" + + // This should not panic and should gracefully return without changing the count + cmw.deleteConfigMapEvent(invalidObj) + + // The count should remain unchanged since the object is invalid + if got, want := foo1.count(), initialCount; got != want { + t.Fatalf("foo1.count = %v, want %d (should not change)", got, want) + } +} From 33ffc8e86ef31a034fb27ca2b8e7b39a59a04669 Mon Sep 17 00:00:00 2001 From: anjalii-28 Date: Wed, 17 Dec 2025 13:29:55 +0530 Subject: [PATCH 2/3] fixing easyCLA --- configmap/informer/informed_watcher_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/configmap/informer/informed_watcher_test.go b/configmap/informer/informed_watcher_test.go index 9f02c22bf0..1fcff1a194 100644 --- a/configmap/informer/informed_watcher_test.go +++ b/configmap/informer/informed_watcher_test.go @@ -489,6 +489,7 @@ func TestWatchWithDefaultAfterStart(t *testing.T) { } } + func TestDeleteConfigMapEventWithDeletedFinalStateUnknown(t *testing.T) { defaultFooCM := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ From e0d3659b43ae031f7316462e5c6dc71905a5e479 Mon Sep 17 00:00:00 2001 From: anjalii-28 Date: Wed, 17 Dec 2025 13:49:41 +0530 Subject: [PATCH 3/3] addressing review comment --- configmap/informer/informed_watcher_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/configmap/informer/informed_watcher_test.go b/configmap/informer/informed_watcher_test.go index 1fcff1a194..9f02c22bf0 100644 --- a/configmap/informer/informed_watcher_test.go +++ b/configmap/informer/informed_watcher_test.go @@ -489,7 +489,6 @@ func TestWatchWithDefaultAfterStart(t *testing.T) { } } - func TestDeleteConfigMapEventWithDeletedFinalStateUnknown(t *testing.T) { defaultFooCM := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{