-
Notifications
You must be signed in to change notification settings - Fork 39
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
Do not bundle a copy of instance-identity
#75
Conversation
@@ -32,21 +32,8 @@ | |||
<revision>3</revision> | |||
<changelist>999999-SNAPSHOT</changelist> | |||
<jenkins.version>2.289.1</jenkins.version> | |||
<java.level>8</java.level> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since we are on 4.40
<dependencies> | ||
<dependency> | ||
<groupId>io.jenkins.tools.bom</groupId> | ||
<artifactId>bom-2.277.x</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong line (the dep is 2.289.x) and anyway this currently has no plugin deps so there is no need for a BOM.
@@ -72,7 +59,7 @@ | |||
<dependency> | |||
<groupId>org.jenkins-ci.modules</groupId> | |||
<artifactId>instance-identity</artifactId> | |||
<version>2.2</version> <!-- We use the module for the moment --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Picking up version from core BOM.
@@ -72,7 +59,7 @@ | |||
<dependency> | |||
<groupId>org.jenkins-ci.modules</groupId> | |||
<artifactId>instance-identity</artifactId> | |||
<version>2.2</version> <!-- We use the module for the moment --> | |||
<scope>provided</scope> <!-- TODO pending JEP-230 we use the module --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main fix—do not include this JAR in WEB-INF/lib/
!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
#38 was incorrect. Noticed when trying to test jenkinsci/jenkins#6585 that even when
instance-identity
is not enabled as a plugin,InstanceIdentityRSAProvider
gets loaded fromplugins/sshd/WEB-INF/lib/instance-identity-2.2.jar
.