-
Notifications
You must be signed in to change notification settings - Fork 9.2k
HADOOP-17092. ABFS: Making AzureADAuthenticator.getToken() throw HttpException if a… #2123
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
Changes from 8 commits
dc0a35f
090410c
5911314
62d1f90
6ac3ef8
6ea50f0
c72183f
a6bc587
2b1886b
0682b22
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 |
|---|---|---|
|
|
@@ -18,9 +18,11 @@ | |
|
|
||
| package org.apache.hadoop.fs.azurebfs.oauth2; | ||
|
|
||
| import java.io.FileNotFoundException; | ||
| import java.io.IOException; | ||
| import java.io.InputStream; | ||
| import java.net.HttpURLConnection; | ||
| import java.net.MalformedURLException; | ||
| import java.net.URL; | ||
| import java.nio.charset.StandardCharsets; | ||
| import java.util.Date; | ||
|
|
@@ -34,6 +36,7 @@ | |
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| import org.apache.hadoop.fs.azurebfs.AbfsConfiguration; | ||
| import org.apache.hadoop.classification.InterfaceAudience; | ||
| import org.apache.hadoop.classification.InterfaceStability; | ||
| import org.apache.hadoop.fs.azurebfs.services.AbfsIoUtils; | ||
|
|
@@ -56,10 +59,16 @@ public final class AzureADAuthenticator { | |
| private static final int CONNECT_TIMEOUT = 30 * 1000; | ||
| private static final int READ_TIMEOUT = 30 * 1000; | ||
|
|
||
| private static ExponentialRetryPolicy TOKEN_FETCH_RETRY_POLICY; | ||
|
|
||
| private AzureADAuthenticator() { | ||
| // no operation | ||
| } | ||
|
|
||
| public static void init(AbfsConfiguration abfsConfiguration) { | ||
| TOKEN_FETCH_RETRY_POLICY = abfsConfiguration.getOauthTokenFetchRetryPolicy(); | ||
| } | ||
|
|
||
| /** | ||
| * gets Azure Active Directory token using the user ID and password of | ||
| * a service principal (that is, Web App in Azure Active Directory). | ||
|
|
@@ -81,8 +90,7 @@ private AzureADAuthenticator() { | |
| * @throws IOException throws IOException if there is a failure in connecting to Azure AD | ||
| */ | ||
| public static AzureADToken getTokenUsingClientCreds(String authEndpoint, | ||
| String clientId, String clientSecret) | ||
| throws IOException { | ||
| String clientId, String clientSecret) throws IOException { | ||
| Preconditions.checkNotNull(authEndpoint, "authEndpoint"); | ||
| Preconditions.checkNotNull(clientId, "clientId"); | ||
| Preconditions.checkNotNull(clientSecret, "clientSecret"); | ||
|
|
@@ -283,13 +291,14 @@ private static AzureADToken getTokenCall(String authEndpoint, String body, | |
| Hashtable<String, String> headers, String httpMethod, boolean isMsi) | ||
| throws IOException { | ||
| AzureADToken token = null; | ||
| ExponentialRetryPolicy retryPolicy | ||
|
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. As every OAuth token provider eventually calls this method to get token, you will just need to create an instance of exponential retry right here with the configured values ?
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. Done
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. Done |
||
| = new ExponentialRetryPolicy(3, 0, 1000, 2); | ||
|
|
||
| int httperror = 0; | ||
| IOException ex = null; | ||
| boolean succeeded = false; | ||
| boolean isRecoverableFailure = true; | ||
| int retryCount = 0; | ||
| boolean shouldRetry; | ||
| LOG.trace("First execution of REST operation getTokenSingleCall"); | ||
| do { | ||
| httperror = 0; | ||
| ex = null; | ||
|
|
@@ -299,17 +308,38 @@ private static AzureADToken getTokenCall(String authEndpoint, String body, | |
| httperror = e.httpErrorCode; | ||
| ex = e; | ||
| } catch (IOException e) { | ||
| ex = e; | ||
| httperror = -1; | ||
| isRecoverableFailure = isRecoverableFailure(e); | ||
| ex = new HttpException(httperror, "", String | ||
| .format("AzureADAuthenticator.getTokenCall threw %s : %s", | ||
| e.getClass().getTypeName(), e.getMessage()), authEndpoint, "", | ||
| ""); | ||
| } | ||
| succeeded = ((httperror == 0) && (ex == null)); | ||
| shouldRetry = !succeeded && isRecoverableFailure | ||
| && TOKEN_FETCH_RETRY_POLICY.shouldRetry(retryCount, httperror); | ||
| retryCount++; | ||
| } while (!succeeded && retryPolicy.shouldRetry(retryCount, httperror)); | ||
| if (shouldRetry) { | ||
| LOG.debug("Retrying getTokenSingleCall. RetryCount = {}", retryCount); | ||
| try { | ||
| Thread.sleep(TOKEN_FETCH_RETRY_POLICY.getRetryInterval(retryCount)); | ||
| } catch (InterruptedException e) { | ||
| Thread.currentThread().interrupt(); | ||
| } | ||
| } | ||
|
|
||
| } while (shouldRetry); | ||
| if (!succeeded) { | ||
| throw ex; | ||
| } | ||
| return token; | ||
| } | ||
|
|
||
| private static boolean isRecoverableFailure(IOException e) { | ||
| return !(e instanceof MalformedURLException | ||
| || e instanceof FileNotFoundException); | ||
| } | ||
|
|
||
| private static AzureADToken getTokenSingleCall(String authEndpoint, | ||
| String payload, Hashtable<String, String> headers, String httpMethod, | ||
| boolean isMsi) | ||
|
|
||
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.
order
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.
done