From 3ecfe6513e3e48b4bec29b2d20ae78e777d9fe12 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sun, 12 Jan 2025 22:02:47 +0200 Subject: [PATCH] [JENKINS-74970] Log advice for build status only Minimize the effect of the previous logging change so that it applies only to posting a build status to Bitbucket Server, not to any other kind of request. This way, the advice can explicitly state that REPO_READ access is needed. --- .../impl/client/AbstractBitbucketApi.java | 39 ++++++++++++-- .../client/BitbucketServerAPIClient.java | 52 ++++++++++--------- .../plugins/bitbucket/Messages.properties | 1 + 3 files changed, 65 insertions(+), 27 deletions(-) diff --git a/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/impl/client/AbstractBitbucketApi.java b/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/impl/client/AbstractBitbucketApi.java index 4787827af..68839e9b8 100644 --- a/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/impl/client/AbstractBitbucketApi.java +++ b/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/impl/client/AbstractBitbucketApi.java @@ -47,6 +47,7 @@ import org.apache.commons.lang.StringUtils; import org.apache.http.Header; import org.apache.http.HttpHost; +import org.apache.http.HttpResponse; import org.apache.http.HttpStatus; import org.apache.http.NameValuePair; import org.apache.http.auth.AuthScope; @@ -97,9 +98,14 @@ protected String truncateMiddle(@CheckForNull String value, int maxLength) { } } - protected BitbucketRequestException buildResponseException(CloseableHttpResponse response, String errorMessage) { + protected BitbucketRequestException buildResponseException(CloseableHttpResponse response, + String errorMessage, + @CheckForNull String advice) { String headers = StringUtils.join(response.getAllHeaders(), "\n"); String message = String.format("HTTP request error.%nStatus: %s%nResponse: %s%n%s", response.getStatusLine(), errorMessage, headers); + if (advice != null) { + message = advice + System.lineSeparator() + message; + } return new BitbucketRequestException(response.getStatusLine().getStatusCode(), message); } @@ -234,6 +240,12 @@ protected CloseableHttpResponse executeMethod(HttpHost host, HttpRequestBase htt } protected String doRequest(HttpRequestBase request, boolean requireAuthentication) throws IOException { + return doRequest(request, requireAuthentication, HttpErrorAdvisor.NULL); + } + + protected String doRequest(HttpRequestBase request, + boolean requireAuthentication, + HttpErrorAdvisor advisor) throws IOException { try (CloseableHttpResponse response = executeMethod(getHost(), request, requireAuthentication)) { int statusCode = response.getStatusLine().getStatusCode(); if (statusCode == HttpStatus.SC_NOT_FOUND) { @@ -247,7 +259,7 @@ protected String doRequest(HttpRequestBase request, boolean requireAuthenticatio String content = getResponseContent(response); EntityUtils.consume(response.getEntity()); if (statusCode != HttpStatus.SC_OK && statusCode != HttpStatus.SC_CREATED) { - throw buildResponseException(response, content); + throw buildResponseException(response, content, advisor.getAdvice(response)); } return content; } catch (BitbucketRequestException e) { @@ -296,7 +308,7 @@ protected InputStream getRequestAsInputStream(String path) throws IOException { } if (statusCode != HttpStatus.SC_OK) { String content = getResponseContent(response); - throw buildResponseException(response, content); + throw buildResponseException(response, content, null); } return new ClosingConnectionInputStream(response, httpget, getConnectionManager()); } @@ -351,4 +363,25 @@ public void close() throws Exception { protected BitbucketAuthenticator getAuthenticator() { return authenticator; } + + /** + * REST API operation methods in classes derived from {@link AbstractBitbucketApi} + * can implement this interface to explain to users why + * {@link #doRequest(HttpRequestBase, boolean, HttpErrorAdvisor)} failed. + */ + @FunctionalInterface + protected interface HttpErrorAdvisor { + /** + * Gets user-readable advice on why Bitbucket returned an error HTTP status. + * + * @param response The HTTP response from Bitbucket. + * @return Advice to the user on why the request failed, or {@code null}. + */ + @CheckForNull String getAdvice(HttpResponse response); + + /** + * A trivial {@link HttpErrorAdvisor} implementation that never has advice. + */ + public static final HttpErrorAdvisor NULL = response -> null; + } } diff --git a/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/server/client/BitbucketServerAPIClient.java b/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/server/client/BitbucketServerAPIClient.java index 902b2d465..74c6c6085 100644 --- a/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/server/client/BitbucketServerAPIClient.java +++ b/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/server/client/BitbucketServerAPIClient.java @@ -23,6 +23,7 @@ */ package com.cloudbees.jenkins.plugins.bitbucket.server.client; +import com.cloudbees.jenkins.plugins.bitbucket.Messages; import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketApi; import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketAuthenticator; import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketBuildStatus; @@ -89,10 +90,13 @@ import org.apache.commons.lang.StringUtils; import org.apache.http.Header; import org.apache.http.HttpHost; +import org.apache.http.HttpResponse; import org.apache.http.HttpStatus; -import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpPost; import org.apache.http.conn.HttpClientConnectionManager; +import org.apache.http.entity.ContentType; +import org.apache.http.entity.StringEntity; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; import org.apache.http.message.BasicNameValuePair; @@ -511,7 +515,11 @@ public void postBuildStatus(@NonNull BitbucketBuildStatus status) throws IOExcep .set("repo", repositoryName) .set("hash", newStatus.getHash()) .expand(); - postRequest(url, JsonParser.toJson(newStatus)); + + HttpPost request = new HttpPost(url); + request.setEntity(new StringEntity(JsonParser.toJson(newStatus), + ContentType.create("application/json", "UTF-8"))); + doRequest(request, true, this::adviceForBuildStatusError); } /** @@ -1053,8 +1061,11 @@ private Map collectLines(String response, final List line return content; } - @Override - protected BitbucketRequestException buildResponseException(CloseableHttpResponse response, String errorMessage) { + // Gets user-visible advice for an HTTP error response when + // Bitbucket Server rejects a build status. + // Implements AbstractBitbucketApi.HttpErrorAdvisor#getAdvice. + @CheckForNull + private String adviceForBuildStatusError(HttpResponse response) { // If the HTTP request failed because of an authorization // problem, then make the exception message also show the // Bitbucket user name with which Jenkins authenticated, @@ -1062,10 +1073,12 @@ protected BitbucketRequestException buildResponseException(CloseableHttpResponse // // Such an authorization problem can occur especially in a // pull request from a personal fork: if Jenkins has been - // granted READ access on the target repository of the PR but - // not on the fork, then it can read the PR information from - // the target repository and check out the files, but cannot - // post a build status to the fork. + // granted REPO_READ access on the target repository of the PR + // but no access on the fork, then it can read the PR + // information from the target repository and check out the + // files, but cannot post a build status to the fork. + // Showing the name of the fork will help the user or + // administrator grant the required access. // // If the HTTP request already includes valid credentials, // but the Bitbucket user has not been granted access on the @@ -1078,24 +1091,15 @@ protected BitbucketRequestException buildResponseException(CloseableHttpResponse Header userNameHeader = response.getFirstHeader("X-AUSERNAME"); if (userNameHeader != null && !userNameHeader.getValue().equals("anonymous")) { - String headers = StringUtils.join(response.getAllHeaders(), "\n"); - - // The message says "sufficient access" because it is - // too difficult for this method to know which level - // of access is actually needed. - // Posting a build status requires READ access, but - // deleting a build status requires ADMIN access. - String message = String.format("HTTP request error.%nPlease verify that the Bitbucket user \"%s\" is granted sufficient access on the repository \"%s/%s\".%nStatus: %s%nResponse: %s%n%s", - userNameHeader.getValue(), - getUserCentricOwner(), - getRepositoryName(), - response.getStatusLine(), - errorMessage, - headers); - return new BitbucketRequestException(httpStatus, message); + // Posting a build status requires REPO_READ access. + // https://docs.atlassian.com/bitbucket-server/rest/7.4.0/bitbucket-rest.html#idp219 + return Messages.BitbucketServerAPIClient_adviceForBuildStatusError( + userNameHeader.getValue(), + getUserCentricOwner(), + getRepositoryName()); } } - return super.buildResponseException(response, errorMessage); + return null; } } diff --git a/src/main/resources/com/cloudbees/jenkins/plugins/bitbucket/Messages.properties b/src/main/resources/com/cloudbees/jenkins/plugins/bitbucket/Messages.properties index fea0c6eed..883ad7a66 100644 --- a/src/main/resources/com/cloudbees/jenkins/plugins/bitbucket/Messages.properties +++ b/src/main/resources/com/cloudbees/jenkins/plugins/bitbucket/Messages.properties @@ -65,3 +65,4 @@ BitbucketTagSCMHead.Pronoun=Tag TagDiscoveryTrait.authorityDisplayName=Trust origin tags BitbucketBuildStatusNotificationsTrait.displayName=Bitbucket build status notifications DiscardOldBranchTrait.displayName=Discard branch older than given days +BitbucketServerAPIClient.adviceForBuildStatusError=Please verify that the Bitbucket user "{0}" is granted REPO_READ access on the repository "{1}/{2}".