Skip to content

Commit fb8691e

Browse files
Merge branch 'master' into remove_use_beta
2 parents e7f7cbf + 6a7ad9f commit fb8691e

File tree

14 files changed

+589
-85
lines changed

14 files changed

+589
-85
lines changed

pom.xml

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
<parent>
66
<groupId>org.jenkins-ci.plugins</groupId>
77
<artifactId>plugin</artifactId>
8-
<version>5.5</version>
8+
<version>5.9</version>
99
<relativePath />
1010
</parent>
1111

@@ -47,7 +47,7 @@
4747
</issueManagement>
4848

4949
<properties>
50-
<revision>1.41.1</revision>
50+
<revision>1.43.1</revision>
5151
<changelist>-SNAPSHOT</changelist>
5252
<gitHubRepo>jenkinsci/${project.artifactId}-plugin</gitHubRepo>
5353
<!-- https://www.jenkins.io/doc/developer/plugin-development/choosing-jenkins-baseline/ -->
@@ -120,6 +120,11 @@
120120
<artifactId>instance-identity</artifactId>
121121
</dependency>
122122

123+
<dependency>
124+
<groupId>io.jenkins.plugins</groupId>
125+
<artifactId>caffeine-api</artifactId>
126+
</dependency>
127+
123128
<!--TEST DEPS-->
124129

125130
<!-- 4.5.3 used by rest-assured -->
@@ -180,9 +185,9 @@
180185

181186
<!--to mock github-->
182187
<dependency>
183-
<groupId>com.github.tomakehurst</groupId>
184-
<artifactId>wiremock-jre8-standalone</artifactId>
185-
<version>2.35.2</version>
188+
<groupId>org.wiremock</groupId>
189+
<artifactId>wiremock-standalone</artifactId>
190+
<version>3.12.1</version>
186191
<scope>test</scope>
187192
</dependency>
188193

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,12 @@ public class GitHubWebHook implements UnprotectedRootAction {
5252
// headers used for testing the endpoint configuration
5353
public static final String URL_VALIDATION_HEADER = "X-Jenkins-Validation";
5454
public static final String X_INSTANCE_IDENTITY = "X-Instance-Identity";
55+
/**
56+
* X-GitHub-Delivery: A globally unique identifier (GUID) to identify the event.
57+
* @see <a href="https://docs.github.com/en/webhooks/webhook-events-and-payloads#delivery-headers">Delivery
58+
* headers</a>
59+
*/
60+
public static final String X_GITHUB_DELIVERY = "X-GitHub-Delivery";
5561

5662
private final transient SequentialExecutionQueue queue = new SequentialExecutionQueue(threadPoolForRemoting);
5763

@@ -117,8 +123,10 @@ public List<Item> reRegisterAllHooks() {
117123
@SuppressWarnings("unused")
118124
@RequirePostWithGHHookPayload
119125
public void doIndex(@NonNull @GHEventHeader GHEvent event, @NonNull @GHEventPayload String payload) {
126+
var currentRequest = Stapler.getCurrentRequest2();
127+
String eventGuid = currentRequest.getHeader(X_GITHUB_DELIVERY);
120128
GHSubscriberEvent subscriberEvent =
121-
new GHSubscriberEvent(SCMEvent.originOf(Stapler.getCurrentRequest2()), event, payload);
129+
new GHSubscriberEvent(eventGuid, SCMEvent.originOf(currentRequest), event, payload);
122130
from(GHEventsSubscriber.all())
123131
.filter(isInterestedIn(event))
124132
.transform(processEvent(subscriberEvent)).toList();
Lines changed: 216 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,216 @@
1+
package org.jenkinsci.plugins.github.admin;
2+
3+
import java.time.Duration;
4+
import java.time.Instant;
5+
import java.util.Set;
6+
import java.util.logging.Logger;
7+
import java.util.stream.Collectors;
8+
9+
import com.github.benmanes.caffeine.cache.Cache;
10+
import com.github.benmanes.caffeine.cache.Caffeine;
11+
import com.github.benmanes.caffeine.cache.Ticker;
12+
import com.google.common.annotations.VisibleForTesting;
13+
14+
import hudson.Extension;
15+
import hudson.ExtensionList;
16+
import hudson.model.AdministrativeMonitor;
17+
import hudson.model.Item;
18+
import jenkins.model.Jenkins;
19+
import org.jenkinsci.plugins.github.Messages;
20+
import org.jenkinsci.plugins.github.extension.GHEventsSubscriber;
21+
import org.jenkinsci.plugins.github.extension.GHSubscriberEvent;
22+
import org.kohsuke.accmod.Restricted;
23+
import org.kohsuke.accmod.restrictions.NoExternalUse;
24+
import org.kohsuke.github.GHEvent;
25+
import org.kohsuke.stapler.HttpResponse;
26+
import org.kohsuke.stapler.WebMethod;
27+
import org.kohsuke.stapler.json.JsonHttpResponse;
28+
import org.kohsuke.stapler.verb.GET;
29+
import edu.umd.cs.findbugs.annotations.Nullable;
30+
import net.sf.json.JSONObject;
31+
32+
@SuppressWarnings("unused")
33+
@Extension
34+
public class GitHubDuplicateEventsMonitor extends AdministrativeMonitor {
35+
36+
@VisibleForTesting
37+
static final String LAST_DUPLICATE_CLICK_HERE_ANCHOR_ID = GitHubDuplicateEventsMonitor.class.getName()
38+
+ ".last-duplicate";
39+
40+
@Override
41+
public String getDisplayName() {
42+
return Messages.duplicate_events_administrative_monitor_displayname();
43+
}
44+
45+
public String getDescription() {
46+
return Messages.duplicate_events_administrative_monitor_description();
47+
}
48+
49+
public String getBlurb() {
50+
return Messages.duplicate_events_administrative_monitor_blurb(
51+
LAST_DUPLICATE_CLICK_HERE_ANCHOR_ID, this.getLastDuplicateUrl());
52+
}
53+
54+
@VisibleForTesting
55+
String getLastDuplicateUrl() {
56+
return this.getUrl() + "/" + "last-duplicate.json";
57+
}
58+
59+
@Override
60+
public boolean isActivated() {
61+
return ExtensionList.lookupSingleton(DuplicateEventsSubscriber.class).isDuplicateEventSeen();
62+
}
63+
64+
@Override
65+
public boolean hasRequiredPermission() {
66+
return Jenkins.get().hasPermission(Jenkins.SYSTEM_READ);
67+
}
68+
69+
@Override
70+
public void checkRequiredPermission() {
71+
Jenkins.get().checkPermission(Jenkins.SYSTEM_READ);
72+
}
73+
74+
@GET
75+
@WebMethod(name = "last-duplicate.json")
76+
public HttpResponse doGetLastDuplicatePayload() {
77+
Jenkins.get().checkPermission(Jenkins.SYSTEM_READ);
78+
JSONObject data;
79+
var lastDuplicate = ExtensionList.lookupSingleton(DuplicateEventsSubscriber.class).getLastDuplicate();
80+
if (lastDuplicate != null) {
81+
data = JSONObject.fromObject(lastDuplicate.ghSubscriberEvent().getPayload());
82+
} else {
83+
data = getLastDuplicateNoEventPayload();
84+
}
85+
return new JsonHttpResponse(data, 200);
86+
}
87+
88+
@VisibleForTesting
89+
static JSONObject getLastDuplicateNoEventPayload() {
90+
return new JSONObject().accumulate("payload", "No duplicate events seen yet");
91+
}
92+
93+
/**
94+
* Tracks duplicate {@link GHEvent} triggering actions in Jenkins.
95+
* Events are tracked for 10 minutes, with the last detected duplicate reference retained for up to 24 hours
96+
* (see {@link #isDuplicateEventSeen}).
97+
* <p>
98+
* Duplicates are stored in-memory only, so a controller restart clears all entries as if none existed.
99+
* Persistent storage is omitted for simplicity, since webhook misconfigurations would likely cause new duplicates.
100+
*/
101+
@Extension
102+
public static final class DuplicateEventsSubscriber extends GHEventsSubscriber {
103+
104+
private static final Logger LOGGER = Logger.getLogger(DuplicateEventsSubscriber.class.getName());
105+
106+
private Ticker ticker = Ticker.systemTicker();
107+
/**
108+
* Caches GitHub event GUIDs for 10 minutes to track recent events to detect duplicates.
109+
* <p>
110+
* Only the keys (event GUIDs) are relevant, as Caffeine automatically handles expiration based
111+
* on insertion time; the value is irrelevant, we put {@link #DUMMY}, as Caffeine doesn't provide any
112+
* Set structures.
113+
* <p>
114+
* Maximum cache size is set to 24k so it doesn't grow unbound (approx. 1MB). Each key takes 36 bytes, and
115+
* timestamp (assuming caffeine internally keeps long) takes 8 bytes; total of 44 bytes
116+
* per entry. So the maximum memory consumed by this cache is 24k * 44 = 1056k = 1.056 MB.
117+
*/
118+
private final Cache<String, Object> eventTracker = Caffeine.newBuilder()
119+
.maximumSize(24_000L)
120+
.expireAfterWrite(Duration.ofMinutes(10))
121+
.ticker(() -> ticker.read())
122+
.build();
123+
private static final Object DUMMY = new Object();
124+
125+
private volatile TrackedDuplicateEvent lastDuplicate;
126+
public record TrackedDuplicateEvent(
127+
String eventGuid, Instant lastUpdated, GHSubscriberEvent ghSubscriberEvent) { }
128+
private static final Duration TWENTY_FOUR_HOURS = Duration.ofHours(24);
129+
130+
@VisibleForTesting
131+
@Restricted(NoExternalUse.class)
132+
void setTicker(Ticker testTicker) {
133+
ticker = testTicker;
134+
}
135+
136+
/**
137+
* This subscriber is not applicable to any item
138+
*
139+
* @param item ignored
140+
* @return always false
141+
*/
142+
@Override
143+
protected boolean isApplicable(@Nullable Item item) {
144+
return false;
145+
}
146+
147+
/**
148+
* {@inheritDoc}
149+
* <p>
150+
* Subscribes to events that trigger actions in Jenkins, such as repository scans or builds.
151+
* <p>
152+
* The {@link GHEvent} enum defines about 63 events, but not all are relevant to Jenkins.
153+
* Tracking unnecessary events increases memory usage, and they occur more frequently than those triggering any
154+
* work.
155+
* <p>
156+
* <a href="https://docs.github.com/en/webhooks/webhook-events-and-payloads">
157+
* Documentation reference (also referenced in {@link GHEvent})</a>
158+
*/
159+
@Override
160+
protected Set<GHEvent> events() {
161+
return Set.of(
162+
GHEvent.CHECK_RUN, // associated with GitHub action Re-run button to trigger build
163+
GHEvent.CHECK_SUITE, // associated with GitHub action Re-run button to trigger build
164+
GHEvent.CREATE, // branch or tag creation
165+
GHEvent.DELETE, // branch or tag deletion
166+
GHEvent.PULL_REQUEST, // PR creation (also PR close or merge)
167+
GHEvent.PUSH // commit push
168+
);
169+
}
170+
171+
@Override
172+
protected void onEvent(final GHSubscriberEvent event) {
173+
String eventGuid = event.getEventGuid();
174+
LOGGER.fine(() -> "Received event with GUID: " + eventGuid);
175+
if (eventGuid == null) {
176+
return;
177+
}
178+
if (eventTracker.getIfPresent(eventGuid) != null) {
179+
lastDuplicate = new TrackedDuplicateEvent(eventGuid, getNow(), event);
180+
}
181+
eventTracker.put(eventGuid, DUMMY);
182+
}
183+
184+
/**
185+
* Checks if a duplicate event was recorded in the past 24 hours.
186+
* <p>
187+
* Events are not stored for 24 hours—only the most recent duplicate is checked within this timeframe.
188+
*
189+
* @return {@code true} if a duplicate was seen in the last 24 hours, {@code false} otherwise.
190+
*/
191+
public boolean isDuplicateEventSeen() {
192+
return lastDuplicate != null
193+
&& Duration.between(lastDuplicate.lastUpdated(), getNow()).compareTo(TWENTY_FOUR_HOURS) < 0;
194+
}
195+
196+
private Instant getNow() {
197+
return Instant.ofEpochSecond(0L, ticker.read());
198+
}
199+
200+
public TrackedDuplicateEvent getLastDuplicate() {
201+
return lastDuplicate;
202+
}
203+
204+
/**
205+
* Caffeine expired keys are not removed immediately. Method returns the non-expired keys;
206+
* required for the tests.
207+
*/
208+
@VisibleForTesting
209+
@Restricted(NoExternalUse.class)
210+
Set<String> getPresentEventKeys() {
211+
return eventTracker.asMap().keySet().stream()
212+
.filter(key -> eventTracker.getIfPresent(key) != null)
213+
.collect(Collectors.toSet());
214+
}
215+
}
216+
}

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

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,32 @@ public class GHSubscriberEvent extends SCMEvent<String> {
1818
*/
1919
private final GHEvent ghEvent;
2020

21+
private final String eventGuid;
22+
23+
/**
24+
* @deprecated use {@link #GHSubscriberEvent(String, String, GHEvent, String)} instead.
25+
*/
26+
@Deprecated
27+
public GHSubscriberEvent(@CheckForNull String origin, @NonNull GHEvent ghEvent, @NonNull String payload) {
28+
this(null, origin, ghEvent, payload);
29+
}
30+
2131
/**
2232
* Constructs a new {@link GHSubscriberEvent}.
23-
*
33+
* @param eventGuid the globally unique identifier (GUID) to identify the event; value of
34+
* request header {@link com.cloudbees.jenkins.GitHubWebHook#X_GITHUB_DELIVERY}.
2435
* @param origin the origin (see {@link SCMEvent#originOf(HttpServletRequest)}) or {@code null}.
2536
* @param ghEvent the type of event received from GitHub.
2637
* @param payload the event payload.
2738
*/
28-
public GHSubscriberEvent(@CheckForNull String origin, @NonNull GHEvent ghEvent, @NonNull String payload) {
39+
public GHSubscriberEvent(
40+
@CheckForNull String eventGuid,
41+
@CheckForNull String origin,
42+
@NonNull GHEvent ghEvent,
43+
@NonNull String payload) {
2944
super(Type.UPDATED, payload, origin);
3045
this.ghEvent = ghEvent;
46+
this.eventGuid = eventGuid;
3147
}
3248

3349
/**
@@ -39,4 +55,8 @@ public GHEvent getGHEvent() {
3955
return ghEvent;
4056
}
4157

58+
@CheckForNull
59+
public String getEventGuid() {
60+
return eventGuid;
61+
}
4262
}

src/main/java/org/jenkinsci/plugins/github/internal/GitHubClientCacheOps.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
import com.google.common.base.Predicate;
66
import com.google.common.hash.Hashing;
77
import edu.umd.cs.findbugs.annotations.NonNull;
8-
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
98
import okhttp3.Cache;
109
import org.apache.commons.io.FileUtils;
1110
import org.jenkinsci.plugins.github.GitHubPlugin;
@@ -96,7 +95,6 @@ public static Path getBaseCacheDir() {
9695
*
9796
* @param configs active server configs to exclude caches from cleanup
9897
*/
99-
@SuppressFBWarnings(value = "RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE")
10098
public static void clearRedundantCaches(List<GitHubServerConfig> configs) {
10199
Path baseCacheDir = getBaseCacheDir();
102100

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

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -73,25 +73,10 @@ protected void onEvent(final GHSubscriberEvent event) {
7373
LOGGER.warn("Received malformed PushEvent: " + event.getPayload(), e);
7474
return;
7575
}
76-
URL repoUrl = push.getRepository().getUrl();
76+
URL htmlUrl = push.getRepository().getHtmlUrl();
7777
final String pusherName = push.getPusher().getName();
78-
LOGGER.info("Received PushEvent for {} from {}", repoUrl, event.getOrigin());
79-
GitHubRepositoryName fromEventRepository = GitHubRepositoryName.create(repoUrl.toExternalForm());
80-
81-
if (fromEventRepository == null) {
82-
// On push event on github.com url === html_url
83-
// this is not consistent with the API docs and with hosted repositories
84-
// see https://goo.gl/c1qmY7
85-
// let's retry with 'html_url'
86-
URL htmlUrl = push.getRepository().getHtmlUrl();
87-
fromEventRepository = GitHubRepositoryName.create(htmlUrl.toExternalForm());
88-
if (fromEventRepository != null) {
89-
LOGGER.debug("PushEvent handling: 'html_url' field "
90-
+ "has been used to retrieve project information (instead of default 'url' field)");
91-
}
92-
}
93-
94-
final GitHubRepositoryName changedRepository = fromEventRepository;
78+
LOGGER.info("Received PushEvent for {} from {}", htmlUrl, event.getOrigin());
79+
final GitHubRepositoryName changedRepository = GitHubRepositoryName.create(htmlUrl.toExternalForm());
9580

9681
if (changedRepository != null) {
9782
// run in high privilege to see all the projects anonymous users don't see.
@@ -128,7 +113,7 @@ public void run() {
128113
}
129114

130115
} else {
131-
LOGGER.warn("Malformed repo url {}", repoUrl);
116+
LOGGER.warn("Malformed repo html url {}", htmlUrl);
132117
}
133118
}
134119
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,9 @@ github.trigger.check.method.warning.details=The webhook for repo {0}/{1} on {2}
1111
More info can be found on the global configuration page. This message will be dismissed if Jenkins receives \
1212
a PING event from repo webhook or if you add the repo to the ignore list in the global configuration.
1313
unknown.error=Unknown error
14+
duplicate.events.administrative.monitor.displayname=GitHub Duplicate Events
15+
duplicate.events.administrative.monitor.description=Warns about duplicate events received from GitHub.
16+
duplicate.events.administrative.monitor.blurb=Duplicate events were received from GitHub, possibly due to \
17+
misconfiguration (e.g., multiple webhooks targeting the same Jenkins controller at the repository or organization \
18+
level), potentially causing redundant builds or at least wasted work. \
19+
<a id="{0}" href="{1}" target="_blank">Click here</a> to inspect the last tracked duplicate event payload.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
<?jelly escape-by-default='true'?>
2+
<j:jelly xmlns:j="jelly:core">
3+
<j:out value="${it.description}"/>
4+
</j:jelly>

0 commit comments

Comments
 (0)