drone-plugins / drone-s3-cache

Caches build artifacts to S3 compatible storage backends
http://plugins.drone.io/drone-plugins/drone-s3-cache
Apache License 2.0
29 stars 30 forks source link

feat: use credentials provider chain #65

Closed kirbyjs closed 3 years ago

kirbyjs commented 3 years ago

Description

kirbyjs commented 3 years ago

@donny-dont This seems to be the way to do it. I'm pretty new to golang tho :smile: But there's a provider change in node.js/java that I'm familiar with and it will go through the chain and try to get credentials from each provider that's defined there.

donny-dont commented 3 years ago

Thanks for the PR!

Now I understand what you're doing here with the chain I'm not a fan of ripping out the error bits. It would be preferable to inform the user on what is going on here especially so they can figure out what's going wrong.

Can you give a bit more info on the file credentials and maybe we can come up with a way to let the user know what's going on?

kirbyjs commented 3 years ago

I'm not a fan of ripping out the error bits

Ah given that this is a plugin, that makes sense.

It just makes it tricky when trying to integrate multiple credentials providers, because it can start to get a little muddy for what provider the user is trying to use.

Can you give a bit more info on the file credentials and maybe we can come up with a way to let the user know what's going on?

This doc shows were the default file location is and what the file looks like. But with FileAWSCredentials you can override the default path and the default profile (which the default is default). You can also set the profile with the env variable AWS_PROFILE.

So maybe we add an option for the aws credentials file location and the profile?

But if it fails, it could be either because the file doesn't exist or the profile doesn't exist in the file. So maybe just a generic error message like "Failed to get AWS credentials from file: $error".

donny-dont commented 3 years ago

Thanks that documentation makes it a lot more clear!

Ok so I don't think we should bother with the chaining bits. I understand what you're trying to do there and if this were just a cli that interacted with s3 I'd be totally with you. In this case we don't want any surprises we want the plugin to do what the user requests which should be a SINGLE mode.

So add a FileCredentials and Profile to the Settings struct in impl.go. During validation you check that the file exists. If it does not then you should error. If the profile is not set then put it as default. There should only be one batch of credentials so if the access key or secret key are set and the file credentials it should error.

That work for you?

donny-dont commented 3 years ago

Thanks @kirbyjs !

kirbyjs commented 3 years ago

@donny-dont np! Any idea when I can expect this to be released?

donny-dont commented 3 years ago

Should already be present in the latest docker image