Add a pure aws-sdk-go auth #4667

sylr · 2021-09-16T15:26:31Z

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

Changes

Added an authentication method for S3 which uses the AWS SDK for go.

The AWS SDK handles more authentication methods than the minio-go lib, e.g:

/etc/thanos/thanos-storage.yml

type: S3
config:
  bucket: "thanos"
  endpoint: "s3.eu-central-1.amazonaws.com"
  aws_sdk_auth: true
  region: "eu-central-1"
  access_key: ""
  insecure: false
  signature_version2: false
  secret_key: ""
  sse_config:
    type: "SSE-KMS"
    kms_key_id: "arn:aws:kms:eu-central-1:99999999999:key/<uuid>"
    kms_encryption_context: {}
    encryption_key: ""

/etc/thanos/aws-config.ini

[profile prometheus]
web_identity_token_file = /var/run/secrets/eks.amazonaws.com/serviceaccount/token
role_arn = arn:aws:iam::111111111111111:role/EKSPrometheusRole

[profile thanos]
source_profile = prometheus
role_arn = arn:aws:iam::99999999999:role/EKSThanosRole

Env vars:

AWS_CONFIG_FILE=/etc/thanos/aws-config.ini
AWS_PROFILE=thanos
AWS_ARN_ROLE=arn:aws:iam::99999999999:role/EKSThanosRole

Here the thanos sidecar running alongside the prometheus container has an AWS session provided via a web identity token file (EKS IRSA feature) and the config file make it so that the thanos profile is configured to use the session from the prometheus profile to assume a role in a different account.

This allows to have cross AWS accounts connectivity without the need for access and secret keys (which are baaaaaad).

Verification

sylr · 2021-11-04T17:10:10Z

@bwplotka could you please review this ?

bwplotka

Nice, thanks! Well implemented in, but I am really curious if it's complex enough to import whole new complex module 🤔 I would add issue on minio-go too, so they are aware of this gap.

pkg/objstore/s3/s3_aws_sdk_auth.go

bwplotka · 2021-11-15T11:50:46Z

pkg/objstore/s3/s3_aws_sdk_auth.go

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


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)

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.

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.

pkg/objstore/s3/s3.go

sylr · 2021-11-29T09:06:35Z

@bwplotka I'm about to fix the conflict, could we merge this before cutting 0.24.0-rc0 ?

sylr · 2021-12-08T15:46:22Z

@bwplotka ?

wiardvanrij

I'm sort of confident that if you don't use an access_key / secret_key and leave everything empty, you can just use it 'as is'. For example we use KIAM roles that places the AWS env vars and this just works out of the box. Without the need for the AWS SDK.

It feels this is an implementation for something that already works? I could however be completely missing a feature or point :)

If this is actually required to work with for example assumed roles (so you don't need KIAM), etc. I think it's actually worth to implement it.

docs/storage.md

sylr · 2021-12-22T08:08:08Z

The purpose of this is indeed to allow Thanos to work with assumed roles based on all the mechanisms implemented in the AWS SDK.

sylr · 2022-01-08T11:38:46Z

Could we please merge this?

GiedriusS · 2022-01-13T14:04:51Z

Added to my TODO list for reviewing

V3ckt0r · 2022-01-13T14:22:29Z

+1 for this. I have this exact usecase as @sylr

This allows to have cross AWS accounts connectivity without the need for access and secret keys (which are baaaaaad).

GiedriusS

Looks good 👍 the logic sounds good to me in https://github.com/thanos-io/thanos/pull/4667/files#r749424917 🤔

GiedriusS · 2022-02-03T08:39:08Z

Could you please rebase on main and add the DCO?

Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>

Signed-off-by: Giedrius Statkevičius <giedriuswork@gmail.com>

docs/storage.md

Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>

sylr · 2022-02-07T10:45:38Z

Is everything good for you @GiedriusS ?

GiedriusS

LGTM

* Add a pure aws-sdk-go auth Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr> * Upgrade github.com/aws/aws-sdk-go-v2 Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr> * Update CHANGELOG.md Signed-off-by: Giedrius Statkevičius <giedriuswork@gmail.com> * Move s3 conf test in validate() Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr> Co-authored-by: Giedrius Statkevičius <giedriuswork@gmail.com> Signed-off-by: Nicholaswang <wzhever@gmail.com>

sylr force-pushed the aws-sdk-auth branch from 02153ba to ed6210a Compare September 16, 2021 15:28

sylr marked this pull request as draft September 16, 2021 15:42

sylr marked this pull request as ready for review September 17, 2021 10:04

sylr mentioned this pull request Sep 21, 2021

#4657 broke query -> sidecar communication #4688

Closed

sylr force-pushed the aws-sdk-auth branch 2 times, most recently from ddd4d2b to 1f874e6 Compare September 23, 2021 15:30

sylr force-pushed the aws-sdk-auth branch from d76b30e to d8c781e Compare October 22, 2021 13:08

bwplotka reviewed Nov 15, 2021

View reviewed changes

sylr force-pushed the aws-sdk-auth branch from fbdfe9f to a2c2012 Compare November 15, 2021 15:52

sylr requested a review from bwplotka December 14, 2021 14:29

wiardvanrij reviewed Dec 21, 2021

View reviewed changes

docs/storage.md Outdated Show resolved Hide resolved

pull-request-size bot added the size/M label Dec 21, 2021

sylr force-pushed the aws-sdk-auth branch from 96a0dda to 65e0139 Compare December 22, 2021 08:04

GiedriusS previously approved these changes Feb 3, 2022

View reviewed changes

Add a pure aws-sdk-go auth

41ae41a

Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>

sylr dismissed GiedriusS’s stale review via 41ae41a February 3, 2022 09:05

sylr force-pushed the aws-sdk-auth branch from 65e0139 to 41ae41a Compare February 3, 2022 09:05

sylr and others added 2 commits February 3, 2022 10:08

Upgrade github.com/aws/aws-sdk-go-v2

3b669dc

Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>

Update CHANGELOG.md

6b6a39f

Signed-off-by: Giedrius Statkevičius <giedriuswork@gmail.com>

GiedriusS reviewed Feb 4, 2022

View reviewed changes

docs/storage.md Show resolved Hide resolved

sylr added 2 commits February 4, 2022 15:55

Move s3 conf test in validate()

7746efd

Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>

Merge branch 'aws-sdk-auth' of github.com:sylr/thanos into aws-sdk-auth

4cac125

Merge remote-tracking branch 'origin/main' into aws-sdk-auth

64a22c9

GiedriusS approved these changes Feb 7, 2022

View reviewed changes

GiedriusS merged commit dffe6ed into thanos-io:main Feb 7, 2022

pjastrzabek mentioned this pull request May 12, 2022

store: Thanos store does not support reloading of configuration #4855

Closed

alvinlin123 mentioned this pull request Oct 6, 2022

Enable AWS SDK for go authentication to fix IMDSv1 issue cortexproject/cortex#4897

Merged

3 tasks

aljoshare mentioned this pull request Aug 1, 2023

Feat/support aws sdk default credential provider chain grafana/mimir#5636

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a pure aws-sdk-go auth #4667

Add a pure aws-sdk-go auth #4667

sylr commented Sep 16, 2021 •

edited

Loading

sylr commented Nov 4, 2021

bwplotka left a comment

bwplotka Nov 15, 2021

sylr Nov 15, 2021

V3ckt0r Jan 13, 2022 •

edited

Loading

sylr commented Nov 29, 2021

sylr commented Dec 8, 2021

wiardvanrij left a comment •

edited

Loading

sylr commented Dec 22, 2021

sylr commented Jan 8, 2022

GiedriusS commented Jan 13, 2022

V3ckt0r commented Jan 13, 2022

GiedriusS left a comment

GiedriusS commented Feb 3, 2022

sylr commented Feb 7, 2022

GiedriusS left a comment

Add a pure aws-sdk-go auth #4667

Add a pure aws-sdk-go auth #4667

Conversation

sylr commented Sep 16, 2021 • edited Loading

Changes

Verification

sylr commented Nov 4, 2021

bwplotka left a comment

Choose a reason for hiding this comment

bwplotka Nov 15, 2021

Choose a reason for hiding this comment

sylr Nov 15, 2021

Choose a reason for hiding this comment

V3ckt0r Jan 13, 2022 • edited Loading

Choose a reason for hiding this comment

sylr commented Nov 29, 2021

sylr commented Dec 8, 2021

wiardvanrij left a comment • edited Loading

Choose a reason for hiding this comment

sylr commented Dec 22, 2021

sylr commented Jan 8, 2022

GiedriusS commented Jan 13, 2022

V3ckt0r commented Jan 13, 2022

GiedriusS left a comment

Choose a reason for hiding this comment

GiedriusS commented Feb 3, 2022

sylr commented Feb 7, 2022

GiedriusS left a comment

Choose a reason for hiding this comment

sylr commented Sep 16, 2021 •

edited

Loading

V3ckt0r Jan 13, 2022 •

edited

Loading

wiardvanrij left a comment •

edited

Loading