-
Notifications
You must be signed in to change notification settings - Fork 28.5k
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
[SPARK-48867][BUILD] Upgrade okhttp
to 4.12.0, okio
to 3.9.0 and esdk-obs-java
to 3.24.3
#47795
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -171,6 +171,41 @@ | |
<groupId>org.apache.hadoop</groupId> | ||
<artifactId>hadoop-cos</artifactId> | ||
</exclusion> | ||
<!-- | ||
HADOOP-19224 / SPARK-48867: com.huaweicloud:esdk-obs-java:jar:3.20.4.2 is | ||
vulnerable due to okhttp 3.x (CVE-2023-0833, CVE-2021-0341), | ||
it has to be upgraded to 3.24.3 which depends on okhttp 4.12.0 | ||
--> | ||
<exclusion> | ||
<groupId>com.huaweicloud</groupId> | ||
<artifactId>esdk-obs-java</artifactId> | ||
</exclusion> | ||
</exclusions> | ||
</dependency> | ||
<dependency> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will it actually work with this removal? if not best to stop trying to restore it and exclude all huaweicloud support with the release note/spark docs saying "explicitly import it" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry for the late reply
I do not have access to Huawei Cloud, therefore I could not test it with an obs bucket. Found this documentation for Spark testing: https://support.huaweicloud.com/intl/en-us/devg-dli/dli_09_0205.html#section6
I did some research and found only Hadoop unit tests but these are disabled by default. Related documentation: Similar configuration has to be created if somebody has such credentials:
Just to be on the safe side, I agree that we should do what @steveloughran suggested above:
@pan3793 / @panbingkun / @dongjoon-hyun / @melin / @bjornjorgensen what is your opinion about this suggestion? If you agre as well, I would like to implement these:
|
||
<groupId>com.huaweicloud</groupId> | ||
<artifactId>esdk-obs-java</artifactId> | ||
<version>${esdk.obs.java.version}</version> | ||
<exclusions> | ||
<exclusion> | ||
<groupId>org.jetbrains.kotlin</groupId> | ||
<artifactId>kotlin-stdlib-jdk8</artifactId> | ||
</exclusion> | ||
<exclusion> | ||
<groupId>org.jetbrains.kotlin</groupId> | ||
<artifactId>kotlin-stdlib</artifactId> | ||
</exclusion> | ||
</exclusions> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.jetbrains.kotlin</groupId> | ||
<artifactId>kotlin-stdlib</artifactId> | ||
<version>${kotlin-stdlib.version}</version> | ||
<exclusions> | ||
<exclusion> | ||
<groupId>org.jetbrains</groupId> | ||
<artifactId>annotations</artifactId> | ||
</exclusion> | ||
</exclusions> | ||
</dependency> | ||
<!-- | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -161,6 +161,12 @@ | |
<aws.java.sdk.v2.version>2.24.6</aws.java.sdk.v2.version> | ||
<!-- the producer is used in tests --> | ||
<aws.kinesis.producer.version>0.12.8</aws.kinesis.producer.version> | ||
<!-- | ||
HADOOP-19224 / SPARK-48867: com.huaweicloud:esdk-obs-java:jar:3.20.4.2 is | ||
vulnerable due to okhttp 3.x (CVE-2023-0833, CVE-2021-0341), | ||
it has to be upgraded to 3.24.3 which depends on okhttp 4.12.0 | ||
--> | ||
<esdk.obs.java.version>3.24.3</esdk.obs.java.version> | ||
<!-- Do not use 3.0.0: https://github.com/GoogleCloudDataproc/hadoop-connectors/issues/1114 --> | ||
<gcs-connector.version>hadoop3-2.2.25</gcs-connector.version> | ||
<!-- org.apache.httpcomponents/httpclient--> | ||
|
@@ -237,7 +243,9 @@ | |
<!-- org.fusesource.leveldbjni will be used except on arm64 platform. --> | ||
<leveldbjni.group>org.fusesource.leveldbjni</leveldbjni.group> | ||
<kubernetes-client.version>6.13.4</kubernetes-client.version> | ||
<okio.version>1.17.6</okio.version> | ||
<okio.version>3.9.0</okio.version> | ||
<okhttp.version>4.12.0</okhttp.version> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. I have uncompressed the kotlin-stdlib-2.0.10.jar file and all class files are part of the kotlin directory / package:
All Spark unit tests have passed in this separate pull request: roczei#4. Currently I can only validate these, I hope it will be enough. |
||
<kotlin-stdlib.version>2.0.10</kotlin-stdlib.version> | ||
|
||
<test.java.home>${java.home}</test.java.home> | ||
|
||
|
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.
it's a little bit crazy to pull another JVM lang runtime into Spark classpath by just consuming an HTTP library.
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.
@pan3793,
yes, I agree that it is bad that we have to add kotlin-stdlib/2.0.10//kotlin-stdlib-2.0.10.jar as a dependency. I have just removed the other two extra dependencies because we do not need them. Related comment: #47795 (comment)
As I mentioned in the pull request's description the kubernetes-client's maintainers do not want upgrade to okhttp 4.x because it's based on Kotlin, they recommend to exclude 3.x. Related documentation:
https://github.com/fabric8io/kubernetes-client/blob/main/doc/KubernetesClientWithIPv6Clusters.md
Currently I do not see other solution to resolve these CVEs: CVE-2021-0341, CVE-2023-0833
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.
kubernetes-httpclient-vertx replace kubernetes-httpclient-okhttp to avoid this problem
fabric8io/kubernetes-client#2764
@pan3793 @roczei
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.
@melin the mockserver is not ported over..
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.
hadoop uses the v3 mockwebserver in test dependencies only, which also seems to be the only place that koitlin comes in.
mmm. maybe they've found koitlin a good language for coding; as more projects use then it will becomes less unusual -instead becoming just another dependency pain point à la guava
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.
@melin / @bjornjorgensen
yes, I confirm it as well
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.
@roczei and @melin her fabric8io/kubernetes-client#5632 is the POC for replacing the MockWebServer but as the PR it is not merged.
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.
@bjornjorgensen,
Thanks!