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

[JENKINS-74842] ClassCastException in ReverseProxySecurityRealm#loadGroupByGroupname2 #153

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

basil
Copy link
Member

@basil basil commented Nov 12, 2024

Sprinkling SetContextClassLoader in yet another place hoping that this fixes JENKINS-74842.

Testing done

None

@basil basil added the bug label Nov 12, 2024
@basil basil requested a review from a team as a code owner November 12, 2024 20:42
Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

Seems harmless, though I would expect any bug like this to be reproducible in tests, perhaps some extension to

String leelaEmail = MailAddressResolver.resolve(j.jenkins.getUser("leela"));
assertEquals("[email protected]", leelaEmail);
?

@basil
Copy link
Member Author

basil commented Nov 12, 2024

Thanks, Jesse! Out of curiosity, why was this plugin not adapted to Spring Security as part of the Acegi removal project?

@jglick
Copy link
Member

jglick commented Nov 12, 2024

why was this plugin not adapted to Spring Security

A good question. At this point I do not really remember. It would have made sense to follow up #40 analogously to jenkinsci/ldap-plugin#49.

@basil
Copy link
Member Author

basil commented Nov 12, 2024

Just as it would have made sense to write a test for this bug. 🤷‍♂️

@basil basil merged commit be58e13 into jenkinsci:master Nov 13, 2024
18 checks passed
@basil basil deleted the JENKINS-74842 branch November 13, 2024 18:36
@basil
Copy link
Member Author

basil commented Nov 13, 2024

@sboardwell Could this please be released?

@sboardwell
Copy link
Contributor

@basil I can do it this weekend. Travelling at the moment and the internet is not the best.

@jglick
Copy link
Member

jglick commented Nov 15, 2024

@sboardwell FYI if you set up https://www.jenkins.io/doc/developer/publishing/releasing-cd/ you can trigger a release from your phone (unless of course you need to do manual testing first).

@sboardwell
Copy link
Contributor

I was going to do that as well at some point. How long does it take to set up approximately? I might do that and then release.

@jglick
Copy link
Member

jglick commented Nov 15, 2024

How long does it take to set up approximately?

Generally not long. If you like, I could file a PR for this repo which you could review & approve. I think I have physical permissions to merge here, probably for historical reasons.

@sboardwell
Copy link
Contributor

That would be great if you have the time.

@jglick
Copy link
Member

jglick commented Nov 15, 2024

@jglick
Copy link
Member

jglick commented Nov 15, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants