Closed jglick closed 3 years ago
Hello @jglick, just to be sure: are you finished working on this, and what is the current state? (working or not, tested or not...)
Thanks for all the work around this btw, abandoning Acegi Security was long overdue, and will simplify this plugin a bit ;-)
are you finished working on this
Yes. If merged and released, the update would become available to users running weeklies but not LTS versions, which is what we would want.
Note that unlike all other security realm plugins I worked on, I did not find a way to produce a single version which works in 2.266+ as well as 2.265-, so a user would need to take care to do the upgrade or core and plugin atomically, which is not possible solely using the GUI AFAIK; otherwise after the core upgrade you would be unable to log in as an admin to request the plugin upgrade, a kind of Catch-22. Can probably manually download the plugin update and save it to the Jenkins controller filesystem just prior to running the core update; untested.
what is the current state? (working or not, tested or not...)
Completely untested. As noted in the PR description, the plugin has no functional or acceptance tests, and I scarcely know that CAS is, much less how to set up a test environment for it. This patch compiles against Jenkins 2.266+, and mostly uses migration idioms proven in other plugins, but if for example there were some typo in a String
argument from the Groovy → Java migration this would only be caught by a manual sanity check.
Glad to see an active maintainer here!
Alright thanks for the feedback, I'll take it over from here. Do you know when a LTS including the Spring Security migration will be released?
when a LTS including the Spring Security migration will be released
Expected around early March. We deliberately merged https://github.com/jenkinsci/jenkins/pull/4848 soon after selecting the baseline for the current LTS (2.263) so that we would have the longest possible “runway” to iron out problems.
@fcrespel Let me know if you'd like help testing this out. I'm currently using the CAS plugin in Jenkins. I don't know much about spring-xstream, but I do know how the CAS protocol itself works.
Thanks for the offer @vwbusguy, I'll try working on this during the weekend, I'll let you know when it's ready for testing.
If anyone cares to try out this PR so far (no warranty), you can save https://repo.jenkins-ci.org/incrementals/org/jenkins-ci/plugins/cas-plugin/1.5.0-rc88.860ce26b5e96/cas-plugin-1.5.0-rc88.860ce26b5e96.hpi as $JENKINS_HOME/plugins/cas-plugin.jpi
and restart Jenkins.
I just pushed an update that works on my test environment, @vwbusguy if you want to test it you can grab it here.
The first version didn't work due to various complications with Spring Security filters, SecurityContext, sessions, etc. I also refactored it to bring back an actual Spring AnnotationConfigApplicationContext and the CasEventListener, which is needed for all this integration to work.
Tested cas-plugin-1.5.0-rc89.828f27fec447.hpi
and it appears to work as expected.
Sorry, I got pulled into other things at work. I'll try to test it out this afternoon.
Good news! 1.5.0 works for me on Jenkins 2.267
Good to hear, thanks for the feedback. I'll merge this now, then push a few more updates and create a release soon.
When you cut a release, please https://github.com/jenkinsci/jep/edit/master/jep/227/compatibility.adoc to reflect the compatible status and the version with the fix. There are lots of plugin entries to use as a model; this one is unique in that there is no plugin version compatible with both 2.266+ and 2.265-, so it deserves a note that upgrading will be tricky if you are not configuring Jenkins as code.
@jglick release 1.5.0 is done, compatibility doc PR is in jenkinsci/jep#338
Attempting to make this compatible with https://github.com/jenkinsci/jenkins/pull/4848. There is no test coverage and the usage of Spring is unusual and complex so this may or may not work at all.