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

Fix reload credentials not working when modified - external PR : #5185 #5259

Closed
wants to merge 15 commits into from
Closed
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 @@ -99,7 +99,7 @@ private InstanceProfileCredentialsProvider(BuilderImpl builder) {
this.asyncCredentialUpdateEnabled = builder.asyncCredentialUpdateEnabled;
this.asyncThreadName = builder.asyncThreadName;
this.profileFile = Optional.ofNullable(builder.profileFile)
.orElseGet(() -> ProfileFileSupplier.fixedProfileFile(ProfileFile.defaultProfileFile()));
.orElseGet(ProfileFileSupplier::defaultSupplier);
this.profileName = Optional.ofNullable(builder.profileName)
.orElseGet(ProfileFileSystemSetting.AWS_PROFILE::getStringValueOrThrow);

Expand Down Expand Up @@ -311,7 +311,7 @@ public interface Builder extends HttpCredentialsProvider.Builder<InstanceProfile
* Configure the profile file used for loading IMDS-related configuration, like the endpoint mode (IPv4 vs IPv6).
*
* <p>By default, {@link ProfileFile#defaultProfileFile()} is used.
*
*
* @see #profileFile(Supplier)
*/
Builder profileFile(ProfileFile profileFile);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ private ProfileCredentialsProvider(BuilderImpl builder) {
.orElseGet(ProfileFileSystemSetting.AWS_PROFILE::getStringValueOrThrow);
selectedProfileSupplier =
Optional.ofNullable(builder.profileFile)
.orElseGet(() -> ProfileFileSupplier.fixedProfileFile(builder.defaultProfileFileLoader.get()));
.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 Down Expand Up @@ -196,7 +196,7 @@ public interface Builder extends CopyableBuilder<Builder, ProfileCredentialsProv
* Define the mechanism for loading profile files.
*
* @param profileFileSupplier Supplier interface for generating a ProfileFile instance.
* @see #profileFile(ProfileFile)
* @see #profileFile(ProfileFile)
*/
Builder profileFile(Supplier<ProfileFile> profileFileSupplier);

Expand All @@ -216,7 +216,7 @@ public interface Builder extends CopyableBuilder<Builder, ProfileCredentialsProv
static final class BuilderImpl implements Builder {
private Supplier<ProfileFile> profileFile;
private String profileName;
private Supplier<ProfileFile> defaultProfileFileLoader = ProfileFile::defaultProfileFile;
private Supplier<ProfileFile> defaultProfileFileLoader = ProfileFileSupplier.defaultSupplier();

BuilderImpl() {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,28 @@
package software.amazon.awssdk.auth.credentials;

import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.List;
import java.util.function.Supplier;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import software.amazon.awssdk.profiles.ProfileFile;
import software.amazon.awssdk.profiles.ProfileFileSupplier;
import software.amazon.awssdk.profiles.ProfileFileSystemSetting;
import software.amazon.awssdk.utils.StringInputStream;
import software.amazon.awssdk.utils.internal.SystemSettingUtilsTestBackdoor;

class DefaultCredentialsProviderTest {

@TempDir
private Path testDirectory;

@Test
void resolveCredentials_ProfileCredentialsProviderWithProfileFile_returnsCredentials() {
DefaultCredentialsProvider provider = DefaultCredentialsProvider
Expand Down Expand Up @@ -97,6 +108,56 @@ void resolveCredentials_ProfileCredentialsProviderWithSupplierProfileFile_return
});
}

@Test
void resolveCredentials_DefaultCredentialProviderWithReloadWhenModified() throws Exception {
Path credentialsFilePath = generateTestCredentialsFile("customAccess", "customSecret");
SystemSettingUtilsTestBackdoor.addEnvironmentVariableOverride(ProfileFileSystemSetting.AWS_SHARED_CREDENTIALS_FILE.environmentVariable(),
credentialsFilePath.toString());
DefaultCredentialsProvider provider = DefaultCredentialsProvider.create();
Thread.sleep(2000);
try {
assertThat(provider.resolveCredentials()).satisfies(awsCredentials -> {
assertThat(awsCredentials.accessKeyId()).isEqualTo("customAccess");
assertThat(awsCredentials.secretAccessKey()).isEqualTo("customSecret");
});
} catch (AssertionError e) {
Thread.sleep(2000);
assertTrue(assertReloadCredentials(provider));
}

generateTestCredentialsFile("modifiedAccess", "modifiedSecret");

// without sleep the assertion fails, as there is delay in config reload
Thread.sleep(1000);
assertThat(provider.resolveCredentials()).satisfies(awsCredentials -> {
assertThat(awsCredentials.accessKeyId()).isEqualTo("modifiedAccess");
assertThat(awsCredentials.secretAccessKey()).isEqualTo("modifiedSecret");
});
SystemSettingUtilsTestBackdoor.clearEnvironmentVariableOverrides();
}

private boolean assertReloadCredentials(AwsCredentialsProvider provider) {
assertThat(provider.resolveCredentials()).satisfies(awsCredentials -> {
assertThat(awsCredentials.accessKeyId()).isEqualTo("customAccess");
assertThat(awsCredentials.secretAccessKey()).isEqualTo("customSecret");
});
}

private Path generateTestFile(String contents, String filename) {
try {
Files.createDirectories(testDirectory);
return Files.write(testDirectory.resolve(filename), contents.getBytes(StandardCharsets.UTF_8));
} catch (IOException e) {
throw new RuntimeException(e);
}
}

private Path generateTestCredentialsFile(String accessKeyId, String secretAccessKey) {
String contents = String.format("[default]\naws_access_key_id = %s\naws_secret_access_key = %s\n",
accessKeyId, secretAccessKey);
return generateTestFile(contents, "credentials.txt");
}

private ProfileFile credentialFile(String credentialFile) {
return ProfileFile.builder()
.content(new StringInputStream(credentialFile))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,15 @@ static ProfileFileSupplier defaultSupplier() {
= ProfileFileLocation.configurationFileLocation()
.map(path -> reloadWhenModified(path, ProfileFile.Type.CONFIGURATION));

ProfileFileSupplier supplier = () -> ProfileFile.builder().build();
ProfileFileSupplier supplier = null;
if (credentialsSupplierOptional.isPresent() && configurationSupplierOptional.isPresent()) {
supplier = aggregate(credentialsSupplierOptional.get(), configurationSupplierOptional.get());
} else if (credentialsSupplierOptional.isPresent()) {
supplier = credentialsSupplierOptional.get();
} else if (configurationSupplierOptional.isPresent()) {
supplier = configurationSupplierOptional.get();
} else {
supplier = fixedProfileFile(ProfileFile.aggregator().build());
}

return supplier;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@
import software.amazon.awssdk.identity.spi.IdentityProviders;
import software.amazon.awssdk.metrics.MetricPublisher;
import software.amazon.awssdk.profiles.ProfileFile;
import software.amazon.awssdk.profiles.ProfileFileSupplier;
import software.amazon.awssdk.profiles.ProfileFileSystemSetting;
import software.amazon.awssdk.profiles.ProfileProperty;
import software.amazon.awssdk.retries.AdaptiveRetryStrategy;
Expand Down Expand Up @@ -273,7 +274,8 @@ protected SdkClientConfiguration mergeChildDefaults(SdkClientConfiguration confi
* Apply global default configuration
*/
private SdkClientConfiguration mergeGlobalDefaults(SdkClientConfiguration configuration) {
Supplier<ProfileFile> defaultProfileFileSupplier = new Lazy<>(ProfileFile::defaultProfileFile)::getValue;
Supplier<ProfileFileSupplier> profileFileSupplierProvider = new Lazy<>(ProfileFileSupplier::defaultSupplier)::getValue;
Supplier<ProfileFile> defaultProfileFileSupplier = profileFileSupplierProvider.get();

configuration = configuration.merge(c -> c.option(EXECUTION_INTERCEPTORS, new ArrayList<>())
.option(METRIC_PUBLISHERS, new ArrayList<>())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ private S3Utilities(Builder builder) {
this.region = Validate.paramNotNull(builder.region, "Region");
this.endpoint = builder.endpoint;
this.profileFile = Optional.ofNullable(builder.profileFile)
.orElseGet(() -> ProfileFileSupplier.fixedProfileFile(ProfileFile.defaultProfileFile()));
.orElseGet(ProfileFileSupplier::defaultSupplier);
this.profileName = builder.profileName;

if (builder.s3Configuration == null) {
Expand Down
Loading