Skip to content

Commit 3ecfe65

Browse files
[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.
1 parent 483d4a2 commit 3ecfe65

File tree

3 files changed

+65
-27
lines changed

3 files changed

+65
-27
lines changed

src/main/java/com/cloudbees/jenkins/plugins/bitbucket/impl/client/AbstractBitbucketApi.java

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import org.apache.commons.lang.StringUtils;
4848
import org.apache.http.Header;
4949
import org.apache.http.HttpHost;
50+
import org.apache.http.HttpResponse;
5051
import org.apache.http.HttpStatus;
5152
import org.apache.http.NameValuePair;
5253
import org.apache.http.auth.AuthScope;
@@ -97,9 +98,14 @@ protected String truncateMiddle(@CheckForNull String value, int maxLength) {
9798
}
9899
}
99100

100-
protected BitbucketRequestException buildResponseException(CloseableHttpResponse response, String errorMessage) {
101+
protected BitbucketRequestException buildResponseException(CloseableHttpResponse response,
102+
String errorMessage,
103+
@CheckForNull String advice) {
101104
String headers = StringUtils.join(response.getAllHeaders(), "\n");
102105
String message = String.format("HTTP request error.%nStatus: %s%nResponse: %s%n%s", response.getStatusLine(), errorMessage, headers);
106+
if (advice != null) {
107+
message = advice + System.lineSeparator() + message;
108+
}
103109
return new BitbucketRequestException(response.getStatusLine().getStatusCode(), message);
104110
}
105111

@@ -234,6 +240,12 @@ protected CloseableHttpResponse executeMethod(HttpHost host, HttpRequestBase htt
234240
}
235241

236242
protected String doRequest(HttpRequestBase request, boolean requireAuthentication) throws IOException {
243+
return doRequest(request, requireAuthentication, HttpErrorAdvisor.NULL);
244+
}
245+
246+
protected String doRequest(HttpRequestBase request,
247+
boolean requireAuthentication,
248+
HttpErrorAdvisor advisor) throws IOException {
237249
try (CloseableHttpResponse response = executeMethod(getHost(), request, requireAuthentication)) {
238250
int statusCode = response.getStatusLine().getStatusCode();
239251
if (statusCode == HttpStatus.SC_NOT_FOUND) {
@@ -247,7 +259,7 @@ protected String doRequest(HttpRequestBase request, boolean requireAuthenticatio
247259
String content = getResponseContent(response);
248260
EntityUtils.consume(response.getEntity());
249261
if (statusCode != HttpStatus.SC_OK && statusCode != HttpStatus.SC_CREATED) {
250-
throw buildResponseException(response, content);
262+
throw buildResponseException(response, content, advisor.getAdvice(response));
251263
}
252264
return content;
253265
} catch (BitbucketRequestException e) {
@@ -296,7 +308,7 @@ protected InputStream getRequestAsInputStream(String path) throws IOException {
296308
}
297309
if (statusCode != HttpStatus.SC_OK) {
298310
String content = getResponseContent(response);
299-
throw buildResponseException(response, content);
311+
throw buildResponseException(response, content, null);
300312
}
301313
return new ClosingConnectionInputStream(response, httpget, getConnectionManager());
302314
}
@@ -351,4 +363,25 @@ public void close() throws Exception {
351363
protected BitbucketAuthenticator getAuthenticator() {
352364
return authenticator;
353365
}
366+
367+
/**
368+
* REST API operation methods in classes derived from {@link AbstractBitbucketApi}
369+
* can implement this interface to explain to users why
370+
* {@link #doRequest(HttpRequestBase, boolean, HttpErrorAdvisor)} failed.
371+
*/
372+
@FunctionalInterface
373+
protected interface HttpErrorAdvisor {
374+
/**
375+
* Gets user-readable advice on why Bitbucket returned an error HTTP status.
376+
*
377+
* @param response The HTTP response from Bitbucket.
378+
* @return Advice to the user on why the request failed, or {@code null}.
379+
*/
380+
@CheckForNull String getAdvice(HttpResponse response);
381+
382+
/**
383+
* A trivial {@link HttpErrorAdvisor} implementation that never has advice.
384+
*/
385+
public static final HttpErrorAdvisor NULL = response -> null;
386+
}
354387
}

src/main/java/com/cloudbees/jenkins/plugins/bitbucket/server/client/BitbucketServerAPIClient.java

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
*/
2424
package com.cloudbees.jenkins.plugins.bitbucket.server.client;
2525

26+
import com.cloudbees.jenkins.plugins.bitbucket.Messages;
2627
import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketApi;
2728
import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketAuthenticator;
2829
import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketBuildStatus;
@@ -89,10 +90,13 @@
8990
import org.apache.commons.lang.StringUtils;
9091
import org.apache.http.Header;
9192
import org.apache.http.HttpHost;
93+
import org.apache.http.HttpResponse;
9294
import org.apache.http.HttpStatus;
93-
import org.apache.http.client.methods.CloseableHttpResponse;
9495
import org.apache.http.client.methods.HttpGet;
96+
import org.apache.http.client.methods.HttpPost;
9597
import org.apache.http.conn.HttpClientConnectionManager;
98+
import org.apache.http.entity.ContentType;
99+
import org.apache.http.entity.StringEntity;
96100
import org.apache.http.impl.client.CloseableHttpClient;
97101
import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
98102
import org.apache.http.message.BasicNameValuePair;
@@ -511,7 +515,11 @@ public void postBuildStatus(@NonNull BitbucketBuildStatus status) throws IOExcep
511515
.set("repo", repositoryName)
512516
.set("hash", newStatus.getHash())
513517
.expand();
514-
postRequest(url, JsonParser.toJson(newStatus));
518+
519+
HttpPost request = new HttpPost(url);
520+
request.setEntity(new StringEntity(JsonParser.toJson(newStatus),
521+
ContentType.create("application/json", "UTF-8")));
522+
doRequest(request, true, this::adviceForBuildStatusError);
515523
}
516524

517525
/**
@@ -1053,19 +1061,24 @@ private Map<String,Object> collectLines(String response, final List<String> line
10531061
return content;
10541062
}
10551063

1056-
@Override
1057-
protected BitbucketRequestException buildResponseException(CloseableHttpResponse response, String errorMessage) {
1064+
// Gets user-visible advice for an HTTP error response when
1065+
// Bitbucket Server rejects a build status.
1066+
// Implements AbstractBitbucketApi.HttpErrorAdvisor#getAdvice.
1067+
@CheckForNull
1068+
private String adviceForBuildStatusError(HttpResponse response) {
10581069
// If the HTTP request failed because of an authorization
10591070
// problem, then make the exception message also show the
10601071
// Bitbucket user name with which Jenkins authenticated,
10611072
// the project name, and the repository name.
10621073
//
10631074
// Such an authorization problem can occur especially in a
10641075
// pull request from a personal fork: if Jenkins has been
1065-
// granted READ access on the target repository of the PR but
1066-
// not on the fork, then it can read the PR information from
1067-
// the target repository and check out the files, but cannot
1068-
// post a build status to the fork.
1076+
// granted REPO_READ access on the target repository of the PR
1077+
// but no access on the fork, then it can read the PR
1078+
// information from the target repository and check out the
1079+
// files, but cannot post a build status to the fork.
1080+
// Showing the name of the fork will help the user or
1081+
// administrator grant the required access.
10691082
//
10701083
// If the HTTP request already includes valid credentials,
10711084
// but the Bitbucket user has not been granted access on the
@@ -1078,24 +1091,15 @@ protected BitbucketRequestException buildResponseException(CloseableHttpResponse
10781091
Header userNameHeader = response.getFirstHeader("X-AUSERNAME");
10791092
if (userNameHeader != null
10801093
&& !userNameHeader.getValue().equals("anonymous")) {
1081-
String headers = StringUtils.join(response.getAllHeaders(), "\n");
1082-
1083-
// The message says "sufficient access" because it is
1084-
// too difficult for this method to know which level
1085-
// of access is actually needed.
1086-
// Posting a build status requires READ access, but
1087-
// deleting a build status requires ADMIN access.
1088-
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",
1089-
userNameHeader.getValue(),
1090-
getUserCentricOwner(),
1091-
getRepositoryName(),
1092-
response.getStatusLine(),
1093-
errorMessage,
1094-
headers);
1095-
return new BitbucketRequestException(httpStatus, message);
1094+
// Posting a build status requires REPO_READ access.
1095+
// https://docs.atlassian.com/bitbucket-server/rest/7.4.0/bitbucket-rest.html#idp219
1096+
return Messages.BitbucketServerAPIClient_adviceForBuildStatusError(
1097+
userNameHeader.getValue(),
1098+
getUserCentricOwner(),
1099+
getRepositoryName());
10961100
}
10971101
}
10981102

1099-
return super.buildResponseException(response, errorMessage);
1103+
return null;
11001104
}
11011105
}

src/main/resources/com/cloudbees/jenkins/plugins/bitbucket/Messages.properties

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,3 +65,4 @@ BitbucketTagSCMHead.Pronoun=Tag
6565
TagDiscoveryTrait.authorityDisplayName=Trust origin tags
6666
BitbucketBuildStatusNotificationsTrait.displayName=Bitbucket build status notifications
6767
DiscardOldBranchTrait.displayName=Discard branch older than given days
68+
BitbucketServerAPIClient.adviceForBuildStatusError=Please verify that the Bitbucket user "{0}" is granted REPO_READ access on the repository "{1}/{2}".

0 commit comments

Comments
 (0)