[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #5617: Add segment encryption on Controller based on table config

[GitBox]

mayankshriv commented on a change in pull request #5617:
URL: https://github.com/apache/incubator-pinot/pull/5617#discussion_r448690902



##########
File path: 
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java
##########
@@ -194,38 +196,32 @@ private SuccessResponse uploadSegment(@Nullable String 
tableName, FormDataMultiP
       ControllerFilePathProvider provider = 
ControllerFilePathProvider.getInstance();
       String tempFileName = TMP_DIR_PREFIX + System.nanoTime();
       tempDecryptedFile = new File(provider.getFileUploadTempDir(), 
tempFileName);
+      tempEncryptedFile = new File(provider.getFileUploadTempDir(), 
tempFileName + ENCRYPTED_SUFFIX);
       tempSegmentDir = new File(provider.getUntarredFileTempDir(), 
tempFileName);
 
-      // Set default crypter to the noop crypter when no crypter header is sent
-      // In this case, the noop crypter will not do any operations, so the 
encrypted and decrypted file will have the same
-      // file path.
-      if (crypterClassName == null) {
-        crypterClassName = NoOpPinotCrypter.class.getSimpleName();
-        tempEncryptedFile = new File(provider.getFileUploadTempDir(), 
tempFileName);
-      } else {
-        tempEncryptedFile = new File(provider.getFileUploadTempDir(), 
tempFileName + ENCRYPTED_SUFFIX);
-      }
-
-      // TODO: Change when metadata upload added
-      String metadataProviderClass = DefaultMetadataExtractor.class.getName();
+      boolean uploadedSegmentIsEncrypted = 
!Strings.isNullOrEmpty(crypterClassNameInHeader);
 
-      SegmentMetadata segmentMetadata;
+      File dstFile = uploadedSegmentIsEncrypted ? tempEncryptedFile : 
tempDecryptedFile;
       FileUploadDownloadClient.FileUploadType uploadType = 
getUploadType(uploadTypeStr);
       switch (uploadType) {
         case URI:
-          segmentMetadata =
-              getMetadataForURI(crypterClassName, downloadUri, 
tempEncryptedFile, tempDecryptedFile, tempSegmentDir,
-                  metadataProviderClass);
+          getSegmentFileFromURI(downloadUri, dstFile);
           break;
         case SEGMENT:
-          getFileFromMultipart(multiPart, tempEncryptedFile);
-          segmentMetadata = getSegmentMetadata(crypterClassName, 
tempEncryptedFile, tempDecryptedFile, tempSegmentDir,
-              metadataProviderClass);
+          getSegmentFileFromMultipart(multiPart, dstFile);
           break;
         default:
           throw new UnsupportedOperationException("Unsupported upload type: " 
+ uploadType);
       }
 
+      if (uploadedSegmentIsEncrypted) {
+        decryptFile(crypterClassNameInHeader, tempEncryptedFile, 
tempDecryptedFile);
+      }
+
+      // TODO: Change when metadata upload added

Review comment:
       FYI, we will need the support for metadata push pretty soon. But that is 
somewhat unrelated here, so just an FYI at this point in time.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org