-
Notifications
You must be signed in to change notification settings - Fork 43
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
JKNS-573: Update jenkins baseline version from 2.426.2 to 2.440.3 #490
base: master
Are you sure you want to change the base?
Conversation
@prdhamdh: This pull request references JKNS-573 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.18.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: prdhamdh The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@adambkaplan @sayan-biswas Hi, Please help to review the change. Thanks ! |
/retest |
1 similar comment
/retest |
pom.xml
Outdated
@@ -136,6 +136,21 @@ | |||
<groupId>org.jenkins-ci.plugins.workflow</groupId> | |||
<artifactId>workflow-support</artifactId> | |||
<version>865.v43e78cc44e0d</version> | |||
</dependency> |
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.
I recommend removing all <dependency>
parts of the XML here if the groupId
starts with org.jenkins-ci.plugins
. Most, if not all, of these have their versions set in the Jenkins Bill of Materials (BOM). This ensures we are always current with what the upstream community considers validated/tested.
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.
Jenkins BOM is set here: https://github.com/jenkinsci/bom
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.
@adambkaplan Hi, I removed all <dependency>
which groupId starts with org.jenkins-ci.plugins
however, I am getting below compilation error while building it. Please suggest, if i am missing anything or how we can handle this.
I also tried with old version of Jenkins BOM and updating dependancies with sequence but still the error message getting reported.
Error:
[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR :
[INFO] -------------------------------------------------------------
[ERROR] <local_path>/jenkins-sync-plugin/src/main/java/io/fabric8/jenkins/openshiftsync/CredentialsUtils.java:[488,16] unreported exception hudson.model.Descriptor.FormException; must be caught or declared to be thrown
[INFO] 1 error
[INFO] -------------------------------------------------------------
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 2.482 s
[INFO] Finished at: 2024-10-15T21:30:59+05:30
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.11.0:compile (default-compile) on project openshift-sync: Compilation failure
[ERROR] <local_path>/jenkins-sync-plugin/src/main/java/io/fabric8/jenkins/openshiftsync/CredentialsUtils.java:[488,16] unreported exception hudson.model.Descriptor.FormException; must be caught or declared to be thrown
Diff :
diff --git a/pom.xml b/pom.xml
index 69b2057..59d116b 100644
--- a/pom.xml
+++ b/pom.xml
@@ -118,7 +118,7 @@
<dependency>
<groupId>io.jenkins.tools.bom</groupId>
<artifactId>bom-2.440.x</artifactId>
- <version>3387.v0f2773fa_3200</version>
+ <version>3435.v238d66a_043fb_</version>
<scope>import</scope>
<type>pom</type>
</dependency>
@@ -127,46 +127,6 @@
<artifactId>joda-time</artifactId>
<version>2.12.5</version>
</dependency>
- <dependency>
- <groupId>org.jenkins-ci.plugins.workflow</groupId>
- <artifactId>workflow-multibranch</artifactId>
- <version>773.vc4fe1378f1d5</version>
- </dependency>
- <dependency>
- <groupId>org.jenkins-ci.plugins.workflow</groupId>
- <artifactId>workflow-support</artifactId>
- <version>865.v43e78cc44e0d</version>
- </dependency>
....
.......
+ </dependency>
+ <dependency>
+ <groupId>io.jenkins.plugins</groupId>
+ <artifactId>asm-api</artifactId>
+ <version>9.6-3.v2e1fa_b_338cd7</version>
+ </dependency>
+ <dependency>
+ <groupId>io.jenkins.plugins.mina-sshd-api</groupId>
+ <artifactId>mina-sshd-api-common</artifactId>
+ <version>2.12.1-113.v4d3ea_5eb_7f72</version>
</dependency>
<dependency>
- <groupId>org.jenkins-ci.plugins</groupId>
- <artifactId>scm-api</artifactId>
- <version>676.v886669a_199a_a_</version>
+ <groupId>io.jenkins.plugins.mina-sshd-api</groupId>
+ <artifactId>mina-sshd-api-core</artifactId>
+ <version>2.12.1-113.v4d3ea_5eb_7f72</version>
</dependency>
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.
[ERROR] <local_path>/jenkins-sync-plugin/src/main/java/io/fabric8/jenkins/openshiftsync/CredentialsUtils.java:[488,16] unreported exception hudson.model.Descriptor.FormException; must be caught or declared to be thrown
The likely cause is that when we updated our dependencies, a function called in CredentialsUtils.java
changed its behavior to throw a hudson.model.Descriptor.FormException
. This is a pretty common thing to encounter when updating dependencies in Java, and similar types of challenges happen in every programming language/ecosystem.
You now need to change CredentialsUtils.java
to handle the exception. A modern IDE like IntelliJ or Visual Studio Code (with Microsoft or Red Hat's Java extension) will likely flag this as a compile error and provide tips on how to address it. I recommend putting this code change in a separate commit - this helps "future you" separate dependency updates from necessary code changes (due to said dependency updates).
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.
For the FormException
bits - I'm concerned that we are blindly re-throwing the exception in sections where it may not make sense. Can we try and minimize the impact to the codebase - for example, if a FormException
is raised, can we recover/mitigate without re-throwing an exception (ex: throw new RuntimeException(e)
)
public interface Lifecyclable { | ||
public void stop(); | ||
public void start(); | ||
public void start() throws FormException; |
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.
I don't love the idea of adding throws FormException
to this interface. Looking at the JavaDoc, this appears to be an error due to a Jenkins form throwing some kind of (validation?) error: https://javadoc.jenkins.io/hudson/model/Descriptor.FormException.html
Can we find the root cause where this Exception could be thrown, and add a try / catch
blocks around those specific locations?
@@ -16,7 +17,7 @@ public class SecretManager { | |||
private final static Logger logger = Logger.getLogger(SecretManager.class.getName()); | |||
private final static ConcurrentHashMap<String, String> trackedSecrets = new ConcurrentHashMap<>(); | |||
|
|||
public static void insertOrUpdateCredentialFromSecret(final Secret secret) { | |||
public static void insertOrUpdateCredentialFromSecret(final Secret secret) throws FormException { |
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.
Where is FormException
being thrown? Can we add a try/catch block around those locations and mitigate?
@@ -52,7 +53,7 @@ static void onInitialSecrets(SecretList secrets) { | |||
} | |||
} | |||
|
|||
protected static void updateCredential(Secret secret) { | |||
protected static void updateCredential(Secret secret) throws FormException { |
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.
Ditto - where is FormException
being thrown?
@@ -182,7 +184,7 @@ public static String upsertCredential(Secret secret) throws IOException { | |||
return null; | |||
} | |||
|
|||
private static String insertOrUpdateCredentialsFromSecret(Secret secret) throws IOException { | |||
private static String insertOrUpdateCredentialsFromSecret(Secret secret) throws IOException, FormException { |
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.
Where is FormException
being thrown? Can we mitigate the exception?
@@ -355,7 +357,7 @@ private static Credentials arbitraryKeyValueTextCredential(Map<String, String> d | |||
new String(Base64.getEncoder().encode(text.getBytes(StandardCharsets.UTF_8)), StandardCharsets.UTF_8)); | |||
} | |||
|
|||
private static Credentials secretToCredentials(Secret secret) { | |||
private static Credentials secretToCredentials(Secret secret) throws FormException { |
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.
Where is FormException
being thrown? Can we mitigate?
pom.xml
Outdated
<version>2.34</version> | ||
<groupId>org.csanchez.jenkins.plugins</groupId> | ||
<artifactId>kubernetes</artifactId> | ||
<version>4290.v93ea_4b_b_26a_61</version> |
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.
This is at least an upgrade from the prior state, so 👍 for now.
/test e2e-aws-jenkins-sync-plugin |
/test e2e-aws-jenkins-sync-plugin |
@prdhamdh: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
- Bump plugins to fix the compatibility issue of \ openshift/jenkins-sync-plugin#490 Signed-off-by: Vinu K <vkochuku@redhat.com>
Update Jenkins baseline version to 2.440.3
Update Jenkins bom in dependency management section to 2.440.x release line.
Remove dependency version overrides as needed.