[
https://issues.apache.org/jira/browse/HADOOP-19736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18033802#comment-18033802
]
ASF GitHub Bot commented on HADOOP-19736:
-----------------------------------------
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
> ABFS: Support for new auth type: User-bound SAS
> -----------------------------------------------
>
> Key: HADOOP-19736
> URL: https://issues.apache.org/jira/browse/HADOOP-19736
> Project: Hadoop Common
> Issue Type: Task
> Components: fs/azure
> Affects Versions: 3.4.1, 3.4.2
> Reporter: Manika Joshi
> Assignee: Manika Joshi
> Priority: Major
> Labels: pull-request-available
>
> Adding support for new authentication type: user bound SAS
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
