Skip to content

Commit 7079e62

Browse files
jianzhangbjzjianzhangbjz
committed
fix OCPBUGS-69688
1 parent 7d2dd62 commit 7079e62

File tree

2 files changed

+359
-11
lines changed

2 files changed

+359
-11
lines changed

‎staging/operator-lifecycle-manager/pkg/lib/event/event.go‎

Lines changed: 84 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ import (
99
"k8s.io/client-go/tools/record"
1010
"k8s.io/klog"
1111

12+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
13+
"k8s.io/apimachinery/pkg/runtime"
14+
1215
"github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned/scheme"
1316
)
1417

@@ -22,24 +25,94 @@ func init() {
2225
}
2326
}
2427

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

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

‎staging/operator-lifecycle-manager/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)