S3 SignatureV4 Without Config File Credentials

[midnightconman]

Changes

This PR fixes a bug in s3 config implementation, which allows the use of SignatureV4 without requiring s3 access_key or secret_key to be in storage object config. This allows the use of environment variables, aws credentials file, etc.

Verification

I tested this change by building locally and verifying SignatureV4 was used if config.SignatureV2 was false.

[jojohappy]

I think it is not necessary, the signature is used by static credentials only. And by default, the following credential providers of minio provided will use SignatureV4.

• EnvAWS
• FileAWSCredentials
• IAM

[midnightconman]

Maybe in theory, but unfortunately not in practice. Using v0.2.1 with the following bucket.yaml and environment variables (AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY), in AWS:

bucket.yaml

type: S3
config:
  bucket: my-secret-bucket-name
  endpoint: s3.us-west-2.amazonaws.com
  insecure: false
  signature_version2: false
  encrypt_sse: false

fails with the following error:

level=info ts=2019-01-15T19:38:03.422651711Z caller=flags.go:75 msg="gossip is disabled"
level=info ts=2019-01-15T19:38:03.422720714Z caller=factory.go:39 msg="loading bucket configuration"
store command failed: bucket store initial sync: sync block: iter: Access Denied

With this patch it works correctly.

[bwplotka]

Hm.. I think I know what is happening (:

So @jojohappy is right here, nothing really changed by moving signature outside of if config.AccessKey body.

I think this patch works for you because recently we fixed S3 auth order: #732 This was after 0.2.1 was released (: I think you accidenly assumed your patch work, but master most likely works for you as well (:

[midnightconman]

Tests are taking too long... on my desktop (with xeon CPU), tests are almost taking 15 mins:

real    14m37.457s
user    1m27.843s
sys     0m29.667s

[bwplotka]

Yup, something is wrong with tests against:

github.com/improbable-eng/thanos/pkg/store 600.006s

We are looking on that: #740

[bwplotka]

Why this is needed?

[bwplotka]

Hm.. I think I know what is happening (:

So @jojohappy is right here, nothing really changed by moving signature outside of if config.AccessKey body.

I think this patch works for you because recently we fixed S3 auth order: #732 This was after 0.2.1 was released (: I think you accidenly assumed your patch work, but master most likely works for you as well (:

[midnightconman]

You guys are right. 😄

I'll go ahead and close this PR. Thank you!