-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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
security: upgrade seata-server spring version #6013
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## 2.x #6013 +/- ##
=========================================
Coverage 49.62% 49.62%
+ Complexity 4769 4768 -1
=========================================
Files 911 911
Lines 31316 31316
Branches 3773 3773
=========================================
Hits 15540 15540
+ Misses 14243 14240 -3
- Partials 1533 1536 +3 |
d3a184d
to
384dacc
Compare
@@ -29,8 +29,9 @@ | |||
<description>console for Seata built with Maven</description> | |||
|
|||
<properties> | |||
<spring-boot-for-server.version>${spring-boot.version}</spring-boot-for-server.version> | |||
<spring-framework-for-server.version>${spring-framework.version}</spring-framework-for-server.version> | |||
<spring-boot-for-server.version>2.7.17</spring-boot-for-server.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why modify here instead of directly modifying the main pom?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The client SDK should try to use lower versions as much as possible to have more users. The server, as a separately deployed process, should try to avoid or minimize security vulnerabilities, which is decoupled from the user program.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why modify here instead of directly modifying the main pom?
@slievrly is right.
<spring-framework-for-server.version>${spring-framework.version}</spring-framework-for-server.version> | ||
<spring-boot-for-server.version>2.7.17</spring-boot-for-server.version> | ||
<spring-framework-for-server.version>5.3.30</spring-framework-for-server.version> | ||
<snakeyaml-for-server.version>2.0</snakeyaml-for-server.version> | ||
</properties> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
.github/workflows/build.yml
Outdated
@@ -44,7 +44,7 @@ jobs: | |||
# job 2: Test on 'arm64v8/ubuntu' OS. | |||
build_arm64-binary: | |||
runs-on: ubuntu-latest | |||
if: ${{ github.event_name == 'push' && (github.ref_name == 'develop' || github.ref_name == 'snapshot' || github.ref_name == '2.x') }} | |||
# if: ${{ github.event_name == 'push' && (github.ref_name == 'develop' || github.ref_name == 'snapshot' || github.ref_name == '2.x') }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be commented out here, we need ci build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be commented out here, we need ci build
Here should be just to test arm build only, because in the previous community tried to upgrade, encountered a high version of spring boot can not be compiled on the ci, but arm local compilation is not a problem
这里应该只是为了测试arm build而已,因为在之前社区尝试过升级,遇到过高版本spring boot无法在ci上通过编译,但是arm本地编译没有问题
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be commented out here, we need ci build
Here should be just to test arm build only, because in the previous community tried to upgrade, encountered a high version of spring boot can not be compiled on the ci, but arm local compilation is not a problem 这里应该只是为了测试arm build而已,因为在之前社区尝试过升级,遇到过高版本spring boot无法在ci上通过编译,但是arm本地编译没有问题
ok,I see
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Ⅰ. Describe what this PR did
upgrade seata-server`s spring version
Ⅱ. Does this pull request fix one issue?
fixes #6011
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews