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

fix kafka CVE-2023-25194, update kafka client to 3.4.0 #2481

Closed
wants to merge 14 commits into from
Closed

fix kafka CVE-2023-25194, update kafka client to 3.4.0 #2481

wants to merge 14 commits into from

Conversation

xie-shujian
Copy link
Contributor

Description

[Describe what this change achieves]

  • Category (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation)
  • Why these changes are required?
  • What is the old behavior before changes and new behavior after changes?

Issues Resolved

[List any issues this PR will resolve]
fix kafka CVE-2023-25194, update kafka client to 3.4.0
Is this a backport? If so, please add backport PR # and/or commits #
upgrade kafka client and spring test framework

Testing

[Please provide details of testing done: unit testing, integration testing and manual testing]
Test pass

Check List

  • New functionality includes testing
  • New functionality has been documented
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@stephen-crawford
Copy link
Contributor

stephen-crawford commented Feb 24, 2023

Hi @xie-shujian, thank you for taking the time to open up a PR to update the Kafka dependency. Edit: Apparently there is a workaround to fix the version mismatch.

Copy link
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

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

Thank you for creating this pull request.

The DCO check failed, learn more about it and how to remedy from the failure https://github.com/opensearch-project/security/pull/2481/checks?check_run_id=11567975502

Also sounds like after this is resolved there will be the problem that @scrawfor99 mentioned, did you have thoughts on if this can be worked around?

.github/workflows/integration-tests.yml Outdated Show resolved Hide resolved
build.gradle Outdated
@@ -24,7 +24,8 @@ buildscript {
opensearch_build = version_tokens[0] + '.0'

common_utils_version = System.getProperty("common_utils.version", '3.0.0.0-SNAPSHOT')
kafka_version = '3.0.2'
kafka_version = '3.4.0'
kafkaVersion = '3.4.0'
Copy link
Member

Choose a reason for hiding this comment

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

Unused, lets remove

@codecov-commenter
Copy link

Codecov Report

Merging #2481 (9741d41) into main (2df8acd) will decrease coverage by 0.02%.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff              @@
##               main    #2481      +/-   ##
============================================
- Coverage     61.19%   61.18%   -0.02%     
+ Complexity     3325     3322       -3     
============================================
  Files           260      260              
  Lines         18494    18494              
  Branches       3268     3268              
============================================
- Hits          11318    11316       -2     
  Misses         5578     5578              
- Partials       1598     1600       +2     
Impacted Files Coverage Δ
...search/security/transport/SecurityInterceptor.java 75.38% <0.00%> (-0.77%) ⬇️
...ensearch/security/compliance/ComplianceConfig.java 82.63% <0.00%> (-0.70%) ⬇️

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

@stephen-crawford
Copy link
Contributor

Hi @xie-shujian, as @peternied, I am happy to help out with this issue if you have any questions. That being said, it looks like I was mistaken about the version we need not being out yet. The website I looked at was outdated...

If you are able to address the comments Peter left, we can get this squared away together. You will want to follow the guidance on this question.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants