Skip to content

Commit 83d1c94

Browse files
authored
fix: nullEventLogger panics on calls to Eventf() (#1214)
Contains AI generated tests * Bump Go version for secvuln
1 parent 0b0b54a commit 83d1c94

File tree

3 files changed

+227
-4
lines changed

3 files changed

+227
-4
lines changed

.go-version

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
1.25.5
1+
1.25.7

vault/client_factory.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1073,8 +1073,18 @@ func InitCachingClientFactory(ctx context.Context, client ctrlclient.Client, con
10731073

10741074
var _ record.EventRecorder = (*nullEventRecorder)(nil)
10751075

1076-
type nullEventRecorder struct {
1077-
record.EventRecorder
1078-
}
1076+
type nullEventRecorder struct{}
10791077

1078+
// Event does nothing, it is just a placeholder to satisfy the
1079+
// record.EventRecorder interface.
10801080
func (n *nullEventRecorder) Event(_ runtime.Object, _, _, _ string) {}
1081+
1082+
// Eventf does nothing, it is just a placeholder to satisfy the
1083+
// record.EventRecorder interface.
1084+
func (n *nullEventRecorder) Eventf(_ runtime.Object, _, _, _ string, _ ...interface{}) {}
1085+
1086+
// AnnotatedEventf is just like eventf, but with annotations attached. It does
1087+
// nothing, it is just a placeholder to satisfy the record.EventRecorder
1088+
// interface.
1089+
func (n *nullEventRecorder) AnnotatedEventf(_ runtime.Object, _ map[string]string, _, reason, _ string, args ...interface{}) {
1090+
}

vault/client_factory_test.go

Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/stretchr/testify/require"
1818
corev1 "k8s.io/api/core/v1"
1919
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
20+
"k8s.io/apimachinery/pkg/runtime"
2021
"k8s.io/utils/keymutex"
2122
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
2223

@@ -774,3 +775,215 @@ func Test_cachingClientFactory_CacheKeyFunc(t *testing.T) {
774775
})
775776
}
776777
}
778+
779+
func Test_nullEventRecorder_Event(t *testing.T) {
780+
t.Parallel()
781+
782+
tests := []struct {
783+
name string
784+
object runtime.Object
785+
eventType string
786+
reason string
787+
message string
788+
}{
789+
{
790+
name: "basic-event",
791+
object: &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "test-secret"}},
792+
eventType: corev1.EventTypeNormal,
793+
reason: "TestReason",
794+
message: "Test message",
795+
},
796+
{
797+
name: "warning-event",
798+
object: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "test-pod"}},
799+
eventType: corev1.EventTypeWarning,
800+
reason: "WarningReason",
801+
message: "Warning message",
802+
},
803+
{
804+
name: "nil-object",
805+
object: nil,
806+
eventType: corev1.EventTypeNormal,
807+
reason: "TestReason",
808+
message: "Test message",
809+
},
810+
{
811+
name: "empty-strings",
812+
object: &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "test-cm"}},
813+
eventType: "",
814+
reason: "",
815+
message: "",
816+
},
817+
}
818+
819+
for _, tt := range tests {
820+
t.Run(tt.name, func(t *testing.T) {
821+
t.Parallel()
822+
823+
recorder := &nullEventRecorder{}
824+
825+
// Verify Event() doesn't panic and is a no-op
826+
assert.NotPanics(t, func() {
827+
recorder.Event(tt.object, tt.eventType, tt.reason, tt.message)
828+
})
829+
})
830+
}
831+
}
832+
833+
func Test_nullEventRecorder_Eventf(t *testing.T) {
834+
t.Parallel()
835+
836+
tests := []struct {
837+
name string
838+
object runtime.Object
839+
eventType string
840+
reason string
841+
message string
842+
args []interface{}
843+
}{
844+
{
845+
name: "formatted-event",
846+
object: &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "test-secret"}},
847+
eventType: corev1.EventTypeNormal,
848+
reason: "TestReason",
849+
message: "Test message with %s and %d",
850+
args: []interface{}{"string", 42},
851+
},
852+
{
853+
name: "no-format-args",
854+
object: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "test-pod"}},
855+
eventType: corev1.EventTypeWarning,
856+
reason: "WarningReason",
857+
message: "Simple message",
858+
args: nil,
859+
},
860+
{
861+
name: "nil-object",
862+
object: nil,
863+
eventType: corev1.EventTypeNormal,
864+
reason: "TestReason",
865+
message: "Test message %v",
866+
args: []interface{}{nil},
867+
},
868+
{
869+
name: "multiple-format-args",
870+
object: &secretsv1beta1.VaultAuth{ObjectMeta: metav1.ObjectMeta{Name: "test-auth"}},
871+
eventType: corev1.EventTypeNormal,
872+
reason: "Reason",
873+
message: "Message with %s, %d, %v, %t",
874+
args: []interface{}{"text", 123, []string{"a", "b"}, true},
875+
},
876+
}
877+
878+
for _, tt := range tests {
879+
t.Run(tt.name, func(t *testing.T) {
880+
t.Parallel()
881+
882+
recorder := &nullEventRecorder{}
883+
884+
// Verify Eventf() doesn't panic and is a no-op
885+
assert.NotPanics(t, func() {
886+
recorder.Eventf(tt.object, tt.eventType, tt.reason, tt.message, tt.args...)
887+
})
888+
})
889+
}
890+
}
891+
892+
func Test_nullEventRecorder_AnnotatedEventf(t *testing.T) {
893+
t.Parallel()
894+
895+
tests := []struct {
896+
name string
897+
object runtime.Object
898+
annotations map[string]string
899+
eventType string
900+
reason string
901+
message string
902+
args []interface{}
903+
}{
904+
{
905+
name: "annotated-event",
906+
object: &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "test-secret"}},
907+
annotations: map[string]string{
908+
"key1": "value1",
909+
"key2": "value2",
910+
},
911+
eventType: corev1.EventTypeNormal,
912+
reason: "TestReason",
913+
message: "Test message with %s",
914+
args: []interface{}{"annotation"},
915+
},
916+
{
917+
name: "nil-annotations",
918+
object: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "test-pod"}},
919+
annotations: nil,
920+
eventType: corev1.EventTypeWarning,
921+
reason: "WarningReason",
922+
message: "Warning message",
923+
args: nil,
924+
},
925+
{
926+
name: "empty-annotations",
927+
object: &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "test-cm"}},
928+
annotations: map[string]string{},
929+
eventType: corev1.EventTypeNormal,
930+
reason: "Reason",
931+
message: "Message",
932+
args: []interface{}{},
933+
},
934+
{
935+
name: "complex-annotations",
936+
object: &secretsv1beta1.VaultConnection{ObjectMeta: metav1.ObjectMeta{Name: "test-conn"}},
937+
annotations: map[string]string{
938+
"app": "vault-secrets-operator",
939+
"version": "v1.0.0",
940+
"environment": "production",
941+
},
942+
eventType: corev1.EventTypeNormal,
943+
reason: "ConnectionEstablished",
944+
message: "Connection to %s established at %s",
945+
args: []interface{}{"vault", "2024-01-01"},
946+
},
947+
{
948+
name: "nil-object",
949+
object: nil,
950+
annotations: map[string]string{"key": "value"},
951+
eventType: corev1.EventTypeNormal,
952+
reason: "TestReason",
953+
message: "Test message",
954+
args: nil,
955+
},
956+
}
957+
958+
for _, tt := range tests {
959+
t.Run(tt.name, func(t *testing.T) {
960+
t.Parallel()
961+
962+
recorder := &nullEventRecorder{}
963+
964+
// Verify AnnotatedEventf() doesn't panic and is a no-op
965+
assert.NotPanics(t, func() {
966+
recorder.AnnotatedEventf(tt.object, tt.annotations, tt.eventType, tt.reason, tt.message, tt.args...)
967+
})
968+
})
969+
}
970+
}
971+
972+
func Test_nullEventRecorder_interface_compliance(t *testing.T) {
973+
t.Parallel()
974+
975+
// Verify that nullEventRecorder implements the EventRecorder interface
976+
_ = (interface{})((*nullEventRecorder)(nil))
977+
978+
// Additional compile-time check
979+
recorder := &nullEventRecorder{}
980+
assert.NotNil(t, recorder)
981+
982+
// Verify all methods can be called without panic
983+
assert.NotPanics(t, func() {
984+
testObj := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "test"}}
985+
recorder.Event(testObj, corev1.EventTypeNormal, "Reason", "Message")
986+
recorder.Eventf(testObj, corev1.EventTypeNormal, "Reason", "Message %s", "formatted")
987+
recorder.AnnotatedEventf(testObj, map[string]string{"key": "value"}, corev1.EventTypeNormal, "Reason", "Message %s", "annotated")
988+
})
989+
}

0 commit comments

Comments
 (0)