Problem: [JENKINS-60901] GitHub manages hooks even when it has not be…

…en 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 <[email protected]>
jenkinsci · Feb 28, 2020 · 71354e0 · 71354e0
1 parent 51dc96b
commit 71354e0
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 0 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/github/webhook/WebhookManager.java b/src/main/java/org/jenkinsci/plugins/github/webhook/WebhookManager.java
@@ -141,6 +141,11 @@ public void run() {
  */
  public void unregisterFor(GitHubRepositoryName name, List<GitHubRepositoryName> aliveRepos) {
  try {
+ if (!(name.resolve(allowedToManageHooks()).iterator().hasNext())) {
+ LOGGER.debug("Skipped adding GitHub webhook for {} because not configured to Manage Hooks", name);
+ return;
+ }
+
  GHRepository repo = checkNotNull(
  from(name.resolve(allowedToManageHooks())).firstMatch(withAdminAccess()).orNull(),
  "There are no credentials with admin access to manage hooks on %s", name
@@ -176,6 +181,12 @@ protected Function<GitHubRepositoryName, GHHook> createHookSubscribedTo(final Li
  @Override
  protected GHHook applyNullSafe(@Nonnull GitHubRepositoryName name) {
  try {
+ if (!(name.resolve(allowedToManageHooks()).iterator().hasNext())) {
+ LOGGER.debug("Skipped adding GitHub webhook for {} because not configured to Manage Hooks",
+ name);
+ return null;
+ }
+
  GHRepository repo = checkNotNull(
  from(name.resolve(allowedToManageHooks())).firstMatch(withAdminAccess()).orNull(),
  "There are no credentials with admin access to manage hooks on %s", name

diff --git a/src/test/java/org/jenkinsci/plugins/github/admin/GitHubHookRegisterProblemMonitorTest.java b/src/test/java/org/jenkinsci/plugins/github/admin/GitHubHookRegisterProblemMonitorTest.java
@@ -5,6 +5,7 @@
 import hudson.model.FreeStyleProject;
 import hudson.model.Item;
 import hudson.plugins.git.GitSCM;
+import org.jenkinsci.plugins.github.config.GitHubServerConfig;
 import org.jenkinsci.plugins.github.extension.GHSubscriberEvent;
 import org.jenkinsci.plugins.github.extension.GHEventsSubscriber;
 import org.jenkinsci.plugins.github.webhook.WebhookManager;
@@ -149,6 +150,9 @@ public void shouldReportAboutHookProblemOnRegister() throws IOException {
  job.addTrigger(new GitHubPushTrigger());
  job.setScm(REPO_GIT_SCM);
 
+ GitHubServerConfig conf = new GitHubServerConfig(WebhookManagerTest.HOOK_ENDPOINT);
+ conf.setManageHooks(true);
+
  WebhookManager.forHookUrl(WebhookManagerTest.HOOK_ENDPOINT)
  .registerFor((Item) job).run();
 
@@ -157,6 +161,9 @@ public void shouldReportAboutHookProblemOnRegister() throws IOException {
 
  @Test
  public void shouldReportAboutHookProblemOnUnregister() {
+ GitHubServerConfig conf = new GitHubServerConfig(WebhookManagerTest.HOOK_ENDPOINT);
+ conf.setManageHooks(true);
+
  WebhookManager.forHookUrl(WebhookManagerTest.HOOK_ENDPOINT)
  .unregisterFor(REPO, Collections.<GitHubRepositoryName>emptyList());