Update parent pom + adjustments #46

Wadeck · 2020-07-10T17:46:23Z

Update the pom.xml to use latest dependencies and adjust the tests to be compatible.

…s no longer recommended

pom.xml

jglick · 2020-07-16T12:51:38Z

pom.xml

+      <dependency>
+        <groupId>org.jenkins-ci.plugins.workflow</groupId>
+        <artifactId>workflow-step-api</artifactId>
+        <version>2.22</version>


+1.
But it's not required for this request.

Will do in a follow-up

.../org/jenkinsci/plugins/authorizeproject/strategy/SpecificUsersAuthorizationStrategyTest.java

jglick · 2020-07-16T12:55:06Z

.../org/jenkinsci/plugins/authorizeproject/strategy/SpecificUsersAuthorizationStrategyTest.java

+            String stderr = result.stderr();
+            int ret = result.returnCode();
+
+            assertEquals(stderr, 0, ret);


There is a custom assertion type in CLICommandInvoker BTW.

+1
Like this: https://github.com/jenkinsci/jenkins/blob/jenkins-2.250/test/src/test/java/hudson/cli/ListPluginsCommandTest.java#L48

…SpecificUsersAuthorizationStrategyTest.java Co-authored-by: Jesse Glick <jglick@cloudbees.com>

Co-authored-by: Jesse Glick <jglick@cloudbees.com>

…SpecificUsersAuthorizationStrategyTest.java Co-authored-by: Jesse Glick <jglick@cloudbees.com>

ikedam · 2020-08-02T04:03:09Z

Backgrounds of changes in tests:

Jenkins.getInstance() and Jenkins.getActiveInstance() are deprecated and replaced with Jenkins.get() in jenkins-2.98: `
User.get() performs ID resolution, and it's preferred to use User.getById() for id for performance reasons.
CLI() is deprecated in jenkins-2.54, and removed in jenkins-2.165
Password fields are no longer implemented with <input type="password"> since jenkins-2.205 (Hide password form fields by default jenkins#3991).
- It is first set to hidden and toggled to password if clicked.

ikedam

The changes look good to me.
But I'm not sure I should merge this request as the latest LTS is not necessary for authorize-project itself. It's only required for #47, which is still in draft.

Is it make sense to create a branch for JENKINS-5303?

ikedam · 2020-08-02T03:16:08Z

Jenkinsfile

@@ -1 +1 @@
-buildPlugin(configurations: buildPlugin.recommendedConfigurations())
+buildPlugin()


Note: The default behavior of buildPlugin:

platform=[linux, windows] x jdkVersion=8 x jenkinsVersions=null

It might be better to add jdk11 and latest LTS, but it's another issue and not required for this request.

ikedam · 2020-08-02T03:17:22Z

pom.xml

@@ -36,17 +36,26 @@
  </licenses>

  <properties>
-    <jenkins.version>2.89.4</jenkins.version>
+    <jenkins.version>2.235.1</jenkins.version>


Note:
The latest LTS as this is the preparation for #47 to accept a upcoming Jenkins core.

ikedam · 2020-08-02T03:18:59Z

pom.xml

+      <dependency>
+        <groupId>org.jenkins-ci.plugins.workflow</groupId>
+        <artifactId>workflow-step-api</artifactId>
+        <version>2.22</version>


+1.
But it's not required for this request.

ikedam · 2020-08-02T03:44:01Z

.../org/jenkinsci/plugins/authorizeproject/strategy/SpecificUsersAuthorizationStrategyTest.java

+            String stderr = result.stderr();
+            int ret = result.returnCode();
+
+            assertEquals(stderr, 0, ret);


+1
Like this: https://github.com/jenkinsci/jenkins/blob/jenkins-2.250/test/src/test/java/hudson/cli/ListPluginsCommandTest.java#L48

Wadeck · 2020-08-03T07:48:59Z

But I'm not sure I should merge

I don't think there is any urgency here :)

jglick · 2020-08-03T23:49:03Z

the latest LTS is not necessary for authorize-project itself

No, but it is generally helpful to keep the source base fresh by making sure everything works on a current version of Jenkins. After all, if you are using anything older than the current LTS, you are deliberately declining to receive patches for disclosed vulnerabilities…so why would you expect to get updates to a security-related plugin?

ikedam · 2020-08-05T04:28:49Z

Availavilities and compatibilities are more important than security for some instances, especially instances in almost-offlined environments. It costs much to keep versions up-to-date in those environments. It’s helpful in those environments if you can update a specific plugin to the latest to use new features or to apply bug fixes of that plugin.
That’s why I prefer to use older core.

“Should plugins use the latest LTS?” is supposed to consist of many issues:

recommendedConfigutation() for buildPlugin() is provided to run tests against the latest core. It assumes that plugins doesn’t use the latest core by default. (anyway, I know recommendedConfigurations are no longer recommended.)
The plugin compatibility tester is to test plugins against various versions of Jenkins core. It assumes that plugins doesn’t use the latest core.
A mechanism like dependabot for Jenkins core may be required.
It might not be applicable to general plugins, but security-releated plugins should be maintained in that policy.
It looks strange to change of the target core for patch releases like fixing documantations.
It’s too old even though I prefer older core. (On the other hand, what version is old but not too old.)

I think I can work on the guideline if there’s a community guideline for the target core version.

jglick · 2020-08-05T22:54:39Z

instances in almost-offlined environments

Remember that even offline servers are not immune to CSRF attacks.

My general preference, FWIW, is to just set a recent LTS baseline in plugin master to make life easier. If and when an important, low-risk bug fix is merged, it is easy enough to cherry-pick it and produce a release compatible with older cores: https://gist.github.com/jglick/86a30894446ed38f918050c1180483e2

ikedam · 2020-08-07T07:15:51Z

@jglick
OK., I agree that Jenkins administrators must always use the latest LTS core (actually, it’s highly recommended, not only for security reasons, but for every aspects).
Whether to consider about administrators using older cores is up to plugin maintainers, but “it’s unnecessary to consider about older cores” is appropriate as the policy of Jenkins community.

But the policy for plugins to use latest LTS cores results maintainers to update and make new releases so often. So the some automated mechanism like the one you planed are required
(actually, I don’t plan to make a new release after merging this request).

Introducing develop branch is good way for your idea:

master branch: Points the latest release version. Bug fixes are applied to this branch and target core is updated automatically.
develop branch: new features for next release are introduced.
master branch is merged into develop when updated.
develop branch is merged into master branch when cutting a new release.

jglick · 2020-08-07T12:51:43Z

I don’t plan to make a new release after merging this request

Does not seem to be marked as being up for adoption yet. It is possible to do this now via a GH repo tag.

A develop branch is a possible workflow. I just have not seen the need for this sort of overhead in practice. Doing everything on master (cutting releases whenever anything of interest is merged) and freshening up master dependencies whenever touching the plugin works fine in my experience, and greatly simplifies project-wide maintenance like running plugin-compat-tester and removing deprecated APIs.

some automated mechanism like the one you planned

Many plugins do use Dependabot for plugin and library deps as well as for parent POM and BOM. Does not currently work for the core dep due to technical limitations.

ikedam · 2020-08-09T03:14:03Z

I thought this is preparation #47, but #47 is closed. I don't know the background of this pull request... (maybe PCT failed?)
Anyway, I don't plan to keep core updated to the latest LTS without automated updating and releasing mechanisms.

This plugin is now adopt-this-plugin and you can take over maintenance.

ikedam · 2020-08-09T03:31:52Z

@jglick I suppose it cannot be automated to pick up commits to backport (FIX in your gist) and to resolve errors when they occurred in releasing.
master / develop management makes it easier. The bot can create a pull request and notify maintainers if errors occur in releasing. There would be no target branch if there are only master.

jglick · 2020-08-19T20:42:57Z

I don't know the background of this pull request

Just that as part of testing the impact of something like jenkinsci/jenkins#4848 it is helpful to be able to take some plugin and run PCT on it, or even more lighter-weight

mvn -f some-plugin test -Djenkins.version=2.xxx-SNAPSHOT

and having a really outdated baseline in the plugin makes that more complicated and painful. The older the baselines in widely used and sensitive plugins, the harder it is to maintain them as part of the whole Jenkins ecosystem.

it cannot be automated to pick up commits to backport

I would only propose a backport of some critical and safe fix to begin with, not every change to trunk.

oleg-nenashev

Time to ship it finally

oleg-nenashev · 2021-03-10T15:40:10Z

pom.xml

+      <dependency>
+        <groupId>org.jenkins-ci.plugins.workflow</groupId>
+        <artifactId>workflow-step-api</artifactId>
+        <version>2.22</version>


Will do in a follow-up

Wadeck added 3 commits July 10, 2020 19:45

Update parent pom + adjustments

9ae2dd4

Update Jenkinsfile to prevent usage of the recommendedSettings that i…

aec8ff6

…s no longer recommended

Remove a TODO and re-trigger tests

f076f12

Wadeck mentioned this pull request Jul 11, 2020

[JENKINS-5303] PoC Acegi facade approach jenkinsci/jenkins#4844

Closed

jglick approved these changes Jul 16, 2020

View reviewed changes

Wadeck and others added 3 commits July 16, 2020 17:35

Update src/test/java/org/jenkinsci/plugins/authorizeproject/strategy/…

311f9e0

…SpecificUsersAuthorizationStrategyTest.java Co-authored-by: Jesse Glick <jglick@cloudbees.com>

Update pom.xml

d47ce9b

Co-authored-by: Jesse Glick <jglick@cloudbees.com>

Update src/test/java/org/jenkinsci/plugins/authorizeproject/strategy/…

03c81fa

…SpecificUsersAuthorizationStrategyTest.java Co-authored-by: Jesse Glick <jglick@cloudbees.com>

ikedam reviewed Aug 2, 2020

View reviewed changes

jglick mentioned this pull request Mar 9, 2021

[JENKINS-64341] Make the plugin compatible with Jenkins 2.266+ configuration form changes #51

Merged

oleg-nenashev approved these changes Mar 10, 2021

View reviewed changes

oleg-nenashev merged commit c78c57e into jenkinsci:master Mar 10, 2021

oleg-nenashev added the chore Reduces maintenance effort by changes not directly visible to users label Mar 10, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update parent pom + adjustments #46

Update parent pom + adjustments #46

Wadeck commented Jul 10, 2020

jglick Jul 16, 2020

ikedam Aug 2, 2020

oleg-nenashev Mar 10, 2021

jglick Jul 16, 2020

ikedam Aug 2, 2020

ikedam commented Aug 2, 2020

ikedam left a comment

ikedam Aug 2, 2020

ikedam Aug 2, 2020

ikedam Aug 2, 2020

ikedam Aug 2, 2020

Wadeck commented Aug 3, 2020

jglick commented Aug 3, 2020

ikedam commented Aug 5, 2020

jglick commented Aug 5, 2020

ikedam commented Aug 7, 2020

jglick commented Aug 7, 2020

ikedam commented Aug 9, 2020

ikedam commented Aug 9, 2020

jglick commented Aug 19, 2020

oleg-nenashev left a comment

oleg-nenashev Mar 10, 2021

		@@ -1 +1 @@
		buildPlugin(configurations: buildPlugin.recommendedConfigurations())
		buildPlugin()

Update parent pom + adjustments #46

Update parent pom + adjustments #46

Conversation

Wadeck commented Jul 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ikedam commented Aug 2, 2020

ikedam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Wadeck commented Aug 3, 2020

jglick commented Aug 3, 2020

ikedam commented Aug 5, 2020

jglick commented Aug 5, 2020

ikedam commented Aug 7, 2020

jglick commented Aug 7, 2020

ikedam commented Aug 9, 2020

ikedam commented Aug 9, 2020

jglick commented Aug 19, 2020

oleg-nenashev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment