[SPARK-29252][BUILD] Upgrade zookeeper to 3.4.14 and fix vulnerabilities. by beliefer · Pull Request #25933 · apache/spark

beliefer · 2019-09-26T03:42:34Z

What changes were proposed in this pull request?

The current code uses org.apache.zookeeper:zookeeper:jar:3.4.6 and it will cause a security vulnerabilities. We could get some security info from https://www.tenable.com/cve/CVE-2019-0201

This reference remind to upgrate the version of zookeeper to 3.4.14 or later.

Why are the changes needed?

This PR fix the security vulnerabilities.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Exists UT.

pom.xml

SparkQA · 2019-09-26T05:39:12Z

Test build #111383 has finished for PR 25933 at commit 39ea96d.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

wangyum · 2019-09-26T06:14:36Z

pom.xml

    <protobuf.version>2.5.0</protobuf.version>
    <yarn.version>${hadoop.version}</yarn.version>
-    <zookeeper.version>3.4.6</zookeeper.version>
+    <zookeeper.version>3.4.14</zookeeper.version>


Is it safe to upgrade zookeeper to 3.4.14 for Hadoop 2.7?

Yes that's a good question @beliefer - what does 2.7 use?
I suspect we can update it. In that case we don't need to special-case the ZK version below anymore.

Hadoop 2.7 uses zookeeper:3.4.6 by default.
It's OK. I reference https://www.cnblogs.com/ywjfx/p/11344345.html.

Could you try to upgrade zookeeper to 3.4.14 for Hadoop 2.7.4, and check if Hadoop test can pass?

beliefer · 2019-09-26T06:33:36Z

Retest this please.

SparkQA · 2019-09-26T07:05:01Z

Test build #111396 has finished for PR 25933 at commit 39ea96d.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

beliefer · 2019-09-26T07:09:50Z

Retest this please.

SparkQA · 2019-09-26T09:25:19Z

Test build #111400 has finished for PR 25933 at commit 39ea96d.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

beliefer · 2019-09-26T09:28:29Z

Retest this please.

SparkQA · 2019-09-26T11:57:42Z

Test build #111418 has finished for PR 25933 at commit 39ea96d.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

srowen · 2019-09-26T13:12:07Z

pom.xml

    <protobuf.version>2.5.0</protobuf.version>
    <yarn.version>${hadoop.version}</yarn.version>
-    <zookeeper.version>3.4.6</zookeeper.version>
+    <zookeeper.version>3.4.14</zookeeper.version>


Yes that's a good question @beliefer - what does 2.7 use?
I suspect we can update it. In that case we don't need to special-case the ZK version below anymore.

srowen · 2019-09-26T13:14:13Z

dev/deps/spark-deps-hadoop-2.7

 arrow-format-0.12.0.jar
 arrow-memory-0.12.0.jar
 arrow-vector-0.12.0.jar
+audience-annotations-0.5.0.jar


This is an ALv2 dependency from Yetus, it seems. It needs a line in LICENSE-binary with other ASF dependencies. We'd also have to check if it has a NOTICE file bundled to append to NOTICE-binary

@srowen Thanks very much! However, I have not fully understood what you mean. Do we need NOTICE file?

I checked the audience-annotations-0.5.0.jar and find the NOTICE file.
The content of the NOTICE file show below:

Apache Yetus - Audience Annotations Copyright 2015-2017 The Apache Software Foundation This product includes software developed at The Apache Software Foundation (http://www.apache.org/).

That's fine, just append this to the bottom of NOTICE-binary

OK. I will append the NOTICE-binary.

srowen · 2019-09-26T13:16:04Z

dev/deps/spark-deps-hadoop-3.2

 spire-platform_2.12-0.17.0-M1.jar
 spire-util_2.12-0.17.0-M1.jar
 spire_2.12-0.17.0-M1.jar
+spotbugs-annotations-3.1.9.jar


Uh oh, this is LGPL. We can't include this. https://spotbugs.github.io/
It looks like ZK specifically excludes it from their binary release: apache/zookeeper@372e713
Therefore, we probably just need to write an exclusion rule to not pull in this dependency, as it sounds like it's not strictly needed at runtime (just annotations)

@srowen Thank you for your very detailed investigation. I learned more. I will try to exclude it.

SparkQA · 2019-09-29T04:54:10Z

Test build #111550 has finished for PR 25933 at commit ce4d184.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-09-29T09:54:05Z

Test build #111559 has finished for PR 25933 at commit ff46c08.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

srowen

Looks OK to me. We could probably exclude the other annotations too without issue, but it's OK to leave them as is.

srowen · 2019-09-29T14:50:07Z

pom.xml

        <hadoop.version>3.2.0</hadoop.version>
        <curator.version>2.13.0</curator.version>
-        <zookeeper.version>3.4.13</zookeeper.version>
+        <zookeeper.version>3.4.14</zookeeper.version>


I think you can just remove this if the profile does not change the value.

Thanks for your remind. I will remove this line.

SparkQA · 2019-09-30T05:05:17Z

Test build #111586 has finished for PR 25933 at commit b72b588.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

beliefer · 2019-10-07T13:34:53Z

@srowen Thanks for your help! @dongjoon-hyun @wangyum Thanks for all your review！

beliefer added 2 commits September 26, 2019 10:12

Upgrade zookeeper to 3.4.14 and fix vulnerabilities.

908efe1

update the manifest.

39ea96d

dongjoon-hyun reviewed Sep 26, 2019

View reviewed changes

pom.xml Show resolved Hide resolved

wangyum reviewed Sep 26, 2019

View reviewed changes

srowen requested changes Sep 26, 2019

View reviewed changes

dongjoon-hyun added the BUILD label Sep 26, 2019

exclude spotbugs-annotations and update NOTICE-binary.

ce4d184

Remove tab.

ff46c08

srowen reviewed Sep 29, 2019

View reviewed changes

Remove profile no change value.

b72b588

srowen approved these changes Sep 30, 2019

View reviewed changes

srowen closed this in 1018390 Sep 30, 2019

Conversation

beliefer commented Sep 26, 2019

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

Uh oh!

SparkQA commented Sep 26, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

beliefer commented Sep 26, 2019

Uh oh!

SparkQA commented Sep 26, 2019

Uh oh!

beliefer commented Sep 26, 2019

Uh oh!

SparkQA commented Sep 26, 2019

Uh oh!

beliefer commented Sep 26, 2019

Uh oh!

SparkQA commented Sep 26, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Sep 29, 2019

Uh oh!

SparkQA commented Sep 29, 2019

Uh oh!

srowen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

beliefer Sep 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Sep 30, 2019

Uh oh!

beliefer commented Oct 7, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

beliefer Sep 30, 2019 •

edited

Loading