aws / aws-sdk-go-v2

AWS SDK for the Go programming language.
https://aws.github.io/aws-sdk-go-v2/docs/
Apache License 2.0
2.68k stars 651 forks source link

provide a region hint of "us-west-2" in GetBucketRegion example #2716

Closed kaovilai closed 4 months ago

kaovilai commented 4 months ago

For changes to files under the /codegen/aws-models folder, and manual edits to autogenerated code (e.g. /service/s3/api.go) please create an Issue instead of a PR for those type of changes.

Related work: https://github.com/vmware-tanzu/velero-plugin-for-aws/pull/210

If the PR addresses an existing bug or feature, please reference it here.

To help speed up the process and reduce the time to merge please ensure that Allow edits by maintainers is checked before submitting your PR. This will allow the project maintainers to make minor adjustments or improvements to the submitted PR, allow us to reduce the roundtrip time for merging your request.

kaovilai commented 4 months ago

The anonymous credentials was added because if you don't have a specified ~/.aws/credentials, this call would fail with the current example prior to this PR.

Error only went away once a credential provider is defined when loading config. If you say it's needed for non-public buckets, I can make a note in the PR. WDYT?

Your feedback above will also help inform my patch elsewhere referenced above.

kaovilai commented 4 months ago

// The request will not be signed, and will not use your AWS credentials.

That's going to cause any requests against non-public buckets to fail here.

So I guess creds could be used on a needed basis then?

lucix-aws commented 4 months ago

I think it entirely depends on the bucket. According to HeadBucket docs, ("authentication and authorization" section) normal auth rules apply for HeadBucket, i.e. unauthorized operations will only work against non-public buckets.

I did notice the docs on our GetBucketRegion say "this will be anonymous, we won't sign the request" which is definitely wrong off the cuff. That should probably be removed.

What context are you using this in? I saw the PR downstream but don't really have any idea what that product is or what it's doing. I assume it's failing without anonymous config because the environment it's running in isn't configured with AWS credentials, but I haven't seen the underlying error. Not sure if that's intended or not. Are the buckets you're trying to query against public?

kaovilai commented 4 months ago

So I have two downstream PRs

I need to dynamically do config depending on env I guess. If in credentialess unit tests, anonymous.. otherwise do not put in anonymous credentials.

lucix-aws commented 4 months ago

That makes sense to me.

So in terms of this PR:

kaovilai commented 4 months ago

Corrected notes on credentials using notes from https://github.com/kaovilai/aws-sdk-go-v2/blob/f7a89262286d7ca7f2307a2df4214d0993d19f5d/aws/credentials.go#L54-L61