Skip to content

[feat][ws] PIP-290 Make WSS support E2E encryption#20958

Merged
poorbarcode merged 15 commits into
apache:masterfrom
poorbarcode:pip_290/impl
Aug 25, 2023
Merged

[feat][ws] PIP-290 Make WSS support E2E encryption#20958
poorbarcode merged 15 commits into
apache:masterfrom
poorbarcode:pip_290/impl

Conversation

@poorbarcode

@poorbarcode poorbarcode commented Aug 8, 2023

Copy link
Copy Markdown
Contributor

Motivation & Modifications

See #20923

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: x

@github-actions github-actions Bot added type/PIP doc-required Your PR changes impact docs and you will update later. labels Aug 8, 2023
@github-actions github-actions Bot removed the type/PIP label Aug 8, 2023
@poorbarcode poorbarcode force-pushed the pip_290/impl branch 5 times, most recently from c39b209 to c79210e Compare August 15, 2023 09:36
@poorbarcode poorbarcode self-assigned this Aug 15, 2023
@poorbarcode poorbarcode changed the title [draft] [improve] [ws] PIP-290 Make WSS support E2E encryption [improve] [ws] PIP-290 Make WSS support E2E encryption Aug 17, 2023
@poorbarcode poorbarcode reopened this Aug 17, 2023
@poorbarcode poorbarcode added this to the 3.2.0 milestone Aug 17, 2023
@codecov-commenter

codecov-commenter commented Aug 17, 2023

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 68.93939% with 41 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.16%. Comparing base (30073db) to head (02bc119).
⚠️ Report is 2221 commits behind head on master.

Files with missing lines Patch % Lines
...a/org/apache/pulsar/websocket/ProducerHandler.java 67.30% 18 Missing and 16 partials ⚠️
...r/websocket/service/WSSDummyMessageCryptoImpl.java 66.66% 5 Missing ⚠️
...he/pulsar/client/api/DummyCryptoKeyReaderImpl.java 33.33% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #20958       +/-   ##
=============================================
+ Coverage     36.73%   73.16%   +36.43%     
- Complexity    12197    32380    +20183     
=============================================
  Files          1696     1887      +191     
  Lines        130039   139836     +9797     
  Branches      14187    15380     +1193     
=============================================
+ Hits          47775   102317    +54542     
+ Misses        75926    29417    -46509     
- Partials       6338     8102     +1764     
Flag Coverage Δ
inttests 24.12% <6.06%> (-0.01%) ⬇️
systests 25.10% <3.03%> (+0.09%) ⬆️
unittests 72.45% <68.93%> (+40.41%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...che/pulsar/client/impl/crypto/MessageCryptoBc.java 68.12% <100.00%> (+2.12%) ⬆️
...va/org/apache/pulsar/client/impl/ConsumerImpl.java 77.00% <100.00%> (+22.04%) ⬆️
...va/org/apache/pulsar/client/impl/ProducerImpl.java 83.52% <100.00%> (+20.62%) ⬆️
...a/org/apache/pulsar/websocket/ConsumerHandler.java 62.50% <ø> (+37.09%) ⬆️
.../apache/pulsar/websocket/data/ProducerMessage.java 89.47% <100.00%> (+83.22%) ⬆️
...he/pulsar/client/api/DummyCryptoKeyReaderImpl.java 33.33% <33.33%> (ø)
...r/websocket/service/WSSDummyMessageCryptoImpl.java 66.66% <66.66%> (ø)
...a/org/apache/pulsar/websocket/ProducerHandler.java 65.51% <67.30%> (+34.48%) ⬆️

... and 1443 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java Outdated
Comment thread pulsar-websocket/src/main/java/org/apache/pulsar/websocket/ProducerHandler.java Outdated
Comment thread pulsar-websocket/src/main/java/org/apache/pulsar/websocket/ProducerHandler.java Outdated
@codelipenghui

Copy link
Copy Markdown
Contributor

It looks like the failed test is related to the change?

@codelipenghui codelipenghui added type/feature The PR added a new feature or issue requested a new feature and removed release/3.0.2 release/2.11.3 labels Aug 22, 2023
@codelipenghui codelipenghui changed the title [improve] [ws] PIP-290 Make WSS support E2E encryption [feat] [ws] PIP-290 Make WSS support E2E encryption Aug 22, 2023
@codelipenghui codelipenghui changed the title [feat] [ws] PIP-290 Make WSS support E2E encryption [feat][ws] PIP-290 Make WSS support E2E encryption Aug 22, 2023
Comment thread pulsar-websocket/src/main/java/org/apache/pulsar/websocket/ProducerHandler.java Outdated

@mattisonchao mattisonchao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall LGTM, left some comments.

@mattisonchao

Copy link
Copy Markdown
Member

Is it possible to add an integration test for this in the future? :)

@poorbarcode poorbarcode merged commit 07eef59 into apache:master Aug 25, 2023
@poorbarcode

Copy link
Copy Markdown
Contributor Author

Is it possible to add an integration test for this in the future? :)

In the test, the Web Socket Proxy is a real server, so I think this is already an integration test?

poorbarcode added a commit to streamnative/pulsar-archived that referenced this pull request Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-required Your PR changes impact docs and you will update later. ready-to-test type/feature The PR added a new feature or issue requested a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants