Skip to content

Prefer Supplier<ProfileFile> over ProfileFileSupplier in APIs #3607

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Dec 12, 2022
Merged

Prefer Supplier<ProfileFile> over ProfileFileSupplier in APIs #3607

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 9 additions & 8 deletions ...uth/src/main/java/software/amazon/awssdk/auth/credentials/DefaultCredentialsProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package software.amazon.awssdk.auth.credentials;

import java.util.Optional;
import java.util.function.Supplier;
import software.amazon.awssdk.annotations.SdkPublicApi;
import software.amazon.awssdk.auth.credentials.internal.LazyAwsCredentialsProvider;
import software.amazon.awssdk.profiles.ProfileFile;
Expand Down Expand Up @@ -53,7 +54,7 @@ public final class DefaultCredentialsProvider

private final LazyAwsCredentialsProvider providerChain;

private final ProfileFileSupplier profileFileSupplier;
private final Supplier<ProfileFile> profileFile;

private final String profileName;

Expand All @@ -65,7 +66,7 @@ public final class DefaultCredentialsProvider
* @see #builder()
*/
private DefaultCredentialsProvider(Builder builder) {
this.profileFileSupplier = builder.profileFileSupplier;
this.profileFile = builder.profileFile;
this.profileName = builder.profileName;
this.reuseLastProviderEnabled = builder.reuseLastProviderEnabled;
this.asyncCredentialUpdateEnabled = builder.asyncCredentialUpdateEnabled;
Expand Down Expand Up @@ -93,15 +94,15 @@ private static LazyAwsCredentialsProvider createChain(Builder builder) {
EnvironmentVariableCredentialsProvider.create(),
WebIdentityTokenFileCredentialsProvider.create(),
ProfileCredentialsProvider.builder()
.profileFile(builder.profileFileSupplier)
.profileFile(builder.profileFile)
.profileName(builder.profileName)
.build(),
ContainerCredentialsProvider.builder()
.asyncCredentialUpdateEnabled(asyncCredentialUpdateEnabled)
.build(),
InstanceProfileCredentialsProvider.builder()
.asyncCredentialUpdateEnabled(asyncCredentialUpdateEnabled)
.profileFile(builder.profileFileSupplier)
.profileFile(builder.profileFile)
.profileName(builder.profileName)
.build()
};
Expand Down Expand Up @@ -146,7 +147,7 @@ public Builder toBuilder() {
* Configuration that defines the {@link DefaultCredentialsProvider}'s behavior.
*/
public static final class Builder implements CopyableBuilder<Builder, DefaultCredentialsProvider> {
private ProfileFileSupplier profileFileSupplier;
private Supplier<ProfileFile> profileFile;
private String profileName;
private Boolean reuseLastProviderEnabled = true;
private Boolean asyncCredentialUpdateEnabled = false;
Expand All @@ -158,7 +159,7 @@ private Builder() {
}

private Builder(DefaultCredentialsProvider credentialsProvider) {
this.profileFileSupplier = credentialsProvider.profileFileSupplier;
this.profileFile = credentialsProvider.profileFile;
this.profileName = credentialsProvider.profileName;
this.reuseLastProviderEnabled = credentialsProvider.reuseLastProviderEnabled;
this.asyncCredentialUpdateEnabled = credentialsProvider.asyncCredentialUpdateEnabled;
Expand All @@ -170,8 +171,8 @@ public Builder profileFile(ProfileFile profileFile) {
.orElse(null));
}

public Builder profileFile(ProfileFileSupplier profileFileSupplier) {
this.profileFileSupplier = profileFileSupplier;
public Builder profileFile(Supplier<ProfileFile> profileFileSupplier) {
this.profileFile = profileFileSupplier;
return this;
}

Expand Down
21 changes: 11 additions & 10 deletions ...main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.Collections;
import java.util.Map;
import java.util.Optional;
import java.util.function.Supplier;
import software.amazon.awssdk.annotations.SdkPublicApi;
import software.amazon.awssdk.annotations.SdkTestInternalApi;
import software.amazon.awssdk.auth.credentials.internal.Ec2MetadataConfigProvider;
Expand Down Expand Up @@ -79,7 +80,7 @@ public final class InstanceProfileCredentialsProvider

private final String asyncThreadName;

private final ProfileFileSupplier profileFileSupplier;
private final Supplier<ProfileFile> profileFile;

private final String profileName;

Expand All @@ -91,13 +92,13 @@ private InstanceProfileCredentialsProvider(BuilderImpl builder) {
this.endpoint = builder.endpoint;
this.asyncCredentialUpdateEnabled = builder.asyncCredentialUpdateEnabled;
this.asyncThreadName = builder.asyncThreadName;
this.profileFileSupplier = builder.profileFileSupplier;
this.profileFile = builder.profileFile;
this.profileName = builder.profileName;

this.httpCredentialsLoader = HttpCredentialsLoader.create();
this.configProvider =
Ec2MetadataConfigProvider.builder()
.profileFile(builder.profileFileSupplier)
.profileFile(builder.profileFile)
.profileName(builder.profileName)
.build();

Expand Down Expand Up @@ -282,7 +283,7 @@ public interface Builder extends HttpCredentialsProvider.Builder<InstanceProfile
*
* <p>By default, {@link ProfileFile#defaultProfileFile()} is used.
*
* @see #profileFile(ProfileFileSupplier)
* @see #profileFile(Supplier)
*/
Builder profileFile(ProfileFile profileFile);

Expand All @@ -292,7 +293,7 @@ public interface Builder extends HttpCredentialsProvider.Builder<InstanceProfile
* @param profileFileSupplier Supplier interface for generating a ProfileFile instance.
* @see #profileFile(ProfileFile)
*/
Builder profileFile(ProfileFileSupplier profileFileSupplier);
Builder profileFile(Supplier<ProfileFile> profileFileSupplier);

/**
* Configure the profile name used for loading IMDS-related configuration, like the endpoint mode (IPv4 vs IPv6).
Expand All @@ -314,7 +315,7 @@ static final class BuilderImpl implements Builder {
private String endpoint;
private Boolean asyncCredentialUpdateEnabled;
private String asyncThreadName;
private ProfileFileSupplier profileFileSupplier;
private Supplier<ProfileFile> profileFile;
private String profileName;

private BuilderImpl() {
Expand All @@ -326,7 +327,7 @@ private BuilderImpl(InstanceProfileCredentialsProvider provider) {
this.endpoint = provider.endpoint;
this.asyncCredentialUpdateEnabled = provider.asyncCredentialUpdateEnabled;
this.asyncThreadName = provider.asyncThreadName;
this.profileFileSupplier = provider.profileFileSupplier;
this.profileFile = provider.profileFile;
this.profileName = provider.profileName;
}

Expand Down Expand Up @@ -377,12 +378,12 @@ public void setProfileFile(ProfileFile profileFile) {
}

@Override
public Builder profileFile(ProfileFileSupplier profileFileSupplier) {
this.profileFileSupplier = profileFileSupplier;
public Builder profileFile(Supplier<ProfileFile> profileFileSupplier) {
this.profileFile = profileFileSupplier;
return this;
}

public void setProfileFile(ProfileFileSupplier profileFileSupplier) {
public void setProfileFile(Supplier<ProfileFile> profileFileSupplier) {
profileFile(profileFileSupplier);
}

Expand Down
31 changes: 16 additions & 15 deletions ...uth/src/main/java/software/amazon/awssdk/auth/credentials/ProfileCredentialsProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public final class ProfileCredentialsProvider

private AwsCredentialsProvider credentialsProvider;
private final RuntimeException loadException;
private final ProfileFileSupplier profileFileSupplier;
private final Supplier<ProfileFile> profileFile;
private volatile ProfileFile currentProfileFile;
private final String profileName;
private final Supplier<ProfileFile> defaultProfileFileLoader;
Expand All @@ -64,15 +64,14 @@ private ProfileCredentialsProvider(BuilderImpl builder) {

RuntimeException thrownException = null;
String selectedProfileName = null;
ProfileFileSupplier selectedProfileSupplier = null;
Supplier<ProfileFile> selectedProfileSupplier = null;

try {
selectedProfileName = Optional.ofNullable(builder.profileName)
.orElseGet(ProfileFileSystemSetting.AWS_PROFILE::getStringValueOrThrow);

selectedProfileSupplier = Optional.ofNullable(builder.profileFileSupplier)
.orElseGet(() -> ProfileFileSupplier
.fixedProfileFile(builder.defaultProfileFileLoader.get()));
selectedProfileSupplier = Optional.ofNullable(builder.profileFile)
.orElseGet(() -> builder.defaultProfileFileLoader);

} catch (RuntimeException e) {
// If we couldn't load the credentials provider for some reason, save an exception describing why. This exception
Expand All @@ -83,7 +82,7 @@ private ProfileCredentialsProvider(BuilderImpl builder) {

this.loadException = thrownException;
this.profileName = selectedProfileName;
this.profileFileSupplier = selectedProfileSupplier;
this.profileFile = selectedProfileSupplier;
}

/**
Expand Down Expand Up @@ -131,7 +130,7 @@ private void handleProfileFileReload(ProfileFile profileFile) {
}

private ProfileFile refreshProfileFile() {
return profileFileSupplier.get();
return profileFile.get();
}

private boolean isNewProfileFile(ProfileFile profileFile) {
Expand All @@ -148,7 +147,9 @@ public String toString() {

@Override
public void close() {
profileFileSupplier.close();
if (profileFile instanceof SdkAutoCloseable) {
((SdkAutoCloseable) profileFile).close();
}
// The delegate credentials provider may be closeable (eg. if it's an STS credentials provider). In this case, we should
// clean it up when this credentials provider is closed.
IoUtils.closeIfCloseable(credentialsProvider, null);
Expand Down Expand Up @@ -178,7 +179,7 @@ public interface Builder extends CopyableBuilder<Builder, ProfileCredentialsProv
/**
* Define the profile file that should be used by this credentials provider. By default, the
* {@link ProfileFile#defaultProfileFile()} is used.
* @see #profileFile(ProfileFileSupplier)
* @see #profileFile(Supplier)
*/
Builder profileFile(ProfileFile profileFile);

Expand All @@ -194,7 +195,7 @@ public interface Builder extends CopyableBuilder<Builder, ProfileCredentialsProv
* @param profileFileSupplier Supplier interface for generating a ProfileFile instance.
* @see #profileFile(ProfileFile)
*/
Builder profileFile(ProfileFileSupplier profileFileSupplier);
Builder profileFile(Supplier<ProfileFile> profileFileSupplier);

/**
* Define the name of the profile that should be used by this credentials provider. By default, the value in
Expand All @@ -210,7 +211,7 @@ public interface Builder extends CopyableBuilder<Builder, ProfileCredentialsProv
}

static final class BuilderImpl implements Builder {
private ProfileFileSupplier profileFileSupplier;
private Supplier<ProfileFile> profileFile;
private String profileName;
private Supplier<ProfileFile> defaultProfileFileLoader = ProfileFile::defaultProfileFile;

Expand All @@ -220,7 +221,7 @@ static final class BuilderImpl implements Builder {
BuilderImpl(ProfileCredentialsProvider provider) {
this.profileName = provider.profileName;
this.defaultProfileFileLoader = provider.defaultProfileFileLoader;
this.profileFileSupplier = provider.profileFileSupplier;
this.profileFile = provider.profileFile;
}

@Override
Expand All @@ -240,12 +241,12 @@ public Builder profileFile(Consumer<ProfileFile.Builder> profileFile) {
}

@Override
public Builder profileFile(ProfileFileSupplier profileFileSupplier) {
this.profileFileSupplier = profileFileSupplier;
public Builder profileFile(Supplier<ProfileFile> profileFileSupplier) {
this.profileFile = profileFileSupplier;
return this;
}

public void setProfileFile(ProfileFileSupplier supplier) {
public void setProfileFile(Supplier<ProfileFile> supplier) {
profileFile(supplier);
}

Expand Down
38 changes: 36 additions & 2 deletions ...src/test/java/software/amazon/awssdk/auth/credentials/DefaultCredentialsProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import java.util.Arrays;
import java.util.List;
import java.util.function.Supplier;
import org.junit.jupiter.api.Test;
import software.amazon.awssdk.profiles.ProfileFile;
import software.amazon.awssdk.profiles.ProfileFileSupplier;
Expand All @@ -27,7 +28,7 @@
class DefaultCredentialsProviderTest {

@Test
void resolveCredentials_requestFallsIntoProfileCredentialsProviderWithProfileFile_returnsCredentials() {
void resolveCredentials_ProfileCredentialsProviderWithProfileFile_returnsCredentials() {
DefaultCredentialsProvider provider = DefaultCredentialsProvider
.builder()
.profileFile(credentialFile("test", "access", "secret"))
Expand All @@ -41,7 +42,7 @@ void resolveCredentials_requestFallsIntoProfileCredentialsProviderWithProfileFil
}

@Test
void resolveCredentials_requestFallsIntoProfileCredentialsProviderWithProfileFileSupplier_returnsCredentials() {
void resolveCredentials_ProfileCredentialsProviderWithProfileFileSupplier_resolvesCredentialsPerCall() {
List<ProfileFile> profileFileList = Arrays.asList(credentialFile("test", "access", "secret"),
credentialFile("test", "modified", "update"));
ProfileFileSupplier profileFileSupplier = supply(profileFileList);
Expand All @@ -63,6 +64,39 @@ void resolveCredentials_requestFallsIntoProfileCredentialsProviderWithProfileFil
});
}

@Test
void resolveCredentials_ProfileCredentialsProviderWithProfileFileSupplier_returnsCredentials() {
ProfileFile profileFile = credentialFile("test", "access", "secret");
ProfileFileSupplier profileFileSupplier = ProfileFileSupplier.fixedProfileFile(profileFile);

DefaultCredentialsProvider provider = DefaultCredentialsProvider
.builder()
.profileFile(profileFileSupplier)
.profileName("test")
.build();

assertThat(provider.resolveCredentials()).satisfies(awsCredentials -> {
assertThat(awsCredentials.accessKeyId()).isEqualTo("access");
assertThat(awsCredentials.secretAccessKey()).isEqualTo("secret");
});
}

@Test
void resolveCredentials_ProfileCredentialsProviderWithSupplierProfileFile_returnsCredentials() {
Supplier<ProfileFile> supplier = () -> credentialFile("test", "access", "secret");

DefaultCredentialsProvider provider = DefaultCredentialsProvider
.builder()
.profileFile(supplier)
.profileName("test")
.build();

assertThat(provider.resolveCredentials()).satisfies(awsCredentials -> {
assertThat(awsCredentials.accessKeyId()).isEqualTo("access");
assertThat(awsCredentials.secretAccessKey()).isEqualTo("secret");
});
}

private ProfileFile credentialFile(String credentialFile) {
return ProfileFile.builder()
.content(new StringInputStream(credentialFile))
Expand Down
42 changes: 42 additions & 0 deletions .../java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import java.time.ZoneOffset;
import java.util.Arrays;
import java.util.List;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import org.junit.AfterClass;
import org.junit.Before;
Expand Down Expand Up @@ -305,6 +306,47 @@ public void resolveCredentials_customProfileFileSupplierAndNameSettingEndpointOv
}
}

@Test
public void resolveCredentials_customSupplierProfileFileAndNameSettingEndpointOverride_usesCorrectEndpointFromSupplier() {
System.clearProperty(SdkSystemSetting.AWS_EC2_METADATA_SERVICE_ENDPOINT.property());
WireMockServer mockMetadataEndpoint_2 = new WireMockServer(WireMockConfiguration.options().dynamicPort());
mockMetadataEndpoint_2.start();
try {
String stubToken = "some-token";
mockMetadataEndpoint_2.stubFor(put(urlPathEqualTo(TOKEN_RESOURCE_PATH)).willReturn(aResponse().withBody(stubToken)));
mockMetadataEndpoint_2.stubFor(get(urlPathEqualTo(CREDENTIALS_RESOURCE_PATH)).willReturn(aResponse().withBody("some-profile")));
mockMetadataEndpoint_2.stubFor(get(urlPathEqualTo(CREDENTIALS_RESOURCE_PATH + "some-profile")).willReturn(aResponse().withBody(STUB_CREDENTIALS)));

String mockServer2Endpoint = "http://localhost:" + mockMetadataEndpoint_2.port();

ProfileFile config = configFile("profile test",
Pair.of(ProfileProperty.EC2_METADATA_SERVICE_ENDPOINT, mockServer2Endpoint));

Supplier<ProfileFile> supplier = () -> config;

InstanceProfileCredentialsProvider provider = InstanceProfileCredentialsProvider
.builder()
.profileFile(supplier)
.profileName("test")
.build();

AwsCredentials awsCredentials1 = provider.resolveCredentials();

assertThat(awsCredentials1).isNotNull();

String userAgentHeader = "User-Agent";
String userAgent = SdkUserAgent.create().userAgent();
mockMetadataEndpoint_2.verify(putRequestedFor(urlPathEqualTo(TOKEN_RESOURCE_PATH)).withHeader(userAgentHeader, equalTo(userAgent)));
mockMetadataEndpoint_2.verify(getRequestedFor(urlPathEqualTo(CREDENTIALS_RESOURCE_PATH)).withHeader(userAgentHeader, equalTo(userAgent)));
mockMetadataEndpoint_2.verify(getRequestedFor(urlPathEqualTo(CREDENTIALS_RESOURCE_PATH + "some-profile")).withHeader(userAgentHeader, equalTo(userAgent)));

// all requests should have gone to the second server, and none to the other one
mockMetadataEndpoint.verify(0, RequestPatternBuilder.allRequests());
} finally {
mockMetadataEndpoint_2.stop();
}
}

@Test
public void resolveCredentials_doesNotFailIfImdsReturnsExpiredCredentials() {
String credentialsResponse =
Expand Down
Loading