Skip to content
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

Don't stop trying dev credentials on failures #35771

Merged
merged 2 commits into from
Jul 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,14 @@ public Mono<AccessToken> getToken(TokenRequestContext request) {
return identityClient.authenticateWithAzureCli(request)
.doOnNext(token -> LoggingUtil.logTokenSuccess(LOGGER, request))
.doOnError(error -> LoggingUtil.logTokenError(LOGGER, identityClient.getIdentityClientOptions(), request,
error));
error))
.onErrorMap(error -> {
if (identityClient.getIdentityClientOptions().isChained()) {
return new CredentialUnavailableException(error.getMessage(), error);
} else {
return error;
}
});
}

@Override
Expand All @@ -94,7 +101,11 @@ public AccessToken getTokenSync(TokenRequestContext request) {
return accessToken;
} catch (Exception e) {
LoggingUtil.logTokenError(LOGGER, identityClient.getIdentityClientOptions(), request, e);
throw e;
if (identityClient.getIdentityClientOptions().isChained()) {
throw new CredentialUnavailableException(e.getMessage(), e);
} else {
throw e;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,14 @@ public Mono<AccessToken> getToken(TokenRequestContext request) {
return identityClient.authenticateWithAzureDeveloperCli(request)
.doOnNext(token -> LoggingUtil.logTokenSuccess(LOGGER, request))
.doOnError(error -> LoggingUtil.logTokenError(LOGGER, identityClient.getIdentityClientOptions(), request,
error));
error))
.onErrorMap(error -> {
if (identityClient.getIdentityClientOptions().isChained()) {
return new CredentialUnavailableException(error.getMessage(), error);
} else {
return error;
}
});
}

@Override
Expand All @@ -94,7 +101,11 @@ public AccessToken getTokenSync(TokenRequestContext request) {
return accessToken;
} catch (Exception e) {
LoggingUtil.logTokenError(LOGGER, identityClient.getIdentityClientOptions(), request, e);
throw e;
if (identityClient.getIdentityClientOptions().isChained()) {
throw new CredentialUnavailableException(e.getMessage(), e);
} else {
throw e;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,13 @@ public Mono<AccessToken> getToken(TokenRequestContext request) {
return identityClient.authenticateWithAzurePowerShell(request)
.doOnNext(token -> LoggingUtil.logTokenSuccess(LOGGER, request))
.doOnError(error -> LoggingUtil.logTokenError(LOGGER, identityClient.getIdentityClientOptions(), request,
error));
error))
.onErrorMap(error -> {
if (identityClient.getIdentityClientOptions().isChained()) {
return new CredentialUnavailableException(error.getMessage(), error);
} else {
return error;
}
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ public class DefaultAzureCredentialBuilder extends CredentialBuilderBase<Default
*/
public DefaultAzureCredentialBuilder() {
this.identityClientOptions.setIdentityLogOptionsImpl(new IdentityLogOptionsImpl(true));
this.identityClientOptions.setChained(true);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,13 @@ public Mono<AccessToken> getToken(TokenRequestContext request) {
})
.doOnNext(token -> LoggingUtil.logTokenSuccess(LOGGER, request))
.doOnError(error -> LoggingUtil.logTokenError(LOGGER, identityClient.getIdentityClientOptions(),
request, error));
request, error))
.onErrorMap(error -> {
if (identityClient.getIdentityClientOptions().isChained()) {
return new CredentialUnavailableException(error.getMessage(), error);
} else {
return error;
}
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,14 @@ public Mono<AccessToken> getToken(TokenRequestContext request) {
.map(this::updateCache)
.doOnNext(token -> LoggingUtil.logTokenSuccess(LOGGER, request))
.doOnError(error -> LoggingUtil.logTokenError(LOGGER, identityClient.getIdentityClientOptions(),
request, error));
request, error))
.onErrorMap(error -> {
if (identityClient.getIdentityClientOptions().isChained()) {
return new CredentialUnavailableException(error.getMessage(), error);
} else {
return error;
}
});
}

private AccessToken updateCache(MsalToken msalToken) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import com.azure.core.util.logging.ClientLogger;
import com.azure.identity.AzureAuthorityHosts;
import com.azure.identity.AuthenticationRecord;
import com.azure.identity.ChainedTokenCredential;
import com.azure.identity.TokenCachePersistenceOptions;
import com.azure.identity.implementation.util.IdentityConstants;
import com.azure.identity.implementation.util.ValidationUtil;
Expand Down Expand Up @@ -72,6 +73,8 @@ public final class IdentityClientOptions implements Cloneable {

private Duration credentialProcessTimeout = Duration.ofSeconds(10);

private boolean isChained;

/**
* Creates an instance of IdentityClientOptions with default settings.
*/
Expand Down Expand Up @@ -708,6 +711,24 @@ public void setCredentialProcessTimeout(Duration credentialProcessTimeout) {
this.credentialProcessTimeout = credentialProcessTimeout;
}

/**
* Indicates whether this options instance is part of a {@link ChainedTokenCredential}.
* @return true if this options instance is part of a {@link ChainedTokenCredential}, false otherwise.
*/
public boolean isChained() {
return this.isChained;
}

/**
* Sets whether this options instance is part of a {@link ChainedTokenCredential}.
* @param isChained
* @return the updated client options
*/
public IdentityClientOptions setChained(boolean isChained) {
this.isChained = isChained;
return this;
}

public IdentityClientOptions clone() {
IdentityClientOptions clone = new IdentityClientOptions()
.setAdditionallyAllowedTenants(this.additionallyAllowedTenants)
Expand Down Expand Up @@ -736,7 +757,8 @@ public IdentityClientOptions clone() {
.setRetryOptions(this.retryOptions)
.setRetryPolicy(this.retryPolicy)
.setPerCallPolicies(this.perCallPolicies)
.setPerRetryPolicies(this.perRetryPolicies);
.setPerRetryPolicies(this.perRetryPolicies)
.setChained(this.isChained);
if (!isInstanceDiscoveryEnabled()) {
clone.disableInstanceDiscovery();
}
Expand Down