Skip to content

Make AwsRegionProviders throw exceptions like AwsCredentialsProviders #514

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

Merged
merged 1 commit into from
Jun 1, 2018
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 @@ -17,6 +17,7 @@

import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.core.SdkSystemSetting;
import software.amazon.awssdk.core.exception.SdkClientException;
import software.amazon.awssdk.profiles.ProfileFile;
import software.amazon.awssdk.profiles.ProfileProperties;
import software.amazon.awssdk.regions.Region;
Expand All @@ -35,7 +36,7 @@ public Region getRegion() {
.profile(profileName)
.map(p -> p.properties().get(ProfileProperties.REGION))
.map(Region::of)
.orElse(null);
.orElseThrow(() -> new SdkClientException("No region provided in profile: " + profileName));
}
}

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

package software.amazon.awssdk.regions.providers;

import software.amazon.awssdk.annotations.ReviewBeforeRelease;
import software.amazon.awssdk.annotations.SdkProtectedApi;
import software.amazon.awssdk.core.exception.SdkClientException;
import software.amazon.awssdk.regions.Region;
Expand All @@ -28,10 +27,10 @@
@FunctionalInterface
public interface AwsRegionProvider {
/**
* @return Region name to use or null if region information is not available.
* Returns the region name to use. If region information is not available, throws an {@link SdkClientException}.
*
* @return Region name to use.
*/
@ReviewBeforeRelease("Should this throw exceptions so that its contract matches that of the credential providers? This is " +
"currently a protected API used in STS, so we should decide before GA.")
Region getRegion() throws SdkClientException;
Region getRegion();

}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
/**
* Composite {@link AwsRegionProvider} that sequentially delegates to a chain of providers looking
* for region information.
*
* Throws an {@link SdkClientException} if region could not be find in any of the providers.
*/
public class AwsRegionProviderChain implements AwsRegionProvider {

Expand All @@ -40,6 +42,8 @@ public AwsRegionProviderChain(AwsRegionProvider... providers) {

@Override
public Region getRegion() throws SdkClientException {
List<String> exceptionMessages = null;

for (AwsRegionProvider provider : providers) {
try {
final Region region = provider.getRegion();
Expand All @@ -49,9 +53,16 @@ public Region getRegion() throws SdkClientException {
} catch (Exception e) {
// Ignore any exceptions and move onto the next provider
log.debug("Unable to load region from {}:{}", provider.toString(), e.getMessage());

String message = provider.toString() + ": " + e.getMessage();
if (exceptionMessages == null) {
exceptionMessages = new ArrayList<>();
}
exceptionMessages.add(message);
}
}

return null;
throw new SdkClientException("Unable to load region from any of the providers in the chain " + this
+ ": " + exceptionMessages);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,13 @@

package software.amazon.awssdk.regions.providers;

import org.slf4j.LoggerFactory;
import software.amazon.awssdk.core.exception.SdkClientException;
import software.amazon.awssdk.regions.Region;
import software.amazon.awssdk.regions.util.EC2MetadataUtils;

/**
* Attempts to load region information from the EC2 Metadata service. If the application is not
* running on EC2 this provider will return null.
* running on EC2 this provider will thrown an exception.
*/
public class InstanceProfileRegionProvider implements AwsRegionProvider {

Expand All @@ -41,16 +40,15 @@ public Region getRegion() throws SdkClientException {
}
}

return region == null ? null : Region.of(region);
if (region == null) {
throw new SdkClientException("Unable to retrieve region information from EC2 Metadata service. "
+ "Please make sure the application is running on EC2.");
}

return Region.of(region);
}

private String tryDetectRegion() {
try {
return EC2MetadataUtils.getEC2InstanceRegion();
} catch (SdkClientException sce) {
LoggerFactory.getLogger(InstanceProfileRegionProvider.class)
.debug("Ignoring failure to retrieve the region: {}", sce.getMessage());
return null;
}
return EC2MetadataUtils.getEC2InstanceRegion();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@
public class SystemSettingsRegionProvider implements AwsRegionProvider {
@Override
public Region getRegion() throws SdkClientException {
return SdkSystemSetting.AWS_REGION.getStringValue().map(Region::of).orElse(null);
return SdkSystemSetting.AWS_REGION.getStringValue()
.map(Region::of)
.orElseThrow(this::exception);
}

private SdkClientException exception() {
return new SdkClientException(String.format("Unable to load region from system settings. Region must be specified either "
+ "via environment variable (%s) or system property (%s).",
SdkSystemSetting.AWS_REGION.environmentVariable(),
SdkSystemSetting.AWS_REGION.property()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@
package software.amazon.awssdk.regions.providers;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import java.net.URISyntaxException;
import java.nio.file.Paths;
import org.junit.Rule;
import org.junit.Test;
import software.amazon.awssdk.core.exception.SdkClientException;
import software.amazon.awssdk.regions.Region;
import software.amazon.awssdk.testutils.EnvironmentVariableHelper;
import software.amazon.awssdk.core.SdkSystemSetting;
Expand All @@ -31,10 +33,12 @@ public class AwsProfileRegionProviderTest {
public EnvironmentVariableHelper settingsHelper = new EnvironmentVariableHelper();

@Test
public void nonExistentDefaultConfigFile_ReturnsNull() {
public void nonExistentDefaultConfigFile_ThrowsException() {
settingsHelper.set(SdkSystemSetting.AWS_CONFIG_FILE, "/var/tmp/this/is/invalid.txt");
settingsHelper.set(SdkSystemSetting.AWS_SHARED_CREDENTIALS_FILE, "/var/tmp/this/is/also.invalid.txt");
assertThat(new AwsProfileRegionProvider().getRegion()).isNull();
assertThatThrownBy(() -> new AwsProfileRegionProvider().getRegion())
.isInstanceOf(SdkClientException.class)
.hasMessageContaining("No region provided in profile: default");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package software.amazon.awssdk.regions.providers;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -77,12 +76,12 @@ public void providerThrowsError_DoesNotContinueChain() {
assertEquals(expectedRegion, chain.getRegion());
}

@Test
public void noProviderGivesRegion_ReturnsNull() {
@Test (expected = SdkClientException.class)
public void noProviderGivesRegion_ThrowsException() {
AwsRegionProviderChain chain = new AwsRegionProviderChain(new NeverAwsRegionProvider(),
new NeverAwsRegionProvider(),
new NeverAwsRegionProvider());
assertNull(chain.getRegion());
chain.getRegion();
}

private static class NeverAwsRegionProvider implements AwsRegionProvider {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package software.amazon.awssdk.regions.providers;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;

import java.io.IOException;
import org.junit.AfterClass;
Expand All @@ -26,6 +25,7 @@
import org.junit.experimental.runners.Enclosed;
import org.junit.runner.RunWith;
import software.amazon.awssdk.core.SdkSystemSetting;
import software.amazon.awssdk.core.exception.SdkClientException;
import software.amazon.awssdk.regions.Region;
import software.amazon.awssdk.regions.util.EC2MetadataUtilsServer;

Expand Down Expand Up @@ -73,9 +73,8 @@ public void metadataServiceRunning_ProvidesCorrectRegion() {
}

/**
* If the EC2 metadata service is not present then the provider should just return null instead
* of failing. This is to allow the provider to be used in a chain context where another
* provider further down the chain may be able to provide the region.
* If the EC2 metadata service is not present then the provider will throw an exception. If the provider is used
* in a {@link AwsRegionProviderChain}, the chain will catch the exception and go on to the next region provider.
*/
public static class MetadataServiceNotRunning {

Expand All @@ -87,9 +86,9 @@ public void setup() {
regionProvider = new InstanceProfileRegionProvider();
}

@Test
public void metadataServiceNotRunning_ProvidesCorrectRegion() {
assertNull(regionProvider.getRegion());
@Test (expected = SdkClientException.class)
public void metadataServiceNotRunning_ThrowsException() {
regionProvider.getRegion();
}

}
Expand Down