-
Notifications
You must be signed in to change notification settings - Fork 939
Add business metrics support for STS and Profile credential providers #6426
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: feature/master/feature-ids-implementation
Are you sure you want to change the base?
Changes from all commits
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 |
---|---|---|
|
@@ -40,6 +40,7 @@ | |
import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; | ||
import software.amazon.awssdk.auth.credentials.SystemPropertyCredentialsProvider; | ||
import software.amazon.awssdk.core.internal.util.ClassLoaderHelper; | ||
import software.amazon.awssdk.core.useragent.BusinessMetricFeatureId; | ||
import software.amazon.awssdk.profiles.Profile; | ||
import software.amazon.awssdk.profiles.ProfileFile; | ||
import software.amazon.awssdk.profiles.ProfileProperty; | ||
|
@@ -161,6 +162,7 @@ private AwsCredentialsProvider basicProfileCredentialsProvider() { | |
.accessKeyId(properties.get(ProfileProperty.AWS_ACCESS_KEY_ID)) | ||
.secretAccessKey(properties.get(ProfileProperty.AWS_SECRET_ACCESS_KEY)) | ||
.accountId(properties.get(ProfileProperty.AWS_ACCOUNT_ID)) | ||
.providerName(BusinessMetricFeatureId.CREDENTIALS_PROFILE.value()) | ||
.build(); | ||
return StaticCredentialsProvider.create(credentials); | ||
} | ||
|
@@ -177,6 +179,7 @@ private AwsCredentialsProvider sessionProfileCredentialsProvider() { | |
.secretAccessKey(properties.get(ProfileProperty.AWS_SECRET_ACCESS_KEY)) | ||
.sessionToken(properties.get(ProfileProperty.AWS_SESSION_TOKEN)) | ||
.accountId(properties.get(ProfileProperty.AWS_ACCOUNT_ID)) | ||
.providerName(BusinessMetricFeatureId.CREDENTIALS_PROFILE.value()) | ||
.build(); | ||
return StaticCredentialsProvider.create(credentials); | ||
} | ||
|
@@ -187,28 +190,36 @@ private AwsCredentialsProvider credentialProcessCredentialsProvider() { | |
return ProcessCredentialsProvider.builder() | ||
.command(properties.get(ProfileProperty.CREDENTIAL_PROCESS)) | ||
.staticAccountId(properties.get(ProfileProperty.AWS_ACCOUNT_ID)) | ||
.source(BusinessMetricFeatureId.CREDENTIALS_PROFILE_PROCESS.value()) | ||
.build(); | ||
} | ||
|
||
/** | ||
* Create the SSO credentials provider based on the related profile properties. | ||
*/ | ||
private AwsCredentialsProvider ssoProfileCredentialsProvider() { | ||
validateRequiredPropertiesForSsoCredentialsProvider(); | ||
boolean isLegacy = validateRequiredPropertiesForSsoCredentialsProvider(); | ||
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. Don't we throw exception if it's invalid in |
||
String source = isLegacy ? | ||
BusinessMetricFeatureId.CREDENTIALS_PROFILE_SSO_LEGACY.value() : | ||
BusinessMetricFeatureId.CREDENTIALS_PROFILE_SSO.value(); | ||
|
||
return ssoCredentialsProviderFactory().create( | ||
ProfileProviderCredentialsContext.builder() | ||
.profile(profile) | ||
.profileFile(profileFile) | ||
.source(source) | ||
.build()); | ||
} | ||
|
||
private void validateRequiredPropertiesForSsoCredentialsProvider() { | ||
private boolean validateRequiredPropertiesForSsoCredentialsProvider() { | ||
requireProperties(ProfileProperty.SSO_ACCOUNT_ID, | ||
ProfileProperty.SSO_ROLE_NAME); | ||
|
||
if (!properties.containsKey(ProfileSection.SSO_SESSION.getPropertyKeyName())) { | ||
requireProperties(ProfileProperty.SSO_REGION, ProfileProperty.SSO_START_URL); | ||
return true; | ||
} | ||
return false; | ||
} | ||
|
||
private AwsCredentialsProvider roleAndWebIdentityTokenProfileCredentialsProvider() { | ||
|
@@ -223,6 +234,7 @@ private AwsCredentialsProvider roleAndWebIdentityTokenProfileCredentialsProvider | |
.roleArn(roleArn) | ||
.roleSessionName(roleSessionName) | ||
.webIdentityTokenFile(webIdentityTokenFile) | ||
.source(BusinessMetricFeatureId.CREDENTIALS_PROFILE_STS_WEB_ID_TOKEN.value()) | ||
.build(); | ||
|
||
return WebIdentityCredentialsUtils.factory().create(credentialProperties); | ||
|
@@ -249,7 +261,8 @@ private AwsCredentialsProvider roleAndSourceProfileBasedProfileCredentialsProvid | |
.credentialsProvider(children)) | ||
.orElseThrow(this::noSourceCredentialsException); | ||
|
||
return stsCredentialsProviderFactory().create(sourceCredentialsProvider, profile); | ||
String source = BusinessMetricFeatureId.CREDENTIALS_PROFILE_SOURCE_PROFILE.value(); | ||
return stsCredentialsProviderFactory().create(sourceCredentialsProvider, profile, source); | ||
} | ||
|
||
/** | ||
|
@@ -260,18 +273,20 @@ private AwsCredentialsProvider roleAndCredentialSourceBasedProfileCredentialsPro | |
requireProperties(ProfileProperty.CREDENTIAL_SOURCE); | ||
|
||
CredentialSourceType credentialSource = CredentialSourceType.parse(properties.get(ProfileProperty.CREDENTIAL_SOURCE)); | ||
AwsCredentialsProvider credentialsProvider = credentialSourceCredentialProvider(credentialSource); | ||
return stsCredentialsProviderFactory().create(credentialsProvider, profile); | ||
String source = BusinessMetricFeatureId.CREDENTIALS_PROFILE_NAMED_PROVIDER.value(); | ||
AwsCredentialsProvider credentialsProvider = credentialSourceCredentialProvider(credentialSource, source); | ||
return stsCredentialsProviderFactory().create(credentialsProvider, profile, source); | ||
} | ||
|
||
private AwsCredentialsProvider credentialSourceCredentialProvider(CredentialSourceType credentialSource) { | ||
private AwsCredentialsProvider credentialSourceCredentialProvider(CredentialSourceType credentialSource, String source) { | ||
switch (credentialSource) { | ||
case ECS_CONTAINER: | ||
return ContainerCredentialsProvider.builder().build(); | ||
return ContainerCredentialsProvider.builder().source(source).build(); | ||
case EC2_INSTANCE_METADATA: | ||
return InstanceProfileCredentialsProvider.builder() | ||
.profileFile(profileFile) | ||
.profileName(name) | ||
.source(source) | ||
.build(); | ||
case ENVIRONMENT: | ||
return AwsCredentialsProviderChain.builder() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,11 +25,13 @@ | |
import software.amazon.awssdk.auth.credentials.AwsCredentials; | ||
import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; | ||
import software.amazon.awssdk.auth.credentials.AwsSessionCredentials; | ||
import software.amazon.awssdk.core.useragent.BusinessMetricFeatureId; | ||
import software.amazon.awssdk.services.sso.SsoClient; | ||
import software.amazon.awssdk.services.sso.internal.SessionCredentialsHolder; | ||
import software.amazon.awssdk.services.sso.model.GetRoleCredentialsRequest; | ||
import software.amazon.awssdk.services.sso.model.RoleCredentials; | ||
import software.amazon.awssdk.utils.SdkAutoCloseable; | ||
import software.amazon.awssdk.utils.StringUtils; | ||
import software.amazon.awssdk.utils.builder.CopyableBuilder; | ||
import software.amazon.awssdk.utils.builder.ToCopyableBuilder; | ||
import software.amazon.awssdk.utils.cache.CachedSupplier; | ||
|
@@ -51,14 +53,15 @@ | |
@SdkPublicApi | ||
public final class SsoCredentialsProvider implements AwsCredentialsProvider, SdkAutoCloseable, | ||
ToCopyableBuilder<SsoCredentialsProvider.Builder, SsoCredentialsProvider> { | ||
private static final String PROVIDER_NAME = "SsoCredentialsProvider"; | ||
private static final String PROVIDER_NAME = BusinessMetricFeatureId.CREDENTIALS_SSO.value(); | ||
|
||
private static final Duration DEFAULT_STALE_TIME = Duration.ofMinutes(1); | ||
private static final Duration DEFAULT_PREFETCH_TIME = Duration.ofMinutes(5); | ||
|
||
private static final String ASYNC_THREAD_NAME = "sdk-sso-credentials-provider"; | ||
|
||
private final Supplier<GetRoleCredentialsRequest> getRoleCredentialsRequestSupplier; | ||
private final String source; | ||
|
||
private final SsoClient ssoClient; | ||
private final Duration staleTime; | ||
|
@@ -77,6 +80,7 @@ private SsoCredentialsProvider(BuilderImpl builder) { | |
|
||
this.staleTime = Optional.ofNullable(builder.staleTime).orElse(DEFAULT_STALE_TIME); | ||
this.prefetchTime = Optional.ofNullable(builder.prefetchTime).orElse(DEFAULT_PREFETCH_TIME); | ||
this.source = builder.source; | ||
|
||
this.asyncCredentialUpdateEnabled = builder.asyncCredentialUpdateEnabled; | ||
CachedSupplier.Builder<SessionCredentialsHolder> cacheBuilder = | ||
|
@@ -95,11 +99,11 @@ private SsoCredentialsProvider(BuilderImpl builder) { | |
*/ | ||
private RefreshResult<SessionCredentialsHolder> updateSsoCredentials() { | ||
SessionCredentialsHolder credentials = getUpdatedCredentials(ssoClient); | ||
Instant acutalTokenExpiration = credentials.sessionCredentialsExpiration(); | ||
Instant actualTokenExpiration = credentials.sessionCredentialsExpiration(); | ||
|
||
return RefreshResult.builder(credentials) | ||
.staleTime(acutalTokenExpiration.minus(staleTime)) | ||
.prefetchTime(acutalTokenExpiration.minus(prefetchTime)) | ||
.staleTime(actualTokenExpiration.minus(staleTime)) | ||
.prefetchTime(actualTokenExpiration.minus(prefetchTime)) | ||
.build(); | ||
} | ||
|
||
|
@@ -112,11 +116,19 @@ private SessionCredentialsHolder getUpdatedCredentials(SsoClient ssoClient) { | |
.secretAccessKey(roleCredentials.secretAccessKey()) | ||
.sessionToken(roleCredentials.sessionToken()) | ||
.accountId(request.accountId()) | ||
.providerName(PROVIDER_NAME) | ||
.providerName(providerName()) | ||
.build(); | ||
return new SessionCredentialsHolder(sessionCredentials, Instant.ofEpochMilli(roleCredentials.expiration())); | ||
} | ||
|
||
private String providerName() { | ||
String providerName = PROVIDER_NAME; | ||
if (!StringUtils.isEmpty(this.source)) { | ||
providerName = String.format("%s,%s", this.source, providerName); | ||
} | ||
Comment on lines
+126
to
+128
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. nit: can we move this logic to ctor so that we just compute this once and not every time? Also, can we use plain string concatenation since String.format is a bit expensive? String.format is okay for exceptional messages/logging statement because the impact is minimal (doesn't affect every request), it seems unnecessary here. Same for other places |
||
return providerName; | ||
} | ||
|
||
/** | ||
* The amount of time, relative to session token expiration, that the cached credentials are considered stale and | ||
* should no longer be used. All threads will block until the value is updated. | ||
|
@@ -206,6 +218,12 @@ public interface Builder extends CopyableBuilder<Builder, SsoCredentialsProvider | |
*/ | ||
Builder refreshRequest(Supplier<GetRoleCredentialsRequest> getRoleCredentialsRequestSupplier); | ||
|
||
/** | ||
* An optional string list of {@link software.amazon.awssdk.core.useragent.BusinessMetricFeatureId} denoting previous | ||
* credentials providers that are chained with this one. | ||
*/ | ||
Builder source(String source); | ||
|
||
/** | ||
* Create a {@link SsoCredentialsProvider} using the configuration applied to this builder. | ||
* @return | ||
|
@@ -220,6 +238,7 @@ protected static final class BuilderImpl implements Builder { | |
private Duration staleTime; | ||
private Duration prefetchTime; | ||
private Supplier<GetRoleCredentialsRequest> getRoleCredentialsRequestSupplier; | ||
private String source; | ||
|
||
BuilderImpl() { | ||
|
||
|
@@ -231,6 +250,7 @@ public BuilderImpl(SsoCredentialsProvider provider) { | |
this.staleTime = provider.staleTime; | ||
this.prefetchTime = provider.prefetchTime; | ||
this.getRoleCredentialsRequestSupplier = provider.getRoleCredentialsRequestSupplier; | ||
this.source = provider.source; | ||
} | ||
|
||
@Override | ||
|
@@ -268,6 +288,12 @@ public Builder refreshRequest(Supplier<GetRoleCredentialsRequest> getRoleCredent | |
return this; | ||
} | ||
|
||
@Override | ||
public Builder source(String source) { | ||
this.source = source; | ||
return this; | ||
} | ||
|
||
@Override | ||
public SsoCredentialsProvider build() { | ||
return new SsoCredentialsProvider(this); | ||
|
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.
Can we create a wrapper object for those parameters? I'm trying to avoid the situation where we need to add a fourth parameter in the future and have to create another constructor.
Something like
Also, we should throw unsupportedoperation exception here otherwise it'd may break customers