S3 repository credentials should use the default credential provider chain if no credentials are provided

Bugs
1

Looking through the source code it looks like the S3 repository support is using the AWS Java SDK.

As it is right now, if no credentials are provided the credentials passed to AmazonS3Client will be null, instead I suggest that the constructor that takes ClientConfiguration as it’s single argument is used if the credentials are null, then the SDK will use the DefaultCredentialsProviderChain which supports (among other things) profile credentials and instance profile credentials (if building on an EC2 instance).

In essence, the suggestion is that row 61 in S3Client.java is changed from:

AmazonS3Client amazonS3Client = new AmazonS3Client(credentials, createConnectionProperties());

to

AmazonS3Client amazonS3Client;
if(credentials != null) {
    amazonS3Client = new AmazonS3Client(credentials, createConnectionProperties());
} else {
    amazonS3Client = new AmazonS3Client(createConnectionProperties());
}

and that calling code (S3ConnectorFactory) supports creating the S3Client with null credentials.

2

@Raniz we defiantly want to allow more authentication mechanisms for S3 (and other) repositories and we have a design spec for it. Using the DefaultCredentialsProviderChain has come up in discussion in the past but we felt it’s not quite the right approach as we want to give build authors the ability to explicitly specify credentials via the DSL.

3

I’m +1 for @Raniz’s patch that allows the AWS SDK to use the default credentialing mechanism when no credentials are supplied. (in fact – we just forked gradle to make this patch)

Specifically, we want/need to use instance credentials instead of a specific user key+secret.

Is it worth supporting both the new credentials design and default credentialing?

4

We want the user to be explicit about the credentials to use, even if that means stating that they want InstanceCredentials to be used.

There is definitely interest in getting this working: but the solution will be more involved than the patch above, and will need to include integration test coverage as well.