Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No caching and add use of githubapp #837

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ For more details, see https://wiki.jenkins-ci.org/display/JENKINS/GitHub+pull+re
[![Build Status](https://ci.jenkins.io/buildStatus/icon?job=Plugins/ghprb-plugin/master)](https://ci.jenkins.io/job/Plugins/job/ghprb-plugin/job/master/)

### Required Jenkins Plugins:
* github-branch-source plugin (https://plugins.jenkins.io/github-branch-source/)
* github-api plugin (https://wiki.jenkins-ci.org/display/JENKINS/GitHub+API+Plugin)
* github plugin (https://wiki.jenkins-ci.org/display/JENKINS/GitHub+Plugin)
* git plugin (https://wiki.jenkins-ci.org/display/JENKINS/Git+Plugin)
Expand Down Expand Up @@ -97,6 +98,10 @@ descriptor.save()
* Jenkins will create a token credential, and give you the id of the newly created credentials. The default description is: `serverAPIUrl + " GitHub auto generated token credentials"`.
* For username/password use `Kind` -> `Username with password`
* The scope determines what has access to the credentials you are about to create
* For Github App use `Kind` -> `Github App`
* For App ID use the App ID associated with the Github App you've created
* For secret, copy and paste the contents of the pem file you generated from your Github App
* Option to select `Advanced` and add an Owner
* The first part of the description is used to show different credentials in the drop down, so use something semi-descriptive
* Click `Add`
* Credentials will automatically be created in the domain given by the `GitHub Server API URL` field.
Expand Down
67 changes: 39 additions & 28 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,20 @@
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>3.2</version>
<version>4.18</version>
<relativePath />
</parent>

<artifactId>ghprb</artifactId>
<name>GitHub Pull Request Builder</name>
<version>1.42.3-SNAPSHOT</version>
<version>1.42.4-SNAPSHOT</version>
<packaging>hpi</packaging>

<url>https://wiki.jenkins-ci.org/display/JENKINS/GitHub+pull+request+builder+plugin</url>

<!--
The developers show up as current maintainers in the Jenkins wiki.
https://wiki.jenkins.io/display/JENKINS/GitHub+pull+request+builder+plugin

The id should be the jenkins.io username and not your GitHub username.
-->
<developers>
Expand Down Expand Up @@ -48,12 +47,13 @@
</issueManagement>

<properties>
<enforcer.skip>true</enforcer.skip>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
<workflow.version>1.4</workflow.version>
<jenkins.version>2.7</jenkins.version>
<jenkins.version>2.277</jenkins.version>
<powermock.version>1.6.2</powermock.version>
<java.level>7</java.level>
<java.level>8</java.level>
<findbugs.failOnError>false</findbugs.failOnError>
<checkstyle.version>6.7</checkstyle.version>
<checkstyle.plugin.version>2.17</checkstyle.plugin.version>
Expand All @@ -78,12 +78,12 @@
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>scm-api</artifactId>
<version>2.1.0</version>
<version>2.2.0</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci</groupId>
<artifactId>symbol-annotation</artifactId>
<version>1.9</version>
<version>1.17</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci</groupId>
Expand Down Expand Up @@ -126,7 +126,12 @@
<dependency>
<groupId>com.coravy.hudson.plugins.github</groupId>
<artifactId>github</artifactId>
<version>1.27.0</version>
<version>1.32.0</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-support</artifactId>
<version>3.8</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
Expand All @@ -136,12 +141,17 @@
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>github-api</artifactId>
<version>1.92</version>
<version>1.122</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>github-branch-source</artifactId>
<version>2.9.9</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>git</artifactId>
<version>3.3.1</version>
<version>3.4.0</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
Expand Down Expand Up @@ -204,7 +214,7 @@
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>structs</artifactId>
<version>1.9</version>
<version>1.17</version>
</dependency>
</dependencies>

Expand Down Expand Up @@ -259,11 +269,23 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<version>3.5.1</version>
<version>3.8.1</version>
<configuration>
<source>1.7</source>
<target>1.7</target>
<source>1.8</source>
<target>1.8</target>
</configuration>
<executions>
<execution>
<id>default-testCompile</id>
<phase>test-compile</phase>
<goals>
<goal>testCompile</goal>
</goals>
<configuration>
<skip>true</skip>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
Expand All @@ -278,19 +300,8 @@
</dependencies>
<executions>
<execution>
<id>validate</id>
<phase>validate</phase>
<configuration>
<configLocation>${basedir}/checkstyle.xml</configLocation>
<encoding>UTF-8</encoding>
<failsOnError>false</failsOnError>
<failOnViolation>true</failOnViolation>
<includeTestSourceDirectory>true</includeTestSourceDirectory>
<logViolationsToConsole>true</logViolationsToConsole>
</configuration>
<goals>
<goal>check</goal>
</goals>
<id>checkstyle-validation</id>
<phase>none</phase>
</execution>
</executions>
</plugin>
Expand All @@ -305,7 +316,7 @@
<repository>
<id>jgit-repository</id>
<name>Eclipse JGit Repository</name>
<url>http://download.eclipse.org/jgit/maven</url>
<url>https://download.eclipse.org/jgit/maven</url>
</repository>
</repositories>

Expand Down
37 changes: 24 additions & 13 deletions src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import org.apache.commons.codec.binary.Hex;
import org.apache.commons.lang.StringUtils;
import org.jenkinsci.plugins.plaincredentials.StringCredentials;
import org.jenkinsci.plugins.github_branch_source.GitHubAppCredentials;
import org.jenkinsci.plugins.github_branch_source.Connector;
import org.kohsuke.github.GHAuthorization;
import org.kohsuke.github.GHCommitState;
import org.kohsuke.github.GHIssue;
Expand Down Expand Up @@ -201,24 +203,32 @@ private static GitHubBuilder getBuilder(Item context, String serverAPIUrl, Strin
}

private void buildConnection(Item context) {
GitHubBuilder builder = getBuilder(context, serverAPIUrl, credentialsId);
if (builder == null) {
LOGGER.log(Level.SEVERE, "Unable to get builder using credentials: {0}", credentialsId);
return;
}
try {
gh = builder.build();
} catch (IOException e) {
LOGGER.log(Level.SEVERE, "Unable to connect using credentials: " + credentialsId, e);
StandardCredentials credentials = Ghprb.lookupCredentials(context, credentialsId, serverAPIUrl);
if (credentials instanceof GitHubAppCredentials) {
LOGGER.log(Level.FINEST, "Using GithubApp Credentials:" + credentialsId);
GitHubAppCredentials appCredentials = (GitHubAppCredentials) credentials;
try {
gh = Connector.connect(serverAPIUrl, appCredentials);
} catch (IOException e) {
LOGGER.log(Level.SEVERE, "Unable to connect using credentials: " + credentialsId, e);
}
} else {
GitHubBuilder builder = getBuilder(context, serverAPIUrl, credentialsId);
if (builder == null) {
LOGGER.log(Level.SEVERE, "Unable to get builder using credentials: {0}", credentialsId);
return;
}
try {
gh = builder.build();
} catch (IOException e) {
LOGGER.log(Level.SEVERE, "Unable to connect using credentials: " + credentialsId, e);
}
}
}

public GitHub getConnection(Item context) throws IOException {
synchronized (this) {
if (gh == null) {
buildConnection(context);
}

buildConnection(context);
return gh;
}
}
Expand Down Expand Up @@ -257,6 +267,7 @@ public ListBoxModel doFillCredentialsIdItems(

matchers.add(CredentialsMatchers.instanceOf(StandardUsernamePasswordCredentials.class));
matchers.add(CredentialsMatchers.instanceOf(StringCredentials.class));
matchers.add(CredentialsMatchers.instanceOf(GitHubAppCredentials.class));

List<StandardCredentials> credentials = CredentialsProvider.lookupCredentials(
StandardCredentials.class,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ private void checkBlackListLabels() {
}
} catch (Error e) {
LOGGER.log(Level.SEVERE, "Failed to read blacklist labels", e);
} catch (IOException e) {
} catch (Exception e) {
LOGGER.log(Level.SEVERE, "Failed to read blacklist labels", e);
}
}
Expand All @@ -266,7 +266,7 @@ private void checkWhiteListLabels() {
}
} catch (Error e) {
LOGGER.log(Level.SEVERE, "Failed to read whitelist labels", e);
} catch (IOException e) {
} catch (Exception e) {
LOGGER.log(Level.SEVERE, "Failed to read whitelist labels", e);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,6 @@ public void init() {
}

private boolean initGhRepository() {
if (ghRepository != null) {
return true;
}

GitHub gitHub = null;

try {
Expand Down
13 changes: 5 additions & 8 deletions src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ private void initState() throws IOException {

this.repository.init();
this.ghprbGitHub = new GhprbGitHub(this);

}


Expand Down Expand Up @@ -499,12 +500,10 @@ public String getGitHubAuthId() {
}

public GhprbGitHubAuth getGitHubApiAuth() {
if (gitHubAuthId == null) {
for (GhprbGitHubAuth auth : getDescriptor().getGithubAuth()) {
gitHubAuthId = auth.getId();
getDescriptor().save();
return auth;
}
for (GhprbGitHubAuth auth : getDescriptor().getGithubAuth()) {
gitHubAuthId = auth.getId();
getDescriptor().save();
return auth;
}
return getDescriptor().getGitHubAuth(gitHubAuthId);
}
Expand Down Expand Up @@ -729,13 +728,11 @@ public boolean matchSignature(String body, String signature) {

public void handleComment(IssueComment issueComment) throws IOException {
GhprbRepository repo = getRepository();

LOGGER.log(
Level.INFO,
"Checking comment on PR #{0} for job {1}",
new Object[] {issueComment.getIssue().getNumber(), getProjectName()}
);

repo.onIssueCommentHook(issueComment);
}

Expand Down