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 build by updating kafka client to 2.2.2 for CVE-2019-12399 #9259

Merged
merged 3 commits into from
Jan 27, 2020

Conversation

clintropolis
Copy link
Member

Travis seems to be failing due to https://nvd.nist.gov/vuln/detail/CVE-2019-12399, though a quick glance doesn't look like something that would affect us, other than the build failure.

To resolve, updates kafka-client to 2.2.2 which is not listed in the CVE, which required a minor change in druid-kafka-extraction-namespace tests.

@@ -78,7 +78,7 @@
<aether.version>0.9.0.M2</aether.version>
<apache.curator.version>4.1.0</apache.curator.version>
<apache.curator.test.version>2.12.0</apache.curator.test.version>
<apache.kafka.version>2.1.1</apache.kafka.version>
<apache.kafka.version>2.2.2</apache.kafka.version>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you also update extensions-core/kafka-indexing-service to use this version as well please

I had a PR to consolidate the kafka version, but never got around to fully testing it - https://github.com/apache/druid/pull/9117/files

Copy link
Member Author

@clintropolis clintropolis Jan 27, 2020

Choose a reason for hiding this comment

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

Ah, missed that it still had a special version property defined distinct from the parent pom, removed.

@clintropolis
Copy link
Member Author

I wonder if there is a better way we can handle this than randomly failing unrelated PRs whenever a CVE pops up, because that is sort of lame behavior ...

licenses.yaml Outdated
license_category: binary
module: extensions/druid-kafka-indexing-service
module: extensions/druid-kafkakafka-indexing-service
Copy link
Contributor

Choose a reason for hiding this comment

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

typo in the module name?

Copy link
Member Author

Choose a reason for hiding this comment

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

hah oops, my search panel must not have opened when searching for other kafkas in this file... actually we need to update the notice section of this file too since this version was released 2019

Copy link
Contributor

@suneet-s suneet-s left a comment

Choose a reason for hiding this comment

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

I wonder if there is a better way we can handle this than randomly failing unrelated PRs whenever a CVE pops up, because that is sort of lame behavior ...

+1 - it really should only prevent merging a PR when you're introducing a new vulnerability. It'd be nice if this ran nightly and filed issues for new CVEs that are being reported so we can go clean them up as needed

@ccaominh
Copy link
Contributor

Didn't see this and made a similar PR (#9261) that has a slightly different change to TestKafkaExtractionCluster.java and doesn't have a typo in licenses.yaml

@clintropolis
Copy link
Member Author

Didn't see this and made a similar PR (#9261) that has a slightly different change to TestKafkaExtractionCluster.java and doesn't have a typo in licenses.yaml

Any idea which is the correct way to fix TestKafkaExtractionCluster? I suppose it doesn't matter so much since it's just for tests

@clintropolis clintropolis mentioned this pull request Jan 27, 2020
2 tasks
Copy link
Contributor

@suneet-s suneet-s left a comment

Choose a reason for hiding this comment

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

I'm ok with either approach - I think .kafkaController() is easier to read

@ccaominh
Copy link
Contributor

I think the changes toTestKafkaExtractionCluster are equivalent, but we can address that later if needed.

@jihoonson jihoonson merged commit c6c8b80 into apache:master Jan 27, 2020
@clintropolis clintropolis deleted the kafka-client-cve-travis-fix branch January 27, 2020 22:55
clintropolis added a commit to implydata/druid-public that referenced this pull request Jan 30, 2020
…e#9259)

* fix build by updating kafka client to 2.2.2 for CVE-2019-12399

* one kafka version to rule them all

* notice
maytasm pushed a commit to implydata/druid-public that referenced this pull request Feb 19, 2020
…e#9259)

* fix build by updating kafka client to 2.2.2 for CVE-2019-12399

* one kafka version to rule them all

* notice
@jihoonson jihoonson added this to the 0.18.0 milestone Mar 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants