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

chore : Upgrade keycloak to 19.0.0 #2952

Merged
merged 1 commit into from
Aug 6, 2022

Conversation

Rajpratik71
Copy link
Contributor

Closes #1009

Signed-off-by: Pratik Raj rajpratik71@gmail.com

@@ -2250,5 +2250,63 @@
},
"clientPolicies": {
"policies": []
}
},
"users": [
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this merge really mandatory? It's not possible to keep files separated with this version?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I really don't like this file merge, it is not possible to keep two files here?

@DamnClin
Copy link
Collaborator

DamnClin commented Aug 5, 2022

Looks like you have a formatting problem. You can run:

npm i
npm run prettier:format

(The manual formatting is only needed here since npm i will install a git hook to handle formatting during commits)

@Rajpratik71 Rajpratik71 force-pushed the keycloak-19-upgrade branch from 2dededa to c632619 Compare August 5, 2022 10:36
@DamnClin
Copy link
Collaborator

DamnClin commented Aug 5, 2022

You need to update OAuth2ModuleFactory to use the new image name at this line:

DockerImage keycloakImage = dockerImages.get("jboss/keycloak");

You then need to update OAuth2ModuleFactoryTest and change mock and expected value (this mock for test is not really great, we should work on something using the real image name and just setting the version)

Signed-off-by: Pratik Raj <rajpratik71@gmail.com>
@Rajpratik71 Rajpratik71 force-pushed the keycloak-19-upgrade branch from c632619 to 6b9fc7a Compare August 5, 2022 12:37
@codecov
Copy link

codecov bot commented Aug 5, 2022

Codecov Report

Merging #2952 (6b9fc7a) into main (278dbb9) will not change coverage.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##                main     #2952   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
  Complexity      1978      1978           
===========================================
  Files            470       470           
  Lines           7995      7990    -5     
  Branches         155       155           
===========================================
- Hits            7995      7990    -5     
Impacted Files Coverage Δ
...rity/oauth2/domain/AngularOauth2ModuleFactory.java 100.00% <ø> (ø)
...ation/springdoc/domain/SpringdocModuleFactory.java 100.00% <100.00%> (ø)
...vc/security/oauth2/domain/OAuth2ModuleFactory.java 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@Rajpratik71 Rajpratik71 requested a review from DamnClin August 5, 2022 13:48
@DamnClin
Copy link
Collaborator

DamnClin commented Aug 6, 2022

Ok, just wasted one hour trying to figure out how to have multiple files for the same realm at startup and that just seems to be a real pain to do (at least the current keycloak codebase don't seems to have something allowing that easily) so this is OK for me.

@pascalgrimaud ok for you (as you probably know this part way better than I am), I merge?

@DamnClin
Copy link
Collaborator

DamnClin commented Aug 6, 2022

How, just realized that I'm not supposed to merge this since there's a Bounty on it!

@pascalgrimaud
Copy link
Member

@DamnClin : if it works, you can merge it, no worry about the bounty as once it's submitted, the admins (Julien, Deepu, Daniel and me) will be notified and can approve it

@DamnClin
Copy link
Collaborator

DamnClin commented Aug 6, 2022

Yep, that works, thanks a lot @Rajpratik71, don't forget to claim your bounty :) https://www.jhipster.tech/bug-bounties/#how-to-get-the-money

@DamnClin DamnClin merged commit 4318827 into jhipster:main Aug 6, 2022
@Rajpratik71 Rajpratik71 deleted the keycloak-19-upgrade branch August 6, 2022 14:55
@Rajpratik71
Copy link
Contributor Author

Yep, that works, thanks a lot @Rajpratik71, don't forget to claim your bounty :) https://www.jhipster.tech/bug-bounties/#how-to-get-the-money

Sure thanks @DamnClin @pascalgrimaud

@Rajpratik71
Copy link
Contributor Author

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.

Upgrade to Keycloak 19
3 participants