Skip to content
Open
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
16 changes: 15 additions & 1 deletion configmap/informer/informed_watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
137 changes: 137 additions & 0 deletions configmap/informer/informed_watcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -487,3 +488,139 @@ func TestWatchWithDefaultAfterStart(t *testing.T) {
t.Fatalf("foo1.count = %v, want %d", got, want)
}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

I guess this was in 33ffc8e to trigger the easyCLA check.
For the future: IIRC this can also be triggered through some command like /easycla

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@creydr removed this extra line, but somehow easyCLA check is not passing.
Can you help ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In #3301 (comment) it says:

So you need to make sure your first commit (f95d534) comes from an email, linked to your GitHub user. Check for example on the linked help articles how to do it

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)
}
}
Loading