Skip to content

MINOR: Upgrade jetty to 9.4.27.v20200227 and jersey to 2.31#8859

Merged
kkonstantine merged 2 commits intoapache:trunkfrom
akatona84:jersey_jetty_upgrade
Jun 17, 2020
Merged

MINOR: Upgrade jetty to 9.4.27.v20200227 and jersey to 2.31#8859
kkonstantine merged 2 commits intoapache:trunkfrom
akatona84:jersey_jetty_upgrade

Conversation

@akatona84
Copy link
Copy Markdown
Contributor

@akatona84 akatona84 commented Jun 12, 2020 •
edited by kkonstantine
Loading

Upgrade jetty to 9.4.27.v20200227 and jersey to 2.31

Also remove the workaround used on previous version from Connect's SSLUtils.
(Reverts KAFKA-9771 - commit ee832d7)

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@akatona84
Copy link
Copy Markdown
Contributor Author

akatona84 commented Jun 12, 2020

@ijuma jersey came out with 2.31 and it is depending on 9.4.27.v20200227 jetty.

Testing with 9.4.26.v20200117 is still in progress, I'm not sure what the difference is between these two in compatibility perspective. I'd rather set to the version which jersey is built with if possible.

Could you check whether this version of jetty is good for rest too?

@akatona84
Copy link
Copy Markdown
Contributor Author

akatona84 commented Jun 12, 2020

cc @viktorsomogyi

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Jun 13, 2020

ok to test

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Jun 13, 2020

Thanks for the PR @akatona84. I think it's fine to use 9.4.27, the reason we downgraded Jetty was due to an incompatibility with Jersey so that shouldn't be an issue in this case.

@C0urante You added a workaround for something that was fixed in a later Jetty version, right? Can we remove this as part of this PR?

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Jun 13, 2020

Do you have a link to the release notes of both projects? Generally good to include that.

@C0urante
Copy link
Copy Markdown
Contributor

C0urante commented Jun 14, 2020

@ijuma yep, the PR can be found here: #8369.

Based on the comments in it, the changes in that PR can be reverted once we upgrade to Jetty 9.4.25 or later, so we should be able to remove the workaround in this PR.

@akatona84 akatona84 force-pushed the jersey_jetty_upgrade branch from eafc584 to baa9dde Compare June 15, 2020 06:30
@kkonstantine
Copy link
Copy Markdown
Contributor

kkonstantine commented Jun 16, 2020

ok to test

@kkonstantine
Copy link
Copy Markdown
Contributor

kkonstantine commented Jun 16, 2020

retest this please

C0urante
C0urante approved these changes Jun 16, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@C0urante C0urante left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, assuming build passes

@kkonstantine
Copy link
Copy Markdown
Contributor

kkonstantine commented Jun 17, 2020

Tests ran but didn't show up here.
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/2984/
org.apache.kafka.streams.integration.StoreUpgradeIntegrationTest.shouldMigratePersistentWindowStoreToTimestampedWindowStoreUsingPapi: 2 failures.
https://builds.apache.org/job/kafka-pr-jdk11-scala2.13/6993/
kafka.admin.ReassignPartitionsUnitTest.testModifyBrokerThrottles 2 failures.
https://builds.apache.org/job/kafka-pr-jdk14-scala2.13/1144/
org.apache.kafka.connect.mirror.MirrorConnectorsIntegrationTest.testReplication
and kafka.admin.ReassignPartitionsUnitTest.testModifyBrokerThrottles

Only flaky failures. We should be good to go.

kkonstantine
kkonstantine approved these changes Jun 17, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@kkonstantine kkonstantine left a comment

Choose a reason for hiding this comment

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

LGTM
Thanks @akatona84 !

ijuma
ijuma approved these changes Jun 17, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@ijuma ijuma left a comment

Choose a reason for hiding this comment

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

LGTM

@kkonstantine kkonstantine changed the title MINOR: Upgrading jersey and jetty versions MINOR: Upgrade jetty to 9.4.27.v20200227 and jersey to 2.31 Jun 17, 2020
@kkonstantine kkonstantine merged commit 492306a into apache:trunk Jun 17, 2020
Kvicii pushed a commit to Kvicii/kafka that referenced this pull request Jun 21, 2020
Merge branch 'trunk' of github.com:apache/kafka into trunk-read
137d186 
* 'trunk' of github.com:apache/kafka:
  KAFKA-10168: fix StreamsConfig parameter name variable (apache#8865)
  MINOR: code cleanup for inconsistent naming (apache#8871)
  KAFKA-10138: Prefer --bootstrap-server for reassign_partitions command in ducktape tests (apache#8898)
  KAFKA-10185: Restoration info logging (apache#8896)
  KAFKA-9891: add integration tests for EOS and StandbyTask (apache#8890)
  MINOR: Reduce build time by gating test coverage plugins behind a flag (apache#8899)
  KAFKA-10141; Add more detail to log segment delete messages (apache#8850)
  KAFKA-10113; Specify fetch offsets correctly in `LogTruncationException` (apache#8822)
  KAFKA-10167: use the admin client to read end-offset (apache#8876)
  MINOR: Upgrade ducktape to 0.7.8 (apache#8879)
  KAFKA-10123; Fix incorrect value for AWAIT_RESET#hasPosition (apache#8841)
  KAFKA-9896: fix flaky StandbyTaskEOSIntegrationTest (apache#8883)
  MINOR: clean up unused checkstyle suppressions for Streams (apache#8861)
  MINOR: reuse toConfigObject(Map) to generate Config (apache#8889)
  MINOR: Upgrade jetty to 9.4.27.v20200227 and jersey to 2.31 (apache#8859)
  MINOR: Fix flaky HighAvailabilityTaskAssignorIntegrationTest (apache#8884)
  KAFKA-10147 MockAdminClient#describeConfigs(Collection<ConfigResource>) is unable to handle broker resource (apache#8853)
  KAFKA-10165: Remove Percentiles from e2e metrics (apache#8882)

# Conflicts:
#	core/src/main/scala/kafka/log/Log.scala
@akatona84
Copy link
Copy Markdown
Contributor Author

akatona84 commented Jun 24, 2020

@ijuma , @kkonstantine could you cherrypick this to 2.5 too please?

@kkonstantine
Copy link
Copy Markdown
Contributor

kkonstantine commented Jun 29, 2020

Hi @akatona84
It's not clear to me why this would go to a patch/bugfix release branch. Do we have a good reason to backport?

Additionally (and independently), note that both 2.5 and 2.6 are accepting only blockers at the moment, since they are in phase or producing release candidates.

(Also, I had to merge #8893 because the method renaming has been reverted, but again, we'd need a compelling reason to backport).

@akatona84
Copy link
Copy Markdown
Contributor Author

akatona84 commented Jun 29, 2020

Hi @kkonstantine , thx for explaining and taking care of these commits.
I was just hoping we could have it in 2.5.1 too.
I'll check CVEs related to the jetty version used in 2.5.0 and get back to you. Most likely it won't be a blocker issue.

@akatona84
Copy link
Copy Markdown
Contributor Author

akatona84 commented Jun 30, 2020

I did not find anything for that version, thanks again.

@kkonstantine
Copy link
Copy Markdown
Contributor

kkonstantine commented Jun 30, 2020

Same here. Thanks for checking!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ijuma ijuma ijuma approved these changes

@C0urante C0urante C0urante approved these changes

@kkonstantine kkonstantine kkonstantine approved these changes

Assignees

No one assigned

Labels

connect

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@akatona84 @ijuma @C0urante @kkonstantine