Hadoop 17912 pr review 2 #12

saxenapranav · 2022-11-14T07:10:25Z

Resolutions to the comments on HADOOP-17912. ABFS: Support for Encryption Context apache/hadoop#3440
Created a utility class NamespaceUtil class which has method isNamespaceEnabled for getting if the account is HNS or non-HNS. Has been moved out from https://github.com/apache/hadoop/blob/trunk/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java#L360-L377. Done so that utility method can be used by AbfsClient while deciding to attach CPK headers or not.

…nKeyRequestHeaders

…atus object

…s://learn.microsoft.com/en-us/rest/api/storageservices/datalakestoragegen2/path/list#pathlist

…is of type ENCYPTION_CONTEXT.

…tion: javadocs in createEncryptedFile, tearDown() impl.

saxenapranav · 2022-12-12T06:23:39Z

hadoop-tools/hadoop-azure/src/site/markdown/abfs.md

 DelegatingSSLSocketFactory.SSLChannelMode. The default value will be
 DelegatingSSLSocketFactory.SSLChannelMode.Default.

-### <a name="encryptionconfigoptions"></a> Encryption Options


Removed the documentation added for 'Customer-Provided Global Key', 'Encryption Context Provider'.

saxenapranav · 2022-12-12T06:28:08Z

...tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java

-    LOG.debug("Get root ACL status");
-    try (AbfsPerfInfo perfInfo = startTracking("getIsNamespaceEnabled",
-        "getAclStatus")) {
-      AbfsRestOperation op = client
-          .getAclStatus(AbfsHttpConstants.ROOT_PATH, tracingContext);
-      perfInfo.registerResult(op.getResult());
-      isNamespaceEnabled = Trilean.getTrilean(true);
-      perfInfo.registerSuccess(true);
-    } catch (AbfsRestOperationException ex) {
-      // Get ACL status is a HEAD request, its response doesn't contain
-      // errorCode
-      // So can only rely on its status code to determine its account type.
-      if (HttpURLConnection.HTTP_BAD_REQUEST != ex.getStatusCode()) {
-        throw ex;
-      }
-
-      isNamespaceEnabled = Trilean.getTrilean(false);
-    }
-


Have moved this piece of utility code to a new utility class: https://github.com/pranavsaxena-microsoft/hadoop/blob/HADOOP-17912-PR-Review-2/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/NamespaceUtil.java

saxenapranav · 2022-12-12T06:39:48Z

...tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java

+          tracingContext);
      final AbfsRestOperation op = client
-          .getPathStatus(getRelativePath(path), true, tracingContext);
+          .getPathStatus(relativePath, true, tracingContext, encryptionAdapter);


This is in-line with apache#3440 (review), where we don't want AbfsClient to have logic to get an encryptionAdapter: https://github.com/sumangala-patki/hadoop/blob/HADOOP-17912/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java#L251-L262

saxenapranav · 2022-12-12T06:40:54Z

...tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java

      final AbfsRestOperation op = client
          .setPathProperties(getRelativePath(path), commaSeparatedProperties,
-              tracingContext);
+              tracingContext, encryptionAdapter);


This is in-line with apache#3440 (review), where we don't want AbfsClient to have logic to get an encryptionAdapter: https://github.com/sumangala-patki/hadoop/blob/HADOOP-17912/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java#L251-L262

saxenapranav · 2022-12-12T06:52:14Z

...tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java

+      *       have encryptionContext field when client's encryptionType is
+      *       ENCRYPTION_CONTEXT.
+      * */
+      if ((fileStatus instanceof VersionedFileStatus) && (


In-line with apache#3440 (comment). Here the expectation is that the:

Fetch List of VersionFileStatus objects by listStatus API of the AzureBlobFileSystem.

Use the fetchedStatus to be sent as an option while doing openWithOptions().

Tested in https://github.com/pranavsaxena-microsoft/hadoop/blob/HADOOP-17912-PR-Review-2/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsCustomEncryption.java#L273-L297.

saxenapranav · 2022-12-12T06:55:37Z

...tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java

-              eTag);
+              eTag,
+              encryptionContext);
    }


Inline with apache#3440 (comment). Need to have encryptionContext in VersionedFileStatus object.

saxenapranav · 2022-12-12T06:56:11Z

...tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java

                          entryPath,
-                          entry.eTag()));
+                          entry.eTag(),
+                          encryptionContext));


Inline with apache#3440 (comment). Need to have encryptionContext in VersionedFileStatus object.

saxenapranav · 2022-12-12T06:56:58Z

...tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java

              hasAcl, false, false);

      this.version = version;
+      this.encryptionContext = encryptionContext;


Inline with apache#3440 (comment). Need to have encryptionContext in the VersionedFileStatus object.

saxenapranav · 2022-12-12T06:58:44Z

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java

      List<AbfsHttpHeader> requestHeaders, boolean isCreateFileRequest,
      EncryptionAdapter encryptionAdapter, TracingContext tracingContext)
      throws IOException {
+    if(!getIsNamespaceEnabled(tracingContext)) {


Only to work with HNS account.

saxenapranav · 2022-12-12T07:00:14Z

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java

-      } else if (encryptionAdapter == null) {
-        // get encryption context from GetPathStatus response header
-        byte[] encryptionContext;
-        try {
-          encryptionContext = getPathStatus(path, false, tracingContext)
-              .getResult().getResponseHeader(X_MS_ENCRYPTION_CONTEXT)
-              .getBytes(StandardCharsets.UTF_8);
-        } catch (NullPointerException e) {
-          LOG.debug("EncryptionContext not present in GetPathStatus response.");
-          throw new IOException(
-              "EncryptionContext not present in GetPathStatus response", e);
-        }
-        try {
-          encryptionAdapter = new EncryptionAdapter(encryptionContextProvider,
-              new Path(path).toUri().getPath(), encryptionContext);
-          encryptionAdapterCreated = true;
-        } catch (IOException e) {
-          LOG.debug("Could not initialize EncryptionAdapter");
-          throw e;
-        }


Inline with apache#3440 (review). AbfsClient to not have heuristic to create encryptionAdapter.

saxenapranav · 2022-12-12T07:00:54Z

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java

    final List<AbfsHttpHeader> requestHeaders = createDefaultHeaders();
    addEncryptionKeyRequestHeaders(path, requestHeaders, false,
-        null, tracingContext);
+        encryptionAdapter, tracingContext);


Inline with apache#3440 (review).

saxenapranav · 2022-12-12T07:01:45Z

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java

      operation = SASTokenProvider.GET_STATUS_OPERATION;
    } else {
-      addEncryptionKeyRequestHeaders(path, requestHeaders, false, null,
+      addEncryptionKeyRequestHeaders(path, requestHeaders, false, encryptionAdapter,


Inline with apache#3440 (review). AbfsClient to not heuristic on creating encryptionAdapter.

saxenapranav · 2022-12-12T07:05:15Z

...ools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsCustomEncryption.java

+  public Boolean isCpkResponseKeyExpected = false;
+
+  @Parameterized.Parameter(8)
+  public Boolean fileSystemListStatusResultToBeUsedForOpeningFile = false;


To test the integration where azureBlobFileSystem.listStatus api response is used as an option for openToReadWithOptions.

Inline with apache#3440 (comment)

saxenapranav · 2022-12-12T07:05:36Z

...ools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsCustomEncryption.java

-        {GLOBAL_KEY, NONE, FSOperationType.DELETE, false, false, false, false},
-        {GLOBAL_KEY, NONE, FSOperationType.SET_PERMISSION, false, false, false, false},
+        {ENCRYPTION_CONTEXT, ENCRYPTION_CONTEXT, FSOperationType.READ, true, false, false, true, false, false},
+        {ENCRYPTION_CONTEXT, ENCRYPTION_CONTEXT, FSOperationType.READ, true, false, false, true, false, true},


Using fileSystemListStatusResultToBeUsedForOpeningFile as true.

saxenapranav · 2022-12-12T07:06:46Z

...ools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsCustomEncryption.java

+        {ENCRYPTION_CONTEXT, ENCRYPTION_CONTEXT, FSOperationType.SET_ACL, false, false, false, false, false, false},
+        {ENCRYPTION_CONTEXT, ENCRYPTION_CONTEXT, FSOperationType.GET_ATTR, true, false, false, true, false, false},
+        {ENCRYPTION_CONTEXT, ENCRYPTION_CONTEXT, FSOperationType.SET_ATTR, false, true, false, true, false, false},
+        {ENCRYPTION_CONTEXT, ENCRYPTION_CONTEXT, FSOperationType.LISTSTATUS, false, false, false, false, true, false},


Having isCpkResponseKeyExpected as true.

saxenapranav · 2022-12-12T07:09:08Z

...ools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsCustomEncryption.java

  public boolean isCpkResponseHdrExpected;

+  @Parameterized.Parameter(7)
+  public Boolean isCpkResponseKeyExpected = false;


if response is going to have cpk related values. Will be used in case of listsStatus. Ref: learn.microsoft.com/en-us/rest/api/storageservices/datalakestoragegen2/path/list#pathlist

This reverts commit bb45ae9.

…ckage-protected methods for setting these fields.

saxenapranav · 2022-12-13T05:33:51Z

:::: AGGREGATED TEST RESULT ::::

HNS-OAuth

[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] TestAccountConfiguration.testConfigPropNotFound:386->testMissingConfigKey:399 Expected a org.apache.hadoop.fs.azurebfs.contracts.exceptions.TokenAccessProviderException to be thrown, but got the result: : "org.apache.hadoop.fs.azurebfs.oauth2.ClientCredsTokenProvider"
[INFO]
[ERROR] Tests run: 107, Failures: 1, Errors: 0, Skipped: 1
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] ITestAzureBlobFileSystemRandomRead.testSkipBounds:218->Assert.assertTrue:42->Assert.fail:89 There should not be any network I/O (elapsedTimeMs=124).
[ERROR] Errors:
[ERROR] ITestAzureBlobFileSystemLease.testAcquireRetry:329 » TestTimedOut test timed o...
[ERROR] ITestAzureBlobFileSystemOauth.testBlobDataContributor:84 » AccessDenied Operat...
[ERROR] ITestAzureBlobFileSystemOauth.testBlobDataReader:143 » AccessDenied Operation ...
[INFO]
[ERROR] Tests run: 575, Failures: 1, Errors: 3, Skipped: 70
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] ITestReadBufferManager.testPurgeBufferManagerForParallelStreams:85 [After closing all streams free list contents should match with [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]]
Expected size:<16> but was:<9> in:
<[7, 8, 6, 4, 3, 5, 2, 1, 0]>
[ERROR] Errors:
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[INFO]
[ERROR] Tests run: 332, Failures: 1, Errors: 7, Skipped: 53

HNS-SharedKey

[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] TestAccountConfiguration.testConfigPropNotFound:386->testMissingConfigKey:399 Expected a org.apache.hadoop.fs.azurebfs.contracts.exceptions.TokenAccessProviderException to be thrown, but got the result: : "org.apache.hadoop.fs.azurebfs.oauth2.ClientCredsTokenProvider"
[INFO]
[ERROR] Tests run: 107, Failures: 1, Errors: 0, Skipped: 2
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] ITestAbfsRestOperationException.testCustomTokenFetchRetryCount:94->testWithDifferentCustomTokenFetchRetry:123->Assert.assertTrue:42->Assert.fail:89 Number of token fetch retries (4) done, does not match with fs.azure.custom.token.fetch.retry.count configured (0)
[ERROR] Errors:
[ERROR] ITestAzureBlobFileSystemLease.testAcquireRetry:329 » TestTimedOut test timed o...
[INFO]
[ERROR] Tests run: 575, Failures: 1, Errors: 1, Skipped: 26
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] ITestReadBufferManager.testPurgeBufferManagerForParallelStreams:85 [After closing all streams free list contents should match with [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]]
Expected size:<16> but was:<14> in:
<[7, 15, 14, 13, 12, 11, 1, 2, 9, 4, 5, 8, 3, 10]>
[ERROR] Errors:
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[INFO]
[ERROR] Tests run: 332, Failures: 1, Errors: 7, Skipped: 41

NonHNS-SharedKey

[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] TestAccountConfiguration.testConfigPropNotFound:386->testMissingConfigKey:399 Expected a org.apache.hadoop.fs.azurebfs.contracts.exceptions.TokenAccessProviderException to be thrown, but got the result: : "org.apache.hadoop.fs.azurebfs.oauth2.ClientCredsTokenProvider"
[INFO]
[ERROR] Tests run: 107, Failures: 1, Errors: 0, Skipped: 2
[INFO] Results:
[INFO]
[ERROR] Errors:
[ERROR] ITestAzureBlobFileSystemLease.testAcquireRetry:336 » TestTimedOut test timed o...
[INFO]
[ERROR] Tests run: 559, Failures: 0, Errors: 1, Skipped: 268
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] ITestAbfsRenameStageFailure>TestRenameStageFailure.testResilienceAsExpected:126 [resilient commit support] expected:<[tru]e> but was:<[fals]e>
[ERROR] ITestReadBufferManager.testPurgeBufferManagerForParallelStreams:81->assertListEmpty:93 [After closing all streams InProgressList should be empty]
Expected size:<0> but was:<6> in:
<[org.apache.hadoop.fs.azurebfs.services.ReadBuffer@3c90b9de,
org.apache.hadoop.fs.azurebfs.services.ReadBuffer@377bbfe,
org.apache.hadoop.fs.azurebfs.services.ReadBuffer@347a2717,
org.apache.hadoop.fs.azurebfs.services.ReadBuffer@4efe3ac3,
org.apache.hadoop.fs.azurebfs.services.ReadBuffer@7719248b,
org.apache.hadoop.fs.azurebfs.services.ReadBuffer@490fd9dd]>
[ERROR] Errors:
[ERROR] ITestAbfsJobThroughManifestCommitter.test_0420_validateJob » OutputValidation ...
[ERROR] ITestAbfsManifestCommitProtocol.testCommitLifecycle » OutputValidation abfs:/... [ERROR] ITestAbfsManifestCommitProtocol.testCommitterWithDuplicatedCommit » OutputValidation [ERROR] ITestAbfsManifestCommitProtocol.testConcurrentCommitTaskWithSubDir » OutputValidation [ERROR] ITestAbfsManifestCommitProtocol.testMapFileOutputCommitter » OutputValidation ... [ERROR] ITestAbfsManifestCommitProtocol.testOutputFormatIntegration » OutputValidation [ERROR] ITestAbfsManifestCommitProtocol.testParallelJobsToAdjacentPaths » OutputValidation [ERROR] ITestAbfsManifestCommitProtocol.testTwoTaskAttemptsCommit » OutputValidation ...
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[INFO]
[ERROR] Tests run: 332, Failures: 2, Errors: 15, Skipped: 44

AppendBlob-HNS-OAuth

[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] TestAccountConfiguration.testConfigPropNotFound:386->testMissingConfigKey:399 Expected a org.apache.hadoop.fs.azurebfs.contracts.exceptions.TokenAccessProviderException to be thrown, but got the result: : "org.apache.hadoop.fs.azurebfs.oauth2.ClientCredsTokenProvider"
[INFO]
[ERROR] Tests run: 107, Failures: 1, Errors: 0, Skipped: 1
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] ITestAzureBlobFileSystemRandomRead.testSkipBounds:218->Assert.assertTrue:42->Assert.fail:89 There should not be any network I/O (elapsedTimeMs=31).
[ERROR] Errors:
[ERROR] ITestAzureBlobFileSystemLease.testAcquireRetry:329 » TestTimedOut test timed o...
[ERROR] ITestAzureBlobFileSystemOauth.testBlobDataContributor:84 » AccessDenied Operat...
[ERROR] ITestAzureBlobFileSystemOauth.testBlobDataReader:143 » AccessDenied Operation ...
[INFO]
[ERROR] Tests run: 575, Failures: 1, Errors: 3, Skipped: 70
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] ITestAbfsFileSystemContractRename>AbstractContractRenameTest.testRenamePopulatesFileAncestors:278->AbstractContractRenameTest.validateAncestorsMoved:291->AbstractFSContractTestBase.assertPathDoesNotExist:330->Assert.fail:89 src path should not exist: unexpectedly found abfss://[email protected]/fork-0006/test/testRenamePopulatesFileAncestors/source/dir1/dir2/dir3/file4 as VersionedFileStatus{VersionedFileStatus{path=abfss://[email protected]/fork-0006/test/testRenamePopulatesFileAncestors/source/dir1/dir2/dir3/file4; isDirectory=false; length=256; replication=1; blocksize=268435456; modification_time=1670908761000; access_time=0; owner=c4ed67a7-0f07-4f6d-bab0-ee764f3ef296; group=azureuser; permission=rw-r--r--; isSymlink=false; hasAcl=false; isEncrypted=false; isErasureCoded=false}; version='0x8DADCC997529B98'}
[ERROR] ITestAzureBlobFileSystemBasics>FileSystemContractBaseTest.testRenameDirectoryMoveToExistingDirectory:521 Nested file2 exists
[ERROR] ITestReadBufferManager.testPurgeBufferManagerForParallelStreams:85 [After closing all streams free list contents should match with [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]]
Expected size:<16> but was:<10> in:
<[0, 15, 14, 13, 12, 11, 10, 9, 8, 7]>
[ERROR] Errors:
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[ERROR] ITestAbfsTerasort.setup:117->AbstractAbfsClusterITest.setup:83->AbstractAbfsClusterITest.demandCreateClusterBinding:215->AbstractAbfsClusterITest.createCluster:172 » YarnRuntime
[INFO]
[ERROR] Tests run: 332, Failures: 3, Errors: 7, Skipped: 53

anmolanmol1234 · 2022-12-19T07:58:49Z

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/NamespaceUtil.java

+import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AzureBlobFileSystemException;
+import org.apache.hadoop.fs.azurebfs.services.AbfsClient;
+
+public class NamespaceUtil {


Javadocs for class and function.

anmolanmol1234 · 2022-12-19T07:59:45Z

...tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java

    }
  }

+  private EncryptionAdapter createEncryptionAdapterFromServerStoreContext(final String path,


Javadoc for function utlity.

anmolanmol1234 · 2022-12-21T10:33:24Z

...tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java


+  private EncryptionAdapter createEncryptionAdapterFromServerStoreContext(final String path,
+      final TracingContext tracingContext) throws IOException {
+    if(client.getEncryptionType() != EncryptionType.ENCRYPTION_CONTEXT) {


nit: space after if

anmolanmol1234 · 2022-12-21T10:34:20Z

...tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java

-          .getPathStatus(getRelativePath(path), true, tracingContext);
+          .getPathStatus(relativePath, true, tracingContext, encryptionAdapter);
      perfInfo.registerResult(op.getResult());
+      if(encryptionAdapter != null) {


nit: space after if

anmolanmol1234 · 2022-12-21T10:36:25Z

...tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java

-      EncryptionAdapter encryptionAdapter = new EncryptionAdapter(
-          client.getEncryptionContextProvider(), getRelativePath(path));
+      EncryptionAdapter encryptionAdapter = null;
+      if(client.getEncryptionType() == EncryptionType.ENCRYPTION_CONTEXT) {


nit: space after if

anmolanmol1234 · 2022-12-21T10:43:58Z

...re/src/main/java/org/apache/hadoop/fs/azurebfs/contracts/services/ListResultEntrySchema.java

+  public String getCustomerProvidedKeySha256() {
+    return customerProvidedKeySha256;
+  }
 }


add a line at the end of file.

anmolanmol1234 · 2022-12-21T10:45:26Z

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java

      List<AbfsHttpHeader> requestHeaders, boolean isCreateFileRequest,
      EncryptionAdapter encryptionAdapter, TracingContext tracingContext)
      throws IOException {
+    if(!getIsNamespaceEnabled(tracingContext)) {


nit: space after if

anmolanmol1234 · 2022-12-21T10:47:14Z

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/EncryptionType.java


 /**
- * Enum EncryptionType to represent the level of encryption applied
+ * Enum EncryptionType to represent the level of encryption applied.


Add . at the end of each sentence.

anmolanmol1234 · 2022-12-21T10:48:20Z

...ools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsCustomEncryption.java

  public boolean isCpkResponseHdrExpected;

+  @Parameterized.Parameter(7)
+  public Boolean isCpkResponseKeyExpected = false;


Add javadocs for use case of params

sreeb-msft · 2022-12-21T07:11:08Z

...tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java

 import org.apache.hadoop.fs.azurebfs.security.EncryptionAdapter;
 import org.apache.hadoop.fs.azurebfs.utils.EncryptionType;
 import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.fs.azurebfs.utils.NamespaceUtil;


Is import order alright? org.apache.hadoop.classification should go before fs.

sreeb-msft · 2022-12-30T11:30:47Z

...tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java

   * The etag is included in the java serialization.
   */
-  private static final class VersionedFileStatus extends FileStatus
+  static final class VersionedFileStatus extends FileStatus


why are we removing the access modifier?

The class is still package-protected. Have removed private as there is an addition of assertion on the values of VersionedFileStatus objects in ITestAbfsCustomEncryption. The constructor to this class is still private -> so only AzureBlobFileSystemStore(parent class of VersionedFileStatus) can create object of this class.

saxenapranav added 19 commits October 6, 2022 22:05

general code refactor

8806ceb

general refactor

ef092dd

Made ABFSKey final

fec93f4

added javadocs

c2d35d8

refactors

1e987a5

added javadocs.

6364cfb

small refactors + javadocs

a1b1906

removed null handling of encryptionAdapter in abfsclient.addEncryptio…

d35a276

…nKeyRequestHeaders

refactored how encryptionAdapter was created in case of versionFileSt…

450324a

…atus object

small refactors

183f1bc

fixed tests

d0a16bd

refactored condition in openFileForRead

a69ee4a

getIsNamespace in abfsclient

061930b

Added NamespaceUtil

00bd8c3

ListResultEntrySchema to have value for x-ms-encryption-context: http…

a642ef3

…s://learn.microsoft.com/en-us/rest/api/storageservices/datalakestoragegen2/path/list#pathlist

added test for the listPath

56f3efa

test for: using result of listStatus to open a file in case the file …

adc82f6

…is of type ENCYPTION_CONTEXT.

Remove public documentation for Encryption Options.

241fb9b

remove non required import in abfsOutputStream; itestAbfsCustomEncryp…

9daea68

…tion: javadocs in createEncryptedFile, tearDown() impl.

saxenapranav commented Dec 12, 2022

View reviewed changes

saxenapranav added 5 commits December 11, 2022 23:14

small refactors

2313790

refactor mock verification so that parameter can be null

aee1c7b

assertions changed for abfsclient namespaceEnabled check

bb45ae9

Revert "assertions changed for abfsclient namespaceEnabled check"

3c6eddb

This reverts commit bb45ae9.

AbfsClientUtils to set certain fields for test. AbfsClient to have pa…

a231592

…ckage-protected methods for setting these fields.

anmolanmol1234 reviewed Dec 19, 2022

View reviewed changes

anmolanmol1234 reviewed Dec 21, 2022

View reviewed changes

sreeb-msft reviewed Dec 21, 2022

View reviewed changes

saxenapranav added 2 commits December 21, 2022 20:31

added javadocs; indentation; comments on sumangala17#12

6ab375c

minor change

aaf1582

anmolanmol1234 approved these changes Dec 28, 2022

View reviewed changes

saxenapranav mentioned this pull request Dec 29, 2022

Hadoop 17912 final PR. apache/hadoop#5261

Closed

sreeb-msft approved these changes Dec 30, 2022

View reviewed changes

sumangala-patki merged commit f9658e0 into sumangala17:HADOOP-17912 Jan 5, 2023

Hadoop 17912 pr review 2 #12

Hadoop 17912 pr review 2 #12

Uh oh!

Conversation

saxenapranav commented Nov 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saxenapranav commented Dec 13, 2022

HNS-OAuth

HNS-SharedKey

NonHNS-SharedKey

AppendBlob-HNS-OAuth

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

saxenapranav commented Nov 14, 2022 •

edited

Loading