Warn about duplicated events received from GitHub via Admin Monitor

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,11 @@ public List<Item> reRegisterAllHooks() {
     @SuppressWarnings("unused")
     @RequirePostWithGHHookPayload
     public void doIndex(@NonNull @GHEventHeader GHEvent event, @NonNull @GHEventPayload String payload) {
+        var currentRequest = Stapler.getCurrentRequest2();
+        // during unit tests currentRequest is null
+        String eventGuid = currentRequest != null ? currentRequest.getHeader(X_GITHUB_DELIVERY) : null;
         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,40 @@
+package org.jenkinsci.plugins.github.admin;
+
+import hudson.Extension;
+import hudson.model.AdministrativeMonitor;
+import jenkins.model.Jenkins;
+import org.jenkinsci.plugins.github.Messages;
+import org.jenkinsci.plugins.github.webhook.subscriber.DuplicateEventsSubscriber;
+
+@SuppressWarnings("unused")
+@Extension
+public class GitHubDuplicateEventsMonitor extends AdministrativeMonitor {
+
+    @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();
+    }
+
+    @Override
+    public boolean isActivated() {
+        return !DuplicateEventsSubscriber.getDuplicateEventCounts().isEmpty();
+    }
+
+    @Override
+    public boolean hasRequiredPermission() {
+        return Jenkins.get().hasPermission(Jenkins.SYSTEM_READ);
+    }
+
+    @Override
+    public void checkRequiredPermission() {
+        Jenkins.get().checkPermission(Jenkins.SYSTEM_READ);
+    }
+}

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

@@ -18,16 +18,37 @@ public class GHSubscriberEvent extends SCMEvent<String> {
      */
     private final GHEvent ghEvent;
 
+    private final String eventGuid;
+
     /**
      * Constructs a new {@link GHSubscriberEvent}.
      *
      * @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.
      */
+    @Deprecated
     public GHSubscriberEvent(@CheckForNull String origin, @NonNull GHEvent ghEvent, @NonNull String payload) {
         super(Type.UPDATED, payload, origin);
         this.ghEvent = ghEvent;
+        this.eventGuid = null;
+    }
+
+    /**
+     * Constructs a new {@link GHSubscriberEvent}.
+     * @param eventGuid the globally unique identifier (GUID) to identify the event.
+     * @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 eventGuid,
+            @CheckForNull String origin,
+            @NonNull GHEvent ghEvent,
+            @NonNull String payload) {
+        super(Type.UPDATED, payload, origin);
+        this.ghEvent = ghEvent;
+        this.eventGuid = eventGuid;
     }
 
     /**
@@ -39,4 +60,8 @@ public GHEvent getGHEvent() {
         return ghEvent;
     }
 
+    @CheckForNull
+    public String getEventGuid() {
+        return eventGuid;
+    }
 }

src/main/java/org/jenkinsci/plugins/github/webhook/subscriber/DuplicateEventsSubscriber.java

@@ -0,0 +1,135 @@
+package org.jenkinsci.plugins.github.webhook.subscriber;
+
+import static com.google.common.collect.Sets.immutableEnumSet;
+
+import com.google.common.annotations.VisibleForTesting;
+import edu.umd.cs.findbugs.annotations.Nullable;
+import hudson.Extension;
+import hudson.model.Item;
+import hudson.model.PeriodicWork;
+import java.time.Instant;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.TimeUnit;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+import java.util.stream.Collectors;
+import org.jenkinsci.plugins.github.extension.GHEventsSubscriber;
+import org.jenkinsci.plugins.github.extension.GHSubscriberEvent;
+import org.kohsuke.github.GHEvent;
+
+@Extension
+public final class DuplicateEventsSubscriber extends GHEventsSubscriber {
+
+    private static final Logger LOGGER = Logger.getLogger(DuplicateEventsSubscriber.class.getName());
+    private static final Map<String, EventCountWithTTL> EVENT_COUNTS_TRACKER = new ConcurrentHashMap<>();
+    private static final long TTL_MILLIS = TimeUnit.MINUTES.toMillis(10);
+
+    @VisibleForTesting
+    record EventCountWithTTL(int count, long lastUpdated) { }
+
+    /**
+     * This method is retained only because it is an abstract method.
+     * It is no longer used to determine event delivery to subscribers.
+     * Instead, {@link #isInterestedIn} and {@link #events()} are now used to
+     * decide whether an event should be delivered to a subscriber.
+     * @see com.cloudbees.jenkins.GitHubWebHook#doIndex
+     */
+    @Override
+    protected boolean isApplicable(@Nullable Item item) {
+        return false;
+    }
+
+    /**
+     * Subscribe to events that can trigger some kind of action within Jenkins, such as repository scan, build launch,
+     * etc.
+     * <p>
+     * There are about 63 specific events mentioned in the {@link GHEvent} enum, but not all of them are useful in
+     * Jenkins. Subscribing to and tracking them in duplicates tracker would cause an increase in memory usage, and
+     * those events' occurrences are likely larger than those that cause an action in Jenkins.
+     * <p>
+     * <a href="https://docs.github.com/en/webhooks/webhook-events-and-payloads">
+     *     Documentation reference (as also referenced in {@link GHEvent})</a>
+     * */
+    @Override
+    protected Set<GHEvent> events() {
+        return immutableEnumSet(
+                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();
+        if (eventGuid == null) {
+            return;
+        }
+        long now = Instant.now().toEpochMilli();
+        EVENT_COUNTS_TRACKER.compute(
+                eventGuid, (key, value) -> new EventCountWithTTL(value == null ? 1 : value.count() + 1, now));
+    }
+
+    public static Map<String, Integer> getDuplicateEventCounts() {
+        return EVENT_COUNTS_TRACKER.entrySet().stream()
+                .filter(entry -> entry.getValue().count() > 1)
+                .collect(Collectors.toMap(
+                        Map.Entry::getKey, entry -> entry.getValue().count()));
+    }
+
+    @VisibleForTesting
+    static void cleanUpOldEntries(long now) {
+        EVENT_COUNTS_TRACKER
+                .entrySet()
+                .removeIf(entry -> (now - entry.getValue().lastUpdated()) > TTL_MILLIS);
+    }
+
+    /**
+     * Only for testing purpose
+     */
+    @VisibleForTesting
+    static Map<String, EventCountWithTTL> getEventCountsTracker() {
+        return new ConcurrentHashMap<>(EVENT_COUNTS_TRACKER);
+    }
+
+    @SuppressWarnings("unused")
+    @Extension
+    public static class EventCountTrackerCleanup extends PeriodicWork {
+
+        /**
+         * At present, as the {@link #TTL_MILLIS} is set to 10 minutes, we consider half of it for cleanup.
+         * This recurrence period is chosen to balance removing stale entries from accumulating in memory vs.
+         * additional load on Jenkins due to a new periodic job execution.
+         * <p>
+         * If we want to keep the stale entries to a minimum, there appear to be three different ways to achieve this:
+         * <ul>
+         *     <li>Increasing the frequency of this periodic task, which will contribute to load</li>
+         *     <li>Event-driven cleanup: for every event from GH, clean up expired entries (need to use
+         *     better data structures and algorithms; simply calling the current {@link #cleanUpOldEntries} will
+         *     result in {@code O(n)} for every {@code insert}, which may lead to slowness in this hot code path)</li>
+         *     <li>Adaptive cleanup: based on the number of stale entries being seen, the system itself will adjust
+         *     the periodic task's frequency (if such adaptive scheduling does not already exist in Jenkins core,
+         *     this wouldn't be a good idea to implement here)
+         *     </li>
+         * </ul>
+         */
+        @Override
+        public long getRecurrencePeriod() {
+            return TTL_MILLIS / 2;
+        }
+
+        @Override
+        protected void doRun() {
+            LOGGER.log(
+                    Level.FINE,
+                    () -> "Cleaning up entries older than " + TTL_MILLIS + "ms, remaining entries: "
+                            + EVENT_COUNTS_TRACKER.size());
+            cleanUpOldEntries(Instant.now().toEpochMilli());
+        }
+    }
+}

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

@@ -11,3 +11,8 @@ 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 job executions.

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/org/jenkinsci/plugins/github/admin/GitHubDuplicateEventsMonitorTest.java

@@ -0,0 +1,98 @@
+package org.jenkinsci.plugins.github.admin;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.not;
+
+import java.io.IOException;
+import java.net.URL;
+
+import org.htmlunit.HttpMethod;
+
+import org.htmlunit.WebRequest;
+import org.jenkinsci.plugins.github.Messages;
+import org.jenkinsci.plugins.github.extension.GHEventsSubscriber;
+import org.jenkinsci.plugins.github.webhook.subscriber.DuplicateEventsSubscriber;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.JenkinsRule.WebClient;
+import org.mockito.Mockito;
+
+public class GitHubDuplicateEventsMonitorTest {
+
+    @Rule
+    public JenkinsRule j = new JenkinsRule();
+    private WebClient wc;
+
+    @Before
+    public void setUp() throws Exception {
+        j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
+        wc = j.createWebClient();
+        wc.login("admin", "admin");
+    }
+
+    @Test
+    public void testAdminMonitorDisplaysForDuplicateEvents() throws Exception {
+        try (var mockSubscriber = Mockito.mockStatic(GHEventsSubscriber.class)) {
+            var subscribers = j.jenkins.getExtensionList(GHEventsSubscriber.class);
+            var nonDuplicateSubscribers = subscribers.stream()
+                                                     .filter(e -> !(e instanceof DuplicateEventsSubscriber))
+                                                     .toList();
+            nonDuplicateSubscribers.forEach(subscribers::remove);
+            mockSubscriber.when(GHEventsSubscriber::all).thenReturn(subscribers);
+
+            // to begin with, monitor doesn't show automatically
+            assertMonitorNotDisplayed();
+
+            // normal case: unique events don't cause admin monitor
+            sendGHEvents(wc, "event1");
+            sendGHEvents(wc, "event2");
+            assertMonitorNotDisplayed();
+
+            // duplicate events cause admin monitor
+            sendGHEvents(wc, "event3");
+            sendGHEvents(wc, "event3");
+            assertMonitorDisplayed();
+        }
+    }
+
+    private void sendGHEvents(WebClient wc, String eventGuid) throws IOException {
+        wc.addRequestHeader("Content-Type", "application/json");
+        wc.addRequestHeader("X-GitHub-Delivery", eventGuid);
+        wc.addRequestHeader("X-Github-Event", "push");
+        String url = j.getURL() + "/github-webhook/";
+        String content = """
+                {
+                    "repository":
+                    {
+                        "url": "http://dummy",
+                        "html_url": "http://dummy"
+                    },
+                    "pusher":
+                    {
+                        "name": "dummy",
+                        "email": "[email protected]"
+                    }
+                }
+            """;
+        var webRequest = new WebRequest(new URL(url), HttpMethod.POST);
+        webRequest.setRequestBody(content);
+        wc.getPage(webRequest).getWebResponse();
+    }
+
+    private void assertMonitorNotDisplayed() throws IOException {
+        String manageUrl = j.getURL() + "/manage";
+        assertThat(
+            wc.getPage(manageUrl).getWebResponse().getContentAsString(),
+            not(containsString(Messages.duplicate_events_administrative_monitor_blurb())));
+    }
+
+    private void assertMonitorDisplayed() throws IOException {
+        String manageUrl = j.getURL() + "/manage";
+        assertThat(
+            wc.getPage(manageUrl).getWebResponse().getContentAsString(),
+            containsString(Messages.duplicate_events_administrative_monitor_blurb()));
+    }
+}