bhattmanish98 commented on code in PR #8051:
URL: https://github.com/apache/hadoop/pull/8051#discussion_r2472593921
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClientHandler.java:
##########
@@ -80,6 +80,25 @@ public AbfsClientHandler(final URL baseUrl,
abfsClientContext);
}
+ public AbfsClientHandler(final URL baseUrl,
Review Comment:
Java doc missing for the constructor
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/AbfsHttpConstants.java:
##########
@@ -187,7 +187,8 @@ public enum ApiVersion {
DEC_12_2019("2019-12-12"),
APR_10_2021("2021-04-10"),
AUG_03_2023("2023-08-03"),
- NOV_04_2024("2024-11-04");
+ NOV_04_2024("2024-11-04"),
+ JULY_05_2025("2025-07-05");
Review Comment:
We should follow the same format: JUL_05_2025, what do you think?
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsDfsClient.java:
##########
@@ -174,6 +174,17 @@ public AbfsDfsClient(final URL baseUrl,
encryptionContextProvider, abfsClientContext, AbfsServiceType.DFS);
}
+ public AbfsDfsClient(final URL baseUrl,
Review Comment:
Java doc missing
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java:
##########
@@ -570,6 +570,11 @@ public void signRequest(final AbfsHttpOperation
httpOperation, int bytesToSign)
// do nothing; the SAS token should already be appended to the query
string
httpOperation.setMaskForSAS(); //mask sig/oid from url for logs
break;
+ case UserboundSASWithOAuth:
+
httpOperation.setRequestProperty(HttpHeaderConfigurations.AUTHORIZATION,
+ client.getAccessToken());
+ httpOperation.setMaskForSAS(); //mask sig/oid from url for logs
Review Comment:
Typo: *sign
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/constants/TestConfigurationKeys.java:
##########
@@ -55,6 +55,9 @@ public final class TestConfigurationKeys {
public static final String FS_AZURE_TEST_APP_SERVICE_PRINCIPAL_OBJECT_ID =
"fs.azure.test.app.service.principal.object.id";
+ public static final String FS_AZURE_END_USER_TENANT_ID =
"fs.azure.test.end.user.tenant.id";
Review Comment:
Rename the variable to FS_AZURE_TEST_END_USER_TENANT_ID
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java:
##########
@@ -1741,7 +1741,15 @@ private void initializeClient(URI uri, String
fileSystemName,
} else if (authType == AuthType.SAS) {
LOG.trace("Fetching SAS Token Provider");
sasTokenProvider = abfsConfiguration.getSASTokenProvider();
- } else {
+ } else if(authType == AuthType.UserboundSASWithOAuth){
+ LOG.trace("Fetching SAS and OAuth Token Provider for user bound SAS");
+ AzureADAuthenticator.init(abfsConfiguration);
+ tokenProvider = abfsConfiguration.getTokenProvider();
+ ExtensionHelper.bind(tokenProvider, uri,
+ abfsConfiguration.getRawConfiguration());
+ sasTokenProvider =
abfsConfiguration.getSASTokenProviderForUserBoundSAS();
+ }
Review Comment:
else can be started in the same line after }, same format we are using at
other places as well.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -363,6 +363,21 @@ public AbfsClient(final URL baseUrl,
this.sasTokenProvider = sasTokenProvider;
}
+ public AbfsClient(final URL baseUrl,
Review Comment:
Java doc missing
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/constants/TestConfigurationKeys.java:
##########
@@ -55,6 +55,9 @@ public final class TestConfigurationKeys {
public static final String FS_AZURE_TEST_APP_SERVICE_PRINCIPAL_OBJECT_ID =
"fs.azure.test.app.service.principal.object.id";
+ public static final String FS_AZURE_END_USER_TENANT_ID =
"fs.azure.test.end.user.tenant.id";
+ public static final String FS_AZURE_END_USER_OBJECT_ID =
"fs.azure.test.end.user.object.id";
Review Comment:
same as above
##########
hadoop-tools/hadoop-azure/src/site/markdown/index.md:
##########
@@ -303,6 +303,7 @@ driven by them.
3. Deployed in-Azure with the Azure VMs providing OAuth 2.0 tokens to the
application, "Managed Instance".
4. Using Shared Access Signature (SAS) tokens provided by a custom
implementation of the SASTokenProvider interface.
5. By directly configuring a fixed Shared Access Signature (SAS) token in the
account configuration settings files.
+6. Using user-bound SAS auth type, which is requires OAuth 2.0 setup (point 2
above) and SAS setup (point 4 above)
Review Comment:
Grammatical mistake: which requires or which is required?
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AuthType.java:
##########
@@ -24,5 +24,6 @@ public enum AuthType {
SharedKey,
OAuth,
Custom,
- SAS
+ SAS,
+ UserboundSASWithOAuth
Review Comment:
Format issue: There is an extra space before UserboundSASWithOAuth
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java:
##########
@@ -1741,7 +1741,15 @@ private void initializeClient(URI uri, String
fileSystemName,
} else if (authType == AuthType.SAS) {
LOG.trace("Fetching SAS Token Provider");
sasTokenProvider = abfsConfiguration.getSASTokenProvider();
- } else {
+ } else if(authType == AuthType.UserboundSASWithOAuth){
Review Comment:
Missing space between if and (
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java:
##########
@@ -1770,7 +1778,12 @@ private void initializeClient(URI uri, String
fileSystemName,
}
LOG.trace("Initializing AbfsClient for {}", baseUrl);
- if (tokenProvider != null) {
+ if(tokenProvider != null && sasTokenProvider != null){
Review Comment:
Space between if and (
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java:
##########
@@ -1770,7 +1778,12 @@ private void initializeClient(URI uri, String
fileSystemName,
}
LOG.trace("Initializing AbfsClient for {}", baseUrl);
- if (tokenProvider != null) {
+ if(tokenProvider != null && sasTokenProvider != null){
+ this.clientHandler = new AbfsClientHandler(baseUrl, creds,
abfsConfiguration,
+ tokenProvider, sasTokenProvider, encryptionContextProvider,
+ populateAbfsClientContext());
+ }
+ else if (tokenProvider != null) {
Review Comment:
same as above
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
