Merge pull request #129790 from aojea/event_name

events: ensure the name is valid

Kubernetes-commit: 052d7a5310b0bab58f81519253e08f456f39e8f3

go.mod

@@ -28,7 +28,7 @@ require (
 	golang.org/x/time v0.7.0
 	google.golang.org/protobuf v1.35.1
 	gopkg.in/evanphx/json-patch.v4 v4.12.0
-	k8s.io/api v0.0.0-20250218234707-8ce7fe8996bd
+	k8s.io/api v0.0.0-20250220033426-193fbe40afa3
 	k8s.io/apimachinery v0.0.0-20250214214420-47e7fa9a40a2
 	k8s.io/klog/v2 v2.130.1
 	k8s.io/kube-openapi v0.0.0-20241212222426-2c72e554b1e7

go.sum

@@ -148,8 +148,8 @@ gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
-k8s.io/api v0.0.0-20250218234707-8ce7fe8996bd h1:Tj1WKOMX+CxUkl0lhoqS+cavQBDEArJEOFfkyYThUAo=
+k8s.io/api v0.0.0-20250220033426-193fbe40afa3 h1:uxx5H9/bTJa9DSAS4iEvcw8LvaVj3VtkGO/oKJomIC4=
-k8s.io/api v0.0.0-20250218234707-8ce7fe8996bd/go.mod h1:j1vwjHcqIjL/8xva/zoPxpzN/saZm5ZqvK7J+cjQJ9A=
+k8s.io/api v0.0.0-20250220033426-193fbe40afa3/go.mod h1:j1vwjHcqIjL/8xva/zoPxpzN/saZm5ZqvK7J+cjQJ9A=
 k8s.io/apimachinery v0.0.0-20250214214420-47e7fa9a40a2 h1:+Wh461h0wCf5qF35OJaZlyItfrbgmuRpIcPdohK3qNQ=
 k8s.io/apimachinery v0.0.0-20250214214420-47e7fa9a40a2/go.mod h1:pvurfgWU15pkR11HFlMI9tdxY59XU+Wzo22Rx2iSD+g=
 k8s.io/klog/v2 v2.130.1 h1:n9Xl7H1Xvksem4KFG4PYbdQCQxqc/tTUyrgXaOhHSzk=

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,

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")
+			}
+
+		})
+	}
+}

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,
 		},

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
+}

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)
+			}
+
+		})
+
+	}
+}