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.

Sign up for GitHub

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

Jump to bottom

S3: Add config option to enforce the minio DNS lookup #5409

Merged
merged 2 commits into from Jun 20, 2022
Merged

S3: Add config option to enforce the minio DNS lookup #5409

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
13bc688
Add config option to enforce the minio DNS lookup
Jakob3xD Jun 8, 2022
46baaa4
Useenums instead of boolean for bucket_lookup_type
Jakob3xD Jun 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -17,6 +17,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re
### Added

- [#5337](https://github.com/thanos-io/thanos/pull/5337) Thanos Object Store: Add the `prefix` option to buckets
- [#5409](https://github.com/thanos-io/thanos/pull/5409) S3: Add option to force DNS style lookup.
- [#5352](https://github.com/thanos-io/thanos/pull/5352) Cache: Add cache metrics to groupcache.
- [#5391](https://github.com/thanos-io/thanos/pull/5391) Receive: Add relabeling support.

Expand Down
3 changes: 3 additions & 0 deletions docs/storage.md
View file
Open in desktop
Expand Up @@ -89,6 +89,7 @@ config:
trace:
enable: false
list_objects_version: ""
dns_style: false
Copy link
Member

Choose a reason for hiding this comment

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

This makes sense, just can we:

  • Find a more specific name?
  • Use enums instead of boolean - it will "scale" better if there will be a 3rd option.

For example:

Suggested change
dns_style: false
dns_style: "virtual-hosted" # Or `"path" read more about it here https://docs.aws.amazon.com/AmazonS3/latest/userguide/VirtualHosting.html

Copy link
Contributor Author

@Jakob3xD Jakob3xD Jun 16, 2022 •
edited

Choose a reason for hiding this comment

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

As requested we changed the name and are using enums.
If this is okay, I can rebase it if needed to a single commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The CI/test are fixed now.

part_size: 67108864
sse_config:
type: ""
Expand Down Expand Up @@ -119,6 +120,8 @@ Set `list_objects_version: "v1"` for S3 compatible APIs that don't support ListO

`http_config.tls_config` allows configuring TLS connections. Please refer to the document of [tls_config](https://prometheus.io/docs/prometheus/latest/configuration/configuration/#tls_config) for detailed information on what each option does.

`dns_style` can be set to enforce the DNS style lookup.

For debug and testing purposes you can set

* `insecure: true` to switch to plain insecure HTTP instead of HTTPS
Expand Down
16 changes: 12 additions & 4 deletions pkg/objstore/s3/s3.go
View file
Open in desktop
Expand Up @@ -83,6 +83,7 @@ type Config struct {
HTTPConfig HTTPConfig `yaml:"http_config"`
TraceConfig TraceConfig `yaml:"trace"`
ListObjectsVersion string `yaml:"list_objects_version"`
DNSStyle bool `yaml:"dns_style"`
// PartSize used for multipart upload. Only used if uploaded object size is known and larger than configured PartSize.
// NOTE we need to make sure this number does not produce more parts than 10 000.
PartSize uint64 `yaml:"part_size"`
Expand Down Expand Up @@ -263,12 +264,19 @@ func NewBucketWithConfig(logger log.Logger, config Config, component string) (*B
return nil, err
}
}
var lookup minio.BucketLookupType
if config.DNSStyle {
lookup = minio.BucketLookupDNS
} else {
lookup = minio.BucketLookupAuto
}

client, err := minio.New(config.Endpoint, &minio.Options{
Creds: credentials.NewChainCredentials(chain),
Secure: !config.Insecure,
Region: config.Region,
Transport: rt,
Creds: credentials.NewChainCredentials(chain),
Secure: !config.Insecure,
Region: config.Region,
Transport: rt,
BucketLookup: lookup,
})
if err != nil {
return nil, errors.Wrap(err, "initialize s3 client")
Expand Down