-
Notifications
You must be signed in to change notification settings - Fork 918
Add Ec2 metadata disabling support via profile configuration #6204
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: master
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 |
---|---|---|
@@ -0,0 +1,6 @@ | ||
{ | ||
"type": "feature", | ||
"category": "AWS SDK for Java v2", | ||
"contributor": "", | ||
"description": "Adding 'disable_ec2_metadata' in the profile property, this can be used to disable IMDS credential fetching." | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,12 +45,14 @@ public final class Ec2MetadataConfigProvider { | |
private final String profileName; | ||
|
||
private final Lazy<Boolean> metadataV1Disabled; | ||
private final Lazy<Boolean> metadataDisabled; | ||
private final Lazy<Long> serviceTimeout; | ||
|
||
private Ec2MetadataConfigProvider(Builder builder) { | ||
this.profileFile = builder.profileFile; | ||
this.profileName = builder.profileName; | ||
this.metadataV1Disabled = new Lazy<>(this::resolveMetadataV1Disabled); | ||
this.metadataDisabled = new Lazy<>(this::resolveMetadataDisabled); | ||
this.serviceTimeout = new Lazy<>(this::resolveServiceTimeout); | ||
} | ||
|
||
|
@@ -120,6 +122,14 @@ public boolean isMetadataV1Disabled() { | |
return metadataV1Disabled.getValue(); | ||
} | ||
|
||
/** | ||
* Resolves whether EC2 Metadata is disabled. | ||
* @return true if EC2 Metadata is disabled, false otherwise. | ||
*/ | ||
public boolean isMetadataDisabled() { | ||
return metadataDisabled.getValue(); | ||
} | ||
|
||
/** | ||
* Resolves the EC2 Metadata Service Timeout in milliseconds. | ||
* @return the timeout value in milliseconds. | ||
|
@@ -137,6 +147,12 @@ private boolean resolveMetadataV1Disabled() { | |
.orElse(false); | ||
} | ||
|
||
// Internal resolution logic for Metadata disabled | ||
private boolean resolveMetadataDisabled() { | ||
return fromProfileFileMetadataDisabled(profileFile, profileName) | ||
.orElse(false); | ||
} | ||
|
||
// Internal resolution logic for Service Timeout | ||
private long resolveServiceTimeout() { | ||
return OptionalUtils.firstPresent( | ||
|
@@ -158,6 +174,15 @@ private static Optional<Boolean> fromProfileFileMetadataV1Disabled(Supplier<Prof | |
.flatMap(p -> p.booleanProperty(ProfileProperty.EC2_METADATA_V1_DISABLED)); | ||
} | ||
|
||
// Profile file resolution for Metadata disabled | ||
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. Can we add some tests? See Ec2MetadataServiceTimeoutResolverTest for reference. |
||
private static Optional<Boolean> fromProfileFileMetadataDisabled(Supplier<ProfileFile> profileFile, String profileName) { | ||
Optional<Profile> profile = profileFile.get().profile(profileName); | ||
if (profile.isPresent()) { | ||
return profile.get().booleanProperty(ProfileProperty.EC2_METADATA_DISABLED); | ||
} | ||
return Optional.empty(); | ||
Comment on lines
+180
to
+183
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 be shortened, see |
||
} | ||
|
||
// System settings resolution for Service Timeout | ||
private static Optional<Long> fromSystemSettingsServiceTimeout() { | ||
return SdkSystemSetting.AWS_METADATA_SERVICE_TIMEOUT.getNonDefaultStringValue() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -319,12 +319,49 @@ void resolveCredentials_metadataLookupDisabled_throws() { | |
try { | ||
assertThatThrownBy(() -> InstanceProfileCredentialsProvider.builder().build().resolveCredentials()) | ||
.isInstanceOf(SdkClientException.class) | ||
.hasMessage("IMDS credentials have been disabled by environment variable or system property."); | ||
.hasMessage("IMDS credentials have been disabled by environment variable, system property, or configuration file profile setting."); | ||
} finally { | ||
System.clearProperty(SdkSystemSetting.AWS_EC2_METADATA_DISABLED.property()); | ||
} | ||
} | ||
|
||
@Test | ||
void resolveCredentials_metadataDisabledThroughConfig_throwsException() { | ||
stubSecureCredentialsResponse(aResponse().withBody(STUB_CREDENTIALS)); | ||
try { | ||
InstanceProfileCredentialsProvider.builder() | ||
.profileFile(configFile("profile test", Pair.of("disable_ec2_metadata", "true"))) | ||
.profileName("test") | ||
.build() | ||
.resolveCredentials(); | ||
ProfileFile config = configFile("profile test", Pair.of("disable_ec2_metadata", "true")); | ||
System.out.println("Test config file: " + config.toString()); | ||
} catch (Exception e) { | ||
assertThat(e).isInstanceOf(SdkClientException.class); | ||
assertThat(e).hasMessage("IMDS credentials have been disabled by environment variable, system property, or configuration file profile setting."); | ||
Comment on lines
+340
to
+341
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. We should use |
||
} | ||
|
||
// Verify that no calls were made to the IMDS endpoints | ||
WireMock.verify(0, putRequestedFor(urlPathEqualTo(TOKEN_RESOURCE_PATH))); | ||
WireMock.verify(0, getRequestedFor(urlPathEqualTo(CREDENTIALS_RESOURCE_PATH))); | ||
} | ||
|
||
@Test | ||
void resolveCredentials_metadataEnabledThroughConfig_returnsCredentials() { | ||
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. This is a bit confusing since it's enabled by default. I think we can remove this test once we address the other test comment |
||
stubSecureCredentialsResponse(aResponse().withBody(STUB_CREDENTIALS)); | ||
|
||
InstanceProfileCredentialsProvider provider = InstanceProfileCredentialsProvider.builder() | ||
.profileFile(configFile("profile test", Pair.of("disable_ec2_metadata", "false"))) | ||
.profileName("test") | ||
.build(); | ||
|
||
AwsCredentials credentials = provider.resolveCredentials(); | ||
assertThat(credentials.accessKeyId()).isEqualTo("ACCESS_KEY_ID"); | ||
assertThat(credentials.secretAccessKey()).isEqualTo("SECRET_ACCESS_KEY"); | ||
|
||
verifyImdsCallWithToken(); | ||
} | ||
|
||
@Test | ||
void resolveCredentials_customProfileFileAndName_usesCorrectEndpoint() { | ||
WireMockServer mockMetadataEndpoint_2 = new WireMockServer(WireMockConfiguration.options().dynamicPort()); | ||
|
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.
Should we check system setting here, like how we do it in
resolveMetadataV1Disabled
? I think we also need to update Ec2MetadataClient to honor this config