You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "LuciferYang (via GitHub)" <gi...@apache.org> on 2023/12/25 06:24:15 UTC
[PR] Test Ivy 2.5.2 [spark]
LuciferYang opened a new pull request, #44477:
URL: https://github.com/apache/spark/pull/44477
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
8. If you want to add or modify an error type or message, please read the guideline first in
'core/src/main/resources/error/README.md'.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
-->
### Was this patch authored or co-authored using generative AI tooling?
<!--
If generative AI tooling has been used in the process of authoring this patch, please include the
phrase: 'Generated-by: ' followed by the name of the tool and its version.
If no, write 'No'.
Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Test Ivy 2.5.2 [spark]
Posted by "bjornjorgensen (via GitHub)" <gi...@apache.org>.
bjornjorgensen commented on PR #44477:
URL: https://github.com/apache/spark/pull/44477#issuecomment-1870169741
> Although there is a CVE issue with a score of 8.2 before Ivy 2.5.1
it's Ivy 2.5.2 not 2.5.1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Test Ivy 2.5.2 [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #44477:
URL: https://github.com/apache/spark/pull/44477#issuecomment-1870766747
> "SECURITY-2924 / https://github.com/advisories/GHSA-2jc4-r94c-rp7h
> Ivy Plugin 2.5 and earlier bundles versions of Apache Ivy vulnerable to
> https://github.com/advisories/GHSA-2jc4-r94c-rp7h.
>
> This allows attackers able to control the input file for the "Trigger the
> build of other projects based on the Ivy dependency management system"
> post-build step to have Jenkins parse a crafted XML document that uses
> external entities for extraction of secrets from the Jenkins controller or
> server-side request forgery."
> https://www.openwall.com/lists/oss-security/2023/09/06/9
For Spark, what specific harm will it suffer if it is not upgraded? Since I don't have a clear understanding of the specific results, I choose a way that does not break compatibility now. Due to my personal knowledge limitations, this decision may be wrong. If upgrading is necessary, could you submit a PR to fix it and explain in detail the specific harm to Spark if it is not upgraded? @bjornjorgensen thanks ~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Test Ivy 2.5.2 [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44477:
URL: https://github.com/apache/spark/pull/44477#discussion_r1436089675
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveExternalCatalogVersionsSuite.scala:
##########
@@ -211,6 +213,10 @@ class HiveExternalCatalogVersionsSuite extends SparkSubmitTestUtils {
tryDownloadSpark(version, sparkTestingDir.getCanonicalPath)
}
+ Files.deleteIfExists(Paths.get(sparkHome.getCanonicalPath, "jars", "ivy-2.5.1.jar"))
+ val ivyUrl = new URL("https://repo1.maven.org/maven2/org/apache/ivy/ivy/2.5.2/ivy-2.5.2.jar")
+ IOUtils.copy(ivyUrl, new File(s"${sparkHome.getCanonicalPath}/jars", "ivy-2.5.2.jar"))
Review Comment:
I have been investigating the case of the previous Ivy upgrade failure recently. I found that if the Spark version being tested in `HiveExternalCatalogVersionsSuite` also uses Ivy 2.5.2, the error like https://github.com/apache/spark/pull/44477#issuecomment-1868917258 will not occur again... But I haven't come up with a better solution yet.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Test Ivy 2.5.2 [spark]
Posted by "bjornjorgensen (via GitHub)" <gi...@apache.org>.
bjornjorgensen commented on PR #44477:
URL: https://github.com/apache/spark/pull/44477#issuecomment-1870610343
So the main issue here is that spark have a to old hive version? Are we going to update that to a newer version for spark 4.0?
"_Although there is a CVE issue with a score of 8.2 before Ivy 2.5.1 (https://github.com/advisories/GHSA-2jc4-r94c-rp7h), it doesn't seem to have a significant impact on Apache Spark._"
"SECURITY-2924 / CVE-2022-46751
Ivy Plugin 2.5 and earlier bundles versions of Apache Ivy vulnerable to
CVE-2022-46751.
This allows attackers able to control the input file for the "Trigger the
build of other projects based on the Ivy dependency management system"
post-build step to have Jenkins parse a crafted XML document that uses
external entities for extraction of secrets from the Jenkins controller or
server-side request forgery."
https://www.openwall.com/lists/oss-security/2023/09/06/9
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Test Ivy 2.5.2 [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #44477:
URL: https://github.com/apache/spark/pull/44477#issuecomment-1868917258
When master use ivy 2.5.2 and the test targets use ivy 2.5.1:
```
[info] : java.lang.RuntimeException: problem during retrieve of org.apache.spark#spark-submit-parent-8bd00540-3ae3-45c0-b8cb-adf54c547a85: java.lang.RuntimeException: Multiple artifacts of the module log4j#log4j;1.2.17 are retrieved to the same file! Update the retrieve pattern to fix this error.
[info] at org.apache.ivy.core.retrieve.RetrieveEngine.retrieve(RetrieveEngine.java:238)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Test Ivy 2.5.2 [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44477:
URL: https://github.com/apache/spark/pull/44477#discussion_r1436096023
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveExternalCatalogVersionsSuite.scala:
##########
@@ -220,6 +221,7 @@ class HiveExternalCatalogVersionsSuite extends SparkSubmitTestUtils {
"--conf", s"${MASTER_REST_SERVER_ENABLED.key}=false",
"--conf", s"${HiveUtils.HIVE_METASTORE_VERSION.key}=$hiveMetastoreVersion",
"--conf", s"${HiveUtils.HIVE_METASTORE_JARS.key}=maven",
+ "--conf", s"${JAR_IVY_REPO_PATH.key}=${ivyTestDir.getCanonicalPath}",
Review Comment:
Let me try isolating the Ivy local repo paths for ivy-2.5.1 and 2.5.2 to see if it works.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Test Ivy 2.5.2 [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #44477:
URL: https://github.com/apache/spark/pull/44477#issuecomment-1869151005
Thank you for the investigation, @LuciferYang .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Test Ivy 2.5.2 [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang closed pull request #44477: Test Ivy 2.5.2
URL: https://github.com/apache/spark/pull/44477
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Test Ivy 2.5.2 [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #44477:
URL: https://github.com/apache/spark/pull/44477#issuecomment-1869971189
Apart from backporting the upgrade to branch-3.4 and branch-3.5, I can't think of a better way to reduce this compatibility impact now. So, shall we skip the upgrade to Ivy 2.5.2? Although there is a CVE issue with a score of 8.2 before Ivy 2.5.1 (CVE-2022-46751), it doesn't seem to have a significant impact on Apache Spark.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Test Ivy 2.5.2 [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #44477:
URL: https://github.com/apache/spark/pull/44477#issuecomment-1870762906
> So the main issue here is that spark have a to old hive version? Are we going to update that to a newer version for spark 4.0?
No, the main issue is `incompatibility between the metadata created by Ivy versions 2.5.1 and 2.5.2`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Test Ivy 2.5.2 [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44477:
URL: https://github.com/apache/spark/pull/44477#discussion_r1436095877
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveExternalCatalogVersionsSuite.scala:
##########
@@ -211,6 +213,10 @@ class HiveExternalCatalogVersionsSuite extends SparkSubmitTestUtils {
tryDownloadSpark(version, sparkTestingDir.getCanonicalPath)
}
+ Files.deleteIfExists(Paths.get(sparkHome.getCanonicalPath, "jars", "ivy-2.5.1.jar"))
Review Comment:
Let me try isolating the Ivy cache paths for ivy-2.5.1 and 2.5.2 to see if it works.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Test Ivy 2.5.2 [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44477:
URL: https://github.com/apache/spark/pull/44477#discussion_r1436116952
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveExternalCatalogVersionsSuite.scala:
##########
@@ -220,6 +221,7 @@ class HiveExternalCatalogVersionsSuite extends SparkSubmitTestUtils {
"--conf", s"${MASTER_REST_SERVER_ENABLED.key}=false",
"--conf", s"${HiveUtils.HIVE_METASTORE_VERSION.key}=$hiveMetastoreVersion",
"--conf", s"${HiveUtils.HIVE_METASTORE_JARS.key}=maven",
+ "--conf", s"${JAR_IVY_REPO_PATH.key}=${ivyTestDir.getCanonicalPath}",
Review Comment:
still failed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Test Ivy 2.5.2 [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #44477:
URL: https://github.com/apache/spark/pull/44477#issuecomment-1870189627
> > Although there is a CVE issue with a score of 8.2 before Ivy 2.5.1
>
> it's Ivy 2.5.2 not 2.5.1
Yes, you are right
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Test Ivy 2.5.2 [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44477:
URL: https://github.com/apache/spark/pull/44477#discussion_r1436089675
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveExternalCatalogVersionsSuite.scala:
##########
@@ -211,6 +213,10 @@ class HiveExternalCatalogVersionsSuite extends SparkSubmitTestUtils {
tryDownloadSpark(version, sparkTestingDir.getCanonicalPath)
}
+ Files.deleteIfExists(Paths.get(sparkHome.getCanonicalPath, "jars", "ivy-2.5.1.jar"))
+ val ivyUrl = new URL("https://repo1.maven.org/maven2/org/apache/ivy/ivy/2.5.2/ivy-2.5.2.jar")
+ IOUtils.copy(ivyUrl, new File(s"${sparkHome.getCanonicalPath}/jars", "ivy-2.5.2.jar"))
Review Comment:
I have been investigating the case of the previous Ivy upgrade failure recently. I found that if the Spark version being tested in `HiveExternalCatalogVersionsSuite` also uses Ivy 2.5.2, the error like https://github.com/apache/spark/pull/44477#issuecomment-1868917258 will not occur again... But I haven't come up with a better solution yet.
cc @bjornjorgensen @dongjoon-hyun
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Test Ivy 2.5.2 [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #44477:
URL: https://github.com/apache/spark/pull/44477#issuecomment-1869925312
@dongjoon-hyun @bjornjorgensen Synchronization:
1. The test failure appears to be due to incompatibility between the metadata created by Ivy versions 2.5.1 and 2.5.2 in the `ivy.cache.dir`. This can be resolved by using a separate `ivy.cache.dir` for the older versions of Spark being tested. The current pull request has already been tested successfully.
2. However, I think this issue is not just limited to testing. I conducted additional functional tests on my local machine as follows:
- Prepared a Spark client using Ivy 2.5.2 with version 4.0.0-SNAPSHOT and another Spark 3.x version, such as Spark 3.4.2, using Ivy 2.5.1.
- After clearing the `~/.ivy2` directory, submitted jobs in the order of 3.4.2, 4.0.0-SNAPSHOT with the config `spark.sql.hive.metastore.jars=maven`. Both versions were able to run successfully.
- After clearing the `~/.ivy2` directory again, submitted jobs in the order of 4.0.0-SNAPSHOT, 3.4.2 with the same config `spark.sql.hive.metastore.jars=maven`. The execution of Spark 3.4.2 failed with the following error:
```
[info] : java.lang.RuntimeException: problem during retrieve of org.apache.spark#spark-submit-parent-8bd00540-3ae3-45c0-b8cb-adf54c547a85: java.lang.RuntimeException: Multiple artifacts of the module log4j#log4j;1.2.17 are retrieved to the same file! Update the retrieve pattern to fix this error.
[info] at org.apache.ivy.core.retrieve.RetrieveEngine.retrieve(RetrieveEngine.java:238)`
```
Therefore, it seems that upgrading to Ivy 2.5.2 can cause issues for end users. If a client with Ivy 2.5.2 is used first and then a client with Ivy 2.5.1 is intended to be used again, it is necessary to specify a different `ivy.cache.dir` or manually clear the `ivy.cache.dir` updated by Ivy 2.5.2 in order to use it again properly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] Test Ivy 2.5.2 [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44477:
URL: https://github.com/apache/spark/pull/44477#discussion_r1436095877
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveExternalCatalogVersionsSuite.scala:
##########
@@ -211,6 +213,10 @@ class HiveExternalCatalogVersionsSuite extends SparkSubmitTestUtils {
tryDownloadSpark(version, sparkTestingDir.getCanonicalPath)
}
+ Files.deleteIfExists(Paths.get(sparkHome.getCanonicalPath, "jars", "ivy-2.5.1.jar"))
Review Comment:
Let me try isolating the Ivy cache paths for ivy-2.5.1 and 2.5.2 to see if it works.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org