Skip to content

Use Quarkus Platform BOM - #21426

Merged
abstractj merged 2 commits into
keycloak:mainfrom
vmuzikar:platform-bom-pzaoral
Jul 6, 2023
Merged

Use Quarkus Platform BOM#21426
abstractj merged 2 commits into
keycloak:mainfrom
vmuzikar:platform-bom-pzaoral

Conversation

@vmuzikar

@vmuzikar vmuzikar commented Jul 4, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Closes #20570
Closes #15870
Closes #9075

Supersedes #21078
Changes from the original PR:

Umbrella for post-22 follow-up issues: #21356

This is currently blocked as we're waiting for the release of Quarkus Platform BOM for 3.2.0.Final

Pepo48 and others added 2 commits July 4, 2023 17:12
@Pepo48 @vmuzikar
Use Quarkus Platform BOM
39c7c81 
Closes keycloak#20570
Closes keycloak#15870

Co-authored-by: Peter Zaoral <pzaoral@redhat.com>
@vmuzikar
Remove elytron and devservices
eb0e23f 
Closes keycloak#9075
@vmuzikar
vmuzikar force-pushed the platform-bom-pzaoral branch from 54a6ec4 to eb0e23f Compare July 4, 2023 15:12
@vmuzikar vmuzikar closed this Jul 5, 2023
@vmuzikar vmuzikar reopened this Jul 5, 2023
@vmuzikar
vmuzikar marked this pull request as ready for review July 5, 2023 10:20
@vmuzikar
vmuzikar requested review from a team as code owners July 5, 2023 10:20
@vmuzikar
vmuzikar requested a review from a team July 5, 2023 10:20
@vmuzikar

vmuzikar commented Jul 5, 2023

Copy link
Copy Markdown
Contributor Author

In order to merge this, we need reviews from @keycloak/store and @keycloak/core.

@miquelsi Could you please confirm the QE pipelines are happy with this change?

@ASzc Could you please confirm it's ok for prod?

@abstractj Can you please double check the dependencies upgrades/downgrades are ok from CVEs perspective?

@pedroigor With regards to #9075, I currently went the simple path and excluded the Quarkus dev services on mvn level (without using profiles). Are you aware of any use cases we need the dev services for during development currently?

@vmuzikar vmuzikar mentioned this pull request Jul 5, 2023
Closed
sguilhen
sguilhen approved these changes Jul 5, 2023
View reviewed changes

@sguilhen sguilhen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thsanks @vmuzikar ! I'm ok with the much needed cleanup being in this PR - don't think we need a separate one just for that.

From store perspective, it should be ok!

ASzc
ASzc approved these changes Jul 5, 2023
View reviewed changes

@ASzc ASzc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see com.fasterxml.jackson.core:jackson-core and com.fasterxml.jackson.core:jackson-annotations are being removed? There are some product build parameters related to those two, specifically forcing version 2.13.4.redhat-00002, I can't remember why. If they're being removed then easy enough to drop these related build parameters too.

I also see there are some new exclusions for deployment dependencies. That's good, in theory should help clean up non-RHBQ jars in our zip.

Otherwise seems ok. Ideally with the volume of pom changes a trial product build would be performed before merging this PR, but we don't have automation for that yet. If you're interested in doing it manually I can guide you, otherwise I'll fix any errors that crop up in the nightly build after this is merged

ahus1
ahus1 approved these changes Jul 5, 2023
View reviewed changes

@ahus1 ahus1 left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed the changes, thank you for this cleanup. I see that the distribution is now down to 168 MB instead of 188 MB, which is great.

I did a short local test, and it looks fine to me. Thanks!

ahus1
ahus1 approved these changes Jul 5, 2023
View reviewed changes

@ahus1 ahus1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed the changes, thank you for this cleanup. I see that the distribution is now down to 168M instead of 188 MB, which is great.

I did a short local test, and it looks fine to me. Thanks!

@ahus1 ahus1 mentioned this pull request Jul 6, 2023
Closed
@abstractj
abstractj merged commit 3f4d971 into keycloak:main Jul 6, 2023
@vmuzikar

vmuzikar commented Jul 10, 2023

Copy link
Copy Markdown
Contributor Author

Thanks for all the reviews!

@miquelsi Just wanted to double check if you were able to run the internal pipelines with this.

@vmuzikar
vmuzikar deleted the platform-bom-pzaoral branch July 10, 2023 09:28
This was referenced Jul 24, 2023
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@abstractj abstractj abstractj approved these changes
@pedroigor pedroigor pedroigor approved these changes
@ahus1 ahus1 ahus1 approved these changes
@miquelsi miquelsi Awaiting requested review from miquelsi
@shawkins shawkins Awaiting requested review from shawkins
+2 more reviewers
@sguilhen sguilhen sguilhen approved these changes
@ASzc ASzc ASzc approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

team/cloud-native

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Use Quarkus Platform BOMs Cleanup unused WildFly dependencies in the distribution Remove any unnecessary dependency from distribution

7 participants

@vmuzikar @abstractj @sguilhen @pedroigor @ASzc @ahus1 @Pepo48