Warn about duplicated events received from GitHub via Admin Monitor

pom.xml

@@ -121,6 +121,11 @@
             <artifactId>instance-identity</artifactId>
         </dependency>
 
+        <dependency>
+            <groupId>io.jenkins.plugins</groupId>
+            <artifactId>caffeine-api</artifactId>
+        </dependency>
+
         <!--TEST DEPS-->
 
         <!-- 4.5.3 used by rest-assured -->

src/main/java/com/cloudbees/jenkins/GitHubWebHook.java

@@ -52,6 +52,12 @@ public class GitHubWebHook implements UnprotectedRootAction {
     // headers used for testing the endpoint configuration
     public static final String URL_VALIDATION_HEADER = "X-Jenkins-Validation";
     public static final String X_INSTANCE_IDENTITY = "X-Instance-Identity";
+    /**
+     * X-GitHub-Delivery: A globally unique identifier (GUID) to identify the event.
+     * @see <a href="https://docs.github.com/en/webhooks/webhook-events-and-payloads#delivery-headers">Delivery
+     * headers</a>
+     */
+    public static final String X_GITHUB_DELIVERY = "X-GitHub-Delivery";
 
     private final transient SequentialExecutionQueue queue = new SequentialExecutionQueue(threadPoolForRemoting);
 
@@ -117,8 +123,10 @@ public List<Item> reRegisterAllHooks() {
     @SuppressWarnings("unused")
     @RequirePostWithGHHookPayload
     public void doIndex(@NonNull @GHEventHeader GHEvent event, @NonNull @GHEventPayload String payload) {
+        var currentRequest = Stapler.getCurrentRequest2();
+        String eventGuid = currentRequest.getHeader(X_GITHUB_DELIVERY);
         GHSubscriberEvent subscriberEvent =
-                new GHSubscriberEvent(SCMEvent.originOf(Stapler.getCurrentRequest2()), event, payload);
+                new GHSubscriberEvent(eventGuid, SCMEvent.originOf(currentRequest), event, payload);
         from(GHEventsSubscriber.all())
                 .filter(isInterestedIn(event))
                 .transform(processEvent(subscriberEvent)).toList();

src/main/java/org/jenkinsci/plugins/github/admin/GitHubDuplicateEventsMonitor.java

@@ -0,0 +1,215 @@
+package org.jenkinsci.plugins.github.admin;
+
+import java.time.Duration;
+import java.time.Instant;
+import java.util.Set;
+import java.util.logging.Logger;
+import java.util.stream.Collectors;
+
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.github.benmanes.caffeine.cache.Ticker;
+import com.google.common.annotations.VisibleForTesting;
+
+import hudson.Extension;
+import hudson.model.AdministrativeMonitor;
+import hudson.model.Item;
+import jenkins.model.Jenkins;
+import org.jenkinsci.plugins.github.Messages;
+import org.jenkinsci.plugins.github.extension.GHEventsSubscriber;
+import org.jenkinsci.plugins.github.extension.GHSubscriberEvent;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
+import org.kohsuke.github.GHEvent;
+import org.kohsuke.stapler.HttpResponse;
+import org.kohsuke.stapler.WebMethod;
+import org.kohsuke.stapler.json.JsonHttpResponse;
+import org.kohsuke.stapler.verb.GET;
+import edu.umd.cs.findbugs.annotations.Nullable;
+import net.sf.json.JSONObject;
+
+@SuppressWarnings("unused")
+@Extension
+public class GitHubDuplicateEventsMonitor extends AdministrativeMonitor {
+
+    @VisibleForTesting
+    static final String LAST_DUPLICATE_CLICK_HERE_ANCHOR_ID = GitHubDuplicateEventsMonitor.class.getName()
+                                                              + ".last-duplicate";
+
+    @Override
+    public String getDisplayName() {
+        return Messages.duplicate_events_administrative_monitor_displayname();
+    }
+
+    public String getDescription() {
+        return Messages.duplicate_events_administrative_monitor_description();
+    }
+
+    public String getBlurb() {
+        return Messages.duplicate_events_administrative_monitor_blurb(
+            LAST_DUPLICATE_CLICK_HERE_ANCHOR_ID, this.getLastDuplicateUrl());
+    }
+
+    @VisibleForTesting
+    String getLastDuplicateUrl() {
+        return this.getUrl() + "/" + "last-duplicate.json";
+    }
+
+    @Override
+    public boolean isActivated() {
+        return DuplicateEventsSubscriber.isDuplicateEventSeen();
+    }
+
+    @Override
+    public boolean hasRequiredPermission() {
+        return Jenkins.get().hasPermission(Jenkins.SYSTEM_READ);
+    }
+
+    @Override
+    public void checkRequiredPermission() {
+        Jenkins.get().checkPermission(Jenkins.SYSTEM_READ);
+    }
+
+    @GET
+    @WebMethod(name = "last-duplicate.json")
+    public HttpResponse doGetLastDuplicatePayload() {
+        Jenkins.get().checkPermission(Jenkins.SYSTEM_READ);
+        JSONObject data;
+        var lastDuplicate = DuplicateEventsSubscriber.getLastDuplicate();
+        if (lastDuplicate != null) {
+            data = JSONObject.fromObject(lastDuplicate.ghSubscriberEvent().getPayload());
+        } else {
+            data = getLastDuplicateNoEventPayload();
+        }
+        return new JsonHttpResponse(data, 200);
+    }
+
+    @VisibleForTesting
+    static JSONObject getLastDuplicateNoEventPayload() {
+        return new JSONObject().accumulate("payload", "No duplicate events seen yet");
+    }
+
+    /**
+     * Tracks duplicate {@link GHEvent} triggering actions in Jenkins.
+     * Events are tracked for 10 minutes, with the last detected duplicate reference retained for up to 24 hours
+     * (see {@link #isDuplicateEventSeen}).
+     * <p>
+     * Duplicates are stored in-memory only, so a controller restart clears all entries as if none existed.
+     * Persistent storage is omitted for simplicity, since webhook misconfigurations would likely cause new duplicates.
+     */
+    @Extension
+    public static final class DuplicateEventsSubscriber extends GHEventsSubscriber {
+
+        private static final Logger LOGGER = Logger.getLogger(DuplicateEventsSubscriber.class.getName());
+
+        private static Ticker ticker = Ticker.systemTicker();
+        /**
+         * Caches GitHub event GUIDs for 10 minutes to track recent events to detect duplicates.
+         * <p>
+         * Only the keys (event GUIDs) are relevant, as Caffeine automatically handles expiration based
+         * on insertion time; the value is irrelevant, we put {@link #DUMMY}, as Caffeine doesn't provide any
+         * Set structures.
+         * <p>
+         * Maximum cache size is set to 24k so it doesn't grow unbound (approx. 1MB). Each key takes 36 bytes, and
+         * timestamp (assuming caffeine internally keeps long) takes 8 bytes; total of 44 bytes
+         * per entry. So the maximum memory consumed by this cache is 24k * 44 = 1056k = 1.056 MB.
+         */
+        private static final Cache<String, Object> EVENT_TRACKER = Caffeine.newBuilder()
+                                                                           .maximumSize(24_000L)
+                                                                           .expireAfterWrite(Duration.ofMinutes(10))
+                                                                           .ticker(() -> ticker.read())
+                                                                           .build();
+        private static final Object DUMMY = new Object();
+
+        private static volatile TrackedDuplicateEvent lastDuplicate;
+        public record TrackedDuplicateEvent(
+            String eventGuid, Instant lastUpdated, GHSubscriberEvent ghSubscriberEvent) { }
+        private static final Duration TWENTY_FOUR_HOURS = Duration.ofHours(24);
+
+        @VisibleForTesting
+        @Restricted(NoExternalUse.class)
+        static void setTicker(Ticker testTicker) {
+            ticker = testTicker;
+        }
+
+        /**
+         * This subscriber is not applicable to any item
+         *
+         * @param item ignored
+         * @return always false
+         */
+        @Override
+        protected boolean isApplicable(@Nullable Item item) {
+            return false;
+        }
+
+        /**
+         * {@inheritDoc}
+         * <p>
+         * Subscribes to events that trigger actions in Jenkins, such as repository scans or builds.
+         * <p>
+         * The {@link GHEvent} enum defines about 63 events, but not all are relevant to Jenkins.
+         * Tracking unnecessary events increases memory usage, and they occur more frequently than those triggering any
+         * work.
+         * <p>
+         * <a href="https://docs.github.com/en/webhooks/webhook-events-and-payloads">
+         *     Documentation reference (also referenced in {@link GHEvent})</a>
+         */
+        @Override
+        protected Set<GHEvent> events() {
+            return Set.of(
+                    GHEvent.CHECK_RUN, // associated with GitHub action Re-run button to trigger build
+                    GHEvent.CHECK_SUITE, // associated with GitHub action Re-run button to trigger build
+                    GHEvent.CREATE, // branch or tag creation
+                    GHEvent.DELETE, // branch or tag deletion
+                    GHEvent.PULL_REQUEST, // PR creation (also PR close or merge)
+                    GHEvent.PUSH // commit push
+                );
+        }
+
+        @Override
+        protected void onEvent(final GHSubscriberEvent event) {
+            String eventGuid = event.getEventGuid();
+            LOGGER.fine(() -> "Received event with GUID: " + eventGuid);
+            if (eventGuid == null) {
+                return;
+            }
+            if (EVENT_TRACKER.getIfPresent(eventGuid) != null) {
+                lastDuplicate = new TrackedDuplicateEvent(eventGuid, getNow(), event);
+            }
+            EVENT_TRACKER.put(eventGuid, DUMMY);
+        }
+
+        /**
+         * Checks if a duplicate event was recorded in the past 24 hours.
+         * <p>
+         * Events are not stored for 24 hours—only the most recent duplicate is checked within this timeframe.
+         *
+         * @return {@code true} if a duplicate was seen in the last 24 hours, {@code false} otherwise.
+         */
+        public static boolean isDuplicateEventSeen() {
+            return lastDuplicate != null
+                   && Duration.between(lastDuplicate.lastUpdated(), getNow()).compareTo(TWENTY_FOUR_HOURS) < 0;
+        }
+
+        private static Instant getNow() {
+            return Instant.ofEpochSecond(0L, ticker.read());
+        }
+
+        public static TrackedDuplicateEvent getLastDuplicate() {
+            return lastDuplicate;
+        }
+
+        /**
+         * Caffeine expired keys are not removed immediately. Method returns the non-expired keys;
+         * required for the tests.
+         */
+        @VisibleForTesting
+        @Restricted(NoExternalUse.class)
+        static Set<String> getEventCountsTracker() {
+            return EVENT_TRACKER.asMap().keySet().stream()
+                                .filter(key -> EVENT_TRACKER.getIfPresent(key) != null)
+                                .collect(Collectors.toSet());
+        }
+    }
+}

src/main/java/org/jenkinsci/plugins/github/extension/GHSubscriberEvent.java

@@ -18,16 +18,32 @@ public class GHSubscriberEvent extends SCMEvent<String> {
      */
     private final GHEvent ghEvent;
 
+    private final String eventGuid;
+
+    /**
+     * @deprecated use {@link #GHSubscriberEvent(String, String, GHEvent, String)} instead.
+     */
+    @Deprecated
+    public GHSubscriberEvent(@CheckForNull String origin, @NonNull GHEvent ghEvent, @NonNull String payload) {
+        this(null, origin, ghEvent, payload);
+    }
+
     /**
      * Constructs a new {@link GHSubscriberEvent}.
-     *
+     * @param eventGuid the globally unique identifier (GUID) to identify the event; value of
+     * request header {@link com.cloudbees.jenkins.GitHubWebHook#X_GITHUB_DELIVERY}.
      * @param origin  the origin (see {@link SCMEvent#originOf(HttpServletRequest)}) or {@code null}.
      * @param ghEvent the type of event received from GitHub.
      * @param payload the event payload.
      */
-    public GHSubscriberEvent(@CheckForNull String origin, @NonNull GHEvent ghEvent, @NonNull String payload) {
+    public GHSubscriberEvent(
+            @CheckForNull String eventGuid,
+            @CheckForNull String origin,
+            @NonNull GHEvent ghEvent,
+            @NonNull String payload) {
         super(Type.UPDATED, payload, origin);
         this.ghEvent = ghEvent;
+        this.eventGuid = eventGuid;
     }
 
     /**
@@ -39,4 +55,8 @@ public GHEvent getGHEvent() {
         return ghEvent;
     }
 
+    @CheckForNull
+    public String getEventGuid() {
+        return eventGuid;
+    }
 }

src/main/resources/org/jenkinsci/plugins/github/Messages.properties

@@ -11,3 +11,9 @@ github.trigger.check.method.warning.details=The webhook for repo {0}/{1} on {2}
   More info can be found on the global configuration page. This message will be dismissed if Jenkins receives \
   a PING event from repo webhook or if you add the repo to the ignore list in the global configuration.
 unknown.error=Unknown error
+duplicate.events.administrative.monitor.displayname=GitHub Duplicate Events
+duplicate.events.administrative.monitor.description=Warns about duplicate events received from GitHub.
+duplicate.events.administrative.monitor.blurb=Duplicate events were received from GitHub, possibly due to \
+  misconfiguration (e.g., multiple webhooks targeting the same Jenkins controller at the repository or organization \
+  level), potentially causing redundant builds or at least wasted work. \
+  <a id="{0}" href="{1}" target="_blank">Click here</a> to inspect the last tracked duplicate event payload.

src/main/resources/org/jenkinsci/plugins/github/admin/GitHubDuplicateEventsMonitor/description.jelly

@@ -0,0 +1,4 @@
+<?jelly escape-by-default='true'?>
+<j:jelly xmlns:j="jelly:core">
+    <j:out value="${it.description}"/>
+</j:jelly>

src/main/resources/org/jenkinsci/plugins/github/admin/GitHubDuplicateEventsMonitor/message.jelly

@@ -0,0 +1,9 @@
+<?jelly escape-by-default='true'?>
+<j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form">
+    <div class="alert alert-warning">
+        <form method="post" action="${rootURL}/${it.url}/disable" name="${it.id}">
+            <f:submit primary="false" clazz="jenkins-!-destructive-color" name="no" value="${%Dismiss}"/>
+        </form>
+        <j:out value="${it.blurb}"/>
+    </div>
+</j:jelly>

src/test/java/com/cloudbees/jenkins/GitHubWebHookTest.java

@@ -3,7 +3,6 @@
 import com.google.inject.Inject;
 
 import hudson.model.Item;
-import hudson.model.Job;
 
 import org.jenkinsci.plugins.github.extension.GHEventsSubscriber;
 import org.junit.Before;
@@ -12,6 +11,11 @@
 import org.jvnet.hudson.test.JenkinsRule;
 import org.jvnet.hudson.test.TestExtension;
 import org.kohsuke.github.GHEvent;
+import org.kohsuke.stapler.Stapler;
+import org.kohsuke.stapler.StaplerRequest2;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.MockitoAnnotations;
 
 import java.util.Set;
 
@@ -41,31 +45,45 @@ public class GitHubWebHookTest {
     @Inject
     private ThrowablePullRequestSubscriber throwablePullRequestSubscriber;
 
+    @Mock
+    private StaplerRequest2 req2;
+
     @Before
     public void setUp() throws Exception {
+        MockitoAnnotations.openMocks(this);
         jenkins.getInstance().getInjector().injectMembers(this);
     }
 
     @Test
     public void shouldCallExtensionInterestedInIssues() throws Exception {
-        new GitHubWebHook().doIndex(GHEvent.ISSUES, PAYLOAD);
-        assertThat("should get interested event", subscriber.lastEvent(), equalTo(GHEvent.ISSUES));
+        try(var mockedStapler = Mockito.mockStatic(Stapler.class)) {
+            mockedStapler.when(Stapler::getCurrentRequest2).thenReturn(req2);
+
+            new GitHubWebHook().doIndex(GHEvent.ISSUES, PAYLOAD);
+            assertThat("should get interested event", subscriber.lastEvent(), equalTo(GHEvent.ISSUES));
+        }
     }
 
     @Test
     public void shouldNotCallAnyExtensionsWithPublicEventIfNotRegistered() throws Exception {
-        new GitHubWebHook().doIndex(GHEvent.PUBLIC, PAYLOAD);
-        assertThat("should not get not interested event", subscriber.lastEvent(), nullValue());
+        try(var mockedStapler = Mockito.mockStatic(Stapler.class)) {
+            mockedStapler.when(Stapler::getCurrentRequest2).thenReturn(req2);
+
+            new GitHubWebHook().doIndex(GHEvent.PUBLIC, PAYLOAD);
+            assertThat("should not get not interested event", subscriber.lastEvent(), nullValue());
+        }
     }
 
     @Test
     public void shouldCatchThrowableOnFailedSubscriber() throws Exception {
-        new GitHubWebHook().doIndex(GHEvent.PULL_REQUEST, PAYLOAD);
-        assertThat("each extension should get event",
-                asList(
-                        pullRequestSubscriber.lastEvent(),
-                        throwablePullRequestSubscriber.lastEvent()
-                ), everyItem(equalTo(GHEvent.PULL_REQUEST)));
+        try(var mockedStapler = Mockito.mockStatic(Stapler.class)) {
+            mockedStapler.when(Stapler::getCurrentRequest2).thenReturn(req2);
+
+            new GitHubWebHook().doIndex(GHEvent.PULL_REQUEST, PAYLOAD);
+            assertThat("each extension should get event",
+                       asList(pullRequestSubscriber.lastEvent(), throwablePullRequestSubscriber.lastEvent()),
+                       everyItem(equalTo(GHEvent.PULL_REQUEST)));
+        }
     }
 
     @TestExtension