Use aws.DetectCreds and don't require aws creds in environment

[garethlewin]

No description provided.

[radeksimko]

See #1049

[garethlewin]

#1049 is a much bigger and more complex change that has obviously been taking time. This is a minor change (really 1 line) that should (I am pretty sure) require no changes to people's already existing workflows but will also support other aws workflows. Packer already does this correctly for example.

[radeksimko]

#1049 is a much bigger and more complex change that has obviously been taking time.

Agreed, it's not trivial solution because the problem is not trivial.

This PR on the other hand can break many existing codebases. De-facto all existing module examples are relying on access_key & secret_key being passed statically. All this is for good reasons. You may also end up using different keys/permissions for each module someday.
https://github.com/terraform-community-modules

Don't get me wrong, I'm all with you on making it happen, I just don't want to cause harm to anyone relying on API from the latest release.

A lot has changed since 0.3.7 and the current master is already in backward-incompatible state as it ignores certain ENV vars which were previously covered by goamz.

I believe this needs to be solved somehow until the next release so it's backward-compatible. // correct me if I'm wrong @catsby

I will try and take a look at the bits that need to be done to move #1049 forward during the weekend unless someone else will be quicker.

[garethlewin]

This PR on the other hand can break many existing codebases. De-facto all existing module examples are relying on access_key & secret_key being passed statically

I might be missing something in the code, but it is relying on them being in the environment statically. And since this method will check the environment first, shouldn't it still work for them? That was my goal.

[catsby]

As it stands, this pull request breaks for anyone using credentials in the configuration (or in modules, etc.). Specifying them that was becomes an error, was that intentional?

[garethlewin]

Ok, I misunderstood the options, my mistake thanks @catsby .

So now my change is even simpler, it removes the requirement from the access_key and secret_key variables, but still leaves them in. DetectCreds will call Creds with whatever value it has if they are not nil so this should act exactly the same for existing setups.

If access_key and secret_key are not set and they don't get values from the environment then it should use the rest of the DetectCreds flow.

[garethlewin]

@catsby does my latest change resolve your concerns?

[BRMatt]

It's worth noting that the core team seem to want the access key/secret key to be "required" so that the user will be prompted for them during demos/initial setup.

[catsby]

@garethlewin yes! Though, I unintentionally duplicated this myself yesterday in #1470 😦 I had forgotten about this PR, sorry!

I think too that we need the key/secret to be required too, so we should probably just close this.
Thank you for the contribution! Sorry I robbed you of credit here

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.