WhoopInc / vagrant-s3auth

Vagrant plugin for private, versioned boxes on Amazon S3.
MIT License
108 stars 23 forks source link

Don't break VagrantFiles that don't require S3 #17

Closed shawnbutts closed 9 years ago

shawnbutts commented 9 years ago

I have one Vagrantfile that uses this plugin for S3 access and it work's perfectly via the "ENV['AWS_PROFILE']" option.

However, Vagrant files that download via any other means are broken as this plugin gives the following error.

Unable to read AWS credentials from the environment.

Ensure the following variables are set in your environment, or set them at the top of your Vagrantfile:

AWS_ACCESS_KEY_ID
AWS_SECRET_ACCESS_KEY

This plugin should not disable Vagrantfiles that do not require this plugin.

At the very least, there should be a way to disable the plugin globally (like https://github.com/dotless-de/vagrant-vbguest#global-configuration).

Thank you

benesch commented 9 years ago

Wow, good point.

benesch commented 9 years ago

I'm going to call this a rough duplicate of #10. If

  1. you've installed vagrant-s3auth,
  2. you're installing a box from an S3 URL, and
  3. an initial download without AWS authentication fails,

I think it's perfectly reasonable for this plugin to demand AWS credentials. As you've discovered, currently vagrant-s3auth won't try an initial public download (3); it'll assume any download from S3 needs credentials and complain if they're missing. So a fix for #10 should also fix this without the need for an explicit global config.

Thanks for another reminder that I need to fix #10. I'll try to get a fix out today. I'm closing this, but feel free to reopen if you disagree with my proposal!

benesch commented 9 years ago

This is released as v1.1. Let me know how it goes.

shawnbutts commented 9 years ago

works great. Thanks

shawnbutts commented 9 years ago

I did find an issue with this approach.

If a download from a non-S3 source fails for any reason, the "Unable to find AWS credentials." is output.

In addition, the error is confusing because this not an S3 source and it's not clear in the message that the error is coming from this plugin.