Skip to content

Adding secure grpc query server support #8207

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.

Sign up for GitHub

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

Jump to bottom
Merged
xiangfu0 merged 1 commit into from
Feb 17, 2022
Merged

Adding secure grpc query server support #8207

xiangfu0 merged 1 commit into from
Feb 17, 2022

Conversation

@xiangfu0
Copy link
Contributor

@xiangfu0 xiangfu0 commented Feb 15, 2022 •
edited
Loading

Description

gRPC uses NettyServer to support JKS.

Configs to enable server-side TLS:

pinot.server.grpctls.enabled=true
pinot.server.grpctls.client.auth.enabled=true
pinot.server.grpctls.keystore.type=JKS
pinot.server.grpctls.keystore.path=/path/to/tls.jks
pinot.server.grpctls.keystore.password=my-password
pinot.server.grpctls.truststore.type=JKS
pinot.server.grpctls.truststore.path=/path/to/tls.jks
pinot.server.grpctls.truststore.password=my-password
pinot.server.grpctls.ssl.provider=JDK

Client-side configs to init GrpcQueryClient:

usePlainText=false
tls.keystore.type=JKS
tls.keystore.path=/path/to/tls.jks
tls.keystore.password=my-password
tls.truststore.type=JKS
tls.truststore.path=/path/to/tls.jks
tls.truststore.password=my-password
tls.ssl.provider=JDK

Upgrade Notes

Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)

  • Yes (Please label as backward-incompat, and complete the section below on Release Notes)

Does this PR fix a zero-downtime upgrade introduced earlier?

  • Yes (Please label this as backward-incompat, and complete the section below on Release Notes)

Does this PR otherwise need attention when creating release notes? Things to consider:

  • New configuration options
  • Deprecation of configurations
  • Signature changes to public methods/interfaces
  • New plugins added or old plugins removed
  • Yes (Please label this PR as release-notes and complete the section on Release Notes)

Release Notes

Documentation

@codecov-commenter
Copy link

codecov-commenter commented Feb 16, 2022 •
edited
Loading

Codecov Report

Merging #8207 (14b2d23) into master (8042408) will decrease coverage by 1.04%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #8207      +/-   ##
============================================
- Coverage     70.97%   69.92%   -1.05%     
+ Complexity     4313     4311       -2     
============================================
  Files          1626     1626              
  Lines         84851    84909      +58     
  Branches      12790    12796       +6     
============================================
- Hits          60221    59372     -849     
- Misses        20496    21426     +930     
+ Partials       4134     4111      -23
Flag Coverage Δ
integration1 28.67% <0.00%> (+<0.01%) ⬆️
integration2 ?
unittests1 67.32% <0.00%> (-0.08%) ⬇️
unittests2 14.12% <0.00%> (-0.03%) ⬇️

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

Impacted Files Coverage Δ
...pache/pinot/common/utils/grpc/GrpcQueryClient.java 0.00% <0.00%> (-94.74%) ⬇️
...che/pinot/core/transport/grpc/GrpcQueryServer.java 0.00% <0.00%> (-42.63%) ⬇️
...rg/apache/pinot/server/starter/ServerInstance.java 65.42% <0.00%> (-21.50%) ⬇️
...t/core/plan/StreamingInstanceResponsePlanNode.java 0.00% <0.00%> (-100.00%) ⬇️
...ore/operator/streaming/StreamingResponseUtils.java 0.00% <0.00%> (-100.00%) ⬇️
...ager/realtime/PeerSchemeSplitSegmentCommitter.java 0.00% <0.00%> (-100.00%) ⬇️
...he/pinot/core/plan/StreamingSelectionPlanNode.java 0.00% <0.00%> (-88.89%) ⬇️
...ator/streaming/StreamingSelectionOnlyOperator.java 0.00% <0.00%> (-87.81%) ⬇️
...re/query/reduce/SelectionOnlyStreamingReducer.java 0.00% <0.00%> (-85.72%) ⬇️
...ller/api/access/BasicAuthAccessControlFactory.java 0.00% <0.00%> (-80.00%) ⬇️
... and 78 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8042408...14b2d23. Read the comment docs.

@xiangfu0 xiangfu0 force-pushed the secure_grpc_server branch 5 times, most recently from 87e9fa9 to 68965bf Compare February 16, 2022 22:36
@xiangfu0 xiangfu0 force-pushed the secure_grpc_server branch 2 times, most recently from 53247c0 to b273514 Compare February 17, 2022 00:17
apucher
apucher approved these changes Feb 17, 2022
View reviewed changes
Copy link
Contributor

@apucher apucher left a comment

Choose a reason for hiding this comment

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

this looks really clean, especially considering the potential messiness of anything TLS

@xiangfu0 xiangfu0 merged commit d672aa2 into apache:master Feb 17, 2022
@xiangfu0 xiangfu0 deleted the secure_grpc_server branch February 17, 2022 01:53
xiangfu0 added a commit to xiangfu0/pinot that referenced this pull request Feb 23, 2022
@xiangfu0
Adding secure grpc query server support (apache#8207)
c0ee672
@walterddr walterddr mentioned this pull request Oct 10, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@apucher apucher apucher approved these changes

@Jackie-Jiang Jackie-Jiang Awaiting requested review from Jackie-Jiang

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@xiangfu0 @codecov-commenter @apucher