From 023460f5a85a1750dcd9916d5872ea7a3c9c5a26 Mon Sep 17 00:00:00 2001 From: Antonio Ojea Date: Thu, 20 Feb 2025 09:13:56 +0000 Subject: [PATCH] events: ensure the name is valid The event Object is created from the referenced Object name, however, there is no guarantee that the name from the referenced Object will be a valid Event Name. For those Objects with name not valid for an Event, generate a new valid name using an UUID. Kubernetes-commit: ee36b817df06f84ce1a48ef4d65ed559c3775404 --- tools/events/event_recorder.go | 2 +- tools/events/event_recorder_test.go | 240 ++++++++++++++++++++++++++++ tools/record/event.go | 2 +- tools/record/util/util.go | 17 ++ tools/record/util/util_test.go | 72 +++++++++ 5 files changed, 331 insertions(+), 2 deletions(-) create mode 100644 tools/events/event_recorder_test.go create mode 100644 tools/record/util/util_test.go diff --git a/tools/events/event_recorder.go b/tools/events/event_recorder.go index 65431788..ba2ec7be 100644 --- a/tools/events/event_recorder.go +++ b/tools/events/event_recorder.go @@ -96,7 +96,7 @@ func (recorder *recorderImpl) makeEvent(refRegarding *v1.ObjectReference, refRel } return &eventsv1.Event{ ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("%v.%x", refRegarding.Name, t.UnixNano()), + Name: util.GenerateEventName(refRegarding.Name, t.UnixNano()), Namespace: namespace, }, EventTime: timestamp, diff --git a/tools/events/event_recorder_test.go b/tools/events/event_recorder_test.go new file mode 100644 index 00000000..0e9aed9c --- /dev/null +++ b/tools/events/event_recorder_test.go @@ -0,0 +1,240 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package events + +import ( + "fmt" + "strings" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + + v1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + + eventsv1 "k8s.io/api/events/v1" + apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + utilvalidation "k8s.io/apimachinery/pkg/util/validation" + "k8s.io/apimachinery/pkg/watch" + testclocks "k8s.io/utils/clock/testing" +) + +func TestEventf(t *testing.T) { + // use a fixed time for generated names that depend on the unix timestamp + fakeClock := testclocks.NewFakeClock(time.Date(2023, time.January, 1, 12, 0, 0, 0, time.UTC)) + + testCases := []struct { + desc string + regarding runtime.Object + related runtime.Object + eventtype string + reason string + action string + note string + args []interface{} + expectedEvent *eventsv1.Event + }{ + { + desc: "normal event", + regarding: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod1", + Namespace: "ns1", + UID: "12345", + }, + }, + eventtype: "Normal", + reason: "Started", + action: "starting", + note: "Pod started", + expectedEvent: &eventsv1.Event{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("pod1.%x", fakeClock.Now().UnixNano()), + Namespace: "ns1", + }, + Regarding: v1.ObjectReference{ + Kind: "Pod", + Name: "pod1", + Namespace: "ns1", + UID: "12345", + APIVersion: "v1", + }, + Type: "Normal", + Reason: "Started", + Action: "starting", + Note: "Pod started", + ReportingController: "c1", + ReportingInstance: "i1", + }, + }, + { + desc: "event with related object and format args", + regarding: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod1", + Namespace: "ns1", + UID: "12345", + }, + }, + related: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node1", + UID: "67890", + }, + }, + eventtype: "Warning", + reason: "FailedScheduling", + action: "scheduling", + + note: "Pod failed to schedule on %s: %s", + args: []interface{}{"node1", "not enough resources"}, + expectedEvent: &eventsv1.Event{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("pod1.%x", fakeClock.Now().UnixNano()), + Namespace: "ns1", + }, + Regarding: v1.ObjectReference{ + Kind: "Pod", + Name: "pod1", + Namespace: "ns1", + UID: "12345", + APIVersion: "v1", + }, + Related: &v1.ObjectReference{ + Kind: "Node", + Name: "node1", + UID: "67890", + APIVersion: "v1", + }, + Type: "Warning", + Reason: "FailedScheduling", + Action: "scheduling", + Note: "Pod failed to schedule on node1: not enough resources", + ReportingController: "c1", + ReportingInstance: "i1", + }, + }, { + desc: "event with invalid Event name", + regarding: &networkingv1.IPAddress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "2001:db8::123", + UID: "12345", + }, + }, + eventtype: "Warning", + reason: "IPAddressNotAllocated", + action: "IPAddressAllocation", + note: "Service default/test appears to have leaked", + + expectedEvent: &eventsv1.Event{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + }, + Regarding: v1.ObjectReference{ + Kind: "IPAddress", + Name: "2001:db8::123", + UID: "12345", + APIVersion: "networking.k8s.io/v1", + }, + Type: "Warning", + Reason: "IPAddressNotAllocated", + Action: "IPAddressAllocation", + Note: "Service default/test appears to have leaked", + ReportingController: "c1", + ReportingInstance: "i1", + }, + }, { + desc: "large event name", + regarding: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: strings.Repeat("x", utilvalidation.DNS1123SubdomainMaxLength*4), + Namespace: "ns1", + UID: "12345", + }, + }, + eventtype: "Normal", + reason: "Started", + action: "starting", + note: "Pod started", + expectedEvent: &eventsv1.Event{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns1", + }, + Regarding: v1.ObjectReference{ + Kind: "Pod", + Name: strings.Repeat("x", utilvalidation.DNS1123SubdomainMaxLength*4), + Namespace: "ns1", + UID: "12345", + APIVersion: "v1", + }, + Type: "Normal", + Reason: "Started", + Action: "starting", + Note: "Pod started", + ReportingController: "c1", + ReportingInstance: "i1", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + broadcaster := watch.NewBroadcaster(1, watch.WaitIfChannelFull) + recorder := &recorderImpl{ + scheme: runtime.NewScheme(), + reportingController: "c1", + reportingInstance: "i1", + Broadcaster: broadcaster, + clock: fakeClock, + } + + if err := v1.AddToScheme(recorder.scheme); err != nil { + t.Fatal(err) + } + if err := networkingv1.AddToScheme(recorder.scheme); err != nil { + t.Fatal(err) + } + ch, err := broadcaster.Watch() + if err != nil { + t.Fatal(err) + } + recorder.Eventf(tc.regarding, tc.related, tc.eventtype, tc.reason, tc.action, tc.note, tc.args...) + + select { + case event := <-ch.ResultChan(): + actualEvent := event.Object.(*eventsv1.Event) + if errs := apimachineryvalidation.NameIsDNSSubdomain(actualEvent.Name, false); len(errs) > 0 { + t.Errorf("Event Name = %s; not a valid name: %v", actualEvent.Name, errs) + } // Overwrite fields that are not relevant for comparison + tc.expectedEvent.EventTime = actualEvent.EventTime + // invalid event names generate random names + if tc.expectedEvent.Name == "" { + actualEvent.Name = "" + } + if diff := cmp.Diff(tc.expectedEvent, actualEvent); diff != "" { + t.Errorf("Unexpected event diff (-want, +got):\n%s", diff) + } + case <-time.After(time.Second): + t.Errorf("Timeout waiting for event") + } + + }) + } +} diff --git a/tools/record/event.go b/tools/record/event.go index 55947d20..f97c5d61 100644 --- a/tools/record/event.go +++ b/tools/record/event.go @@ -489,7 +489,7 @@ func (recorder *recorderImpl) makeEvent(ref *v1.ObjectReference, annotations map } return &v1.Event{ ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("%v.%x", ref.Name, t.UnixNano()), + Name: util.GenerateEventName(ref.Name, t.UnixNano()), Namespace: namespace, Annotations: annotations, }, diff --git a/tools/record/util/util.go b/tools/record/util/util.go index afcc6a6a..7a82db0c 100644 --- a/tools/record/util/util.go +++ b/tools/record/util/util.go @@ -17,10 +17,14 @@ limitations under the License. package util import ( + "fmt" "net/http" + "github.com/google/uuid" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" + apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" ) // ValidateEventType checks that eventtype is an expected type of event @@ -38,3 +42,16 @@ func IsKeyNotFoundError(err error) bool { return statusErr != nil && statusErr.Status().Code == http.StatusNotFound } + +// GenerateEventName generates a valid Event name from the referenced name and the passed UNIX timestamp. +// The referenced Object name may not be a valid name for Events and cause the Event to fail +// to be created, so we need to generate a new one in that case. +// Ref: https://issues.k8s.io/127594 +func GenerateEventName(refName string, unixNano int64) string { + name := fmt.Sprintf("%s.%x", refName, unixNano) + if errs := apimachineryvalidation.NameIsDNSSubdomain(name, false); len(errs) > 0 { + // Using an uuid guarantees uniqueness and correctness + name = uuid.New().String() + } + return name +} diff --git a/tools/record/util/util_test.go b/tools/record/util/util_test.go new file mode 100644 index 00000000..d6be791e --- /dev/null +++ b/tools/record/util/util_test.go @@ -0,0 +1,72 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "strings" + "testing" + + apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" +) + +func TestGenerateEventName(t *testing.T) { + timestamp := int64(105999103295324396) + testCases := []struct { + name string + refName string + expected string + }{ + { + name: "valid name", + refName: "test-pod", + expected: "test-pod.178959f726d80ec", + }, + { + name: "invalid name - too long", + refName: strings.Repeat("x", 300), + }, + { + name: "invalid name - upper case", + refName: "test.POD", + }, + { + name: "invalid name - special chars", + refName: "test.pod/invalid!chars?", + }, + { + name: "invalid name - special chars and non alphanumeric starting character", + refName: "--test.pod/invalid!chars?", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + actual := GenerateEventName(tc.refName, timestamp) + + if errs := apimachineryvalidation.NameIsDNSSubdomain(actual, false); len(errs) > 0 { + t.Errorf("generateEventName(%s) = %s; not a valid name: %v", tc.refName, actual, errs) + + } + + if tc.expected != "" && (actual != tc.expected) { + t.Errorf("generateEventName(%s) returned %s expected %s", tc.refName, actual, tc.expected) + } + + }) + + } +}