Skip to content
This repository was archived by the owner on Apr 1, 2024. It is now read-only.

Commit ce191fb

Browse files
addisonjcodelipenghui
authored andcommitted
[tiered-storage] Allow AWS credentials to be refreshed (apache#9387)
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, it does a simple validation to ensure that the underlying credentials can change via AWS SystemPropertyCredentialProvider (cherry picked from commit 562b2e7)
1 parent 5e21470 commit ce191fb

File tree

2 files changed

+62
-42
lines changed

2 files changed

+62
-42
lines changed

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

Lines changed: 30 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;
@@ -279,37 +280,46 @@ public ProviderMetadata getProviderMetadata() {
279280

280281
static final CredentialBuilder AWS_CREDENTIAL_BUILDER = (TieredStorageConfiguration config) -> {
281282
if (config.getCredentials() == null) {
282-
AWSCredentials awsCredentials = null;
283+
final AWSCredentialsProvider authChain;
283284
try {
284285
if (Strings.isNullOrEmpty(config.getConfigProperty(S3_ROLE_FIELD))) {
285-
awsCredentials = DefaultAWSCredentialsProviderChain.getInstance().getCredentials();
286+
authChain = DefaultAWSCredentialsProviderChain.getInstance();
286287
} else {
287-
awsCredentials =
288+
authChain =
288289
new STSAssumeRoleSessionCredentialsProvider.Builder(
289290
config.getConfigProperty(S3_ROLE_FIELD),
290291
config.getConfigProperty(S3_ROLE_SESSION_NAME_FIELD)
291-
).build().getCredentials();
292-
}
293-
294-
if (awsCredentials instanceof AWSSessionCredentials) {
295-
// if we have session credentials, we need to send the session token
296-
// this allows us to support EC2 metadata credentials
297-
SessionCredentials sessionCredentials = SessionCredentials.builder()
298-
.accessKeyId(awsCredentials.getAWSAccessKeyId())
299-
.secretAccessKey(awsCredentials.getAWSSecretKey())
300-
.sessionToken(((AWSSessionCredentials) awsCredentials).getSessionToken())
301-
.build();
302-
config.setProviderCredentials(() -> sessionCredentials);
303-
} else {
304-
Credentials credentials = new Credentials(
305-
awsCredentials.getAWSAccessKeyId(), awsCredentials.getAWSSecretKey());
306-
config.setProviderCredentials(() -> credentials);
292+
).build();
307293
}
308294

295+
// Important! Delay the building of actual credentials
296+
// until later to support tokens that may be refreshed
297+
// such as all session tokens
298+
config.setProviderCredentials(() -> {
299+
AWSCredentials newCreds = authChain.getCredentials();
300+
Credentials jcloudCred = null;
301+
302+
if (newCreds instanceof AWSSessionCredentials) {
303+
// if we have session credentials, we need to send the session token
304+
// this allows us to support EC2 metadata credentials
305+
jcloudCred = SessionCredentials.builder()
306+
.accessKeyId(newCreds.getAWSAccessKeyId())
307+
.secretAccessKey(newCreds.getAWSSecretKey())
308+
.sessionToken(((AWSSessionCredentials) newCreds).getSessionToken())
309+
.build();
310+
} else {
311+
// in the event we hit this branch, we likely don't have expiring
312+
// credentials, however, this still allows for the user to update
313+
// profiles creds or some other mechanism
314+
jcloudCred = new Credentials(
315+
newCreds.getAWSAccessKeyId(), newCreds.getAWSSecretKey());
316+
}
317+
return jcloudCred;
318+
});
309319
} catch (Exception e) {
310320
// allowed, some mock s3 service do not need credential
311321
log.warn("Exception when get credentials for s3 ", e);
312322
}
313323
}
314324
};
315-
}
325+
}

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)