Skip to content
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

Use default provider chains to find AWS credentials and region #16

Merged
merged 2 commits into from
Apr 28, 2020
Merged

Conversation

RoKish
Copy link
Contributor

@RoKish RoKish commented Apr 9, 2020

The pull request contains the changes to use default provider chains in case AWS credentials or a region are not defined in the plugin configuration. Closes issue #1.

For AWS credentials, the following order is used to find the AWS access keys:

  1. Use ~/.m2/settings.xml if serverId is defined in the plugin configuration.
  2. Use awsAccessKey and awsSecretAccessKey if they're defined.
  3. Use the order defined byDefaultAWSCredentialsProviderChain.

To infer the region, we simply check if the region configuration parameter is defined. If the parameter isn't defined, use the order defined byDefaultAwsRegionProviderChain.

Please note that the pull request contains two commits. The second one doesn't bring any new features and is optional as it just generalizes the way AWS clients are created.

Copy link
Owner

@davidmoten davidmoten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @RoKish for another contribution. I'm happy with it except for the method of enabling default provider chains. Leaving out the <region> element is too cryptic for me. I think it should be enabled by a <useDefaultProviderChain> (or whatever name) true or false element that can exist with the other elements but when is true is preferred. Can you do that?

@RoKish
Copy link
Contributor Author

RoKish commented Apr 27, 2020

@davidmoten Thank you for the feedback. I just want to make sure that we're on the same page regarding the way it's enabled.

If region is present, we just use it, otherwise, the default region provider chain is used. I find it quite logical as it works the same way in AWS SDK (i.e. if you don't provide a region while building a client, the default provider chain is used). It doesn't affect the authentication configuration and it's backward compatible as all existing clients have region specified.

The authentication configuration works the same. If serverId or awsAccessKey and awsSecretAccessKey are defined, we just use them (the priority order is the same as it used to be). Otherwise, we use the default credentials provider. It's the same pattern that is currently in use (i.e. there's no property like useMavenSettingsAuthentication, if serverId is defined, it just overrides the credentials provided by awsAccessKey and awsSecretAccessKey).

Please note that if we stick to the approach you suggested, 2 additional properties will be required: useDefaultRegionProvider and useDefaultCredentialsProvider. I think it makes configuration more complex because of the number of properties and the fact that the properties actually depend on each other (i. e. you always update useDefaultRegionProvider and region together, updating just one of them just doesn't make sense).

@davidmoten
Copy link
Owner

@RoKish thanks for the clarification. I didn't pay enough attention to your original explanation. I'm fine with it as is then.

@davidmoten davidmoten merged commit 98ab1c5 into davidmoten:master Apr 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants