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

Add option to dynamically update the baseline of the RJRInit plugin #780

Conversation

jtnord
Copy link
Member

@jtnord jtnord commented Jun 5, 2024

if the version of Jenkins used by RealJenkinsRuleInit has detached plugins in the version of Jenkins used for the test then this can unnecessarily pull in detached plugins.

In most cases this is harmless, but in some cases these plugins may be prevented from running under the test (e.g. testing FIPS and one of the detached plugins like eddsa is not FIPS complaint and blocks start-up).

The plugin itself has limited exposure to the Jenkins api (via hudson.plugin.Plugin and RealJenkinsRule$Init2).

Code running from the plugin tests inside RealJenkinsRule uses the uberClassloader and if the plugin was targeting a lower core (where the dependencies have been detached, they should still be included as this change only affects the version of the RealJenkinsRuleInit plugin.

this is a "future proof" (but potentially more risky) version of #779 (nb. the changes are not mutually exclusive)

consumed in jenkinsci/jenkins#9353

Testing done

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

if the version of Jenkins used by RealJenkinsRuleInit has detached
plugins in the version of Jenknis used for the test then this can
uncessescarily pull in detached plugins.

In most cases this is harmless, but in some cases these plugins may be
prevented from running under the test (e.g. testing FIPS and one of the
detached plugins like eddsa is not FIPS complaint and blocks startup).

The plugin itself has limited exposure to the Jenkins api (via
hudson.plugin.Plugin and RealJenkinsRule$Init2).

Code running from the plugin tests inside RealJenkinsRule use the
uberClassloader and if the plugin was targetting a lower core (where the
dependencies have been deteached, they should still be included as this
change only affects the version of the RealJenkinsRuleInit plugin.
@@ -199,6 +200,7 @@ public final class RealJenkinsRule implements TestRule {

private boolean prepareHomeLazily;
private boolean provisioned;
private boolean updateRealJenkinsRuleInitPluginBaseline;
Copy link
Member Author

Choose a reason for hiding this comment

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

defaulting to false as

  1. it changes the current behaviour and
  2. if it does turn out to impact plugins I do not want to be on the hook for fixing them all

Copy link
Member

Choose a reason for hiding this comment

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

is it worth just running this through bom with it enabled by default? this doesn't sound like something you want to have to try dig out when it should "just work".

Copy link
Member Author

Choose a reason for hiding this comment

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

#781 I expect anything using a JNLP based agent and an updated baseline to fail...

Copy link
Member Author

Choose a reason for hiding this comment

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

* The intended use case of this is to prevent any detached plugins being dragged in when they would not normally be.
* @param updateRealJenkinsRuleInitPluginBaseline {@code true} to update the plugin, false to use the plugin as is.
*/
public RealJenkinsRule updateRealJenkinsRuleInitPluginBaseline(boolean updateRealJenkinsRuleInitPluginBaseline) {
Copy link
Member Author

Choose a reason for hiding this comment

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

naming is hard....

FileUtils.copyURLToFile(RealJenkinsRule.class.getResource("RealJenkinsRuleInit.jpi"), new File(plugins, "RealJenkinsRuleInit.jpi"));
if (updateRealJenkinsRuleInitPluginBaseline) {
try (JarFile jf = new JarFile(war);
InputStream resourceAsStream = RealJenkinsRule.class.getResourceAsStream("RealJenkinsRuleInit.jpi");
Copy link
Member

Choose a reason for hiding this comment

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

Actually with #781 (comment) we could clean up some longstanding tech debt and get rid of the .jpi file in Git: just compile the one class from sources in this harness (include in src/main/java/) and pack the plugin on the fly.

@jtnord
Copy link
Member Author

jtnord commented Jun 7, 2024

closing in favor of #782

@jtnord jtnord closed this Jun 7, 2024
@jtnord jtnord deleted the add-option-to-bump-baseline-of-rjrinit-plugin branch June 7, 2024 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants