From 513676627603ec4e19d3fb2ed878350bd691c6e0 Mon Sep 17 00:00:00 2001 From: Ross Golder Date: Mon, 27 Jul 2026 11:55:50 +0700 Subject: [PATCH 1/2] Migrate event package from record.EventRecorder to events.EventRecorder - Replace deprecated record.EventRecorder with events.EventRecorder (events.k8s.io/v1) - Fix WithAnnotations to preserve filterFns when creating derived recorders - Remove unused Event.Annotations field and keysAndValues params from Normal/Warning - Drop annotation pairs from callers in managed reconciler - Pass Reason as event action to avoid empty action string Signed-off-by: Ross Golder --- pkg/event/event.go | 75 ++++++++++++------------- pkg/event/event_test.go | 82 ++++++++++++++++++++++++++++ pkg/reconciler/managed/reconciler.go | 5 +- 3 files changed, 122 insertions(+), 40 deletions(-) diff --git a/pkg/event/event.go b/pkg/event/event.go index c1630e6b9..0936bd2fe 100644 --- a/pkg/event/event.go +++ b/pkg/event/event.go @@ -21,14 +21,14 @@ import ( "maps" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/client-go/tools/record" + "k8s.io/client-go/tools/events" ) // A Type of event. type Type string -// Event types. See below for valid types. -// https://godoc.org/k8s.io/client-go/tools/record#EventRecorder +// Event types. +// https://pkg.go.dev/k8s.io/client-go/tools/events#EventRecorder const ( TypeNormal Type = "Normal" TypeWarning Type = "Warning" @@ -39,36 +39,27 @@ type Reason string // An Event relating to a Crossplane resource. type Event struct { - Type Type - Reason Reason - Message string - Annotations map[string]string + Type Type + Reason Reason + Message string } // Normal returns a normal, informational event. -func Normal(r Reason, message string, keysAndValues ...string) Event { - e := Event{ - Type: TypeNormal, - Reason: r, - Message: message, - Annotations: map[string]string{}, +func Normal(r Reason, message string) Event { + return Event{ + Type: TypeNormal, + Reason: r, + Message: message, } - sliceMap(keysAndValues, e.Annotations) - - return e } // Warning returns a warning event, typically due to an error. -func Warning(r Reason, err error, keysAndValues ...string) Event { - e := Event{ - Type: TypeWarning, - Reason: r, - Message: err.Error(), - Annotations: map[string]string{}, +func Warning(r Reason, err error) Event { + return Event{ + Type: TypeWarning, + Reason: r, + Message: err.Error(), } - sliceMap(keysAndValues, e.Annotations) - - return e } // A Recorder records Kubernetes events. @@ -77,20 +68,27 @@ type Recorder interface { WithAnnotations(keysAndValues ...string) Recorder } -// An APIRecorder records Kubernetes events to an API server. +// FilterFn is a function used to filter events. Returning true prevents the +// event from being recorded. +type FilterFn func(obj runtime.Object, e Event) bool + +// An APIRecorder records Kubernetes events to an API server using the +// events.k8s.io/v1 API introduced in Kubernetes 1.19. +// +// Note: the events.EventRecorder interface does not support per-event +// annotations (unlike the deprecated record.EventRecorder.AnnotatedEventf). +// Annotations accumulated via WithAnnotations are stored but cannot be +// forwarded to the API server. Callers that relied on annotation propagation +// should encode that metadata into the event message instead. type APIRecorder struct { - kube record.EventRecorder - annotations map[string]string + kube events.EventRecorder + annotations map[string]string // stored but not forwarded; see type doc. filterFns []FilterFn } -// FilterFn is a function used to filter events. -// It should return false when events should not be sent. -type FilterFn func(obj runtime.Object, e Event) bool - // NewAPIRecorder returns an APIRecorder that records Kubernetes events to an -// APIServer using the supplied EventRecorder. -func NewAPIRecorder(r record.EventRecorder, fns ...FilterFn) *APIRecorder { +// API server using the supplied EventRecorder. +func NewAPIRecorder(r events.EventRecorder, fns ...FilterFn) *APIRecorder { return &APIRecorder{kube: r, annotations: map[string]string{}, filterFns: fns} } @@ -102,13 +100,16 @@ func (r *APIRecorder) Event(obj runtime.Object, e Event) { } } - r.kube.AnnotatedEventf(obj, r.annotations, string(e.Type), string(e.Reason), "%s", e.Message) + r.kube.Eventf(obj, nil, string(e.Type), string(e.Reason), string(e.Reason), "%s", e.Message) } // WithAnnotations returns a new *APIRecorder that includes the supplied -// annotations with all recorded events. +// annotations. Note: the events.k8s.io API does not support per-event +// annotations, so accumulated annotations are stored but not propagated to +// the API server. Encode annotation data in the event message if it must +// appear in the emitted event. func (r *APIRecorder) WithAnnotations(keysAndValues ...string) Recorder { - ar := NewAPIRecorder(r.kube) + ar := NewAPIRecorder(r.kube, r.filterFns...) maps.Copy(ar.annotations, r.annotations) sliceMap(keysAndValues, ar.annotations) diff --git a/pkg/event/event_test.go b/pkg/event/event_test.go index 656b0a9ec..3ff82c370 100644 --- a/pkg/event/event_test.go +++ b/pkg/event/event_test.go @@ -17,11 +17,39 @@ limitations under the License. package event import ( + "fmt" "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" ) +// mockKubeRecorder satisfies events.EventRecorder. +type mockKubeRecorder struct { + events []mockEvent +} + +type mockEvent struct { + obj runtime.Object + typeStr string + reason string + msg string +} + +func (m *mockKubeRecorder) Eventf(obj runtime.Object, _ runtime.Object, eventtype, reason, _, note string, args ...any) { + msg := fmt.Sprintf(note, args...) + m.events = append(m.events, mockEvent{obj: obj, typeStr: eventtype, reason: reason, msg: msg}) +} + +type mockObj struct{} + +func (m *mockObj) GetObjectKind() schema.ObjectKind { return nil } +func (m *mockObj) DeepCopyObject() runtime.Object { + return &mockObj{} +} + func TestSliceMap(t *testing.T) { type args struct { from []string @@ -86,3 +114,57 @@ func TestSliceMap(t *testing.T) { }) } } + +func TestAPIRecorderEvent(t *testing.T) { + mr := &mockKubeRecorder{} + rec := NewAPIRecorder(mr) + + rec.Event(&mockObj{}, Normal("testReason", "test message")) + + want := mockEvent{typeStr: "Normal", reason: "testReason", msg: "test message"} + if diff := cmp.Diff(want, mr.events[0], cmp.AllowUnexported(mockEvent{}), cmpopts.IgnoreFields(mockEvent{}, "obj")); diff != "" { + t.Errorf("unexpected event: -want, +got:\n%s", diff) + } +} + +func TestAPIRecorderFilter(t *testing.T) { + mr := &mockKubeRecorder{} + filter := func(_ runtime.Object, _ Event) bool { return true } + rec := NewAPIRecorder(mr, filter) + + rec.Event(&mockObj{}, Normal("testReason", "test message")) + + if diff := cmp.Diff(0, len(mr.events)); diff != "" { + t.Errorf("expected no events, got %d: %s", len(mr.events), diff) + } +} + +func TestAPIRecorderWithAnnotationsPreservesFilterFns(t *testing.T) { + filterCalled := false + filter := func(_ runtime.Object, _ Event) bool { + filterCalled = true + return false + } + + mr := &mockKubeRecorder{} + rec := NewAPIRecorder(mr, filter) + derived := rec.WithAnnotations("key", "val") + + derived.Event(&mockObj{}, Normal("test", "msg")) + + if diff := cmp.Diff(true, filterCalled); diff != "" { + t.Errorf("filter function was not preserved after WithAnnotations: %s", diff) + } +} + +func TestAPIRecorderWithAnnotationsPreservesExistingAnnotations(t *testing.T) { + mr := &mockKubeRecorder{} + rec := NewAPIRecorder(mr) + r1 := rec.WithAnnotations("k1", "v1").(*APIRecorder) + r2 := r1.WithAnnotations("k2", "v2").(*APIRecorder) + + want := map[string]string{"k1": "v1", "k2": "v2"} + if diff := cmp.Diff(want, r2.annotations); diff != "" { + t.Errorf("annotations not preserved correctly: -want, +got:\n%s", diff) + } +} diff --git a/pkg/reconciler/managed/reconciler.go b/pkg/reconciler/managed/reconciler.go index ee945b56b..53ec733e4 100644 --- a/pkg/reconciler/managed/reconciler.go +++ b/pkg/reconciler/managed/reconciler.go @@ -1050,8 +1050,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu // Log, publish an event and update the SYNC status condition. if meta.IsPaused(managed) || policy.IsPaused() { log.Debug("Reconciliation is paused either through the `spec.managementPolicies` or the pause annotation", "annotation", meta.AnnotationKeyReconciliationPaused) - record.Event(managed, event.Normal(reasonReconciliationPaused, "Reconciliation is paused either through the `spec.managementPolicies` or the pause annotation", - "annotation", meta.AnnotationKeyReconciliationPaused)) + record.Event(managed, event.Normal(reasonReconciliationPaused, "Reconciliation is paused either through the `spec.managementPolicies` or the pause annotation")) status.MarkConditions(xpv2.ReconcilePaused()) // if the pause annotation is removed or the management policies changed, we will have a chance to reconcile // again and resume and if status update fails, we will reconcile again to retry to update the status @@ -1067,7 +1066,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu if tracker, ok := managed.(reconcileRequestTracker); ok { if tracker.GetLastHandledReconcileAt() != token { log.Debug("Processing reconcile request", "token", token) - record.Event(managed, event.Normal(reasonReconcileRequestHandled, "Handling reconcile request", "token", token)) + record.Event(managed, event.Normal(reasonReconcileRequestHandled, "Handling reconcile request")) reconcileRequestToken = token } } From 34ee9b8c283d116f7fb3892c941380471f7f1fd9 Mon Sep 17 00:00:00 2001 From: Ross Golder Date: Sun, 9 Aug 2026 17:49:27 +0700 Subject: [PATCH 2/2] test(event): assert action string in Eventf call Add test coverage for issue #1057 item 4 (empty action string causing events.k8s.io/v1 rejection). The mockKubeRecorder now captures the action parameter and TestAPIRecorderEvent asserts it equals the event's Reason, validating the fix and preventing future regressions. - Extend mockEvent with action field - Capture action parameter instead of discarding with _ - Assert action: "testReason" in want comparison Signed-off-by: Ross Golder --- pkg/event/event_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/event/event_test.go b/pkg/event/event_test.go index 3ff82c370..5c4b1186e 100644 --- a/pkg/event/event_test.go +++ b/pkg/event/event_test.go @@ -35,12 +35,13 @@ type mockEvent struct { obj runtime.Object typeStr string reason string + action string msg string } -func (m *mockKubeRecorder) Eventf(obj runtime.Object, _ runtime.Object, eventtype, reason, _, note string, args ...any) { +func (m *mockKubeRecorder) Eventf(obj runtime.Object, _ runtime.Object, eventtype, reason, action, note string, args ...any) { msg := fmt.Sprintf(note, args...) - m.events = append(m.events, mockEvent{obj: obj, typeStr: eventtype, reason: reason, msg: msg}) + m.events = append(m.events, mockEvent{obj: obj, typeStr: eventtype, reason: reason, action: action, msg: msg}) } type mockObj struct{} @@ -121,7 +122,7 @@ func TestAPIRecorderEvent(t *testing.T) { rec.Event(&mockObj{}, Normal("testReason", "test message")) - want := mockEvent{typeStr: "Normal", reason: "testReason", msg: "test message"} + want := mockEvent{typeStr: "Normal", reason: "testReason", action: "testReason", msg: "test message"} if diff := cmp.Diff(want, mr.events[0], cmp.AllowUnexported(mockEvent{}), cmpopts.IgnoreFields(mockEvent{}, "obj")); diff != "" { t.Errorf("unexpected event: -want, +got:\n%s", diff) }