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

Add a pure aws-sdk-go auth #4667

Merged
merged 6 commits into from
Feb 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re
- [#4974](https://github.com/thanos-io/thanos/pull/4974) Store: Support tls_config configuration for connecting with Azure storage.
- [#4999](https://github.com/thanos-io/thanos/pull/4999) COS: Support `endpoint` configuration for vpc internal endpoint.
- [#5059](https://github.com/thanos-io/thanos/pull/5059) Compactor: Adding minimum retention flag validation for downsampling retention.
- [#4667](https://github.com/thanos-io/thanos/pull/4667) Add a pure aws-sdk auth for s3 storage.
- [#5111](https://github.com/thanos-io/thanos/pull/5111) Add matcher support to Query Rules endpoint.

### Fixed
Expand Down
3 changes: 3 additions & 0 deletions docs/storage.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ config:
bucket: ""
endpoint: ""
region: ""
aws_sdk_auth: false
access_key: ""
insecure: false
signature_version2: false
Expand Down Expand Up @@ -99,6 +100,8 @@ config:

At a minimum, you will need to provide a value for the `bucket`, `endpoint`, `access_key`, and `secret_key` keys. The rest of the keys are optional.

However if you set `aws_sdk_auth: true` Thanos will use the default authentication methods of the AWS SDK for go based on [known environment variables](https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-envvars.html) (`AWS_PROFILE`, `AWS_WEB_IDENTITY_TOKEN_FILE` ... etc) and known AWS config files (~/.aws/config). If you turn this on, then the `bucket` and `endpoint` are the required config keys.
sylr marked this conversation as resolved.
Show resolved Hide resolved

The AWS region to endpoint mapping can be found in this [link](https://docs.aws.amazon.com/general/latest/gr/s3.html).

Make sure you use a correct signature version. Currently AWS requires signature v4, so it needs `signature_version2: false`. If you don't specify it, you will get an `Access Denied` error. On the other hand, several S3 compatible APIs use `signature_version2: true`.
Expand Down
11 changes: 11 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ require (
github.com/alecthomas/units v0.0.0-20210927113745-59d0afb8317a
github.com/alicebob/miniredis/v2 v2.14.3
github.com/aliyun/aliyun-oss-go-sdk v2.0.4+incompatible
github.com/aws/aws-sdk-go-v2 v1.13.0
github.com/aws/aws-sdk-go-v2/config v1.13.1
github.com/baidubce/bce-sdk-go v0.9.81
github.com/blang/semver/v4 v4.0.0
github.com/bradfitz/gomemcache v0.0.0-20190913173617-a41fca850d0b
Expand Down Expand Up @@ -105,6 +107,15 @@ require (
github.com/armon/go-radix v1.0.0 // indirect
github.com/asaskevich/govalidator v0.0.0-20200907205600-7a23bdc65eef // indirect
github.com/aws/aws-sdk-go v1.42.8 // indirect
github.com/aws/aws-sdk-go-v2/credentials v1.8.0 // indirect
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.10.0 // indirect
github.com/aws/aws-sdk-go-v2/internal/configsources v1.1.4 // indirect
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.2.0 // indirect
github.com/aws/aws-sdk-go-v2/internal/ini v1.3.5 // indirect
github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.7.0 // indirect
github.com/aws/aws-sdk-go-v2/service/sso v1.9.0 // indirect
github.com/aws/aws-sdk-go-v2/service/sts v1.14.0 // indirect
github.com/aws/smithy-go v1.10.0 // indirect
github.com/beorn7/perks v1.0.1 // indirect
github.com/census-instrumentation/opencensus-proto v0.2.1 // indirect
github.com/cncf/udpa/go v0.0.0-20201120205902-5459f2c99403 // indirect
Expand Down
22 changes: 22 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,30 @@ github.com/aws/aws-sdk-go v1.42.8 h1:Tj2RP4Fas1mYchwbmw0qWLJIEATAseyp5iTa1D+LWYQ
github.com/aws/aws-sdk-go v1.42.8/go.mod h1:585smgzpB/KqRA+K3y/NL/oYRqQvpNJYvLm+LY1U59Q=
github.com/aws/aws-sdk-go-v2 v0.18.0/go.mod h1:JWVYvqSMppoMJC0x5wdwiImzgXTI9FuZwxzkQq9wy+g=
github.com/aws/aws-sdk-go-v2 v1.9.1/go.mod h1:cK/D0BBs0b/oWPIcX/Z/obahJK1TT7IPVjy53i/mX/4=
github.com/aws/aws-sdk-go-v2 v1.13.0 h1:1XIXAfxsEmbhbj5ry3D3vX+6ZcUYvIqSm4CWWEuGZCA=
github.com/aws/aws-sdk-go-v2 v1.13.0/go.mod h1:L6+ZpqHaLbAaxsqV0L4cvxZY7QupWJB4fhkf8LXvC7w=
github.com/aws/aws-sdk-go-v2/config v1.13.1 h1:yLv8bfNoT4r+UvUKQKqRtdnvuWGMK5a82l4ru9Jvnuo=
github.com/aws/aws-sdk-go-v2/config v1.13.1/go.mod h1:Ba5Z4yL/UGbjQUzsiaN378YobhFo0MLfueXGiOsYtEs=
github.com/aws/aws-sdk-go-v2/credentials v1.8.0 h1:8Ow0WcyDesGNL0No11jcgb1JAtE+WtubqXjgxau+S0o=
github.com/aws/aws-sdk-go-v2/credentials v1.8.0/go.mod h1:gnMo58Vwx3Mu7hj1wpcG8DI0s57c9o42UQ6wgTQT5to=
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.10.0 h1:NITDuUZO34mqtOwFWZiXo7yAHj7kf+XPE+EiKuCBNUI=
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.10.0/go.mod h1:I6/fHT/fH460v09eg2gVrd8B/IqskhNdpcLH0WNO3QI=
github.com/aws/aws-sdk-go-v2/internal/configsources v1.1.4 h1:CRiQJ4E2RhfDdqbie1ZYDo8QtIo75Mk7oTdJSfwJTMQ=
github.com/aws/aws-sdk-go-v2/internal/configsources v1.1.4/go.mod h1:XHgQ7Hz2WY2GAn//UXHofLfPXWh+s62MbMOijrg12Lw=
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.2.0 h1:3ADoioDMOtF4uiK59vCpplpCwugEU+v4ZFD29jDL3RQ=
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.2.0/go.mod h1:BsCSJHx5DnDXIrOcqB8KN1/B+hXLG/bi4Y6Vjcx/x9E=
github.com/aws/aws-sdk-go-v2/internal/ini v1.3.5 h1:ixotxbfTCFpqbuwFv/RcZwyzhkxPSYDYEMcj4niB5Uk=
github.com/aws/aws-sdk-go-v2/internal/ini v1.3.5/go.mod h1:R3sWUqPcfXSiF/LSFJhjyJmpg9uV6yP2yv3YZZjldVI=
github.com/aws/aws-sdk-go-v2/service/cloudwatch v1.8.1/go.mod h1:CM+19rL1+4dFWnOQKwDc7H1KwXTz+h61oUSHyhV0b3o=
github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.7.0 h1:4QAOB3KrvI1ApJK14sliGr3Ie2pjyvNypn/lfzDHfUw=
github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.7.0/go.mod h1:K/qPe6AP2TGYv4l6n7c88zh9jWBDf6nHhvg1fx/EWfU=
github.com/aws/aws-sdk-go-v2/service/sso v1.9.0 h1:1qLJeQGBmNQW3mBNzK2CFmrQNmoXWrscPqsrAaU1aTA=
github.com/aws/aws-sdk-go-v2/service/sso v1.9.0/go.mod h1:vCV4glupK3tR7pw7ks7Y4jYRL86VvxS+g5qk04YeWrU=
github.com/aws/aws-sdk-go-v2/service/sts v1.14.0 h1:ksiDXhvNYg0D2/UFkLejsaz3LqpW5yjNQ8Nx9Sn2c0E=
github.com/aws/aws-sdk-go-v2/service/sts v1.14.0/go.mod h1:u0xMJKDvvfocRjiozsoZglVNXRG19043xzp3r2ivLIk=
github.com/aws/smithy-go v1.8.0/go.mod h1:SObp3lf9smib00L/v3U2eAKG8FyQ7iLrJnQiAmR5n+E=
github.com/aws/smithy-go v1.10.0 h1:gsoZQMNHnX+PaghNw4ynPsyGP7aUCqx5sY2dlPQsZ0w=
github.com/aws/smithy-go v1.10.0/go.mod h1:SObp3lf9smib00L/v3U2eAKG8FyQ7iLrJnQiAmR5n+E=
github.com/baidubce/bce-sdk-go v0.9.81 h1:n8KfThLG9fvGv3A+RtTt/jKhg/FPPRpo+iNnS2r+iPI=
github.com/baidubce/bce-sdk-go v0.9.81/go.mod h1:zbYJMQwE4IZuyrJiFO8tO8NbtYiKTFTbwh4eIsqjVdg=
github.com/baiyubin/aliyun-sts-go-sdk v0.0.0-20180326062324-cfa1a18b161f h1:ZNv7On9kyUzm7fvRZumSyy/IUiSC7AzL0I1jKKtwooA=
Expand Down
12 changes: 11 additions & 1 deletion pkg/objstore/s3/s3.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ type Config struct {
Bucket string `yaml:"bucket"`
Endpoint string `yaml:"endpoint"`
Region string `yaml:"region"`
AWSSDKAuth bool `yaml:"aws_sdk_auth"`
AccessKey string `yaml:"access_key"`
Insecure bool `yaml:"insecure"`
SignatureV2 bool `yaml:"signature_version2"`
Expand Down Expand Up @@ -224,7 +225,12 @@ func NewBucketWithConfig(logger log.Logger, config Config, component string) (*B
if err := validate(config); err != nil {
return nil, err
}
if config.AccessKey != "" {

if config.AWSSDKAuth {
chain = []credentials.Provider{
wrapCredentialsProvider(&AWSSDKAuth{Region: config.Region}),
}
} else if config.AccessKey != "" {
sylr marked this conversation as resolved.
Show resolved Hide resolved
chain = []credentials.Provider{wrapCredentialsProvider(&credentials.Static{
Value: credentials.Value{
AccessKeyID: config.AccessKey,
Expand Down Expand Up @@ -336,6 +342,10 @@ func validate(conf Config) error {
return errors.New("no s3 endpoint in config file")
}

if conf.AWSSDKAuth && conf.AccessKey != "" {
return errors.New("aws_sdk_auth and access_key are mutually exclusive configurations")
}

if conf.AccessKey == "" && conf.SecretKey != "" {
return errors.New("no s3 access_key specified while secret_key is present in config file; either both should be present in config or envvars/IAM should be used.")
}
Expand Down
54 changes: 54 additions & 0 deletions pkg/objstore/s3/s3_aws_sdk_auth.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// Copyright (c) The Thanos Authors.
// Licensed under the Apache License 2.0.

package s3

import (
"context"

aws "github.com/aws/aws-sdk-go-v2/aws"
awsconfig "github.com/aws/aws-sdk-go-v2/config"
"github.com/minio/minio-go/v7/pkg/credentials"
"github.com/pkg/errors"
)

// AWSSDKAuth retrieves credentials from the aws-sdk-go.
type AWSSDKAuth struct {
Region string
creds aws.Credentials
}

// NewAWSSDKAuth returns a pointer to a new Credentials object
// wrapping the environment variable provider.
func NewAWSSDKAuth(region string) *credentials.Credentials {
return credentials.New(&AWSSDKAuth{
Copy link
Member

Choose a reason for hiding this comment

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

Is implementation that complex that is worth to import the aws-sdk-go?

Essentially we need to compare the maintenance work.

  • Is it better for us to copy (potentially tiny) code and maintain it? As far as I understand this, it's just taking correct environment variables, that's it?
  • Is it better to contribute such logic to minio-go?
  • Is it better to import whole aws-sdk-go (dependency hell)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used aws-sdk-go-v2 which contrary to the first aws-sdk-go has split modules for every services so we should import only what is required for the features we need.

It does much more than taking env vars.

It also leverage the well known AWS config files allowing to automatically assume roles based on other roles via the STS API. All that logic is well and consistently implemented across all AWS SDKs so I see no reason we should have our own implementation.

Copy link
Contributor

@V3ckt0r V3ckt0r Jan 13, 2022

Choose a reason for hiding this comment

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

here is the LoadDefaultConfig credential chain we get by importing this.

1. Environment variables.
1.1 Static Credentials (AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, AWS_SESSION_TOKEN)
1.2 Web Identity Token (AWS_WEB_IDENTITY_TOKEN_FILE)
2. Shared configuration files.
2.1 SDK defaults to credentials file under .aws folder that is placed in the home folder on your computer.
2.2 SDK defaults to config file under .aws folder that is placed in the home folder on your computer.
3. If your application uses an ECS task definition or RunTask API operation, IAM role for tasks.
4. If your application is running on an Amazon EC2 instance, IAM role for Amazon EC2.

Region: region,
})
}

// Retrieve retrieves the keys from the environment.
func (a *AWSSDKAuth) Retrieve() (credentials.Value, error) {
cfg, err := awsconfig.LoadDefaultConfig(context.TODO(), awsconfig.WithRegion(a.Region))
if err != nil {
return credentials.Value{}, errors.Wrap(err, "load AWS SDK config")
}

creds, err := cfg.Credentials.Retrieve(context.TODO())
if err != nil {
return credentials.Value{}, errors.Wrap(err, "retrieve AWS SDK credentials")
}

a.creds = creds

return credentials.Value{
AccessKeyID: creds.AccessKeyID,
SecretAccessKey: creds.SecretAccessKey,
SessionToken: creds.SessionToken,
SignerType: credentials.SignatureV4,
}, nil
}

// IsExpired returns if the credentials have been retrieved.
func (a *AWSSDKAuth) IsExpired() bool {
return a.creds.Expired()
}