pinot icon indicating copy to clipboard operation
pinot copied to clipboard

Adding ClientEncryptionS3PinotFS

Open xiangfu0 opened this issue 3 years ago • 5 comments

https://docs.amazonaws.cn/en_us/sdk-for-java/v1/developer-guide/examples-crypto-masterkey.html

To use client side encryption, you can specify either

  • kmsCmkId which contains the id of the KMS key as the value
  • aesHexSecret which contains a custom generated AES 256 key.

The Filesystem for scheme s3 also needs to be configured to org.apache.pinot.plugin.filesystem.ClientEncryptionS3PinotFS

Example -

Controller conf for deep storage

controller.data.dir=s3://my-bucket/quickstart-data
controller.local.temp.dir=/tmp/pinot/data/controller/index

pinot.controller.storage.factory.class.s3=org.apache.pinot.plugin.filesystem.ClientEncryptionS3PinotFS
pinot.controller.segment.fetcher.protocols=file,http,s3
pinot.controller.segment.fetcher.s3.class=org.apache.pinot.common.utils.fetcher.PinotFSSegmentFetcher
pinot.controller.storage.factory.s3.region=us-east-1
pinot.controller.storage.factory.s3.accessKey=XXXX
pinot.controller.storage.factory.s3.secretKey=XXXX
pinot.controller.storage.factory.s3.kmsCmkId=703373a8-3e4b-4eb8-8a3f-03699a0d0927

Ingestion spec

pinotFSSpecs:
    - scheme: s3
      className: org.apache.pinot.plugin.filesystem.S3PinotFS
      configs:
        region: 'us-east-1'
        accessKey: 'XXXX'
        secretKey: 'XXXX'
        kmsCmkId: '703373a8-3e4b-4eb8-8a3f-03699a0d0927'

xiangfu0 avatar Jun 20 '22 21:06 xiangfu0

Weird that AWS Java SDK 2 doesn't have encryption based S3 client. The issue seems to be open from last 5 years - https://github.com/aws/aws-sdk-java-v2/issues/34

KKcorps avatar Jun 21 '22 11:06 KKcorps

This also requires some refactor on the common utils for S3PinotFS as well.

xiangfu0 avatar Jun 21 '22 20:06 xiangfu0

Weird that AWS Java SDK 2 doesn't have encryption based S3 client. The issue seems to be open from last 5 years - aws/aws-sdk-java-v2#34

Right, so for the client-side encryption, we need to use new lib and write a new PinotFS implementation 🤦

xiangfu0 avatar Jun 21 '22 20:06 xiangfu0

Added some unit tests as well.

KKcorps avatar Jun 29 '22 14:06 KKcorps

Codecov Report

Merging #8933 (0316f6c) into master (93fa0e7) will decrease coverage by 0.09%. The diff coverage is 37.45%.

@@             Coverage Diff              @@
##             master    #8933      +/-   ##
============================================
- Coverage     70.39%   70.30%   -0.10%     
- Complexity     5694     5724      +30     
============================================
  Files          1991     1992       +1     
  Lines        107634   107907     +273     
  Branches      16361    16402      +41     
============================================
+ Hits          75773    75868      +95     
- Misses        26553    26710     +157     
- Partials       5308     5329      +21     
Flag Coverage Δ
integration1 24.55% <0.00%> (-0.08%) :arrow_down:
integration2 24.25% <0.00%> (-0.07%) :arrow_down:
unittests1 67.97% <ø> (-0.02%) :arrow_down:
unittests2 13.66% <37.45%> (+0.04%) :arrow_up:

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

Impacted Files Coverage Δ
...a/org/apache/pinot/plugin/filesystem/S3Config.java 0.00% <0.00%> (ø)
.../org/apache/pinot/plugin/filesystem/S3PinotFS.java 40.92% <ø> (+0.26%) :arrow_up:
...t/plugin/filesystem/ClientEncryptionS3PinotFS.java 38.00% <38.00%> (ø)
...ore/query/scheduler/resources/ResourceManager.java 88.46% <0.00%> (-11.54%) :arrow_down:
...n/java/org/apache/pinot/common/utils/URIUtils.java 66.66% <0.00%> (-7.41%) :arrow_down:
...tream/kafka20/server/KafkaDataServerStartable.java 72.91% <0.00%> (-6.25%) :arrow_down:
.../predicate/NotEqualsPredicateEvaluatorFactory.java 68.75% <0.00%> (-5.36%) :arrow_down:
...elix/core/periodictask/ControllerPeriodicTask.java 66.07% <0.00%> (-5.36%) :arrow_down:
.../filter/predicate/InPredicateEvaluatorFactory.java 74.43% <0.00%> (-4.52%) :arrow_down:
.../pinot/core/query/scheduler/PriorityScheduler.java 77.77% <0.00%> (-2.78%) :arrow_down:
... and 25 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

codecov-commenter avatar Jun 29 '22 17:06 codecov-commenter

Close since no further actions.

xiangfu0 avatar Mar 17 '23 03:03 xiangfu0

I recommend also looking into a key rotation strategy

rino-kadijk avatar Mar 17 '23 08:03 rino-kadijk

Close since no further actions.

Not sure to understand why it didn't got merged ? The S3 plugin doesn't support AES 256 encryption currently right ?

vmarchaud avatar Mar 30 '23 19:03 vmarchaud