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 awsProfile in s3region and s3credentials #57

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ addSbtPlugin("ohnosequences" % "sbt-s3-resolver" % "<version>")
| Key | Type | Default |
|:-----------------|:---------------------------:|:--------------------------------|
| `s3credentials` | [`AWSCredentialsProvider`] | see [below](#credentials) |
| `awsProfile` | `String` | `"default"` |
| `awsProfile` | `Option[String]` | `None` |
| `s3region` | [`Region`] | `DefaultAwsRegionProviderChain` |
| `s3acl` | [`CannedAccessControlList`] | `PublicRead` |
| `s3storageClass` | [`StorageClass`] | `Standard` |
Expand Down Expand Up @@ -115,10 +115,10 @@ i.e. without using this plugin. Or if you're using it anyway, you can write:
The **default credentials** chain in this plugin is

```scala
awsProfile := "default"
awsProfile := Some("default")

s3credentials :=
new ProfileCredentialsProvider(awsProfile.value) |
new ProfileCredentialsProvider(awsProfile.value.orNull) |
new EnvironmentVariableCredentialsProvider() |
new InstanceProfileCredentialsProvider()
```
Expand All @@ -132,14 +132,14 @@ You can find other types of credentials providers in the [AWS Java SDK docs][`AW
If you have [different profiles](http://docs.aws.amazon.com/AWSSdkDocsJava/latest/DeveloperGuide/credentials.html#credentials-file-format) in your `~/.aws/credentials` file, you can choose the one you need by setting

```scala
awsProfile := "bob"
awsProfile := Some("bob")
```

Or if you would like to use profile credentials and have your env vars override if they exist. This is handy if you have both a local dev environment as well as a CI environment where you need to use env vars.

```scala
s3credentials :=
new ProfileCredentialsProvider(awsProfile.value) |
new ProfileCredentialsProvider(awsProfile.value.orNull) |
new EnvironmentVariableCredentialsProvider()
```

Expand Down
17 changes: 11 additions & 6 deletions src/main/scala/SBTS3Resolver.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ package ohnosequences.sbt
import sbt._
import Keys._
import java.util.Optional
import com.amazonaws.auth._, profile._
import com.amazonaws.regions.{ Region, Regions, RegionUtils, AwsRegionProvider }

import com.amazonaws.auth._
import com.amazonaws.auth.profile.ProfileCredentialsProvider
import com.amazonaws.regions._
import com.amazonaws.services.s3.AmazonS3


Expand Down Expand Up @@ -85,7 +87,7 @@ object SbtS3Resolver extends AutoPlugin {
implicit def fromProviderToAWSRegion(provider: AwsRegionProvider): Region = regionFromString(provider.getRegion())

// Adding setting keys
lazy val awsProfile = settingKey[String]("AWS credentials profile")
lazy val awsProfile = settingKey[Option[String]]("AWS credentials profile")
Copy link
Contributor

@laughedelic laughedelic Oct 16, 2017

Choose a reason for hiding this comment

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

Regarding your concern about breaking change: take a look at the trick we did in #55 (comment) (here). You can add an implicit conversion from String to Option[String] and a deprecation message, which explains that you should write explicit Some(...). This way users that didn't use the setting, won't notice the change and those who did will get a warning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added an implicit conversion from String to Option[String] according to acl2Option

lazy val s3credentials = settingKey[AWSCredentialsProvider]("AWS credentials provider to access S3")
lazy val s3region = settingKey[Region]("AWS Region for your S3 resolvers")
lazy val s3overwrite = settingKey[Boolean]("Controls whether publishing resolver can overwrite artifacts")
Expand Down Expand Up @@ -129,12 +131,15 @@ object SbtS3Resolver extends AutoPlugin {

// Default settings
override def projectSettings: Seq[Setting[_]] = Seq(
awsProfile := "default",
awsProfile := None,
s3credentials :=
new ProfileCredentialsProvider(awsProfile.value) |
new ProfileCredentialsProvider(awsProfile.value.orNull) |
new EnvironmentVariableCredentialsProvider() |
InstanceProfileCredentialsProvider.getInstance(),
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to change s3credentials default similarly to the new default for s3region. I think consistency here is more important than preserving compatibility:

  • the default will be mentioned in the readme
  • the change will be mentioned in the release notes
  • I think it won't affect most people, because usually you either rely on the default credentials chain, or set credentials explicitly. Also if you were using awsProfile before, then you also shouldn't notice the change.

So change this, please, in the same way as the s3region.

s3region := new com.amazonaws.regions.DefaultAwsRegionProviderChain(),
s3region := (awsProfile.value match {
case Some(profile) => new AwsProfileRegionProvider(profile)
case _ => new DefaultAwsRegionProviderChain()
}),
s3overwrite := isSnapshot.value,
s3sse := false,
s3acl := Some(com.amazonaws.services.s3.model.CannedAccessControlList.PublicRead),
Expand Down