diff --git a/staging/operator-lifecycle-manager/pkg/lib/event/event.go b/staging/operator-lifecycle-manager/pkg/lib/event/event.go index 34382d35f8..97e286ac64 100644 --- a/staging/operator-lifecycle-manager/pkg/lib/event/event.go +++ b/staging/operator-lifecycle-manager/pkg/lib/event/event.go @@ -9,6 +9,9 @@ import ( "k8s.io/client-go/tools/record" "k8s.io/klog" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned/scheme" ) @@ -22,24 +25,94 @@ func init() { } } +// safeSpamKeyFunc builds a spam key from event fields with nil checks to prevent panics. +// This protects against nil pointer dereferences when event.InvolvedObject fields are empty. +func safeSpamKeyFunc(event *v1.Event) string { + if event == nil { + return "unknown/unknown/unknown/unknown" + } + + kind := event.InvolvedObject.Kind + namespace := event.InvolvedObject.Namespace + name := event.InvolvedObject.Name + reason := event.Reason + + // Provide defaults for empty fields to avoid issues + if kind == "" { + kind = "Unknown" + } + if name == "" { + name = "unknown" + } + + return fmt.Sprintf("%s/%s/%s/%s", kind, namespace, name, reason) +} + +// SafeEventRecorder wraps record.EventRecorder with nil checks to prevent panics +// when recording events for objects with nil or invalid metadata. +type SafeEventRecorder struct { + recorder record.EventRecorder +} + +// isValidObject checks if the object has valid metadata required for event recording. +func isValidObject(object runtime.Object) bool { + if object == nil { + return false + } + + // Check if object implements metav1.Object interface + accessor, ok := object.(metav1.Object) + if !ok { + return false + } + + // Ensure the object has a valid name (required for event recording) + if accessor.GetName() == "" { + return false + } + + return true +} + +// Event records an event for the given object, with nil checks. +func (s *SafeEventRecorder) Event(object runtime.Object, eventtype, reason, message string) { + if !isValidObject(object) { + klog.V(4).Infof("Skipping event recording: invalid object (nil or missing name), reason=%s, message=%s", reason, message) + return + } + s.recorder.Event(object, eventtype, reason, message) +} + +// Eventf records a formatted event for the given object, with nil checks. +func (s *SafeEventRecorder) Eventf(object runtime.Object, eventtype, reason, messageFmt string, args ...interface{}) { + if !isValidObject(object) { + klog.V(4).Infof("Skipping event recording: invalid object (nil or missing name), reason=%s, messageFmt=%s", reason, messageFmt) + return + } + s.recorder.Eventf(object, eventtype, reason, messageFmt, args...) +} + +// AnnotatedEventf records a formatted event with annotations for the given object, with nil checks. +func (s *SafeEventRecorder) AnnotatedEventf(object runtime.Object, annotations map[string]string, eventtype, reason, messageFmt string, args ...interface{}) { + if !isValidObject(object) { + klog.V(4).Infof("Skipping event recording: invalid object (nil or missing name), reason=%s, messageFmt=%s", reason, messageFmt) + return + } + s.recorder.AnnotatedEventf(object, annotations, eventtype, reason, messageFmt, args...) +} + // NewRecorder returns an EventRecorder type that can be // used to post Events to different object's lifecycles. +// The returned recorder includes nil checks to prevent panics from invalid objects. func NewRecorder(event typedcorev1.EventInterface) (record.EventRecorder, error) { eventBroadcaster := record.NewBroadcasterWithCorrelatorOptions(record.CorrelatorOptions{ - BurstSize: 10, - SpamKeyFunc: func(event *v1.Event) string { - return fmt.Sprintf( - "%s/%s/%s/%s", - event.InvolvedObject.Kind, - event.InvolvedObject.Namespace, - event.InvolvedObject.Name, - event.Reason, - ) - }, + BurstSize: 10, + SpamKeyFunc: safeSpamKeyFunc, }) eventBroadcaster.StartLogging(klog.Infof) eventBroadcaster.StartRecordingToSink(&typedcorev1.EventSinkImpl{Interface: event}) recorder := eventBroadcaster.NewRecorder(s, v1.EventSource{Component: component}) - return recorder, nil + // Wrap the recorder with SafeEventRecorder for nil protection + return &SafeEventRecorder{recorder: recorder}, nil } diff --git a/staging/operator-lifecycle-manager/pkg/lib/event/event_test.go b/staging/operator-lifecycle-manager/pkg/lib/event/event_test.go new file mode 100644 index 0000000000..4fed2f9825 --- /dev/null +++ b/staging/operator-lifecycle-manager/pkg/lib/event/event_test.go @@ -0,0 +1,275 @@ +package event + +import ( + "testing" + + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/record" +) + +func TestSafeSpamKeyFunc(t *testing.T) { + tests := []struct { + name string + event *v1.Event + expected string + }{ + { + name: "nil event", + event: nil, + expected: "unknown/unknown/unknown/unknown", + }, + { + name: "empty event", + event: &v1.Event{ + InvolvedObject: v1.ObjectReference{}, + }, + expected: "Unknown//unknown/", + }, + { + name: "valid event", + event: &v1.Event{ + InvolvedObject: v1.ObjectReference{ + Kind: "Pod", + Namespace: "default", + Name: "test-pod", + }, + Reason: "Created", + }, + expected: "Pod/default/test-pod/Created", + }, + { + name: "event with empty kind", + event: &v1.Event{ + InvolvedObject: v1.ObjectReference{ + Namespace: "default", + Name: "test-pod", + }, + Reason: "Created", + }, + expected: "Unknown/default/test-pod/Created", + }, + { + name: "event with empty name", + event: &v1.Event{ + InvolvedObject: v1.ObjectReference{ + Kind: "Pod", + Namespace: "default", + }, + Reason: "Created", + }, + expected: "Pod/default/unknown/Created", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := safeSpamKeyFunc(tt.event) + if result != tt.expected { + t.Errorf("safeSpamKeyFunc() = %q, expected %q", result, tt.expected) + } + }) + } +} + +func TestIsValidObject(t *testing.T) { + tests := []struct { + name string + object runtime.Object + expected bool + }{ + { + name: "nil object", + object: nil, + expected: false, + }, + { + name: "valid pod", + object: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "default", + }, + }, + expected: true, + }, + { + name: "pod with empty name", + object: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + }, + }, + expected: false, + }, + { + name: "valid namespace", + object: &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ns", + }, + }, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isValidObject(tt.object) + if result != tt.expected { + t.Errorf("isValidObject() = %v, expected %v", result, tt.expected) + } + }) + } +} + +// mockEventRecorder is a mock implementation of record.EventRecorder for testing +type mockEventRecorder struct { + events []string +} + +func (m *mockEventRecorder) Event(object runtime.Object, eventtype, reason, message string) { + m.events = append(m.events, reason+":"+message) +} + +func (m *mockEventRecorder) Eventf(object runtime.Object, eventtype, reason, messageFmt string, args ...interface{}) { + m.events = append(m.events, reason+":"+messageFmt) +} + +func (m *mockEventRecorder) AnnotatedEventf(object runtime.Object, annotations map[string]string, eventtype, reason, messageFmt string, args ...interface{}) { + m.events = append(m.events, reason+":"+messageFmt) +} + +// Ensure mockEventRecorder implements record.EventRecorder +var _ record.EventRecorder = &mockEventRecorder{} + +func TestSafeEventRecorder_Event(t *testing.T) { + tests := []struct { + name string + object runtime.Object + expectRecorded bool + }{ + { + name: "nil object - should not record", + object: nil, + expectRecorded: false, + }, + { + name: "valid object - should record", + object: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "default", + }, + }, + expectRecorded: true, + }, + { + name: "object with empty name - should not record", + object: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + }, + }, + expectRecorded: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mock := &mockEventRecorder{} + safe := &SafeEventRecorder{recorder: mock} + + safe.Event(tt.object, v1.EventTypeNormal, "TestReason", "Test message") + + if tt.expectRecorded && len(mock.events) != 1 { + t.Errorf("Expected event to be recorded, but got %d events", len(mock.events)) + } + if !tt.expectRecorded && len(mock.events) != 0 { + t.Errorf("Expected no events to be recorded, but got %d events", len(mock.events)) + } + }) + } +} + +func TestSafeEventRecorder_Eventf(t *testing.T) { + tests := []struct { + name string + object runtime.Object + expectRecorded bool + }{ + { + name: "nil object - should not record", + object: nil, + expectRecorded: false, + }, + { + name: "valid object - should record", + object: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "default", + }, + }, + expectRecorded: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mock := &mockEventRecorder{} + safe := &SafeEventRecorder{recorder: mock} + + safe.Eventf(tt.object, v1.EventTypeNormal, "TestReason", "Test message %s", "arg") + + if tt.expectRecorded && len(mock.events) != 1 { + t.Errorf("Expected event to be recorded, but got %d events", len(mock.events)) + } + if !tt.expectRecorded && len(mock.events) != 0 { + t.Errorf("Expected no events to be recorded, but got %d events", len(mock.events)) + } + }) + } +} + +func TestSafeEventRecorder_AnnotatedEventf(t *testing.T) { + tests := []struct { + name string + object runtime.Object + expectRecorded bool + }{ + { + name: "nil object - should not record", + object: nil, + expectRecorded: false, + }, + { + name: "valid object - should record", + object: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "default", + }, + }, + expectRecorded: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mock := &mockEventRecorder{} + safe := &SafeEventRecorder{recorder: mock} + + annotations := map[string]string{"key": "value"} + safe.AnnotatedEventf(tt.object, annotations, v1.EventTypeNormal, "TestReason", "Test message %s", "arg") + + if tt.expectRecorded && len(mock.events) != 1 { + t.Errorf("Expected event to be recorded, but got %d events", len(mock.events)) + } + if !tt.expectRecorded && len(mock.events) != 0 { + t.Errorf("Expected no events to be recorded, but got %d events", len(mock.events)) + } + }) + } +} diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/lib/event/event.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/lib/event/event.go index 34382d35f8..97e286ac64 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/lib/event/event.go +++ b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/lib/event/event.go @@ -9,6 +9,9 @@ import ( "k8s.io/client-go/tools/record" "k8s.io/klog" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned/scheme" ) @@ -22,24 +25,94 @@ func init() { } } +// safeSpamKeyFunc builds a spam key from event fields with nil checks to prevent panics. +// This protects against nil pointer dereferences when event.InvolvedObject fields are empty. +func safeSpamKeyFunc(event *v1.Event) string { + if event == nil { + return "unknown/unknown/unknown/unknown" + } + + kind := event.InvolvedObject.Kind + namespace := event.InvolvedObject.Namespace + name := event.InvolvedObject.Name + reason := event.Reason + + // Provide defaults for empty fields to avoid issues + if kind == "" { + kind = "Unknown" + } + if name == "" { + name = "unknown" + } + + return fmt.Sprintf("%s/%s/%s/%s", kind, namespace, name, reason) +} + +// SafeEventRecorder wraps record.EventRecorder with nil checks to prevent panics +// when recording events for objects with nil or invalid metadata. +type SafeEventRecorder struct { + recorder record.EventRecorder +} + +// isValidObject checks if the object has valid metadata required for event recording. +func isValidObject(object runtime.Object) bool { + if object == nil { + return false + } + + // Check if object implements metav1.Object interface + accessor, ok := object.(metav1.Object) + if !ok { + return false + } + + // Ensure the object has a valid name (required for event recording) + if accessor.GetName() == "" { + return false + } + + return true +} + +// Event records an event for the given object, with nil checks. +func (s *SafeEventRecorder) Event(object runtime.Object, eventtype, reason, message string) { + if !isValidObject(object) { + klog.V(4).Infof("Skipping event recording: invalid object (nil or missing name), reason=%s, message=%s", reason, message) + return + } + s.recorder.Event(object, eventtype, reason, message) +} + +// Eventf records a formatted event for the given object, with nil checks. +func (s *SafeEventRecorder) Eventf(object runtime.Object, eventtype, reason, messageFmt string, args ...interface{}) { + if !isValidObject(object) { + klog.V(4).Infof("Skipping event recording: invalid object (nil or missing name), reason=%s, messageFmt=%s", reason, messageFmt) + return + } + s.recorder.Eventf(object, eventtype, reason, messageFmt, args...) +} + +// AnnotatedEventf records a formatted event with annotations for the given object, with nil checks. +func (s *SafeEventRecorder) AnnotatedEventf(object runtime.Object, annotations map[string]string, eventtype, reason, messageFmt string, args ...interface{}) { + if !isValidObject(object) { + klog.V(4).Infof("Skipping event recording: invalid object (nil or missing name), reason=%s, messageFmt=%s", reason, messageFmt) + return + } + s.recorder.AnnotatedEventf(object, annotations, eventtype, reason, messageFmt, args...) +} + // NewRecorder returns an EventRecorder type that can be // used to post Events to different object's lifecycles. +// The returned recorder includes nil checks to prevent panics from invalid objects. func NewRecorder(event typedcorev1.EventInterface) (record.EventRecorder, error) { eventBroadcaster := record.NewBroadcasterWithCorrelatorOptions(record.CorrelatorOptions{ - BurstSize: 10, - SpamKeyFunc: func(event *v1.Event) string { - return fmt.Sprintf( - "%s/%s/%s/%s", - event.InvolvedObject.Kind, - event.InvolvedObject.Namespace, - event.InvolvedObject.Name, - event.Reason, - ) - }, + BurstSize: 10, + SpamKeyFunc: safeSpamKeyFunc, }) eventBroadcaster.StartLogging(klog.Infof) eventBroadcaster.StartRecordingToSink(&typedcorev1.EventSinkImpl{Interface: event}) recorder := eventBroadcaster.NewRecorder(s, v1.EventSource{Component: component}) - return recorder, nil + // Wrap the recorder with SafeEventRecorder for nil protection + return &SafeEventRecorder{recorder: recorder}, nil }