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.

Sign up for GitHub

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

Jump to bottom

Move to Java 8 & Jenkins 2.60 #389

Merged
merged 4 commits into from Jan 31, 2019
Merged

Move to Java 8 & Jenkins 2.60 #389

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
79eaa99
Move to java 8
Jan 2, 2019
65424d5
Upgrade Jenkins Core to 2.14 and use AutoCloseable ACL's
Jan 3, 2019
b5b2904
Use Jenkins new API since we use Jenkins core 2.14
Jan 4, 2019
ebbc526
Upgrade Jenkins core to 2.60
Jan 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 7 additions & 6 deletions README.md
View file
Open in desktop
Expand Up @@ -28,12 +28,10 @@ Each job can be configured with one Gerrit server.

# Environments
* `linux`
* `java-1.7`
* `java-1.8`
* `maven-3.3.3`

* Java 1.7: minimum development environment.
* Java 8: Works.
* Java 6 Runtime compatible
* Java 8: needed development environment.

You should have no problem running the plugin on a Windows server.

Expand All @@ -52,7 +50,7 @@ The _(build-config)_ directory contains "special" CheckStyle configurations and
fail during the verification phase if you don't follow them.

mvn clean package

Run findbugs for future reference or to make sure you haven't introduced any
new warnings

Expand All @@ -68,6 +66,10 @@ To test in a local Jenkins instance

mvn hpi:run

# Clean test environment

mvn clean
rm /tmp/jenkins-testkey* hostkey.ser # Needed when changing SSH components versions

# License

Expand All @@ -93,4 +95,3 @@ To test in a local Jenkins instance
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
THE SOFTWARE.

51 changes: 35 additions & 16 deletions pom.xml
View file
Open in desktop
Expand Up @@ -3,7 +3,7 @@
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>2.30</version>
<version>3.32</version>
<relativePath />
</parent>

Expand Down Expand Up @@ -53,17 +53,17 @@
</developers>

<properties>
<jenkins.version>1.609.3</jenkins.version>
<!--<jenkins.version>2.5-SNAPSHOT</jenkins.version>-->
<java.level>6</java.level>
<!-- <jenkins.version>2.14</jenkins.version> First version with auto-closeable ACLs / 2016 -->
<jenkins.version>2.60.1</jenkins.version> <!-- First LTS for java 8 only / 2017 -->
<java.level>8</java.level>
<no-test-jar>false</no-test-jar>
<findbugs.failOnError>false</findbugs.failOnError>
<concurrency>1C</concurrency>
<powermock.version>1.6.2</powermock.version>
<checkstyle.version>2.13</checkstyle.version>
<mockito.version>1.10.19</mockito.version>
<workflow.version>1.14</workflow.version>
<workflow.version.test>1.14.1-beta-1</workflow.version.test>
<workflow.version>1.14.2</workflow.version>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we need to bump this to some 2.x version of workflow and split up the versions to whatever latest that supports core 2.60. 1.14 of workflow is ancient.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All right, I check that.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need some advice for this point. If I use workflow-step-api plugin versions targeted to java 8 (core 2.60), I get the last version of this plugin. I can't wonder if it's a problem or not. If we do that, gerrit-trigger-plugin users will be forced to use the last workflow-step-api version to update gerrit-trigger-plugin ?

May be not an issue, but I can't guess ...

Version of workflow-step-api needing java 8 / jenkins core 2.60 : 2.18 (january 2019)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

workflow-aggregator needed for some tests introduce a lot of dependencies problems (maven enforcer). Do you think there is a good way to replace it for test, or we really need to deal with all depencies management ? Maybe we could get an example of dependencies management from other project using it ?

Or ... keep this versions of workflow plugins ? Since we do need new versions features, and their versions are drived Jenkins plugin management, it's an important issue ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have to go with the latest bleeding edge versions but something 2.x so that debugging etc gets easier to do.
Also as of workflow suite 2.0 there is no longer a mono repo of all the basics of workflow instead each plugin in the suite has it's own release cycle so we can't use the agregator any more.
See #390 for examples.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we've also seen from usage statistics that those who update they update everything most of the time. And those who don't update won't care either way since they won't get this version that requires them to update everything :)

<workflow.version.test>1.14.2</workflow.version.test>
</properties>

<dependencies>
Expand All @@ -88,13 +88,12 @@
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>git</artifactId>
<version>2.3</version>
<version>2.4.3</version>
<optional>true</optional>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>git-client</artifactId>
<version>1.11.1</version>
<optional>true</optional>
</dependency>
<dependency>
Expand All @@ -106,7 +105,7 @@
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>rabbitmq-consumer</artifactId>
<version>2.2</version>
<version>2.8</version>
<optional>true</optional>
</dependency>
<dependency>
Expand All @@ -118,7 +117,7 @@
<dependency>
<groupId>commons-net</groupId>
<artifactId>commons-net</artifactId>
<version>2.0</version>
<version>3.6</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
Expand All @@ -138,6 +137,13 @@
<version>${workflow.version}</version>
<optional>true</optional>
</dependency>
<dependency>
<!-- Used for test with matrix permissions -->
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>matrix-auth</artifactId>
<version>2.3</version>
<optional>true</optional>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this then be a test dependency instead?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I fix it.

</dependency>
<dependency>
<groupId>org.easymock</groupId>
<artifactId>easymock</artifactId>
Expand All @@ -147,17 +153,16 @@
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
<version>1.6.1</version>
<version>1.7.25</version>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-jdk14</artifactId>
<version>1.6.1</version>
<version>1.7.25</version>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>4.11</version>
<scope>test</scope>
</dependency>
<dependency>
Expand Down Expand Up @@ -187,7 +192,7 @@
<dependency>
<groupId>org.apache.sshd</groupId>
<artifactId>sshd-core</artifactId>
<version>0.13.0</version>
<version>0.14.0</version>
<scope>test</scope>
</dependency>
<dependency>
Expand All @@ -208,6 +213,20 @@
<scope>provided</scope>
</dependency>
</dependencies>
<dependencyManagement>
<dependencies>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>git-client</artifactId>
<version>1.19.6</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>script-security</artifactId>
<version>1.16</version>
</dependency>
</dependencies>
</dependencyManagement>
<repositories>
<repository>
<id>repo.jenkins-ci.org</id>
Expand Down Expand Up @@ -305,8 +324,8 @@
<configuration>
<signature>
<groupId>org.codehaus.mojo.signature</groupId>
<artifactId>java16</artifactId>
<version>1.1</version>
<artifactId>java18</artifactId>
<version>1.0</version>
</signature>
</configuration>
<executions>
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer.java
View file
Open in desktop
Expand Up @@ -1329,7 +1329,7 @@ public FormValidation doIntegerCheck(
Integer.parseInt(value);
return FormValidation.ok();
} catch (NumberFormatException e) {
return FormValidation.error(hudson.model.Messages.Hudson_NotANumber());
return FormValidation.error(Messages.NotANumber());
}
}

Expand All @@ -1349,7 +1349,7 @@ public FormValidation doEmptyOrIntegerCheck(
Integer.parseInt(value);
return FormValidation.ok();
} catch (NumberFormatException e) {
return FormValidation.error(hudson.model.Messages.Hudson_NotANumber());
return FormValidation.error(Messages.NotANumber());
}
}
}
Expand Down
8 changes: 6 additions & 2 deletions src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/JenkinsAwareGerritHandler.java
View file
Open in desktop
Expand Up @@ -23,6 +23,8 @@
*/
package com.sonyericsson.hudson.plugins.gerrit.trigger;

import hudson.security.ACL;
import hudson.security.ACLContext;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -73,8 +75,10 @@ public void notifyListeners(GerritEvent event) {
}
}

// The read deal
super.notifyListeners(event);
try (ACLContext ctx = ACL.as(ACL.SYSTEM)) {
// The read deal
super.notifyListeners(event);
}

// //Notify lifecycle listeners.
if (event instanceof GerritEventLifecycle) {
Expand Down
9 changes: 2 additions & 7 deletions src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/SystemEventThread.java
View file
Open in desktop
Expand Up @@ -23,10 +23,8 @@
*/
package com.sonyericsson.hudson.plugins.gerrit.trigger;

import org.acegisecurity.context.SecurityContext;
import org.acegisecurity.context.SecurityContextHolder;

import hudson.security.ACL;
import hudson.security.ACLContext;

import com.sonymobile.tools.gerrit.gerritevents.workers.Coordinator;
import com.sonymobile.tools.gerrit.gerritevents.workers.EventThread;
Expand Down Expand Up @@ -61,11 +59,8 @@ public SystemEventThread(Coordinator coordinator) {
*/
@Override
public void run() {
SecurityContext old = ACL.impersonate(ACL.SYSTEM);
try {
try (ACLContext ctx = ACL.as(ACL.SYSTEM)) {
super.run();
} finally {
SecurityContextHolder.setContext(old);
}
}
}
9 changes: 2 additions & 7 deletions ...n/hudson/plugins/gerrit/trigger/gerritnotifier/job/rest/BuildCompletedRestCommandJob.java
View file
Open in desktop
Expand Up @@ -38,14 +38,12 @@

import hudson.model.TaskListener;
import hudson.security.ACL;
import hudson.security.ACLContext;

import java.util.ArrayList;
import java.util.Collection;
import java.util.List;

import org.acegisecurity.context.SecurityContext;
import org.acegisecurity.context.SecurityContextHolder;

/**
* A job for the {@link com.sonymobile.tools.gerrit.gerritevents.GerritSendCommandQueue} that
* sends a build completed message.
Expand Down Expand Up @@ -79,8 +77,7 @@ public BuildCompletedRestCommandJob(IGerritHudsonTriggerConfig config, BuildMemo

@Override
protected ReviewInput createReview() {
SecurityContext old = ACL.impersonate(ACL.SYSTEM);
try {
try (ACLContext ctx = ACL.as(ACL.SYSTEM)) {
String message = parameterExpander.getBuildCompletedMessage(memoryImprint, listener);
Collection<ReviewLabel> scoredLabels = new ArrayList<ReviewLabel>();
if (memoryImprint.getEvent().isScorable()) {
Expand Down Expand Up @@ -119,8 +116,6 @@ protected ReviewInput createReview() {

return new ReviewInput(message, scoredLabels, commentedFiles).setNotify(notificationLevel)
.setTag(Constants.TAG_VALUE);
} finally {
SecurityContextHolder.setContext(old);
}
}
}
9 changes: 2 additions & 7 deletions ...icsson/hudson/plugins/gerrit/trigger/gerritnotifier/job/ssh/BuildCompletedCommandJob.java
View file
Open in desktop
Expand Up @@ -25,9 +25,6 @@

package com.sonyericsson.hudson.plugins.gerrit.trigger.gerritnotifier.job.ssh;

import org.acegisecurity.context.SecurityContext;
import org.acegisecurity.context.SecurityContextHolder;

import com.sonymobile.tools.gerrit.gerritevents.workers.cmd.AbstractSendCommandJob;
import com.sonyericsson.hudson.plugins.gerrit.trigger.config.IGerritHudsonTriggerConfig;
import com.sonyericsson.hudson.plugins.gerrit.trigger.gerritnotifier.GerritNotifier;
Expand All @@ -36,6 +33,7 @@

import hudson.model.TaskListener;
import hudson.security.ACL;
import hudson.security.ACLContext;

/**
* A send-command-job that calculates and sends the builds completed command.
Expand Down Expand Up @@ -64,13 +62,10 @@ public BuildCompletedCommandJob(IGerritHudsonTriggerConfig config,

@Override
public void run() {
SecurityContext old = ACL.impersonate(ACL.SYSTEM);
try {
try (ACLContext ctx = ACL.as(ACL.SYSTEM)) {
GerritNotifier notifier = NotificationFactory.getInstance()
.createGerritNotifier((IGerritHudsonTriggerConfig)getConfig(), this);
notifier.buildCompleted(memoryImprint, listener);
} finally {
SecurityContextHolder.setContext(old);
}
}
}
8 changes: 2 additions & 6 deletions ...ericsson/hudson/plugins/gerrit/trigger/gerritnotifier/job/ssh/BuildStartedCommandJob.java
View file
Open in desktop
Expand Up @@ -26,8 +26,6 @@
package com.sonyericsson.hudson.plugins.gerrit.trigger.gerritnotifier.job.ssh;

import hudson.model.Run;
import org.acegisecurity.context.SecurityContext;
import org.acegisecurity.context.SecurityContextHolder;

import com.sonymobile.tools.gerrit.gerritevents.dto.events.GerritTriggeredEvent;
import com.sonymobile.tools.gerrit.gerritevents.workers.cmd.AbstractSendCommandJob;
Expand All @@ -38,6 +36,7 @@

import hudson.model.TaskListener;
import hudson.security.ACL;
import hudson.security.ACLContext;

/**
* A send-command-job that calculates and sends the build started command.
Expand Down Expand Up @@ -73,13 +72,10 @@ public BuildStartedCommandJob(IGerritHudsonTriggerConfig config, Run build,

@Override
public void run() {
SecurityContext old = ACL.impersonate(ACL.SYSTEM);
try {
try (ACLContext ctx = ACL.as(ACL.SYSTEM)) {
GerritNotifier notifier = NotificationFactory.getInstance()
.createGerritNotifier((IGerritHudsonTriggerConfig)getConfig(), this);
notifier.buildStarted(build, taskListener, event, stats);
} finally {
SecurityContextHolder.setContext(old);
}
}
}