-
Notifications
You must be signed in to change notification settings - Fork 9.2k
HADOOP-19736: ABFS. Support for new auth type: User-bound SAS #8051
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Changes from all commits
05b52e4
fc9251f
4c181df
0411d9b
a33f682
c9a994e
0a168e9
5c9cfd5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -337,22 +337,24 @@ private AbfsClient(final URL baseUrl, | |
| LOG.trace("primaryUserGroup is {}", this.primaryUserGroup); | ||
| } | ||
|
|
||
| public AbfsClient(final URL baseUrl, | ||
| final SharedKeyCredentials sharedKeyCredentials, | ||
| final AbfsConfiguration abfsConfiguration, | ||
| final AccessTokenProvider tokenProvider, | ||
| final EncryptionContextProvider encryptionContextProvider, | ||
| final AbfsClientContext abfsClientContext, | ||
| final AbfsServiceType abfsServiceType) | ||
| throws IOException { | ||
| this(baseUrl, sharedKeyCredentials, abfsConfiguration, | ||
| encryptionContextProvider, abfsClientContext, abfsServiceType); | ||
| this.tokenProvider = tokenProvider; | ||
| } | ||
|
|
||
| /** | ||
| * Constructs an AbfsClient instance with all authentication and configuration options. | ||
| * | ||
| * @param baseUrl The base URL for the ABFS endpoint. | ||
| * @param sharedKeyCredentials Shared key credentials for authentication. | ||
| * @param abfsConfiguration The ABFS configuration. | ||
| * @param tokenProvider The access token provider for OAuth authentication. | ||
| * @param sasTokenProvider The SAS token provider for SAS authentication. | ||
| * @param encryptionContextProvider The encryption context provider. | ||
| * @param abfsClientContext The client context | ||
| * @param abfsServiceType The ABFS service type (e.g., Blob, DFS). | ||
| * @throws IOException if initialization fails. | ||
| */ | ||
| public AbfsClient(final URL baseUrl, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Java doc missing
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we simply combine the 3 constructors to accept both AccessTokenProvider, SASTokenProvider and the caller can set what it has and null as other?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense, added |
||
| final SharedKeyCredentials sharedKeyCredentials, | ||
| final AbfsConfiguration abfsConfiguration, | ||
| final AccessTokenProvider tokenProvider, | ||
| final SASTokenProvider sasTokenProvider, | ||
| final EncryptionContextProvider encryptionContextProvider, | ||
| final AbfsClientContext abfsClientContext, | ||
|
|
@@ -361,6 +363,7 @@ public AbfsClient(final URL baseUrl, | |
| this(baseUrl, sharedKeyCredentials, abfsConfiguration, | ||
| encryptionContextProvider, abfsClientContext, abfsServiceType); | ||
| this.sasTokenProvider = sasTokenProvider; | ||
| this.tokenProvider = tokenProvider; | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -1157,7 +1160,7 @@ protected String appendSASTokenToQuery(String path, | |
| String cachedSasToken) | ||
| throws SASTokenProviderException { | ||
| String sasToken = null; | ||
| if (this.authType == AuthType.SAS) { | ||
| if (this.authType == AuthType.SAS || this.authType == AuthType.UserboundSASWithOAuth) { | ||
| try { | ||
| LOG.trace("Fetch SAS token for {} on {}", operation, path); | ||
| if (cachedSasToken == null) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,24 +47,26 @@ public class AbfsClientHandler implements Closeable { | |
| private final AbfsDfsClient dfsAbfsClient; | ||
| private final AbfsBlobClient blobAbfsClient; | ||
|
|
||
| public AbfsClientHandler(final URL baseUrl, | ||
| final SharedKeyCredentials sharedKeyCredentials, | ||
| final AbfsConfiguration abfsConfiguration, | ||
| final AccessTokenProvider tokenProvider, | ||
| final EncryptionContextProvider encryptionContextProvider, | ||
| final AbfsClientContext abfsClientContext) throws IOException { | ||
| this.dfsAbfsClient = createDfsClient(baseUrl, sharedKeyCredentials, | ||
| abfsConfiguration, tokenProvider, null, encryptionContextProvider, | ||
| abfsClientContext); | ||
| this.blobAbfsClient = createBlobClient(baseUrl, sharedKeyCredentials, | ||
| abfsConfiguration, tokenProvider, null, encryptionContextProvider, | ||
| abfsClientContext); | ||
| initServiceType(abfsConfiguration); | ||
| } | ||
|
|
||
| /** | ||
| * Constructs an AbfsClientHandler instance. | ||
| * | ||
| * Initializes the default and ingress service types from the provided configuration, | ||
| * then creates both DFS and Blob clients using the given params | ||
| * | ||
| * @param baseUrl the base URL for the file system. | ||
| * @param sharedKeyCredentials credentials for shared key authentication. | ||
| * @param abfsConfiguration the ABFS configuration. | ||
| * @param tokenProvider the access token provider, may be null. | ||
| * @param sasTokenProvider the SAS token provider, may be null. | ||
| * @param encryptionContextProvider the encryption context provider | ||
| * @param abfsClientContext the ABFS client context. | ||
| * @throws IOException if client creation or URL conversion fails. | ||
| */ | ||
| public AbfsClientHandler(final URL baseUrl, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Java doc missing for the constructor
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above. We can combine into a single constructor. |
||
| final SharedKeyCredentials sharedKeyCredentials, | ||
| final AbfsConfiguration abfsConfiguration, | ||
| final AccessTokenProvider tokenProvider, | ||
| final SASTokenProvider sasTokenProvider, | ||
| final EncryptionContextProvider encryptionContextProvider, | ||
| final AbfsClientContext abfsClientContext) throws IOException { | ||
|
|
@@ -73,10 +75,10 @@ public AbfsClientHandler(final URL baseUrl, | |
| // only for default client. | ||
| initServiceType(abfsConfiguration); | ||
| this.dfsAbfsClient = createDfsClient(baseUrl, sharedKeyCredentials, | ||
| abfsConfiguration, null, sasTokenProvider, encryptionContextProvider, | ||
| abfsConfiguration, tokenProvider, sasTokenProvider, encryptionContextProvider, | ||
| abfsClientContext); | ||
| this.blobAbfsClient = createBlobClient(baseUrl, sharedKeyCredentials, | ||
| abfsConfiguration, null, sasTokenProvider, encryptionContextProvider, | ||
| abfsConfiguration, tokenProvider, sasTokenProvider, encryptionContextProvider, | ||
| abfsClientContext); | ||
| } | ||
|
|
||
|
|
@@ -154,7 +156,15 @@ private AbfsDfsClient createDfsClient(final URL baseUrl, | |
| final EncryptionContextProvider encryptionContextProvider, | ||
| final AbfsClientContext abfsClientContext) throws IOException { | ||
| URL dfsUrl = changeUrlFromBlobToDfs(baseUrl); | ||
| if (tokenProvider != null) { | ||
| if (tokenProvider != null && sasTokenProvider != null) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apart from UDS, can there be a case where caller can send both as not null? Makes ure no flow leads to this and also add a test around it.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are initialising both the token providers only for UBS auth type. Added a test for token provider expectations (null or non-null) according to the auth type
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All these conditions can again be removed by creating a single constructor for AbfsDfsClient and AbfsBlobClient that sends both access token provider and sas token provider (like we did for client handler and parent constructors) |
||
| LOG.debug( | ||
| "Creating AbfsDfsClient with both access token provider and SAS token provider using the URL: {}", | ||
| dfsUrl); | ||
| return new AbfsDfsClient(dfsUrl, creds, abfsConfiguration, | ||
| tokenProvider, sasTokenProvider, encryptionContextProvider, | ||
| abfsClientContext); | ||
| } | ||
| else if (tokenProvider != null) { | ||
| LOG.debug("Creating AbfsDfsClient with access token provider using the URL: {}", dfsUrl); | ||
| return new AbfsDfsClient(dfsUrl, creds, abfsConfiguration, | ||
| tokenProvider, encryptionContextProvider, | ||
|
|
@@ -188,12 +198,21 @@ private AbfsBlobClient createBlobClient(final URL baseUrl, | |
| final EncryptionContextProvider encryptionContextProvider, | ||
| final AbfsClientContext abfsClientContext) throws IOException { | ||
| URL blobUrl = changeUrlFromDfsToBlob(baseUrl); | ||
| if (tokenProvider != null) { | ||
| if (tokenProvider != null && sasTokenProvider != null) { | ||
| LOG.debug( | ||
| "Creating AbfsBlobClient with both access token provider and SAS token provider using the URL: {}", | ||
| blobUrl); | ||
| return new AbfsBlobClient(blobUrl, creds, abfsConfiguration, | ||
| tokenProvider, sasTokenProvider, encryptionContextProvider, | ||
| abfsClientContext); | ||
| } | ||
| else if (tokenProvider != null) { | ||
| LOG.debug("Creating AbfsBlobClient with access token provider using the URL: {}", blobUrl); | ||
| return new AbfsBlobClient(blobUrl, creds, abfsConfiguration, | ||
| tokenProvider, encryptionContextProvider, | ||
| abfsClientContext); | ||
| } else { | ||
| } | ||
| else { | ||
| LOG.debug("Creating AbfsBlobClient with SAS token provider using the URL: {}", blobUrl); | ||
| return new AbfsBlobClient(blobUrl, creds, abfsConfiguration, | ||
| sasTokenProvider, encryptionContextProvider, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar check for OAuth also needed right?
For UBS, Cx must configure OAuth as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We were getting OAuth token provider separately. Added a single method to get both now