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

s3: add DisableDualstack to config #112

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

narqo
Copy link

@narqo narqo commented Mar 11, 2024

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

Relates to grafana/mimir#7382.

Due to internal policies, enterprises deploy VPC Interface Endpoints with Private DNS option, which overrides s3.region.amazonaws.com, pointing it to the local endpoint. Because S3 VPC Endpoints do not support dualstack, this adds extra requirements to the infrastructure (refer to minio/minio-go#1766 (comment) for details).

This PR adds a new configuration to s3 client, allowing to disable the client from using AWS S3 dual-stack endpoints.

@@ -1,6 +1,6 @@
module github.com/thanos-io/objstore

go 1.20
go 1.21
Copy link
Author

Choose a reason for hiding this comment

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

Note, this one comes from latest minio-go/v7, which is not ideal, but shouldn't be a big deal

go: github.com/minio/minio-go/v7@v7.0.69 requires go@1.21, but 1.20 is requested

Copy link
Contributor

@matej-g matej-g left a comment

Choose a reason for hiding this comment

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

Thanks for this @narqo! Looks good to me, but it seems like we'll also need to bump the Go version in our setup. Would you mind bumping the version in the .go-version file to ensure also our CI passes?

@narqo narqo requested a review from matej-g March 26, 2024 12:41
Signed-off-by: Vladimir Varankin <vladimir@varank.in>
Signed-off-by: Vladimir Varankin <vladimir@varank.in>
@narqo
Copy link
Author

narqo commented Apr 26, 2024

@matej-g could you have a glance over the latest changes. Is there anything I can do to help with landing them to mainline?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants