From ba5fc795968c5b934e3feb2a68830cda1a32d37c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 27 Oct 2026 17:44:39 +0100 Subject: [PATCH 1/5] fix: event recorder aggregation, long names and cluster scoped event namespace - aggregating onto an event with an explicit name also updates type, reason and action, and rejects a change of them when the event carries an eventTime, which the API server treats as immutable - supplied names longer than the Kubernetes limit are shortened with a hash of the full name instead of plainly truncated, so distinct names stay distinct - events about cluster scoped resources in an overridden namespace carry an eventTime (and an action), as the API server refuses them otherwise --- .../api/event/DefaultEventRecorder.java | 39 ++- .../operator/api/event/DefaultEventSink.java | 32 ++- .../api/event/DefaultEventRecorderTest.java | 30 ++- .../event/EventRecorderRegressionTest.java | 244 ++++++++++++++++++ 4 files changed, 332 insertions(+), 13 deletions(-) create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderRegressionTest.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorder.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorder.java index 67764b7146..1d070d7e4e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorder.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorder.java @@ -21,6 +21,8 @@ import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.time.Instant; +import java.time.ZoneOffset; +import java.time.format.DateTimeFormatter; import java.time.temporal.ChronoUnit; import java.util.HexFormat; import java.util.Objects; @@ -32,6 +34,7 @@ import io.fabric8.kubernetes.api.model.Event; import io.fabric8.kubernetes.api.model.EventBuilder; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.MicroTime; import io.fabric8.kubernetes.api.model.ObjectReference; import io.fabric8.kubernetes.api.model.ObjectReferenceBuilder; import io.javaoperatorsdk.operator.api.config.LeaderElectionConfiguration; @@ -75,6 +78,12 @@ public class DefaultEventRecorder implements EventRecorder { private static final int IDENTITY_HASH_LENGTH = 32; + /** + * The fixed width format of {@code MicroTime}, which the API server refuses to read otherwise. + */ + private static final DateTimeFormatter EVENT_TIME_FORMAT = + DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSSSS'Z'").withZone(ZoneOffset.UTC); + /** What the API server accepts as an object name, see RFC 1123 on DNS subdomains. */ private static final Pattern RFC_1123_SUBDOMAIN = Pattern.compile("[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*"); @@ -211,11 +220,12 @@ protected Event toEvent(Context context, EventRecord record) { var regarding = context.getPrimaryResource(); var now = Instant.now().truncatedTo(ChronoUnit.SECONDS).toString(); var involvedObject = objectReferenceFor(regarding); + var namespace = eventNamespace(regarding, context); var builder = new EventBuilder() .withNewMetadata() .withName(eventName(regarding, record, controllerName)) - .withNamespace(eventNamespace(regarding, context)) + .withNamespace(namespace) .withLabels(record.labels()) .withAnnotations(record.annotations()) .endMetadata() @@ -256,9 +266,26 @@ protected Event toEvent(Context context, EventRecord record) { .endMetadata(); } record.action().ifPresent(builder::withAction); + if (requiresEventTime(regarding, namespace)) { + builder.withEventTime(new MicroTime(EVENT_TIME_FORMAT.format(Instant.now()))); + if (record.action().isEmpty()) { + builder.withAction(record.reason()); + } + } return builder.build(); } + /** + * The API server only accepts an event without {@code eventTime} for a cluster scoped object if + * the event lives in the {@code default} namespace. Events that carry an {@code eventTime} are + * not subject to that restriction, but then have to set an {@code action} too. + */ + private static boolean requiresEventTime(HasMetadata regarding, String eventNamespace) { + return regarding.getMetadata().getNamespace() == null + && !CLUSTER_SCOPED_EVENT_NAMESPACE.equals(eventNamespace) + && !eventNamespace.isEmpty(); + } + private String eventNamespace(HasMetadata regarding, Context context) { var namespace = regarding.getMetadata().getNamespace(); if (namespace != null) { @@ -295,15 +322,17 @@ private static String truncateToMaxNameLength(String name) { if (name.length() <= MAX_NAME_LENGTH) { return name; } - var truncated = name.substring(0, MAX_NAME_LENGTH); + var suffix = "." + identityDigest(name); + var truncated = name.substring(0, MAX_NAME_LENGTH - suffix.length()); while (truncated.endsWith("-") || truncated.endsWith(".")) { truncated = truncated.substring(0, truncated.length() - 1); } + var shortened = truncated + suffix; log.warn( - "Truncated the name of event {} to {} to stay within the Kubernetes name limit", + "Shortened the name of event {} to {} to stay within the Kubernetes name limit", name, - truncated); - return truncated; + shortened); + return shortened; } /** diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventSink.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventSink.java index c2f5461ae8..8269759e95 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventSink.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventSink.java @@ -15,6 +15,8 @@ */ package io.javaoperatorsdk.operator.api.event; +import java.util.Objects; + import io.fabric8.kubernetes.api.model.Event; import io.fabric8.kubernetes.api.model.EventBuilder; import io.fabric8.kubernetes.client.KubernetesClient; @@ -30,8 +32,12 @@ *

An event that does not exist yet is created. An event that does is a repeat of one already * recorded, since the name of an event is derived from everything that identifies it (see {@link * DefaultEventRecorder}), and is aggregated onto the recorded one by patching its {@code count} and - * {@code lastTimestamp}. That is what makes {@code kubectl describe} report a repeating event once, - * as {@code (x12 over 3m)}, rather than filling the event list with copies of it. + * {@code lastTimestamp}, along with its {@code message}, {@code type}, {@code reason} and {@code + * action}, which events sharing a name explicitly may differ in, unless the event carries an {@code + * eventTime} (see {@link DefaultEventRecorder}), in which case the API server treats them as + * immutable and recording an event that differs in them is rejected. That is what makes {@code + * kubectl describe} report a repeating event once, as {@code (x12 over 3m)}, rather than filling + * the event list with copies of it. * *

Recording therefore needs {@code get}, {@code create} and {@code patch} on {@code events} in * the core API group. The patch is made on the resource version the event was read at, so two @@ -55,13 +61,31 @@ public void emit(Event event, Context context) { if (existing == null) { events.resource(event).create(); } else { - var aggregated = + if (existing.getEventTime() != null && differsInImmutableFields(existing, event)) { + throw new IllegalStateException( + "Event " + + name + + " carries an eventTime, so its type, reason and action cannot change, but the" + + " event to record differs in them: use distinct event names for them"); + } + var builder = new EventBuilder(existing) .withCount(requireNonNullElse(existing.getCount(), 1) + 1) .withLastTimestamp(event.getLastTimestamp()) .withMessage(event.getMessage()) - .build(); + .withType(event.getType()) + .withReason(event.getReason()); + if (event.getAction() != null) { + builder.withAction(event.getAction()); + } + var aggregated = builder.build(); events.resource(existing).patch(aggregated); } } + + private static boolean differsInImmutableFields(Event existing, Event event) { + return !Objects.equals(existing.getType(), event.getType()) + || !Objects.equals(existing.getReason(), event.getReason()) + || (event.getAction() != null && !Objects.equals(existing.getAction(), event.getAction())); + } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorderTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorderTest.java index 388217b9be..3cedc6cc9f 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorderTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorderTest.java @@ -436,19 +436,41 @@ void stripsTrailingSeparatorsFromTruncatedNames() { EventRecord.builder() .reason("Created") .message("created") - .name("a".repeat(252) + "." + "b".repeat(47)) + .name("a".repeat(219) + "." + "b".repeat(47)) .build(), context(configMap())); recorder.record( EventRecord.builder() .reason("Created") .message("created") - .name("b".repeat(252) + "-" + "c".repeat(47)) + .name("b".repeat(219) + "-" + "c".repeat(47)) .build(), context(configMap())); - assertThat(emitted.get(0).getMetadata().getName()).isEqualTo("a".repeat(252)); - assertThat(emitted.get(1).getMetadata().getName()).isEqualTo("b".repeat(252)); + assertThat(emitted.get(0).getMetadata().getName()).matches("a{219}\\.[0-9a-f]{32}"); + assertThat(emitted.get(1).getMetadata().getName()).matches("b{219}\\.[0-9a-f]{32}"); + } + + @Test + void keepsLongSuppliedNamesSharingTheirPrefixDistinct() { + recorder.record( + EventRecord.builder() + .reason("Created") + .message("created") + .name("a".repeat(300) + "-one") + .build(), + context(configMap())); + recorder.record( + EventRecord.builder() + .reason("Created") + .message("created") + .name("a".repeat(300) + "-two") + .build(), + context(configMap())); + + assertThat(emitted.get(0).getMetadata().getName()).hasSize(253); + assertThat(emitted.get(0).getMetadata().getName()) + .isNotEqualTo(emitted.get(1).getMetadata().getName()); } @Test diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderRegressionTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderRegressionTest.java new file mode 100644 index 0000000000..4a1020e23f --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderRegressionTest.java @@ -0,0 +1,244 @@ +/* + * Copyright Java Operator SDK 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 io.javaoperatorsdk.operator.api.event; + +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicReference; + +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ConfigMapBuilder; +import io.fabric8.kubernetes.api.model.Event; +import io.fabric8.kubernetes.api.model.EventList; +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.NamespaceBuilder; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.dsl.MixedOperation; +import io.fabric8.kubernetes.client.dsl.NonNamespaceOperation; +import io.fabric8.kubernetes.client.dsl.Resource; +import io.fabric8.kubernetes.client.dsl.V1APIGroupDSL; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.Context; + +import static io.javaoperatorsdk.operator.api.config.LeaderElectionConfigurationBuilder.aLeaderElectionConfiguration; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** Regression tests for the aggregation, naming and namespacing of recorded events. */ +class EventRecorderRegressionTest { + + private final List emitted = new ArrayList<>(); + + /** + * The core/v1 API server (legacyValidateEvent in pkg/apis/core/validation/events.go) rejects an + * event without eventTime whose involvedObject.namespace is empty unless the event itself lives + * in "" or "default". Overriding the cluster scoped event namespace, as + * ConfigurationService#clusterScopedEventNamespace recommends, therefore requires the event to + * carry an eventTime, and with it an action. + */ + @Test + void overriddenClusterScopedEventNamespaceProducesEventsTheCoreApiAccepts() { + var recorder = new DefaultEventRecorder((event, context) -> emitted.add(event)); + var namespace = + new NamespaceBuilder() + .withNewMetadata() + .withName("ns1") + .withUid("uid-2") + .endMetadata() + .build(); + + recorder.record(EventRecord.normal("Created", "created"), context(namespace, "operator-ns")); + + var event = emitted.get(0); + assertThat(event.getInvolvedObject().getNamespace()).isNull(); + assertThat(event.getMetadata().getNamespace()).isEqualTo("operator-ns"); + assertThat(event.getEventTime().getTime()) + .matches("\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d{6}Z"); + assertThat(event.getAction()).isEqualTo("Created"); + } + + @Test + void clusterScopedEventsInTheDefaultNamespaceDoNotCarryAnEventTime() { + var recorder = new DefaultEventRecorder((event, context) -> emitted.add(event)); + var namespace = + new NamespaceBuilder() + .withNewMetadata() + .withName("ns1") + .withUid("uid-2") + .endMetadata() + .build(); + + recorder.record(EventRecord.normal("Created", "created"), context(namespace, "default")); + + assertThat(emitted.get(0).getEventTime()).isNull(); + assertThat(emitted.get(0).getMetadata().getNamespace()).isEqualTo("default"); + } + + /** + * A naming strategy (here the one EventNamingStrategy's javadoc suggests, a fixed suffix per + * object) makes the name the aggregation identity on its own, so records with different type and + * reason resolve to the same event. DefaultEventSink then only patches count, lastTimestamp and + * message: the Warning is published under the type and reason of the earlier Normal event. + */ + @Test + void aggregatingOntoANamedEventKeepsTheTypeAndReasonOfTheFirstOccurrence() { + var stored = new AtomicReference(); + var patched = new AtomicReference(); + var sink = new DefaultEventSink(mockClient(stored, patched)); + var recorder = + DefaultEventRecorder.builder(sink) + .namingStrategy( + (regarding, record) -> + Optional.of(regarding.getMetadata().getName() + "-status-report")) + .build(); + var context = context(configMap("test1"), "default"); + + recorder.record(EventRecord.normal("Ready", "all good"), context); + recorder.record(EventRecord.warning("Degraded", "replica down"), context); + + assertThat(patched.get()).isNotNull(); + assertThat(patched.get().getCount()).isEqualTo(2); + assertThat(patched.get().getMessage()).isEqualTo("replica down"); + assertThat(patched.get().getType()).isEqualTo("Warning"); + assertThat(patched.get().getReason()).isEqualTo("Degraded"); + } + + @Test + void aggregatingOntoAnEventWithAnEventTimeRejectsChangedImmutableFields() { + var stored = new AtomicReference(); + var patched = new AtomicReference(); + var sink = new DefaultEventSink(mockClient(stored, patched)); + var recorder = + DefaultEventRecorder.builder(sink) + .namingStrategy((regarding, record) -> Optional.of("fixed-name")) + .build(); + var context = context(clusterScoped("ns1"), "operator-ns"); + + recorder.record(EventRecord.normal("Ready", "all good"), context); + recorder.record(EventRecord.warning("Degraded", "replica down"), context); + + assertThat(patched.get()).isNull(); + assertThat(stored.get().getType()).isEqualTo("Normal"); + + recorder.record(EventRecord.normal("Ready", "still good"), context); + + assertThat(patched.get().getCount()).isEqualTo(2); + } + + /** + * Supplied names are truncated to 253 characters, which silently maps distinct names onto the + * same event, so two different events of a long-named object are aggregated as one. + */ + @Test + void truncatingSuppliedNamesMergesDistinctEvents() { + var recorder = + DefaultEventRecorder.builder((event, context) -> emitted.add(event)) + .namingStrategy( + (regarding, record) -> + Optional.of( + regarding.getMetadata().getName() + "-" + record.reason().toLowerCase())) + .build(); + var context = context(configMap("a".repeat(245)), "default"); + + recorder.record(EventRecord.warning("BackoffLimitExceeded", "job failed"), context); + recorder.record(EventRecord.warning("BackoffStarted", "job retrying"), context); + + assertThat(emitted.get(0).getMetadata().getName()) + .isNotEqualTo(emitted.get(1).getMetadata().getName()); + } + + @SuppressWarnings({"unchecked", "rawtypes"}) + private KubernetesClient mockClient( + AtomicReference stored, AtomicReference patched) { + var client = mock(KubernetesClient.class); + var v1 = mock(V1APIGroupDSL.class); + MixedOperation> events = mock(MixedOperation.class); + NonNamespaceOperation> inNs = + mock(NonNamespaceOperation.class); + when(client.v1()).thenReturn(v1); + when(v1.events()).thenReturn(events); + when(events.inNamespace(anyString())).thenReturn(inNs); + Resource byName = mock(Resource.class); + when(inNs.withName(anyString())).thenReturn(byName); + when(byName.get()).thenAnswer(i -> stored.get()); + when(inNs.resource(any(Event.class))) + .thenAnswer( + i -> { + Event target = i.getArgument(0); + Resource r = mock(Resource.class); + when(r.create()) + .thenAnswer( + c -> { + stored.set(target); + return target; + }); + when(r.patch(any(Event.class))) + .thenAnswer( + p -> { + Event e = p.getArgument(0); + patched.set(e); + stored.set(e); + return e; + }); + return r; + }); + return client; + } + + @SuppressWarnings("rawtypes") + private Context context(HasMetadata primaryResource, String clusterScopedEventNamespace) { + var configurationService = mock(ConfigurationService.class); + when(configurationService.getLeaderElectionConfiguration()) + .thenReturn( + Optional.of(aLeaderElectionConfiguration("lease").withIdentity("pod-1").build())); + when(configurationService.clusterScopedEventNamespace()) + .thenReturn(clusterScopedEventNamespace); + ControllerConfiguration controllerConfiguration = mock(ControllerConfiguration.class); + when(controllerConfiguration.getName()).thenReturn("testcontroller"); + when(controllerConfiguration.getConfigurationService()).thenReturn(configurationService); + Context context = mock(Context.class); + when(context.getPrimaryResource()).thenReturn(primaryResource); + when(context.getControllerConfiguration()).thenReturn(controllerConfiguration); + return context; + } + + private HasMetadata clusterScoped(String name) { + return new NamespaceBuilder() + .withNewMetadata() + .withName(name) + .withUid("uid-3") + .endMetadata() + .build(); + } + + private ConfigMap configMap(String name) { + return new ConfigMapBuilder() + .withNewMetadata() + .withName(name) + .withNamespace("ns1") + .withUid("uid-1") + .withResourceVersion("42") + .endMetadata() + .build(); + } +} From 5402953c3c0bb4c069fccc0b285957c002e6a84f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 3 Nov 2026 14:55:04 -0800 Subject: [PATCH 2/5] fix: address event recorder review comments - drop the immutable field check when aggregating events: the core API does not enforce it, and the exception made the sink silently drop the event - aggregate the action along with type, reason and message, clearing it when absent - only carry an eventTime for cluster scoped events in kube-system, the one namespace besides default the API server accepts, and fix the related javadocs and docs - strip trailing separators when shortening generated event names - add an integration test recording events about a cluster scoped resource --- .../documentation/operations/configuration.md | 7 +- .../api/config/ConfigurationService.java | 8 +- .../config/ConfigurationServiceOverrider.java | 4 +- .../api/event/DefaultEventRecorder.java | 34 +++---- .../operator/api/event/DefaultEventSink.java | 35 ++----- .../api/event/EventNamingStrategy.java | 5 +- .../event/EventRecorderRegressionTest.java | 93 +++++++++++++------ .../ClusterScopedEventRecorderIT.java | 82 ++++++++++++++++ .../ClusterScopedEventRecorderReconciler.java | 36 +++++++ 9 files changed, 226 insertions(+), 78 deletions(-) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/eventrecorder/ClusterScopedEventRecorderIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/eventrecorder/ClusterScopedEventRecorderReconciler.java diff --git a/docs/content/en/docs/documentation/operations/configuration.md b/docs/content/en/docs/documentation/operations/configuration.md index 831c1aa60b..0b469e8312 100644 --- a/docs/content/en/docs/documentation/operations/configuration.md +++ b/docs/content/en/docs/documentation/operations/configuration.md @@ -379,7 +379,7 @@ All operator-level keys are prefixed with `josdk.`. | Key | Type | Description | |---|---|---| -| `josdk.events.cluster-scoped-namespace` | `String` | Namespace to record events about cluster scoped resources in (defaults to `default`) | +| `josdk.events.cluster-scoped-namespace` | `String` | Namespace to record events about cluster scoped resources in (defaults to `default`); the API server only accepts `default` and `kube-system` | Recording events requires the operator's service account to be allowed `get`, `create` and `patch` on `events` in the core (`""`) API group. All three are used: an event that does not exist yet is @@ -395,6 +395,11 @@ therefore has to grant the permission in every namespace events are recorded in `default`, or whatever `josdk.events.cluster-scoped-namespace` is set to, if the operator reconciles cluster scoped resources. A `ClusterRole` covers all of them at once. +The API server only accepts events about cluster scoped objects in the `default` and `kube-system` +namespaces, so those are the only values `josdk.events.cluster-scoped-namespace` can usefully be set +to. In particular, the namespace the operator itself runs in is rejected, and the event dropped with +a warning in the operator log. + #### Leader Election Leader election is activated when at least one `josdk.leader-election.*` key is present. diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java index 92d1729619..6a7f2f93d5 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java @@ -389,9 +389,11 @@ default Optional getLeaderElectionConfiguration() { *

Defaults to the {@code default} namespace, following the Kubernetes convention, which is * also what the Go client does, so that events recorded by an operator end up alongside the ones * recorded by the built-in controllers. Operators whose RBAC does not allow creating events in - * the {@code default} namespace should override this, typically with the namespace the operator - * itself runs in. Note that recording an event is best effort, so a missing permission results in - * the event being dropped with a warning in the log rather than in an error. + * the {@code default} namespace can override this with {@code kube-system}. The API server + * rejects events about cluster scoped resources in any other namespace, such as the namespace the + * operator itself runs in. Note that recording an event is best effort, so a missing permission + * or a rejected namespace results in the event being dropped with a warning in the log rather + * than in an error. * * @return the namespace to record events about cluster scoped resources in */ diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java index 05112b4da7..c95b9cc54a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java @@ -201,8 +201,8 @@ public ConfigurationServiceOverrider withLeaderElectionConfiguration( /** * Sets the namespace in which Kubernetes events about cluster scoped resources are recorded. Use - * this when the operator is not allowed to create events in the {@code default} namespace, - * passing for example the namespace the operator itself runs in. + * this when the operator is not allowed to create events in the {@code default} namespace. The + * API server only accepts {@code default} and {@code kube-system} for such events. * * @param namespace the namespace to record events about cluster scoped resources in * @return this {@link ConfigurationServiceOverrider} for chained customization diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorder.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorder.java index 1d070d7e4e..dea3b075df 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorder.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorder.java @@ -50,7 +50,8 @@ *

Events for cluster scoped objects have to live in some namespace: by default the {@value * #CLUSTER_SCOPED_EVENT_NAMESPACE} namespace is used, following the Kubernetes convention, but it * can be overridden, see {@link - * io.javaoperatorsdk.operator.api.config.ConfigurationService#clusterScopedEventNamespace()}. + * io.javaoperatorsdk.operator.api.config.ConfigurationService#clusterScopedEventNamespace()}. The + * API server only accepts such events in the {@code default} and {@code kube-system} namespaces. * *

By default, events are named deterministically, after the object they are about plus a hash of * everything that identifies the event, so that recording the same event again resolves to the @@ -64,6 +65,8 @@ public class DefaultEventRecorder implements EventRecorder { public static final String CLUSTER_SCOPED_EVENT_NAMESPACE = "default"; + private static final String KUBE_SYSTEM_NAMESPACE = "kube-system"; + /** * Kubernetes limits object names to 253 characters, as they have to be valid RFC 1123 DNS * subdomains. @@ -277,13 +280,13 @@ protected Event toEvent(Context context, EventRecord record) { /** * The API server only accepts an event without {@code eventTime} for a cluster scoped object if - * the event lives in the {@code default} namespace. Events that carry an {@code eventTime} are - * not subject to that restriction, but then have to set an {@code action} too. + * the event lives in the {@code default} namespace. An event that carries an {@code eventTime} + * (and with it an {@code action}) is also accepted in the {@code kube-system} namespace. No other + * namespace is accepted for such an event, whether it has an {@code eventTime} or not. */ private static boolean requiresEventTime(HasMetadata regarding, String eventNamespace) { return regarding.getMetadata().getNamespace() == null - && !CLUSTER_SCOPED_EVENT_NAMESPACE.equals(eventNamespace) - && !eventNamespace.isEmpty(); + && KUBE_SYSTEM_NAMESPACE.equals(eventNamespace); } private String eventNamespace(HasMetadata regarding, Context context) { @@ -323,11 +326,7 @@ private static String truncateToMaxNameLength(String name) { return name; } var suffix = "." + identityDigest(name); - var truncated = name.substring(0, MAX_NAME_LENGTH - suffix.length()); - while (truncated.endsWith("-") || truncated.endsWith(".")) { - truncated = truncated.substring(0, truncated.length() - 1); - } - var shortened = truncated + suffix; + var shortened = truncate(name, MAX_NAME_LENGTH - suffix.length()) + suffix; log.warn( "Shortened the name of event {} to {} to stay within the Kubernetes name limit", name, @@ -335,6 +334,14 @@ private static String truncateToMaxNameLength(String name) { return shortened; } + private static String truncate(String name, int maxLength) { + var truncated = name.substring(0, Math.min(name.length(), maxLength)); + while (truncated.endsWith("-") || truncated.endsWith(".")) { + truncated = truncated.substring(0, truncated.length() - 1); + } + return truncated; + } + /** * Names events {@code .}, following the convention of the Go client, hashing * everything that makes two events the same event: the object, the type, the reason, the @@ -366,12 +373,7 @@ private String identityHashName( .orElseGet(() -> requireNonNullElse(record.message(), ""))); var suffix = "." + identityDigest(identity); - var prefix = metadata.getName(); - var maxPrefixLength = MAX_NAME_LENGTH - suffix.length(); - if (prefix.length() > maxPrefixLength) { - prefix = prefix.substring(0, maxPrefixLength); - } - return prefix + suffix; + return truncate(metadata.getName(), MAX_NAME_LENGTH - suffix.length()) + suffix; } /** diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventSink.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventSink.java index 8269759e95..5f44b1af43 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventSink.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventSink.java @@ -15,8 +15,6 @@ */ package io.javaoperatorsdk.operator.api.event; -import java.util.Objects; - import io.fabric8.kubernetes.api.model.Event; import io.fabric8.kubernetes.api.model.EventBuilder; import io.fabric8.kubernetes.client.KubernetesClient; @@ -33,11 +31,11 @@ * recorded, since the name of an event is derived from everything that identifies it (see {@link * DefaultEventRecorder}), and is aggregated onto the recorded one by patching its {@code count} and * {@code lastTimestamp}, along with its {@code message}, {@code type}, {@code reason} and {@code - * action}, which events sharing a name explicitly may differ in, unless the event carries an {@code - * eventTime} (see {@link DefaultEventRecorder}), in which case the API server treats them as - * immutable and recording an event that differs in them is rejected. That is what makes {@code - * kubectl describe} report a repeating event once, as {@code (x12 over 3m)}, rather than filling - * the event list with copies of it. + * action}, which events sharing a name explicitly may differ in: they describe the latest + * occurrence. The API server only makes them immutable for {@code events.k8s.io/v1} requests, not + * for the core group this sink writes to. That is what makes {@code kubectl describe} report a + * repeating event once, as {@code (x12 over 3m)}, rather than filling the event list with copies of + * it. * *

Recording therefore needs {@code get}, {@code create} and {@code patch} on {@code events} in * the core API group. The patch is made on the resource version the event was read at, so two @@ -61,31 +59,16 @@ public void emit(Event event, Context context) { if (existing == null) { events.resource(event).create(); } else { - if (existing.getEventTime() != null && differsInImmutableFields(existing, event)) { - throw new IllegalStateException( - "Event " - + name - + " carries an eventTime, so its type, reason and action cannot change, but the" - + " event to record differs in them: use distinct event names for them"); - } - var builder = + var aggregated = new EventBuilder(existing) .withCount(requireNonNullElse(existing.getCount(), 1) + 1) .withLastTimestamp(event.getLastTimestamp()) .withMessage(event.getMessage()) .withType(event.getType()) - .withReason(event.getReason()); - if (event.getAction() != null) { - builder.withAction(event.getAction()); - } - var aggregated = builder.build(); + .withReason(event.getReason()) + .withAction(event.getAction()) + .build(); events.resource(existing).patch(aggregated); } } - - private static boolean differsInImmutableFields(Event existing, Event event) { - return !Objects.equals(existing.getType(), event.getType()) - || !Objects.equals(existing.getReason(), event.getReason()) - || (event.getAction() != null && !Objects.equals(existing.getAction(), event.getAction())); - } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/EventNamingStrategy.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/EventNamingStrategy.java index 80f6f33e82..4fb3e7ae64 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/EventNamingStrategy.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/EventNamingStrategy.java @@ -30,8 +30,9 @@ * *

A name must be a valid RFC 1123 DNS subdomain: at most 253 lowercase alphanumeric characters, * {@code -} or {@code .}, starting and ending with an alphanumeric character. Names longer than the - * limit are truncated. A name derived from the object and a fixed lowercase suffix (such as {@code - * -status-report}) satisfies all of this by construction. + * limit are shortened, keeping a hash of the full name so distinct names stay distinct. A name + * derived from the object and a fixed lowercase suffix (such as {@code -status-report}) + * satisfies all of this by construction. * *

An empty result or an invalid name falls back to the default {@code .} * name, rather than the event being lost to the API server rejecting the name. diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderRegressionTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderRegressionTest.java index 4a1020e23f..f44dad6086 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderRegressionTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderRegressionTest.java @@ -51,44 +51,47 @@ class EventRecorderRegressionTest { /** * The core/v1 API server (legacyValidateEvent in pkg/apis/core/validation/events.go) rejects an - * event without eventTime whose involvedObject.namespace is empty unless the event itself lives - * in "" or "default". Overriding the cluster scoped event namespace, as - * ConfigurationService#clusterScopedEventNamespace recommends, therefore requires the event to - * carry an eventTime, and with it an action. + * event whose involvedObject.namespace is empty unless the event itself lives in "" or "default", + * or in "kube-system" if it carries an eventTime, and with it an action. No other namespace is + * accepted. */ @Test - void overriddenClusterScopedEventNamespaceProducesEventsTheCoreApiAccepts() { + void clusterScopedEventsInTheKubeSystemNamespaceCarryAnEventTimeAndAnAction() { var recorder = new DefaultEventRecorder((event, context) -> emitted.add(event)); - var namespace = - new NamespaceBuilder() - .withNewMetadata() - .withName("ns1") - .withUid("uid-2") - .endMetadata() - .build(); - recorder.record(EventRecord.normal("Created", "created"), context(namespace, "operator-ns")); + recorder.record( + EventRecord.normal("Created", "created"), context(clusterScoped("ns1"), "kube-system")); var event = emitted.get(0); assertThat(event.getInvolvedObject().getNamespace()).isNull(); - assertThat(event.getMetadata().getNamespace()).isEqualTo("operator-ns"); + assertThat(event.getMetadata().getNamespace()).isEqualTo("kube-system"); assertThat(event.getEventTime().getTime()) .matches("\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d{6}Z"); assertThat(event.getAction()).isEqualTo("Created"); } + @Test + void aSuppliedActionIsKeptOnClusterScopedEventsInTheKubeSystemNamespace() { + var recorder = new DefaultEventRecorder((event, context) -> emitted.add(event)); + + recorder.record( + EventRecord.builder() + .type(EventType.NORMAL) + .reason("Created") + .message("created") + .action("Create") + .build(), + context(clusterScoped("ns1"), "kube-system")); + + assertThat(emitted.get(0).getAction()).isEqualTo("Create"); + } + @Test void clusterScopedEventsInTheDefaultNamespaceDoNotCarryAnEventTime() { var recorder = new DefaultEventRecorder((event, context) -> emitted.add(event)); - var namespace = - new NamespaceBuilder() - .withNewMetadata() - .withName("ns1") - .withUid("uid-2") - .endMetadata() - .build(); - recorder.record(EventRecord.normal("Created", "created"), context(namespace, "default")); + recorder.record( + EventRecord.normal("Created", "created"), context(clusterScoped("ns1"), "default")); assertThat(emitted.get(0).getEventTime()).isNull(); assertThat(emitted.get(0).getMetadata().getNamespace()).isEqualTo("default"); @@ -124,7 +127,7 @@ void aggregatingOntoANamedEventKeepsTheTypeAndReasonOfTheFirstOccurrence() { } @Test - void aggregatingOntoAnEventWithAnEventTimeRejectsChangedImmutableFields() { + void aggregatingOntoAnEventWithAnEventTimeUpdatesTypeReasonAndAction() { var stored = new AtomicReference(); var patched = new AtomicReference(); var sink = new DefaultEventSink(mockClient(stored, patched)); @@ -132,17 +135,51 @@ void aggregatingOntoAnEventWithAnEventTimeRejectsChangedImmutableFields() { DefaultEventRecorder.builder(sink) .namingStrategy((regarding, record) -> Optional.of("fixed-name")) .build(); - var context = context(clusterScoped("ns1"), "operator-ns"); + var context = context(clusterScoped("ns1"), "kube-system"); recorder.record(EventRecord.normal("Ready", "all good"), context); recorder.record(EventRecord.warning("Degraded", "replica down"), context); - assertThat(patched.get()).isNull(); - assertThat(stored.get().getType()).isEqualTo("Normal"); + assertThat(stored.get().getEventTime()).isNotNull(); + assertThat(patched.get().getCount()).isEqualTo(2); + assertThat(patched.get().getType()).isEqualTo("Warning"); + assertThat(patched.get().getReason()).isEqualTo("Degraded"); + assertThat(patched.get().getAction()).isEqualTo("Degraded"); + } + + @Test + void aggregatingOntoANamedEventWithoutAnActionClearsTheActionOfTheEarlierOccurrence() { + var stored = new AtomicReference(); + var patched = new AtomicReference(); + var sink = new DefaultEventSink(mockClient(stored, patched)); + var recorder = + DefaultEventRecorder.builder(sink) + .namingStrategy((regarding, record) -> Optional.of("fixed-name")) + .build(); + var context = context(configMap("test1"), "default"); + + recorder.record( + EventRecord.builder() + .type(EventType.NORMAL) + .reason("Created") + .message("created") + .action("Create") + .build(), + context); + recorder.record(EventRecord.warning("Degraded", "replica down"), context); + + assertThat(patched.get().getAction()).isNull(); + } - recorder.record(EventRecord.normal("Ready", "still good"), context); + @Test + void shortenedGeneratedNamesDoNotEndInASeparatorBeforeTheHash() { + var recorder = new DefaultEventRecorder((event, context) -> emitted.add(event)); - assertThat(patched.get().getCount()).isEqualTo(2); + recorder.record( + EventRecord.normal("Created", "created"), + context(configMap("a".repeat(219) + "-" + "b".repeat(10)), "default")); + + assertThat(emitted.get(0).getMetadata().getName()).matches("a{219}\\.[0-9a-f]{32}"); } /** diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/eventrecorder/ClusterScopedEventRecorderIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/eventrecorder/ClusterScopedEventRecorderIT.java new file mode 100644 index 0000000000..550320afe3 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/eventrecorder/ClusterScopedEventRecorderIT.java @@ -0,0 +1,82 @@ +/* + * Copyright Java Operator SDK 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 io.javaoperatorsdk.operator.baseapi.eventrecorder; + +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.Event; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.baseapi.clusterscopedresource.ClusterScopedCustomResource; +import io.javaoperatorsdk.operator.baseapi.clusterscopedresource.ClusterScopedCustomResourceSpec; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +class ClusterScopedEventRecorderIT { + + public static final String TEST_RESOURCE_NAME = "event-recorder-test1"; + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder() + .withReconciler(new ClusterScopedEventRecorderReconciler()) + .build(); + + @Test + void recordsAndCountsEventsAboutAClusterScopedResourceInTheDefaultNamespace() { + var resource = extension.create(testResource()); + await().untilAsserted(() -> assertThat(eventsForTestResource()).hasSize(1)); + + resource.getMetadata().setAnnotations(Map.of("reconcile", "again")); + extension.replace(resource); + + await() + .untilAsserted( + () -> { + var events = eventsForTestResource(); + assertThat(events).hasSize(1); + assertThat(events.get(0).getReason()) + .isEqualTo(ClusterScopedEventRecorderReconciler.REASON); + assertThat(events.get(0).getInvolvedObject().getNamespace()).isNull(); + assertThat(events.get(0).getCount()).isGreaterThanOrEqualTo(2); + }); + } + + private List eventsForTestResource() { + return extension + .getKubernetesClient() + .v1() + .events() + .inNamespace("default") + .withField("involvedObject.name", TEST_RESOURCE_NAME) + .list() + .getItems(); + } + + ClusterScopedCustomResource testResource() { + var resource = new ClusterScopedCustomResource(); + resource.setMetadata(new ObjectMetaBuilder().withName(TEST_RESOURCE_NAME).build()); + resource.setSpec(new ClusterScopedCustomResourceSpec()); + resource.getSpec().setTargetNamespace(extension.getNamespace()); + resource.getSpec().setData("data"); + return resource; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/eventrecorder/ClusterScopedEventRecorderReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/eventrecorder/ClusterScopedEventRecorderReconciler.java new file mode 100644 index 0000000000..988c46f597 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/eventrecorder/ClusterScopedEventRecorderReconciler.java @@ -0,0 +1,36 @@ +/* + * Copyright Java Operator SDK 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 io.javaoperatorsdk.operator.baseapi.eventrecorder; + +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.javaoperatorsdk.operator.baseapi.clusterscopedresource.ClusterScopedCustomResource; + +@ControllerConfiguration(generationAwareEventProcessing = false) +public class ClusterScopedEventRecorderReconciler + implements Reconciler { + + public static final String REASON = "Reconciled"; + + @Override + public UpdateControl reconcile( + ClusterScopedCustomResource resource, Context context) { + context.eventRecorder().normal(REASON, "cluster scoped resource reconciled"); + return UpdateControl.noUpdate(); + } +} From 8b511c4fbec8fd281ddbe67f01d24a04ad481856 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Wed, 4 Nov 2026 14:48:23 -0800 Subject: [PATCH 3/5] fix: keep an action on aggregated events with an eventTime and scope the ITs to the resource - aggregating onto an event that carries an eventTime without an action falls back to the reason, as the API server requires an action for such events - the cluster scoped event recorder IT filters events by the uid of the resource, so events left over from earlier runs cannot match - add an IT recording events about a cluster scoped resource in kube-system --- .../operator/api/event/DefaultEventSink.java | 13 ++- .../event/EventRecorderRegressionTest.java | 29 +++++++ .../ClusterScopedEventRecorderIT.java | 9 +- ...lusterScopedKubeSystemEventRecorderIT.java | 87 +++++++++++++++++++ 4 files changed, 130 insertions(+), 8 deletions(-) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/eventrecorder/ClusterScopedKubeSystemEventRecorderIT.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventSink.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventSink.java index 5f44b1af43..ddb0b074bb 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventSink.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventSink.java @@ -33,9 +33,10 @@ * {@code lastTimestamp}, along with its {@code message}, {@code type}, {@code reason} and {@code * action}, which events sharing a name explicitly may differ in: they describe the latest * occurrence. The API server only makes them immutable for {@code events.k8s.io/v1} requests, not - * for the core group this sink writes to. That is what makes {@code kubectl describe} report a - * repeating event once, as {@code (x12 over 3m)}, rather than filling the event list with copies of - * it. + * for the core group this sink writes to. An event that carries an {@code eventTime} needs an + * {@code action}, so the reason stands in for it when the occurrence has none. That is what makes + * {@code kubectl describe} report a repeating event once, as {@code (x12 over 3m)}, rather than + * filling the event list with copies of it. * *

Recording therefore needs {@code get}, {@code create} and {@code patch} on {@code events} in * the core API group. The patch is made on the resource version the event was read at, so two @@ -59,6 +60,10 @@ public void emit(Event event, Context context) { if (existing == null) { events.resource(event).create(); } else { + var action = event.getAction(); + if (action == null && existing.getEventTime() != null) { + action = event.getReason(); + } var aggregated = new EventBuilder(existing) .withCount(requireNonNullElse(existing.getCount(), 1) + 1) @@ -66,7 +71,7 @@ public void emit(Event event, Context context) { .withMessage(event.getMessage()) .withType(event.getType()) .withReason(event.getReason()) - .withAction(event.getAction()) + .withAction(action) .build(); events.resource(existing).patch(aggregated); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderRegressionTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderRegressionTest.java index f44dad6086..367d93a707 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderRegressionTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderRegressionTest.java @@ -25,8 +25,10 @@ import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.ConfigMapBuilder; import io.fabric8.kubernetes.api.model.Event; +import io.fabric8.kubernetes.api.model.EventBuilder; import io.fabric8.kubernetes.api.model.EventList; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.MicroTime; import io.fabric8.kubernetes.api.model.NamespaceBuilder; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.dsl.MixedOperation; @@ -171,6 +173,33 @@ void aggregatingOntoANamedEventWithoutAnActionClearsTheActionOfTheEarlierOccurre assertThat(patched.get().getAction()).isNull(); } + @Test + void aggregatingOntoAnEventWithAnEventTimeWithoutAnActionFallsBackToTheReason() { + var stored = new AtomicReference(); + var patched = new AtomicReference(); + stored.set( + new EventBuilder() + .withNewMetadata() + .withName("fixed-name") + .withNamespace("default") + .endMetadata() + .withEventTime(new MicroTime("2026-01-01T00:00:00.000000Z")) + .withAction("Create") + .withCount(1) + .build()); + var sink = new DefaultEventSink(mockClient(stored, patched)); + var recorder = + DefaultEventRecorder.builder(sink) + .namingStrategy((regarding, record) -> Optional.of("fixed-name")) + .build(); + + recorder.record( + EventRecord.warning("Degraded", "replica down"), context(configMap("test1"), "default")); + + assertThat(patched.get().getEventTime()).isNotNull(); + assertThat(patched.get().getAction()).isEqualTo("Degraded"); + } + @Test void shortenedGeneratedNamesDoNotEndInASeparatorBeforeTheHash() { var recorder = new DefaultEventRecorder((event, context) -> emitted.add(event)); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/eventrecorder/ClusterScopedEventRecorderIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/eventrecorder/ClusterScopedEventRecorderIT.java index 550320afe3..7af90b3373 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/eventrecorder/ClusterScopedEventRecorderIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/eventrecorder/ClusterScopedEventRecorderIT.java @@ -43,7 +43,8 @@ class ClusterScopedEventRecorderIT { @Test void recordsAndCountsEventsAboutAClusterScopedResourceInTheDefaultNamespace() { var resource = extension.create(testResource()); - await().untilAsserted(() -> assertThat(eventsForTestResource()).hasSize(1)); + var uid = resource.getMetadata().getUid(); + await().untilAsserted(() -> assertThat(eventsAbout(uid)).hasSize(1)); resource.getMetadata().setAnnotations(Map.of("reconcile", "again")); extension.replace(resource); @@ -51,7 +52,7 @@ void recordsAndCountsEventsAboutAClusterScopedResourceInTheDefaultNamespace() { await() .untilAsserted( () -> { - var events = eventsForTestResource(); + var events = eventsAbout(uid); assertThat(events).hasSize(1); assertThat(events.get(0).getReason()) .isEqualTo(ClusterScopedEventRecorderReconciler.REASON); @@ -60,13 +61,13 @@ void recordsAndCountsEventsAboutAClusterScopedResourceInTheDefaultNamespace() { }); } - private List eventsForTestResource() { + private List eventsAbout(String uid) { return extension .getKubernetesClient() .v1() .events() .inNamespace("default") - .withField("involvedObject.name", TEST_RESOURCE_NAME) + .withField("involvedObject.uid", uid) .list() .getItems(); } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/eventrecorder/ClusterScopedKubeSystemEventRecorderIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/eventrecorder/ClusterScopedKubeSystemEventRecorderIT.java new file mode 100644 index 0000000000..eebe152fd3 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/eventrecorder/ClusterScopedKubeSystemEventRecorderIT.java @@ -0,0 +1,87 @@ +/* + * Copyright Java Operator SDK 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 io.javaoperatorsdk.operator.baseapi.eventrecorder; + +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.Event; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.baseapi.clusterscopedresource.ClusterScopedCustomResource; +import io.javaoperatorsdk.operator.baseapi.clusterscopedresource.ClusterScopedCustomResourceSpec; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +class ClusterScopedKubeSystemEventRecorderIT { + + public static final String TEST_RESOURCE_NAME = "event-recorder-test2"; + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder() + .withReconciler(new ClusterScopedEventRecorderReconciler()) + .withConfigurationService(o -> o.withClusterScopedEventNamespace("kube-system")) + .build(); + + @Test + void recordsAndCountsEventsAboutAClusterScopedResourceInTheKubeSystemNamespace() { + var resource = extension.create(testResource()); + var uid = resource.getMetadata().getUid(); + await().untilAsserted(() -> assertThat(eventsAbout(uid)).hasSize(1)); + + resource.getMetadata().setAnnotations(Map.of("reconcile", "again")); + extension.replace(resource); + + await() + .untilAsserted( + () -> { + var events = eventsAbout(uid); + assertThat(events).hasSize(1); + assertThat(events.get(0).getReason()) + .isEqualTo(ClusterScopedEventRecorderReconciler.REASON); + assertThat(events.get(0).getInvolvedObject().getNamespace()).isNull(); + assertThat(events.get(0).getEventTime()).isNotNull(); + assertThat(events.get(0).getAction()) + .isEqualTo(ClusterScopedEventRecorderReconciler.REASON); + assertThat(events.get(0).getCount()).isGreaterThanOrEqualTo(2); + }); + } + + private List eventsAbout(String uid) { + return extension + .getKubernetesClient() + .v1() + .events() + .inNamespace("kube-system") + .withField("involvedObject.uid", uid) + .list() + .getItems(); + } + + ClusterScopedCustomResource testResource() { + var resource = new ClusterScopedCustomResource(); + resource.setMetadata(new ObjectMetaBuilder().withName(TEST_RESOURCE_NAME).build()); + resource.setSpec(new ClusterScopedCustomResourceSpec()); + resource.getSpec().setTargetNamespace(extension.getNamespace()); + resource.getSpec().setData("data"); + return resource; + } +} From 692f91444390a779ca9191e3ef125abd5bc78097 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Wed, 4 Nov 2026 14:59:58 -0800 Subject: [PATCH 4/5] test: record events against a real API server Adds DefaultEventRecorderIntegrationTest, which uses kube-api-test to check that events are counted, aggregated and accepted or rejected by a real API server: namespaced and cluster scoped resources, the default and kube-system namespaces, a rejected namespace, long names. --- .../DefaultEventRecorderIntegrationTest.java | 235 ++++++++++++++++++ 1 file changed, 235 insertions(+) create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorderIntegrationTest.java diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorderIntegrationTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorderIntegrationTest.java new file mode 100644 index 0000000000..9c60ac6b4e --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorderIntegrationTest.java @@ -0,0 +1,235 @@ +/* + * Copyright Java Operator SDK 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 io.javaoperatorsdk.operator.api.event; + +import java.util.List; +import java.util.Optional; +import java.util.UUID; + +import org.junit.jupiter.api.Test; + +import io.fabric8.kubeapitest.junit.EnableKubeAPIServer; +import io.fabric8.kubernetes.api.model.ConfigMapBuilder; +import io.fabric8.kubernetes.api.model.Event; +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.NamespaceBuilder; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.KubernetesClientException; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.Context; + +import static io.javaoperatorsdk.operator.api.config.LeaderElectionConfigurationBuilder.aLeaderElectionConfiguration; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * Records events against a real API server, which validates and aggregates them as in a cluster. + */ +@EnableKubeAPIServer +class DefaultEventRecorderIntegrationTest { + + static KubernetesClient client; + + @Test + void countsRepeatedEventsAboutANamespacedResource() { + var configMap = configMap("test1"); + var context = context(configMap, "default"); + + recorder().record(EventRecord.normal("Ready", "all good"), context); + recorder().record(EventRecord.normal("Ready", "all good"), context); + + var events = eventsAbout(configMap, configMap.getMetadata().getNamespace()); + assertThat(events).hasSize(1); + assertThat(events.get(0).getCount()).isEqualTo(2); + } + + @Test + void countsRepeatedEventsAboutAClusterScopedResourceInTheDefaultNamespace() { + var namespace = clusterScoped(); + var context = context(namespace, "default"); + + recorder().record(EventRecord.normal("Ready", "all good"), context); + recorder().record(EventRecord.normal("Ready", "all good"), context); + + var events = eventsAbout(namespace, "default"); + assertThat(events).hasSize(1); + assertThat(events.get(0).getCount()).isEqualTo(2); + assertThat(events.get(0).getEventTime()).isNull(); + } + + @Test + void countsRepeatedEventsAboutAClusterScopedResourceInTheKubeSystemNamespace() { + var namespace = clusterScoped(); + var context = context(namespace, "kube-system"); + + recorder().record(EventRecord.normal("Ready", "all good"), context); + recorder().record(EventRecord.normal("Ready", "all good"), context); + + var events = eventsAbout(namespace, "kube-system"); + assertThat(events).hasSize(1); + assertThat(events.get(0).getCount()).isEqualTo(2); + assertThat(events.get(0).getEventTime()).isNotNull(); + assertThat(events.get(0).getAction()).isEqualTo("Ready"); + } + + @Test + void clusterScopedEventsInOtherNamespacesAreRejectedByTheApiServer() { + client + .namespaces() + .resource( + new NamespaceBuilder().withNewMetadata().withName("operator-ns").endMetadata().build()) + .serverSideApply(); + var namespace = clusterScoped(); + var context = context(namespace, "operator-ns"); + var event = recorder().toEvent(context, EventRecord.normal("Ready", "all good")); + + assertThatThrownBy(() -> new DefaultEventSink(client).emit(event, context)) + .isInstanceOf(KubernetesClientException.class); + assertThat(eventsAbout(namespace, "operator-ns")).isEmpty(); + } + + @Test + void aggregatingOntoANamedEventTakesTheTypeReasonAndActionOfTheLatestOccurrence() { + var configMap = configMap("test2"); + var context = context(configMap, "default"); + var recorder = namedRecorder("named-event-" + UUID.randomUUID()); + + recorder.record( + EventRecord.builder() + .type(EventType.NORMAL) + .reason("Ready") + .message("all good") + .action("Create") + .build(), + context); + recorder.record(EventRecord.warning("Degraded", "replica down"), context); + + var events = eventsAbout(configMap, "default"); + assertThat(events).hasSize(1); + var event = events.get(0); + assertThat(event.getCount()).isEqualTo(2); + assertThat(event.getType()).isEqualTo("Warning"); + assertThat(event.getReason()).isEqualTo("Degraded"); + assertThat(event.getMessage()).isEqualTo("replica down"); + assertThat(event.getAction()).isNull(); + } + + @Test + void aggregatingOntoANamedEventWithAnEventTimeTakesTheTypeReasonAndActionOfTheLatestOccurrence() { + var namespace = clusterScoped(); + var context = context(namespace, "kube-system"); + var recorder = namedRecorder("named-event-" + UUID.randomUUID()); + + recorder.record(EventRecord.normal("Ready", "all good"), context); + recorder.record(EventRecord.warning("Degraded", "replica down"), context); + + var events = eventsAbout(namespace, "kube-system"); + assertThat(events).hasSize(1); + var event = events.get(0); + assertThat(event.getCount()).isEqualTo(2); + assertThat(event.getType()).isEqualTo("Warning"); + assertThat(event.getReason()).isEqualTo("Degraded"); + assertThat(event.getAction()).isEqualTo("Degraded"); + } + + @Test + void longSuppliedNamesAreAcceptedAndStayDistinct() { + var configMap = configMap("test3"); + var context = context(configMap, "default"); + var prefix = "a".repeat(219) + "-"; + var recorder = + DefaultEventRecorder.builder(new DefaultEventSink(client)) + .namingStrategy( + (regarding, record) -> Optional.of(prefix + record.reason().toLowerCase() + "x")) + .build(); + + recorder.record(EventRecord.normal("First", "first"), context); + recorder.record(EventRecord.normal("Second", "second"), context); + + var events = eventsAbout(configMap, "default"); + assertThat(events).hasSize(2); + assertThat(events) + .allSatisfy(event -> assertThat(event.getMetadata().getName()).hasSizeLessThan(254)); + } + + @Test + void generatedNamesShortenedAtASeparatorAreAccepted() { + var configMap = configMap("a".repeat(219) + "-" + "b".repeat(10)); + var context = context(configMap, "default"); + + recorder().record(EventRecord.normal("Ready", "all good"), context); + + assertThat(eventsAbout(configMap, "default")).hasSize(1); + } + + private static DefaultEventRecorder recorder() { + return new DefaultEventRecorder(new DefaultEventSink(client)); + } + + private static DefaultEventRecorder namedRecorder(String name) { + return DefaultEventRecorder.builder(new DefaultEventSink(client)) + .namingStrategy((regarding, record) -> Optional.of(name)) + .build(); + } + + private static List eventsAbout(HasMetadata resource, String namespace) { + return client + .v1() + .events() + .inNamespace(namespace) + .withField("involvedObject.uid", resource.getMetadata().getUid()) + .list() + .getItems(); + } + + private static HasMetadata configMap(String name) { + return new ConfigMapBuilder() + .withNewMetadata() + .withName(name) + .withNamespace("default") + .withUid(UUID.randomUUID().toString()) + .endMetadata() + .build(); + } + + private static HasMetadata clusterScoped() { + return new NamespaceBuilder() + .withNewMetadata() + .withName("ns-" + UUID.randomUUID()) + .withUid(UUID.randomUUID().toString()) + .endMetadata() + .build(); + } + + @SuppressWarnings({"rawtypes", "unchecked"}) + private static Context context(HasMetadata primaryResource, String clusterScopedNamespace) { + var configurationService = mock(ConfigurationService.class); + when(configurationService.getLeaderElectionConfiguration()) + .thenReturn( + Optional.of(aLeaderElectionConfiguration("lease").withIdentity("pod-1").build())); + when(configurationService.clusterScopedEventNamespace()).thenReturn(clusterScopedNamespace); + ControllerConfiguration controllerConfiguration = mock(ControllerConfiguration.class); + when(controllerConfiguration.getName()).thenReturn("testcontroller"); + when(controllerConfiguration.getConfigurationService()).thenReturn(configurationService); + Context context = mock(Context.class); + when(context.getPrimaryResource()).thenReturn(primaryResource); + when(context.getControllerConfiguration()).thenReturn(controllerConfiguration); + return context; + } +} From 1fa8565aa3042affb76e621a6818760d4d6bf489 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 5 Nov 2026 18:48:47 +0100 Subject: [PATCH 5/5] Apply batched suggestions from code review Co-authored-by: Antonio <122279781+afalhambra-hivemq@users.noreply.github.com> --- .../operator/api/event/DefaultEventRecorder.java | 5 +++-- .../api/event/EventRecorderRegressionTest.java | 12 ++++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorder.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorder.java index dea3b075df..9bd54ef96c 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorder.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorder.java @@ -268,10 +268,11 @@ protected Event toEvent(Context context, EventRecord record) { .endOwnerReference() .endMetadata(); } - record.action().ifPresent(builder::withAction); + var action = record.action().filter(a -> !a.isBlank()); + action.ifPresent(builder::withAction); if (requiresEventTime(regarding, namespace)) { builder.withEventTime(new MicroTime(EVENT_TIME_FORMAT.format(Instant.now()))); - if (record.action().isEmpty()) { + if (action.isEmpty()) { builder.withAction(record.reason()); } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderRegressionTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderRegressionTest.java index 367d93a707..582b379859 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderRegressionTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderRegressionTest.java @@ -102,11 +102,11 @@ void clusterScopedEventsInTheDefaultNamespaceDoNotCarryAnEventTime() { /** * A naming strategy (here the one EventNamingStrategy's javadoc suggests, a fixed suffix per * object) makes the name the aggregation identity on its own, so records with different type and - * reason resolve to the same event. DefaultEventSink then only patches count, lastTimestamp and - * message: the Warning is published under the type and reason of the earlier Normal event. + * reason resolve to the same event. DefaultEventSink patches type and reason along with count, + * lastTimestamp and message, so the event describes the latest occurrence. */ @Test - void aggregatingOntoANamedEventKeepsTheTypeAndReasonOfTheFirstOccurrence() { + void aggregatingOntoANamedEventTakesTheTypeAndReasonOfTheLatestOccurrence() { var stored = new AtomicReference(); var patched = new AtomicReference(); var sink = new DefaultEventSink(mockClient(stored, patched)); @@ -212,11 +212,11 @@ void shortenedGeneratedNamesDoNotEndInASeparatorBeforeTheHash() { } /** - * Supplied names are truncated to 253 characters, which silently maps distinct names onto the - * same event, so two different events of a long-named object are aggregated as one. + * Supplied names longer than 253 characters are shortened with a hash of the full name, so two + * different events of a long-named object are not merged into one. */ @Test - void truncatingSuppliedNamesMergesDistinctEvents() { + void longSuppliedNamesStayDistinct() { var recorder = DefaultEventRecorder.builder((event, context) -> emitted.add(event)) .namingStrategy(