Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
6 changes: 6 additions & 0 deletions .changes/next-release/bugfix-AWSSDKforJavav2-1c5bd30.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "bugfix",
"category": "AWS SDK for Java v2",
"contributor": "",
"description": "Fix credential reloading in defaults when shared credential/config files are modified."
}
6 changes: 6 additions & 0 deletions .changes/next-release/bugfix-AWSSDKforJavav2-b3c8f2c.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "bugfix",
"category": "AWS SDK for Java v2",
"contributor": "",
"description": "Gracefully handle missing file in ProfileFileSupplier.reloadWhenModified."
}
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,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);
this.sourceChain = builder.sourceChain;
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()));
.orElse(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 @@ -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 @@ -17,9 +17,15 @@

import static org.assertj.core.api.AssertionsForClassTypes.assertThat;

import com.google.common.jimfs.Jimfs;
import java.io.IOException;
import java.nio.file.FileSystem;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.List;
import java.util.function.Supplier;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
Comment thread
alextwoods marked this conversation as resolved.
Outdated
import org.junit.jupiter.api.Test;
import software.amazon.awssdk.profiles.ProfileFile;
import software.amazon.awssdk.profiles.ProfileFileSupplier;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.nio.file.FileSystem;
import java.nio.file.Files;
import java.nio.file.Path;
import java.time.Instant;
Comment thread
alextwoods marked this conversation as resolved.
Outdated
import java.util.Optional;
import java.util.function.Supplier;
import org.junit.jupiter.api.AfterAll;
Expand Down Expand Up @@ -243,6 +244,33 @@ void resolveCredentials_presentSupplierProfileFile_returnsCredentials() {
});
}

@Test
void resolveCredentials_defaultProfileFileSupplier_refreshesCredentials() throws InterruptedException {
Path credentialsFile = generateTestCredentialsFile("defaultAccessKey", "defaultSecretAccessKey");

ProfileCredentialsProvider provider = new ProfileCredentialsProvider
.BuilderImpl()
.defaultProfileFileLoader(ProfileFileSupplier.reloadWhenModified(credentialsFile, ProfileFile.Type.CREDENTIALS))
.profileName("default")
.build();

assertThat(provider.resolveCredentials()).satisfies(credentials -> {
assertThat(credentials.accessKeyId()).isEqualTo("defaultAccessKey");
assertThat(credentials.secretAccessKey()).isEqualTo("defaultSecretAccessKey");
});

// modify the file
generateTestCredentialsFile("updatedAccessKey", "updatedSecretAccessKey");

// ProfileFileRefresher has a stale time of 1000 ms.
Thread.sleep(1000);

assertThat(provider.resolveCredentials()).satisfies(credentials -> {
assertThat(credentials.accessKeyId()).isEqualTo("updatedAccessKey");
assertThat(credentials.secretAccessKey()).isEqualTo("updatedSecretAccessKey");
});
}

@Test
void create_noProfileName_returnsProfileCredentialsProviderToResolveWithDefaults() {
ProfileCredentialsProvider provider = ProfileCredentialsProvider.create();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

package software.amazon.awssdk.profiles;

import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Collections;
import java.util.LinkedHashMap;
Expand All @@ -38,7 +39,7 @@ public interface ProfileFileSupplier extends Supplier<ProfileFile> {

/**
* Creates a {@link ProfileFileSupplier} capable of producing multiple profile objects by aggregating the default
* credentials and configuration files as determined by {@link ProfileFileLocation#credentialsFileLocation()} abd
* credentials and configuration files as determined by {@link ProfileFileLocation#credentialsFileLocation()} and
* {@link ProfileFileLocation#configurationFileLocation()}. This supplier will return a new ProfileFile instance only once
* either disk file has been modified. Multiple calls to the supplier while both disk files are unchanged will return the
* same object.
Expand Down Expand Up @@ -81,13 +82,17 @@ static ProfileFileSupplier defaultSupplier() {
*/
static ProfileFileSupplier reloadWhenModified(Path path, ProfileFile.Type type) {
return new ProfileFileSupplier() {

final ProfileFile.Builder builder = ProfileFile.builder()
.content(path)
.type(type);
Supplier<ProfileFile> profileFileSupplier = () -> {
if (Files.isRegularFile(path) && Files.isReadable(path)) {
return ProfileFile.builder()
.content(path)
.type(type).build();
}
return ProfileFile.empty();
};

final ProfileFileRefresher refresher = ProfileFileRefresher.builder()
.profileFile(builder::build)
.profileFile(profileFileSupplier)
.profileFilePath(path)
.build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

package software.amazon.awssdk.profiles;

import java.nio.file.Files;
import java.nio.file.Path;
import java.time.Clock;
import java.util.Objects;
Expand All @@ -33,10 +34,14 @@ final class ProfileFileSupplierBuilder {
private Consumer<ProfileFile> onProfileFileLoad;

public ProfileFileSupplierBuilder reloadWhenModified(Path path, ProfileFile.Type type) {
ProfileFile.Builder builder = ProfileFile.builder()
.content(path)
.type(type);
this.profileFile = builder::build;
this.profileFile = () -> {
if (Files.isRegularFile(path) && Files.isReadable(path)) {
return ProfileFile.builder()
.content(path)
.type(type).build();
}
return ProfileFile.empty();
};
this.profileFilePath = path;
this.reloadingSupplier = true;
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ private boolean isNewProfileFile(ProfileFile profileFile) {
}

private boolean canReloadProfileFile() {
if (Objects.isNull(profileFilePath)) {
if (Objects.isNull(profileFilePath) || !Files.exists(profileFilePath)) {
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,28 @@ public void defaultSupplier_noCredentialsFiles_returnsEmptyProvider() {
});
}

@Test
public void reloadWhenModified_noCredentialsFiles_returnsEmptyProvider_andRefreshes() throws IOException {
Path credentialsFilePath = getTestCredentialsFilePath();
Files.deleteIfExists(credentialsFilePath);

AdjustableClock clock = new AdjustableClock();
ProfileFileSupplier supplier = builderWithClock(clock)
.reloadWhenModified(credentialsFilePath, ProfileFile.Type.CREDENTIALS)
.build();

assertThat(supplier.get().profiles()).isEmpty();

generateTestCredentialsFile("modifiedAccessKey", "modifiedSecretAccessKey", "modifiedAccountId");
updateModificationTime(credentialsFilePath, clock.instant().plusMillis(1));

clock.tickForward(Duration.ofSeconds(10));

// supplied ProfileFile should refreshed and now have data under the `default` profile
Optional<Profile> fileOptional = supplier.get().profile("default");
assertThat(fileOptional).isPresent();
}

private Path writeTestFile(String contents, Path path) {
try {
Files.createDirectories(testDirectory);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,13 @@
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.api.RetryStrategy;
import software.amazon.awssdk.utils.AttributeMap;
import software.amazon.awssdk.utils.AttributeMap.LazyValueSource;
import software.amazon.awssdk.utils.Either;
import software.amazon.awssdk.utils.Lazy;
import software.amazon.awssdk.utils.OptionalUtils;
import software.amazon.awssdk.utils.StringUtils;
import software.amazon.awssdk.utils.ThreadFactoryBuilder;
Expand Down Expand Up @@ -280,7 +280,7 @@ protected SdkClientConfiguration mergeChildDefaults(SdkClientConfiguration confi
* Apply global default configuration
*/
private SdkClientConfiguration mergeGlobalDefaults(SdkClientConfiguration configuration) {
Supplier<ProfileFile> defaultProfileFileSupplier = new Lazy<>(ProfileFile::defaultProfileFile)::getValue;
Supplier<ProfileFile> defaultProfileFileSupplier = ProfileFileSupplier.defaultSupplier();

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 @@ -44,8 +44,13 @@
import java.beans.BeanInfo;
import java.beans.Introspector;
import java.beans.PropertyDescriptor;
import java.io.File;
import java.io.IOException;
import java.lang.reflect.Method;
import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -59,7 +64,9 @@
import java.util.function.Supplier;
import org.assertj.core.api.Assertions;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.Mockito;
Expand All @@ -81,6 +88,7 @@
import software.amazon.awssdk.metrics.MetricCollection;
import software.amazon.awssdk.metrics.MetricPublisher;
import software.amazon.awssdk.profiles.ProfileFile;
import software.amazon.awssdk.testutils.EnvironmentVariableHelper;
import software.amazon.awssdk.utils.AttributeMap;
import software.amazon.awssdk.utils.StringInputStream;

Expand All @@ -100,6 +108,9 @@ public class DefaultClientBuilderTest {
private static final URI ENDPOINT = URI.create("https://example.com");
private static final NoOpSigner TEST_SIGNER = new NoOpSigner();

@Rule
public TemporaryFolder tempFolder = new TemporaryFolder();

@Mock
private SdkHttpClient.Builder defaultHttpClientFactory;

Expand Down Expand Up @@ -419,6 +430,39 @@ public void defaultProfileFileSupplier_isStaticOrHasIdentityCaching() {
assertThat(secondGet).isSameAs(firstGet);
}

@Test
public void defaultProfileFileSupplier_refreshesWhenFileModified() {
EnvironmentVariableHelper.run(env -> {
try {
File credentialFile = tempFolder.newFile();
writeTestCredentialsFile(credentialFile, "akid1", "sak");
env.set("AWS_SHARED_CREDENTIALS_FILE", credentialFile.getPath());
SdkClientConfiguration config =
testClientBuilder().build().clientConfiguration;

Supplier<ProfileFile> defaultProfileFileSupplier = config.option(PROFILE_FILE_SUPPLIER);
ProfileFile firstGet = defaultProfileFileSupplier.get();

writeTestCredentialsFile(credentialFile, "updatedAkid", "updatedSak");
Thread.sleep(1000);
ProfileFile secondGet = defaultProfileFileSupplier.get();

assertThat(secondGet).isNotSameAs(firstGet);
assertThat(secondGet.profile("default")).isPresent();
assertThat(secondGet.profile("default").get()).satisfies(profile -> {
assertThat(profile.property("aws_access_key_id"))
.isEqualTo(Optional.of("updatedAkid"));
assertThat(profile.property("aws_secret_access_key"))
.isEqualTo(Optional.of("updatedSak"));
});

}
catch (IOException | InterruptedException e) {
throw new RuntimeException(e);
}
});
}

private SdkDefaultClientBuilder<TestClientBuilder, TestClient> testClientBuilder() {
ClientOverrideConfiguration overrideConfig =
ClientOverrideConfiguration.builder()
Expand All @@ -437,6 +481,13 @@ private SdkDefaultClientBuilder<TestAsyncClientBuilder, TestAsyncClient> testAsy
return new TestAsyncClientBuilder().overrideConfiguration(overrideConfig);
}

private void writeTestCredentialsFile(File file, String accessKeyId, String secretAccessKey)
throws IOException {
String contents = String.format("[default]\naws_access_key_id = %s\naws_secret_access_key = %s\n",
accessKeyId, secretAccessKey);
Files.write(file.toPath(), contents.getBytes(StandardCharsets.UTF_8));
}

private static class TestClient {
private final SdkClientConfiguration clientConfiguration;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,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