-
Notifications
You must be signed in to change notification settings - Fork 57
Conversation
akouznetchik
commented
Apr 19, 2023
•
edited by DuMaM
Loading
edited by DuMaM
- Added SSO token auth caching, can be enabled via web UI. This idea is originally from Adding caching for crowd tokens. #34 and Fix cache, add token auth caching #162 but modified to work. By default, the authentication-filter is mapped to /* therefore all requests including for /static where the header contains an SSO token are validated. This can significantly degrade page loading times depending on latency and Crowd server load. In our testing, after clearing the content cache, enabling this feature improved performance by a factor of 10-20x. The token validation cache will remain valid for the duration of the specified cache TTL, as is the case with other caches.
I will release new version with fix only. |
7e31abe
to
4cb7a17
Compare
In this weekend i will merge this if everything works as expected. |
.../resources/de/theit/jenkins/crowd/CrowdSecurityRealm/CacheConfiguration/config_pl.properties
Outdated
Show resolved
Hide resolved
src/main/resources/de/theit/jenkins/crowd/CrowdSecurityRealm/CacheConfiguration/config.jelly
Outdated
Show resolved
Hide resolved
...ain/resources/de/theit/jenkins/crowd/CrowdSecurityRealm/CacheConfiguration/config.properties
Outdated
Show resolved
Hide resolved
@akouznetchik Could you make it work? This checkbox is not rendering. |
Seems to work for me. Try an |
This is a bit confusing to me. I'm doing a clean build every time with docker image.
This should be updated in separated PR.
Yes, but maybe in feature it's not mandatory now. I misunderstand your change because it was not rendering for me. |
@akouznetchik ? :) |
Hey man, I’m just on vacation and didn’t grab my laptop, so I won’t be able to give docker a go. I was testing using the Jenkins maven hpi plugin, hpi run, because I have my own crowd server to test against and didn’t need the docker image. I will give this a try next weekend. |
@DuMaM ok so this pull request is fine. The reason you do not see the changes is because our current build script does not override anything that was already deployed via See: https://github.com/jenkinsci/docker/blob/master/README.md#installing-custom-plugins We can fix this by copying the plugin HPI file with
I will add a separate PR for this. Essentially our latest plugin was being copied from the You can verify this if you connect to the docker container and do
They will differ. |
@akouznetchik #169 (comment) To be fair im waiting for it to start test it |
i would be grateful if you can hop in and help me maintain this plugin |
Would be my pleasure. |
@DuMaM okay so I opened two PRs, test them out, then let's merge them, and rebase this. Then we need to remove the additional null check that I added, as you see you already re-added it to master. Anything else? |
Please post request on https://github.com/jenkins-infra/helpdesk |
Please rebase this now ;) |
47688e2
to
0ff9fbd
Compare
Why you closed this PR? |
Mistake, re-opened |
@DuMaM Let me know when you have done your testing. Also there are 'Changes requested' pending, but all seem resolved, please double check. |
lgtm 😉 |
@akouznetchik We need to check if this is a potential breaking change. |
Just tested this by upgrading via UI on a Jenkins master that had production plugin v4.0.0 without I also downgraded from my 4.0.1-SNAPSHOT down to 4.0.0 and the extra node was removed without issue. Looks OK to me. |
Tomorrow I will test this out on my prod env and if everything works, then I will perform release |
@akouznetchik could you please take a look on this PR |
@DuMaM can we release this? |
Yes we can. |