Skip to content

Commit 71354e0

Browse files
committed
Problem: [JENKINS-60901] GitHub manages hooks even when it has not been configured to do it (and complains a lot in the log)
Solution: Check if configured to manage hooks before trying to register or unregister them (which in case of no credential set up, results in a meaningful exception message plus a lengthy but somewhat useless stacktrace). If nothing else, this reduces I/O and storage overhead due to hook not configured and disabled. Signed-off-by: Jim Klimov <jimklimov@gmail.com>
1 parent 51dc96b commit 71354e0

File tree

2 files changed

+18
-0
lines changed

2 files changed

+18
-0
lines changed

src/main/java/org/jenkinsci/plugins/github/webhook/WebhookManager.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,11 @@ public void run() {
141141
*/
142142
public void unregisterFor(GitHubRepositoryName name, List<GitHubRepositoryName> aliveRepos) {
143143
try {
144+
if (!(name.resolve(allowedToManageHooks()).iterator().hasNext())) {
145+
LOGGER.debug("Skipped adding GitHub webhook for {} because not configured to Manage Hooks", name);
146+
return;
147+
}
148+
144149
GHRepository repo = checkNotNull(
145150
from(name.resolve(allowedToManageHooks())).firstMatch(withAdminAccess()).orNull(),
146151
"There are no credentials with admin access to manage hooks on %s", name
@@ -176,6 +181,12 @@ protected Function<GitHubRepositoryName, GHHook> createHookSubscribedTo(final Li
176181
@Override
177182
protected GHHook applyNullSafe(@Nonnull GitHubRepositoryName name) {
178183
try {
184+
if (!(name.resolve(allowedToManageHooks()).iterator().hasNext())) {
185+
LOGGER.debug("Skipped adding GitHub webhook for {} because not configured to Manage Hooks",
186+
name);
187+
return null;
188+
}
189+
179190
GHRepository repo = checkNotNull(
180191
from(name.resolve(allowedToManageHooks())).firstMatch(withAdminAccess()).orNull(),
181192
"There are no credentials with admin access to manage hooks on %s", name

src/test/java/org/jenkinsci/plugins/github/admin/GitHubHookRegisterProblemMonitorTest.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import hudson.model.FreeStyleProject;
66
import hudson.model.Item;
77
import hudson.plugins.git.GitSCM;
8+
import org.jenkinsci.plugins.github.config.GitHubServerConfig;
89
import org.jenkinsci.plugins.github.extension.GHSubscriberEvent;
910
import org.jenkinsci.plugins.github.extension.GHEventsSubscriber;
1011
import org.jenkinsci.plugins.github.webhook.WebhookManager;
@@ -149,6 +150,9 @@ public void shouldReportAboutHookProblemOnRegister() throws IOException {
149150
job.addTrigger(new GitHubPushTrigger());
150151
job.setScm(REPO_GIT_SCM);
151152

153+
GitHubServerConfig conf = new GitHubServerConfig(WebhookManagerTest.HOOK_ENDPOINT);
154+
conf.setManageHooks(true);
155+
152156
WebhookManager.forHookUrl(WebhookManagerTest.HOOK_ENDPOINT)
153157
.registerFor((Item) job).run();
154158

@@ -157,6 +161,9 @@ public void shouldReportAboutHookProblemOnRegister() throws IOException {
157161

158162
@Test
159163
public void shouldReportAboutHookProblemOnUnregister() {
164+
GitHubServerConfig conf = new GitHubServerConfig(WebhookManagerTest.HOOK_ENDPOINT);
165+
conf.setManageHooks(true);
166+
160167
WebhookManager.forHookUrl(WebhookManagerTest.HOOK_ENDPOINT)
161168
.unregisterFor(REPO, Collections.<GitHubRepositoryName>emptyList());
162169

0 commit comments

Comments
 (0)