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.

Sign up for GitHub

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

Jump to bottom

Security fix release #115

Merged
merged 1 commit into from
Oct 28, 2022
Merged

Security fix release #115

merged 1 commit into from
Oct 28, 2022

Conversation

dkasimovskiy
Copy link
Contributor

@dkasimovskiy dkasimovskiy commented Oct 27, 2022 •
edited
Loading

Bump cartridge-java version to 0.9.1
Bump spring to 5.3.23.RELEASE
Bump spring-boot to 2.6.3

Closes #113

ArtDu
ArtDu requested changes Oct 28, 2022
View reviewed changes
pom.xml Outdated
@@ -60,10 +60,10 @@
</scm>

<properties>
<spring.version>5.2.13.RELEASE</spring.version>
<spring.version>5.2.22.RELEASE</spring.version>
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure the 5.2.22.RELEASE doesn't have spring-beans critical CVE?

Copy link
Contributor

Choose a reason for hiding this comment

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

nexus tells us
image

but may be bump to 5.3.22 or 5.3.23

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bumped to 5.3.23.RELEASE

pom.xml Outdated
@@ -60,10 +60,10 @@
</scm>

<properties>
<spring.version>5.2.13.RELEASE</spring.version>
<spring.version>5.2.22.RELEASE</spring.version>
<spring-boot.version>2.3.2.RELEASE</spring-boot.version>
Copy link
Contributor

Choose a reason for hiding this comment

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

May be we should bump it too?

Copy link
Contributor

Choose a reason for hiding this comment

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

@akudiyar I don't get it why we have spring-boot variable but it's used in spring-data-commons?

Copy link
Contributor

Choose a reason for hiding this comment

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

@ArtDu because spring-data-commons in spring-boot sequence version numbers, take a look Compile Dependencies for spring-data-commons

Copy link
Contributor

@ArtDu ArtDu Oct 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@ArtDu because spring-data-commons in spring-boot sequence version numbers, take a look Compile Dependencies for spring-data-commons

@Elishtar I can't find something like spring-boot in there https://github.com/spring-projects/spring-data-commons/blob/main/pom.xml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also bumped to 2.6.3

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ArtDu when I added this library, Sping Boot had different version numbers from the Sping framework. Have they unified it by this time?

Copy link
Contributor

Choose a reason for hiding this comment

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

Noop, I'm not talking about version difference between spring boot and spring framework. I don't understand why we use variable spring-boot-version in spring-data-commons library

@ArtDu
Copy link
Contributor

ArtDu commented Oct 28, 2022 •
edited
Loading

@dkasimovskiy Remove github ticket from title name in commit and PR and move it below in description Closes #113

@dkasimovskiy dkasimovskiy force-pushed the bump/cartridge-java-0.9.1 branch from 23c76fc to 8b0e47f Compare October 28, 2022 13:37
@dkasimovskiy dkasimovskiy requested a review from ArtDu October 28, 2022 13:37
ArtDu
ArtDu reviewed Oct 28, 2022
View reviewed changes
CHANGELOG.md
@@ -2,6 +2,13 @@

## [Unreleased]

## [0.5.2] - 2022-10-27
Copy link
Contributor

Choose a reason for hiding this comment

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

Add version also in README

@dkasimovskiy dkasimovskiy force-pushed the bump/cartridge-java-0.9.1 branch from 8b0e47f to f218169 Compare October 28, 2022 14:08
@dkasimovskiy dkasimovskiy requested a review from ArtDu October 28, 2022 14:08
ArtDu
ArtDu requested changes Oct 28, 2022
View reviewed changes
Copy link
Contributor

@ArtDu ArtDu left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@dkasimovskiy Please remove github ticket from title name in commit and PR and move it below in description Closes #113

@dkasimovskiy dkasimovskiy changed the title gh-113 Security fix release Security fix release Oct 28, 2022
@dkasimovskiy dkasimovskiy requested a review from ArtDu October 28, 2022 14:11
ArtDu
ArtDu previously approved these changes Oct 28, 2022
View reviewed changes
Copy link
Contributor

@ArtDu ArtDu left a comment

Choose a reason for hiding this comment

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

Removing the ticket number in the commit title would be great too

Elishtar
Elishtar previously approved these changes Oct 28, 2022
View reviewed changes
akudiyar
akudiyar requested changes Oct 28, 2022
View reviewed changes
pom.xml
@@ -112,12 +112,6 @@
<version>1.2.3</version>
<scope>test</scope>
</dependency>
<dependency>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please be careful here. Our testcontainers module has an old version of this library too, and it has some major bugs. We need to change this dependency to 1.17.3 or higher, but should not remove it.

Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion we need to have this dependency only in cartridge-java-testcontainers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

Choose a reason for hiding this comment

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

@akudiyar @dkasimovskiy
image
Now it's look shit crutch

@dkasimovskiy
gh-113 Security fix release
8471c87 
Bump cartridge-java version to 0.9.1
Bump spring to 5.3.23.RELEASE
Bump spring-boot to 2.6.3
@dkasimovskiy dkasimovskiy dismissed stale reviews from Elishtar and ArtDu via 8471c87 October 28, 2022 15:34
@dkasimovskiy dkasimovskiy force-pushed the bump/cartridge-java-0.9.1 branch from f218169 to 8471c87 Compare October 28, 2022 15:34
akudiyar
akudiyar approved these changes Oct 28, 2022
View reviewed changes
Copy link
Collaborator

@akudiyar akudiyar left a comment

Choose a reason for hiding this comment

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

LGTM

@akudiyar akudiyar merged commit 299e2fb into master Oct 28, 2022
@ArtDu
Copy link
Contributor

ArtDu commented Oct 28, 2022

Removing the ticket number in the commit title would be great too

@dkasimovskiy Please remove github ticket from title name in commit and PR and move it below in description Closes #113

I don't see my comments have been fixed

@akudiyar @dkasimovskiy I don't like that we merge PR before all changes or comments are approved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Elishtar Elishtar Elishtar approved these changes

@akudiyar akudiyar akudiyar approved these changes

@ArtDu ArtDu Awaiting requested review from ArtDu

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bump all dependencies to the latest version
4 participants
@dkasimovskiy @ArtDu @Elishtar @akudiyar