Skip to content

Commit f40bcd6

Browse files
jianzhangbjzjianzhangbjz
committed
Fix nil pointer dereference panic in event recorder
1 parent e3391c6 commit f40bcd6

File tree

2 files changed

+358
-11
lines changed

2 files changed

+358
-11
lines changed

‎pkg/lib/event/event.go‎

Lines changed: 83 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"fmt"
55

66
v1 "k8s.io/api/core/v1"
7+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
8+
"k8s.io/apimachinery/pkg/runtime"
79
kscheme "k8s.io/client-go/kubernetes/scheme"
810
typedcorev1 "k8s.io/client-go/kubernetes/typed/core/v1"
911
"k8s.io/client-go/tools/record"
@@ -22,24 +24,94 @@ func init() {
2224
}
2325
}
2426

27+
// safeSpamKeyFunc builds a spam key from event fields with nil checks to prevent panics.
28+
// This protects against nil pointer dereferences when event.InvolvedObject fields are empty.
29+
func safeSpamKeyFunc(event *v1.Event) string {
30+
if event == nil {
31+
return "unknown/unknown/unknown/unknown"
32+
}
33+
34+
kind := event.InvolvedObject.Kind
35+
namespace := event.InvolvedObject.Namespace
36+
name := event.InvolvedObject.Name
37+
reason := event.Reason
38+
39+
// Provide defaults for empty fields to avoid issues
40+
if kind == "" {
41+
kind = "Unknown"
42+
}
43+
if name == "" {
44+
name = "unknown"
45+
}
46+
47+
return fmt.Sprintf("%s/%s/%s/%s", kind, namespace, name, reason)
48+
}
49+
50+
// SafeEventRecorder wraps record.EventRecorder with nil checks to prevent panics
51+
// when recording events for objects with nil or invalid metadata.
52+
type SafeEventRecorder struct {
53+
recorder record.EventRecorder
54+
}
55+
56+
// isValidObject checks if the object has valid metadata required for event recording.
57+
func isValidObject(object runtime.Object) bool {
58+
if object == nil {
59+
return false
60+
}
61+
62+
// Check if object implements metav1.Object interface
63+
accessor, ok := object.(metav1.Object)
64+
if !ok {
65+
return false
66+
}
67+
68+
// Ensure the object has a valid name (required for event recording)
69+
if accessor.GetName() == "" {
70+
return false
71+
}
72+
73+
return true
74+
}
75+
76+
// Event records an event for the given object, with nil checks.
77+
func (s *SafeEventRecorder) Event(object runtime.Object, eventtype, reason, message string) {
78+
if !isValidObject(object) {
79+
klog.V(4).Infof("Skipping event recording: invalid object (nil or missing name), reason=%s, message=%s", reason, message)
80+
return
81+
}
82+
s.recorder.Event(object, eventtype, reason, message)
83+
}
84+
85+
// Eventf records a formatted event for the given object, with nil checks.
86+
func (s *SafeEventRecorder) Eventf(object runtime.Object, eventtype, reason, messageFmt string, args ...interface{}) {
87+
if !isValidObject(object) {
88+
klog.V(4).Infof("Skipping event recording: invalid object (nil or missing name), reason=%s, messageFmt=%s", reason, messageFmt)
89+
return
90+
}
91+
s.recorder.Eventf(object, eventtype, reason, messageFmt, args...)
92+
}
93+
94+
// AnnotatedEventf records a formatted event with annotations for the given object, with nil checks.
95+
func (s *SafeEventRecorder) AnnotatedEventf(object runtime.Object, annotations map[string]string, eventtype, reason, messageFmt string, args ...interface{}) {
96+
if !isValidObject(object) {
97+
klog.V(4).Infof("Skipping event recording: invalid object (nil or missing name), reason=%s, messageFmt=%s", reason, messageFmt)
98+
return
99+
}
100+
s.recorder.AnnotatedEventf(object, annotations, eventtype, reason, messageFmt, args...)
101+
}
102+
25103
// NewRecorder returns an EventRecorder type that can be
26104
// used to post Events to different object's lifecycles.
105+
// The returned recorder includes nil checks to prevent panics from invalid objects.
27106
func NewRecorder(event typedcorev1.EventInterface) (record.EventRecorder, error) {
28107
eventBroadcaster := record.NewBroadcasterWithCorrelatorOptions(record.CorrelatorOptions{
29-
BurstSize: 10,
30-
SpamKeyFunc: func(event *v1.Event) string {
31-
return fmt.Sprintf(
32-
"%s/%s/%s/%s",
33-
event.InvolvedObject.Kind,
34-
event.InvolvedObject.Namespace,
35-
event.InvolvedObject.Name,
36-
event.Reason,
37-
)
38-
},
108+
BurstSize: 10,
109+
SpamKeyFunc: safeSpamKeyFunc,
39110
})
40111
eventBroadcaster.StartLogging(klog.Infof)
41112
eventBroadcaster.StartRecordingToSink(&typedcorev1.EventSinkImpl{Interface: event})
42113
recorder := eventBroadcaster.NewRecorder(s, v1.EventSource{Component: component})
43114

44-
return recorder, nil
115+
// Wrap the recorder with SafeEventRecorder for nil protection
116+
return &SafeEventRecorder{recorder: recorder}, nil
45117
}

‎pkg/lib/event/event_test.go‎

Lines changed: 275 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,275 @@
1+
package event
2+
3+
import (
4+
"testing"
5+
6+
v1 "k8s.io/api/core/v1"
7+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
8+
"k8s.io/apimachinery/pkg/runtime"
9+
"k8s.io/client-go/tools/record"
10+
)
11+
12+
func TestSafeSpamKeyFunc(t *testing.T) {
13+
tests := []struct {
14+
name string
15+
event *v1.Event
16+
expected string
17+
}{
18+
{
19+
name: "nil event",
20+
event: nil,
21+
expected: "unknown/unknown/unknown/unknown",
22+
},
23+
{
24+
name: "empty event",
25+
event: &v1.Event{
26+
InvolvedObject: v1.ObjectReference{},
27+
},
28+
expected: "Unknown//unknown/",
29+
},
30+
{
31+
name: "valid event",
32+
event: &v1.Event{
33+
InvolvedObject: v1.ObjectReference{
34+
Kind: "Pod",
35+
Namespace: "default",
36+
Name: "test-pod",
37+
},
38+
Reason: "Created",
39+
},
40+
expected: "Pod/default/test-pod/Created",
41+
},
42+
{
43+
name: "event with empty kind",
44+
event: &v1.Event{
45+
InvolvedObject: v1.ObjectReference{
46+
Namespace: "default",
47+
Name: "test-pod",
48+
},
49+
Reason: "Created",
50+
},
51+
expected: "Unknown/default/test-pod/Created",
52+
},
53+
{
54+
name: "event with empty name",
55+
event: &v1.Event{
56+
InvolvedObject: v1.ObjectReference{
57+
Kind: "Pod",
58+
Namespace: "default",
59+
},
60+
Reason: "Created",
61+
},
62+
expected: "Pod/default/unknown/Created",
63+
},
64+
}
65+
66+
for _, tt := range tests {
67+
t.Run(tt.name, func(t *testing.T) {
68+
result := safeSpamKeyFunc(tt.event)
69+
if result != tt.expected {
70+
t.Errorf("safeSpamKeyFunc() = %q, expected %q", result, tt.expected)
71+
}
72+
})
73+
}
74+
}
75+
76+
func TestIsValidObject(t *testing.T) {
77+
tests := []struct {
78+
name string
79+
object runtime.Object
80+
expected bool
81+
}{
82+
{
83+
name: "nil object",
84+
object: nil,
85+
expected: false,
86+
},
87+
{
88+
name: "valid pod",
89+
object: &v1.Pod{
90+
ObjectMeta: metav1.ObjectMeta{
91+
Name: "test-pod",
92+
Namespace: "default",
93+
},
94+
},
95+
expected: true,
96+
},
97+
{
98+
name: "pod with empty name",
99+
object: &v1.Pod{
100+
ObjectMeta: metav1.ObjectMeta{
101+
Namespace: "default",
102+
},
103+
},
104+
expected: false,
105+
},
106+
{
107+
name: "valid namespace",
108+
object: &v1.Namespace{
109+
ObjectMeta: metav1.ObjectMeta{
110+
Name: "test-ns",
111+
},
112+
},
113+
expected: true,
114+
},
115+
}
116+
117+
for _, tt := range tests {
118+
t.Run(tt.name, func(t *testing.T) {
119+
result := isValidObject(tt.object)
120+
if result != tt.expected {
121+
t.Errorf("isValidObject() = %v, expected %v", result, tt.expected)
122+
}
123+
})
124+
}
125+
}
126+
127+
// mockEventRecorder is a mock implementation of record.EventRecorder for testing
128+
type mockEventRecorder struct {
129+
events []string
130+
}
131+
132+
func (m *mockEventRecorder) Event(object runtime.Object, eventtype, reason, message string) {
133+
m.events = append(m.events, reason+":"+message)
134+
}
135+
136+
func (m *mockEventRecorder) Eventf(object runtime.Object, eventtype, reason, messageFmt string, args ...interface{}) {
137+
m.events = append(m.events, reason+":"+messageFmt)
138+
}
139+
140+
func (m *mockEventRecorder) AnnotatedEventf(object runtime.Object, annotations map[string]string, eventtype, reason, messageFmt string, args ...interface{}) {
141+
m.events = append(m.events, reason+":"+messageFmt)
142+
}
143+
144+
// Ensure mockEventRecorder implements record.EventRecorder
145+
var _ record.EventRecorder = &mockEventRecorder{}
146+
147+
func TestSafeEventRecorder_Event(t *testing.T) {
148+
tests := []struct {
149+
name string
150+
object runtime.Object
151+
expectRecorded bool
152+
}{
153+
{
154+
name: "nil object - should not record",
155+
object: nil,
156+
expectRecorded: false,
157+
},
158+
{
159+
name: "valid object - should record",
160+
object: &v1.Pod{
161+
ObjectMeta: metav1.ObjectMeta{
162+
Name: "test-pod",
163+
Namespace: "default",
164+
},
165+
},
166+
expectRecorded: true,
167+
},
168+
{
169+
name: "object with empty name - should not record",
170+
object: &v1.Pod{
171+
ObjectMeta: metav1.ObjectMeta{
172+
Namespace: "default",
173+
},
174+
},
175+
expectRecorded: false,
176+
},
177+
}
178+
179+
for _, tt := range tests {
180+
t.Run(tt.name, func(t *testing.T) {
181+
mock := &mockEventRecorder{}
182+
safe := &SafeEventRecorder{recorder: mock}
183+
184+
safe.Event(tt.object, v1.EventTypeNormal, "TestReason", "Test message")
185+
186+
if tt.expectRecorded && len(mock.events) != 1 {
187+
t.Errorf("Expected event to be recorded, but got %d events", len(mock.events))
188+
}
189+
if !tt.expectRecorded && len(mock.events) != 0 {
190+
t.Errorf("Expected no events to be recorded, but got %d events", len(mock.events))
191+
}
192+
})
193+
}
194+
}
195+
196+
func TestSafeEventRecorder_Eventf(t *testing.T) {
197+
tests := []struct {
198+
name string
199+
object runtime.Object
200+
expectRecorded bool
201+
}{
202+
{
203+
name: "nil object - should not record",
204+
object: nil,
205+
expectRecorded: false,
206+
},
207+
{
208+
name: "valid object - should record",
209+
object: &v1.Pod{
210+
ObjectMeta: metav1.ObjectMeta{
211+
Name: "test-pod",
212+
Namespace: "default",
213+
},
214+
},
215+
expectRecorded: true,
216+
},
217+
}
218+
219+
for _, tt := range tests {
220+
t.Run(tt.name, func(t *testing.T) {
221+
mock := &mockEventRecorder{}
222+
safe := &SafeEventRecorder{recorder: mock}
223+
224+
safe.Eventf(tt.object, v1.EventTypeNormal, "TestReason", "Test message %s", "arg")
225+
226+
if tt.expectRecorded && len(mock.events) != 1 {
227+
t.Errorf("Expected event to be recorded, but got %d events", len(mock.events))
228+
}
229+
if !tt.expectRecorded && len(mock.events) != 0 {
230+
t.Errorf("Expected no events to be recorded, but got %d events", len(mock.events))
231+
}
232+
})
233+
}
234+
}
235+
236+
func TestSafeEventRecorder_AnnotatedEventf(t *testing.T) {
237+
tests := []struct {
238+
name string
239+
object runtime.Object
240+
expectRecorded bool
241+
}{
242+
{
243+
name: "nil object - should not record",
244+
object: nil,
245+
expectRecorded: false,
246+
},
247+
{
248+
name: "valid object - should record",
249+
object: &v1.Pod{
250+
ObjectMeta: metav1.ObjectMeta{
251+
Name: "test-pod",
252+
Namespace: "default",
253+
},
254+
},
255+
expectRecorded: true,
256+
},
257+
}
258+
259+
for _, tt := range tests {
260+
t.Run(tt.name, func(t *testing.T) {
261+
mock := &mockEventRecorder{}
262+
safe := &SafeEventRecorder{recorder: mock}
263+
264+
annotations := map[string]string{"key": "value"}
265+
safe.AnnotatedEventf(tt.object, annotations, v1.EventTypeNormal, "TestReason", "Test message %s", "arg")
266+
267+
if tt.expectRecorded && len(mock.events) != 1 {
268+
t.Errorf("Expected event to be recorded, but got %d events", len(mock.events))
269+
}
270+
if !tt.expectRecorded && len(mock.events) != 0 {
271+
t.Errorf("Expected no events to be recorded, but got %d events", len(mock.events))
272+
}
273+
})
274+
}
275+
}

0 commit comments

Comments
 (0)