Skip to content

Commit 5180dbb

Browse files
addisonjaddisonj
committed
[tiered-storage] Allow AWS credentials to be refreshed
With the refactor of support azure, a regression occured where the AWS credentials were fetched once and then used through the entire process. This is a problem in AWS, where it is commonplace to use credentials that expire. The AWS credential provider chain takes care of this problem, but when intgrating with JClouds, that means we need the credential Supplier to return a new set of credentials each time. Luckily, AWS should intelligently cache this so we aren't thrashing the underlying credential mechanisms. This also adds a test to ensure this isn't broken in the future
1 parent 0edcaa0 commit 5180dbb

File tree

2 files changed

+60
-42
lines changed

2 files changed

+60
-42
lines changed

‎tiered-storage/jcloud/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/JCloudBlobStoreProvider.java

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import static org.apache.bookkeeper.mledger.offload.jcloud.provider.TieredStorageConfiguration.S3_ROLE_SESSION_NAME_FIELD;
2424

2525
import com.amazonaws.auth.AWSCredentials;
26+
import com.amazonaws.auth.AWSCredentialsProvider;
2627
import com.amazonaws.auth.AWSSessionCredentials;
2728
import com.amazonaws.auth.DefaultAWSCredentialsProviderChain;
2829
import com.amazonaws.auth.STSAssumeRoleSessionCredentialsProvider;
@@ -304,33 +305,40 @@ public ProviderMetadata getProviderMetadata() {
304305

305306
static final CredentialBuilder AWS_CREDENTIAL_BUILDER = (TieredStorageConfiguration config) -> {
306307
if (config.getCredentials() == null) {
307-
AWSCredentials awsCredentials = null;
308+
AWSCredentialsProvider authChain = null;
308309
try {
309310
if (Strings.isNullOrEmpty(config.getConfigProperty(S3_ROLE_FIELD))) {
310-
awsCredentials = DefaultAWSCredentialsProviderChain.getInstance().getCredentials();
311+
authChain = DefaultAWSCredentialsProviderChain.getInstance();
311312
} else {
312-
awsCredentials =
313+
authChain =
313314
new STSAssumeRoleSessionCredentialsProvider.Builder(
314315
config.getConfigProperty(S3_ROLE_FIELD),
315316
config.getConfigProperty(S3_ROLE_SESSION_NAME_FIELD)
316-
).build().getCredentials();
317-
}
318-
319-
if (awsCredentials instanceof AWSSessionCredentials) {
320-
// if we have session credentials, we need to send the session token
321-
// this allows us to support EC2 metadata credentials
322-
SessionCredentials sessionCredentials = SessionCredentials.builder()
323-
.accessKeyId(awsCredentials.getAWSAccessKeyId())
324-
.secretAccessKey(awsCredentials.getAWSSecretKey())
325-
.sessionToken(((AWSSessionCredentials) awsCredentials).getSessionToken())
326-
.build();
327-
config.setProviderCredentials(() -> sessionCredentials);
328-
} else {
329-
Credentials credentials = new Credentials(
330-
awsCredentials.getAWSAccessKeyId(), awsCredentials.getAWSSecretKey());
331-
config.setProviderCredentials(() -> credentials);
317+
).build();
332318
}
333319

320+
// Important! Delay the building of actual credentials
321+
// until later to support tokens that may be refreshed
322+
// such as all session tokens
323+
AWSCredentialsProvider finalAuthChain = authChain;
324+
config.setProviderCredentials(() -> {
325+
AWSCredentials newCreds = finalAuthChain.getCredentials();
326+
Credentials jcloudCred = null;
327+
328+
if (newCreds instanceof AWSSessionCredentials) {
329+
// if we have session credentials, we need to send the session token
330+
// this allows us to support EC2 metadata credentials
331+
jcloudCred = SessionCredentials.builder()
332+
.accessKeyId(newCreds.getAWSAccessKeyId())
333+
.secretAccessKey(newCreds.getAWSSecretKey())
334+
.sessionToken(((AWSSessionCredentials) newCreds).getSessionToken())
335+
.build();
336+
} else {
337+
jcloudCred = new Credentials(
338+
newCreds.getAWSAccessKeyId(), newCreds.getAWSSecretKey());
339+
}
340+
return jcloudCred;
341+
});
334342
} catch (Exception e) {
335343
// allowed, some mock s3 service do not need credential
336344
log.warn("Exception when get credentials for s3 ", e);
@@ -391,4 +399,4 @@ public ProviderMetadata getProviderMetadata() {
391399
config.setProviderCredentials(() -> credentials);
392400
};
393401

394-
}
402+
}

‎tiered-storage/jcloud/src/test/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/TieredStorageConfigurationTests.java

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@
2323
import java.util.HashMap;
2424
import java.util.List;
2525
import java.util.Map;
26+
import org.apache.pulsar.jcloud.shade.com.google.common.base.Supplier;
2627

28+
import org.jclouds.domain.Credentials;
2729
import org.testng.annotations.Test;
2830

2931
public class TieredStorageConfigurationTests {
@@ -113,7 +115,36 @@ public final void awsS3BackwardCompatiblePropertiesTest() {
113115
assertEquals(config.getReadBufferSizeInBytes(), new Integer(500));
114116
assertEquals(config.getServiceEndpoint(), "http://some-url:9093");
115117
}
116-
118+
119+
/**
120+
* Confirm that with AWS we create different instances of the credentials
121+
* object each time we call the supplier, this ensure that we get fresh credentials
122+
* if the aws credential provider changes
123+
*/
124+
@Test
125+
public final void awsS3CredsProviderTest() {
126+
Map<String, String> map = new HashMap<>();
127+
map.put(TieredStorageConfiguration.BLOB_STORE_PROVIDER_KEY, JCloudBlobStoreProvider.AWS_S3.getDriver());
128+
TieredStorageConfiguration config = new TieredStorageConfiguration(map);
129+
130+
// set the aws properties with fake creds so the defaultProviderChain works
131+
System.setProperty("aws.accessKeyId", "fakeid1");
132+
System.setProperty("aws.secretKey", "fakekey1");
133+
Credentials creds1 = config.getProviderCredentials().get();
134+
assertEquals(creds1.identity, "fakeid1");
135+
assertEquals(creds1.credential, "fakekey1");
136+
137+
// reset the properties and ensure we get different values by re-evaluating the chain
138+
System.setProperty("aws.accessKeyId", "fakeid2");
139+
System.setProperty("aws.secretKey", "fakekey2");
140+
Credentials creds2 = config.getProviderCredentials().get();
141+
assertEquals(creds2.identity, "fakeid2");
142+
assertEquals(creds2.credential, "fakekey2");
143+
144+
System.clearProperty("aws.accessKeyId");
145+
System.clearProperty("aws.secretKey");
146+
}
147+
117148
/**
118149
* Confirm that both property options are available for GCS
119150
*/
@@ -177,25 +208,4 @@ public final void gcsBackwardCompatiblePropertiesTest() {
177208
assertEquals(config.getMaxBlockSizeInBytes(), new Integer(12));
178209
assertEquals(config.getReadBufferSizeInBytes(), new Integer(500));
179210
}
180-
181-
/**
182-
* Confirm that we can configure AWS using the old properties
183-
*/
184-
@Test
185-
public final void s3BackwardCompatiblePropertiesTest() {
186-
Map<String, String> map = new HashMap<String,String>();
187-
map.put(TieredStorageConfiguration.BLOB_STORE_PROVIDER_KEY, JCloudBlobStoreProvider.AWS_S3.getDriver());
188-
map.put(BC_S3_BUCKET, "test bucket");
189-
map.put(BC_S3_ENDPOINT, "http://some-url:9093");
190-
map.put(BC_S3_MAX_BLOCK_SIZE, "12");
191-
map.put(BC_S3_READ_BUFFER_SIZE, "500");
192-
map.put(BC_S3_REGION, "test region");
193-
TieredStorageConfiguration config = new TieredStorageConfiguration(map);
194-
195-
assertEquals(config.getRegion(), "test region");
196-
assertEquals(config.getBucket(), "test bucket");
197-
assertEquals(config.getMaxBlockSizeInBytes(), new Integer(12));
198-
assertEquals(config.getReadBufferSizeInBytes(), new Integer(500));
199-
assertEquals(config.getServiceEndpoint(), "http://some-url:9093");
200-
}
201211
}

0 commit comments

Comments
 (0)