fix: Prevent panic in EventEmitter when logger is nil

* Added nil checks for the logger before use in EmitMessage.
* Prevented a potential panic if the EventEmitter was initialized without a
logger.
* Added a test case to ensure the function does not panic with a nil logger.

Co-authored-by: Claude <[email protected]>
Signed-off-by: Chmouel Boudjnah <[email protected]>

Author: chmouel

pkg/events/emit.go

@@ -34,20 +34,24 @@ func (e *EventEmitter) EmitMessage(repo *v1alpha1.Repository, loggerLevel zapcor
 	if repo != nil {
 		event := makeEvent(repo, loggerLevel, reason, message)
 		if _, err := e.client.CoreV1().Events(event.Namespace).Create(context.Background(), event, metav1.CreateOptions{}); err != nil {
-			e.logger.Infof("Cannot create event: %s", err.Error())
+			if e.logger != nil {
+				e.logger.Infof("Cannot create event: %s", err.Error())
+			}
 		}
 	}
 
-	//nolint
-	switch loggerLevel {
-	case zapcore.DebugLevel:
-		e.logger.Debug(message)
-	case zapcore.ErrorLevel:
-		e.logger.Error(message)
-	case zapcore.InfoLevel:
-		e.logger.Info(message)
-	case zapcore.WarnLevel:
-		e.logger.Warn(message)
+	if e.logger != nil {
+		//nolint
+		switch loggerLevel {
+		case zapcore.DebugLevel:
+			e.logger.Debug(message)
+		case zapcore.ErrorLevel:
+			e.logger.Error(message)
+		case zapcore.InfoLevel:
+			e.logger.Info(message)
+		case zapcore.WarnLevel:
+			e.logger.Warn(message)
+		}
 	}
 }
 

pkg/events/emit_test.go

@@ -20,12 +20,14 @@ func TestEventEmitter_EmitMessage(t *testing.T) {
 	observer, _ := zapobserver.New(zap.InfoLevel)
 	fakelogger := zap.New(observer).Sugar()
 	tests := []struct {
-		name        string
-		repo        *v1alpha1.Repository
-		message     string
-		reason      string
-		logLevel    zapcore.Level
-		expectEvent bool
+		name         string
+		repo         *v1alpha1.Repository
+		message      string
+		reason       string
+		logLevel     zapcore.Level
+		expectEvent  bool
+		expectedType string
+		useNilLogger bool
 	}{
 		{
 			name: "repo exists",
@@ -36,9 +38,10 @@ func TestEventEmitter_EmitMessage(t *testing.T) {
 				},
 				Spec: v1alpha1.RepositorySpec{},
 			},
-			message:     "info-message",
-			logLevel:    zap.InfoLevel,
-			expectEvent: true,
+			message:      "info-message",
+			logLevel:     zap.InfoLevel,
+			expectEvent:  true,
+			expectedType: v1.EventTypeNormal,
 		},
 		{
 			name: "event with a reason",
@@ -50,17 +53,34 @@ func TestEventEmitter_EmitMessage(t *testing.T) {
 				},
 				Spec: v1alpha1.RepositorySpec{},
 			},
-			message:     "info-message",
-			logLevel:    zap.InfoLevel,
-			expectEvent: true,
-			reason:      "aintnosunshine",
+			message:      "info-message",
+			logLevel:     zap.InfoLevel,
+			expectEvent:  true,
+			expectedType: v1.EventTypeNormal,
+			reason:       "aintnosunshine",
 		},
 		{
-			name:        "repo doesn't exists",
-			repo:        nil,
-			message:     "error-message",
-			logLevel:    zap.ErrorLevel,
-			expectEvent: false,
+			name:         "repo doesn't exists",
+			repo:         nil,
+			message:      "error-message",
+			logLevel:     zap.ErrorLevel,
+			expectEvent:  false,
+			expectedType: v1.EventTypeWarning,
+		},
+		{
+			name: "nil logger doesn't cause panic",
+			repo: &v1alpha1.Repository{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-repo",
+					Namespace: "test-ns",
+				},
+				Spec: v1alpha1.RepositorySpec{},
+			},
+			message:      "error-message",
+			logLevel:     zap.ErrorLevel,
+			expectEvent:  true,
+			expectedType: v1.EventTypeWarning,
+			useNilLogger: true,
 		},
 	}
 
@@ -69,14 +89,22 @@ func TestEventEmitter_EmitMessage(t *testing.T) {
 			ctx, _ := rtesting.SetupFakeContext(t)
 			stdata, _ := testclient.SeedTestData(t, ctx, testclient.Data{})
 
-			// emit event
-			NewEventEmitter(stdata.Kube, fakelogger).EmitMessage(tt.repo, tt.logLevel, tt.reason, tt.message)
+			// Use nil logger for specific test case
+			var logger *zap.SugaredLogger
+			if tt.useNilLogger {
+				logger = nil
+			} else {
+				logger = fakelogger
+			}
+
+			// emit event - this should not panic even with nil logger
+			NewEventEmitter(stdata.Kube, logger).EmitMessage(tt.repo, tt.logLevel, tt.reason, tt.message)
 
 			if tt.expectEvent {
 				events, err := stdata.Kube.CoreV1().Events(tt.repo.Namespace).List(context.Background(), metav1.ListOptions{})
 				assert.NilError(t, err)
 				assert.Equal(t, events.Items[0].Message, tt.message)
-				assert.Equal(t, events.Items[0].Type, v1.EventTypeNormal)
+				assert.Equal(t, events.Items[0].Type, tt.expectedType)
 				assert.Equal(t, events.Items[0].Reason, tt.reason)
 				assert.Equal(t, events.Items[0].InvolvedObject.Name, tt.repo.Name)
 				assert.Equal(t, events.Items[0].InvolvedObject.Namespace, tt.repo.Namespace)