From 715c9e3b2b9aa16e94d2c0346b791f618fc23de3 Mon Sep 17 00:00:00 2001 From: Keith Duncan Date: Tue, 19 Oct 2021 17:34:56 +1000 Subject: [PATCH 01/12] Migrate from aws-sdk-go to aws-sdk-go-v2 --- s3secrets-helper/go.mod | 8 +++- s3secrets-helper/go.sum | 29 +++++++++++++ s3secrets-helper/s3/s3.go | 90 +++++++++++++++++---------------------- 3 files changed, 76 insertions(+), 51 deletions(-) diff --git a/s3secrets-helper/go.mod b/s3secrets-helper/go.mod index 4baa6e0..0b81719 100644 --- a/s3secrets-helper/go.mod +++ b/s3secrets-helper/go.mod @@ -2,4 +2,10 @@ module github.com/buildkite/elastic-ci-stack-s3-secrets-hooks/s3secrets-helper/v go 1.15 -require github.com/aws/aws-sdk-go v1.35.14 +require ( + github.com/aws/aws-sdk-go-v2 v1.9.2 // indirect + github.com/aws/aws-sdk-go-v2/config v1.8.3 // indirect + github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.5.4 // indirect + github.com/aws/aws-sdk-go-v2/service/s3 v1.16.1 // indirect + github.com/aws/smithy-go v1.8.0 // indirect +) diff --git a/s3secrets-helper/go.sum b/s3secrets-helper/go.sum index 1005132..4888ace 100644 --- a/s3secrets-helper/go.sum +++ b/s3secrets-helper/go.sum @@ -1,7 +1,35 @@ github.com/aws/aws-sdk-go v1.35.14 h1:nucVVXXjAr9UkmYCBWxQWRuYa5KOlaXjuJGg2ulW0K0= github.com/aws/aws-sdk-go v1.35.14/go.mod h1:tlPOdRjfxPBpNIwqDj61rmsnA85v9jc0Ps9+muhnW+k= +github.com/aws/aws-sdk-go-v2 v1.9.2 h1:dUFQcMNZMLON4BOe273pl0filK9RqyQMhCK/6xssL6s= +github.com/aws/aws-sdk-go-v2 v1.9.2/go.mod h1:cK/D0BBs0b/oWPIcX/Z/obahJK1TT7IPVjy53i/mX/4= +github.com/aws/aws-sdk-go-v2/config v1.8.3 h1:o5583X4qUfuRrOGOgmOcDgvr5gJVSu57NK08cWAhIDk= +github.com/aws/aws-sdk-go-v2/config v1.8.3/go.mod h1:4AEiLtAb8kLs7vgw2ZV3p2VZ1+hBavOc84hqxVNpCyw= +github.com/aws/aws-sdk-go-v2/credentials v1.4.3 h1:LTdD5QhK073MpElh9umLLP97wxphkgVC/OjQaEbBwZA= +github.com/aws/aws-sdk-go-v2/credentials v1.4.3/go.mod h1:FNNC6nQZQUuyhq5aE5c7ata8o9e4ECGmS4lAXC7o1mQ= +github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.6.0 h1:9tfxW/icbSu98C2pcNynm5jmDwU3/741F11688B6QnU= +github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.6.0/go.mod h1:gqlclDEZp4aqJOancXK6TN24aKhT0W0Ae9MHk3wzTMM= +github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.5.4 h1:TnU1cY51027j/MQeFy7DIgk1UuzJY+wLFYqXceY/fiE= +github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.5.4/go.mod h1:Ex7XQmbFmgFHrjUX6TN3mApKW5Hglyga+F7wZHTtYhA= +github.com/aws/aws-sdk-go-v2/internal/ini v1.2.4 h1:leSJ6vCqtPpTmBIgE7044B1wql1E4n//McF+mEgNrYg= +github.com/aws/aws-sdk-go-v2/internal/ini v1.2.4/go.mod h1:ZcBrrI3zBKlhGFNYWvju0I3TR93I7YIgAfy82Fh4lcQ= +github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.3.0 h1:gceOysEWNNwLd6cki65IMBZ4WAM0MwgBQq2n7kejoT8= +github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.3.0/go.mod h1:v8ygadNyATSm6elwJ/4gzJwcFhri9RqS8skgHKiwXPU= +github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.3.2 h1:r7jel2aa4d9Duys7wEmWqDd5ebpC9w6Kxu6wIjjp18E= +github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.3.2/go.mod h1:72HRZDLMtmVQiLG2tLfQcaWLCssELvGl+Zf2WVxMmR8= +github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.7.2 h1:RnZjLgtCGLsF2xYYksy0yrx6xPvKG9BYv29VfK4p/J8= +github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.7.2/go.mod h1:np7TMuJNT83O0oDOSF8i4dF3dvGqA6hPYYo6YYkzgRA= +github.com/aws/aws-sdk-go-v2/service/s3 v1.16.1 h1:z+P3r4LrwdudLKBoEVWxIORrk4sVg4/iqpG3+CS53AY= +github.com/aws/aws-sdk-go-v2/service/s3 v1.16.1/go.mod h1:CQe/KvWV1AqRc65KqeJjrLzr5X2ijnFTTVzJW0VBRCI= +github.com/aws/aws-sdk-go-v2/service/sso v1.4.2 h1:pZwkxZbspdqRGzddDB92bkZBoB7lg85sMRE7OqdB3V0= +github.com/aws/aws-sdk-go-v2/service/sso v1.4.2/go.mod h1:NBvT9R1MEF+Ud6ApJKM0G+IkPchKS7p7c2YPKwHmBOk= +github.com/aws/aws-sdk-go-v2/service/sts v1.7.2 h1:ol2Y5DWqnJeKqNd8th7JWzBtqu63xpOfs1Is+n1t8/4= +github.com/aws/aws-sdk-go-v2/service/sts v1.7.2/go.mod h1:8EzeIqfWt2wWT4rJVu3f21TfrhJ8AEMzVybRNSb/b4g= +github.com/aws/smithy-go v1.8.0 h1:AEwwwXQZtUwP5Mz506FeXXrKBe0jA8gVM+1gEcSRooc= +github.com/aws/smithy-go v1.8.0/go.mod h1:SObp3lf9smib00L/v3U2eAKG8FyQ7iLrJnQiAmR5n+E= github.com/buildkite/elastic-ci-stack-s3-secrets-hooks v1.3.0 h1:CRdPqLjYECJY0cgs24E4ZwUJ2qlmcavN7W1jo+5ujnQ= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/jmespath/go-jmespath v0.4.0 h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9YPoQUg= github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHWvzYPziyZiYoo= github.com/jmespath/go-jmespath/internal/testify v1.5.1/go.mod h1:L3OGu8Wl2/fWfCI6z80xFu9LTZmf1ZRjMHUOPmWr69U= @@ -12,5 +40,6 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk golang.org/x/net v0.0.0-20200202094626-16171245cfb2/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= diff --git a/s3secrets-helper/s3/s3.go b/s3secrets-helper/s3/s3.go index f1dd0c6..3218f42 100644 --- a/s3secrets-helper/s3/s3.go +++ b/s3secrets-helper/s3/s3.go @@ -1,61 +1,58 @@ package s3 import ( + "context" + "errors" "log" "io/ioutil" "os" - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" - "github.com/aws/aws-sdk-go/aws/ec2metadata" - "github.com/aws/aws-sdk-go/aws/session" - "github.com/aws/aws-sdk-go/service/s3" - "github.com/aws/aws-sdk-go/service/s3/s3manager" + "github.com/aws/aws-sdk-go-v2/config" + "github.com/aws/aws-sdk-go-v2/feature/s3/manager" + "github.com/aws/aws-sdk-go-v2/service/s3" + "github.com/aws/aws-sdk-go-v2/service/s3/types" + "github.com/aws/smithy-go" "github.com/buildkite/elastic-ci-stack-s3-secrets-hooks/s3secrets-helper/v2/sentinel" ) -const envDefaultRegion = "AWS_DEFAULT_REGION" - type Client struct { - s3 *s3.S3 + s3 *s3.Client bucket string } func New(log *log.Logger, bucket string) (*Client, error) { - sess, err := session.NewSession() + ctx := context.Background() + + // Using the current region (or a guess) find where the bucket lives + managerCfg, err := config.LoadDefaultConfig(ctx, + // TODO confirm this region algorithm is equivalent to before, + // these are not processed in order, and are disjoint configs, region + // vs default region. The default loader considers imds region + // automatically too. + config.WithRegion(os.Getenv("AWS_DEFAULT_REGION")), + config.WithDefaultRegion("us-east-1"), + ) if err != nil { return nil, err } - currentRegion := os.Getenv(envDefaultRegion) - // Discover our executing region using the IMDS - if currentRegion == "" { - idms := ec2metadata.New(sess) - currentRegion, _ = idms.Region() - } - // Fall back to us-east-1 :( - if currentRegion == "" { - currentRegion = "us-east-1" - } - - log.Printf("Discovered current region as %q\n", currentRegion) + log.Printf("Discovered current region as %q\n", managerCfg.Region) - // Using the current region (or a guess) find where the bucket lives - bucketRegion, err := s3manager.GetBucketRegion(aws.BackgroundContext(), sess, bucket, currentRegion) + managerClient := s3.NewFromConfig(managerCfg) + bucketRegion, err := manager.GetBucketRegion(ctx, managerClient, bucket) if err != nil { return nil, err } log.Printf("Discovered bucket region as %q\n", bucketRegion) - sess, err = session.NewSession(&aws.Config{ - Region: &bucketRegion, - }) + s3Cfg, err := config.LoadDefaultConfig(ctx, config.WithRegion(bucketRegion)) if err != nil { return nil, err } + return &Client{ - s3: s3.New(sess), + s3: s3.NewFromConfig(s3Cfg), bucket: bucket, }, nil } @@ -69,23 +66,26 @@ func (c *Client) Bucket() (string) { // sentinel.ErrNotFound and sentinel.ErrForbidden are returned for those cases. // Other errors are returned verbatim. func (c *Client) Get(key string) ([]byte, error) { - out, err := c.s3.GetObject(&s3.GetObjectInput{ + out, err := c.s3.GetObject(context.Background(), &s3.GetObjectInput{ Bucket: &c.bucket, Key: &key, }) if err != nil { - if aerr, ok := err.(awserr.Error); ok { - switch aerr.Code() { - case "NoSuchKey": - return nil, sentinel.ErrNotFound - case "Forbidden": + var noSuchKey *types.NoSuchKey + if errors.As(err, &noSuchKey) { + return nil, sentinel.ErrNotFound + } + + var apiErr smithy.APIError + if errors.As(err, &apiErr) { + code := apiErr.ErrorCode() + // TODO confirm "Forbidden" is a member of the set of values this can return + if code == "Forbidden" { return nil, sentinel.ErrForbidden - default: - return nil, aerr } - } else { - return nil, err } + + return nil, err } defer out.Body.Close() // we probably should return io.Reader or io.ReadCloser rather than []byte, @@ -98,18 +98,8 @@ func (c *Client) Get(key string) ([]byte, error) { // 404 Not Found and 403 Forbidden return false without error. // Other errors result in false with an error. func (c *Client) BucketExists() (bool, error) { - if _, err := c.s3.HeadBucket(&s3.HeadBucketInput{Bucket: &c.bucket}); err != nil { - if aerr, ok := err.(awserr.Error); ok { - switch aerr.Code() { - // https://github.com/aws/aws-sdk-go/issues/2593#issuecomment-491436818 - case "NoSuchBucket", "NotFound": - return false, nil - default: // e.g. NoCredentialProviders, Forbidden - return false, aerr - } - } else { - return false, err - } + if _, err := c.s3.HeadBucket(context.Background(), &s3.HeadBucketInput{Bucket: &c.bucket}); err != nil { + return false, err } return true, nil } From 5f73933bb8c3652fcd4af2357982ee0bf326061c Mon Sep 17 00:00:00 2001 From: Keith Duncan Date: Tue, 19 Oct 2021 17:42:09 +1000 Subject: [PATCH 02/12] Remove comment about region algorithm being equivalent after checking --- s3secrets-helper/s3/s3.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/s3secrets-helper/s3/s3.go b/s3secrets-helper/s3/s3.go index 3218f42..a079551 100644 --- a/s3secrets-helper/s3/s3.go +++ b/s3secrets-helper/s3/s3.go @@ -25,10 +25,6 @@ func New(log *log.Logger, bucket string) (*Client, error) { // Using the current region (or a guess) find where the bucket lives managerCfg, err := config.LoadDefaultConfig(ctx, - // TODO confirm this region algorithm is equivalent to before, - // these are not processed in order, and are disjoint configs, region - // vs default region. The default loader considers imds region - // automatically too. config.WithRegion(os.Getenv("AWS_DEFAULT_REGION")), config.WithDefaultRegion("us-east-1"), ) From bcca48aac6b0e0efef8cc737e4d31470dfe97187 Mon Sep 17 00:00:00 2001 From: Keith Duncan Date: Tue, 19 Oct 2021 17:47:15 +1000 Subject: [PATCH 03/12] Add WithEC2IMDSRegion which is not on by default --- s3secrets-helper/s3/s3.go | 1 + 1 file changed, 1 insertion(+) diff --git a/s3secrets-helper/s3/s3.go b/s3secrets-helper/s3/s3.go index a079551..786ef00 100644 --- a/s3secrets-helper/s3/s3.go +++ b/s3secrets-helper/s3/s3.go @@ -26,6 +26,7 @@ func New(log *log.Logger, bucket string) (*Client, error) { // Using the current region (or a guess) find where the bucket lives managerCfg, err := config.LoadDefaultConfig(ctx, config.WithRegion(os.Getenv("AWS_DEFAULT_REGION")), + config.WithEC2IMDSRegion(), config.WithDefaultRegion("us-east-1"), ) if err != nil { From 4ea9abe3cd9443b4eb61d978b7df9e735c06b938 Mon Sep 17 00:00:00 2001 From: Keith Duncan Date: Tue, 19 Oct 2021 17:49:17 +1000 Subject: [PATCH 04/12] Fix transcription error in errors --- s3secrets-helper/sentinel/errors.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/s3secrets-helper/sentinel/errors.go b/s3secrets-helper/sentinel/errors.go index ccff7d3..8a5396b 100644 --- a/s3secrets-helper/sentinel/errors.go +++ b/s3secrets-helper/sentinel/errors.go @@ -6,8 +6,8 @@ import "errors" var ( // ErrNotFound indicates something was not found - ErrNotFound = errors.New("Forbidden") + ErrNotFound = errors.New("NotFound") // ErrForbidden indicates something was forbidden - ErrForbidden = errors.New("NotFound") + ErrForbidden = errors.New("Forbidden") ) From 958bcd2df3fb0503d4360081ff0e616f055bc03a Mon Sep 17 00:00:00 2001 From: Keith Duncan Date: Tue, 19 Oct 2021 17:54:59 +1000 Subject: [PATCH 05/12] Run go mod tidy --- s3secrets-helper/go.mod | 8 ++++---- s3secrets-helper/go.sum | 15 +++++++-------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/s3secrets-helper/go.mod b/s3secrets-helper/go.mod index 0b81719..5cd2dda 100644 --- a/s3secrets-helper/go.mod +++ b/s3secrets-helper/go.mod @@ -4,8 +4,8 @@ go 1.15 require ( github.com/aws/aws-sdk-go-v2 v1.9.2 // indirect - github.com/aws/aws-sdk-go-v2/config v1.8.3 // indirect - github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.5.4 // indirect - github.com/aws/aws-sdk-go-v2/service/s3 v1.16.1 // indirect - github.com/aws/smithy-go v1.8.0 // indirect + github.com/aws/aws-sdk-go-v2/config v1.8.3 + github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.5.4 + github.com/aws/aws-sdk-go-v2/service/s3 v1.16.1 + github.com/aws/smithy-go v1.8.0 ) diff --git a/s3secrets-helper/go.sum b/s3secrets-helper/go.sum index 4888ace..38c93dd 100644 --- a/s3secrets-helper/go.sum +++ b/s3secrets-helper/go.sum @@ -1,5 +1,3 @@ -github.com/aws/aws-sdk-go v1.35.14 h1:nucVVXXjAr9UkmYCBWxQWRuYa5KOlaXjuJGg2ulW0K0= -github.com/aws/aws-sdk-go v1.35.14/go.mod h1:tlPOdRjfxPBpNIwqDj61rmsnA85v9jc0Ps9+muhnW+k= github.com/aws/aws-sdk-go-v2 v1.9.2 h1:dUFQcMNZMLON4BOe273pl0filK9RqyQMhCK/6xssL6s= github.com/aws/aws-sdk-go-v2 v1.9.2/go.mod h1:cK/D0BBs0b/oWPIcX/Z/obahJK1TT7IPVjy53i/mX/4= github.com/aws/aws-sdk-go-v2/config v1.8.3 h1:o5583X4qUfuRrOGOgmOcDgvr5gJVSu57NK08cWAhIDk= @@ -26,20 +24,21 @@ github.com/aws/aws-sdk-go-v2/service/sts v1.7.2 h1:ol2Y5DWqnJeKqNd8th7JWzBtqu63x github.com/aws/aws-sdk-go-v2/service/sts v1.7.2/go.mod h1:8EzeIqfWt2wWT4rJVu3f21TfrhJ8AEMzVybRNSb/b4g= github.com/aws/smithy-go v1.8.0 h1:AEwwwXQZtUwP5Mz506FeXXrKBe0jA8gVM+1gEcSRooc= github.com/aws/smithy-go v1.8.0/go.mod h1:SObp3lf9smib00L/v3U2eAKG8FyQ7iLrJnQiAmR5n+E= -github.com/buildkite/elastic-ci-stack-s3-secrets-hooks v1.3.0 h1:CRdPqLjYECJY0cgs24E4ZwUJ2qlmcavN7W1jo+5ujnQ= +github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/google/go-cmp v0.5.6 h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ= github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/jmespath/go-jmespath v0.4.0 h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9YPoQUg= github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHWvzYPziyZiYoo= +github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGwWFoC7ycTf1rcQZHOlsJ6N8= github.com/jmespath/go-jmespath/internal/testify v1.5.1/go.mod h1:L3OGu8Wl2/fWfCI6z80xFu9LTZmf1ZRjMHUOPmWr69U= -github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= -golang.org/x/net v0.0.0-20200202094626-16171245cfb2/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= -golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= -golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= From affae464a804b97ccd7172a50162394dd2878f67 Mon Sep 17 00:00:00 2001 From: Keith Duncan Date: Wed, 20 Oct 2021 16:10:58 +1000 Subject: [PATCH 06/12] Reuse the config once discovered --- s3secrets-helper/s3/s3.go | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/s3secrets-helper/s3/s3.go b/s3secrets-helper/s3/s3.go index 786ef00..fca34d9 100644 --- a/s3secrets-helper/s3/s3.go +++ b/s3secrets-helper/s3/s3.go @@ -24,7 +24,7 @@ func New(log *log.Logger, bucket string) (*Client, error) { ctx := context.Background() // Using the current region (or a guess) find where the bucket lives - managerCfg, err := config.LoadDefaultConfig(ctx, + config, err := config.LoadDefaultConfig(ctx, config.WithRegion(os.Getenv("AWS_DEFAULT_REGION")), config.WithEC2IMDSRegion(), config.WithDefaultRegion("us-east-1"), @@ -33,23 +33,19 @@ func New(log *log.Logger, bucket string) (*Client, error) { return nil, err } - log.Printf("Discovered current region as %q\n", managerCfg.Region) + log.Printf("Discovered current region as %q\n", config.Region) - managerClient := s3.NewFromConfig(managerCfg) - bucketRegion, err := manager.GetBucketRegion(ctx, managerClient, bucket) + bucketRegion, err := manager.GetBucketRegion(ctx, s3.NewFromConfig(config), bucket) if err != nil { return nil, err } log.Printf("Discovered bucket region as %q\n", bucketRegion) - s3Cfg, err := config.LoadDefaultConfig(ctx, config.WithRegion(bucketRegion)) - if err != nil { - return nil, err - } + config.Region = bucketRegion return &Client{ - s3: s3.NewFromConfig(s3Cfg), + s3: s3.NewFromConfig(config), bucket: bucket, }, nil } From 6d6c38a968b5f957d8ccead9d8b1d7b0bbf15d20 Mon Sep 17 00:00:00 2001 From: Keith Duncan Date: Wed, 20 Oct 2021 16:21:17 +1000 Subject: [PATCH 07/12] Remove the os.Getenv(AWS_DEFAULT_REGION) and rely on the default EnvConfig behaviour for resolveRegion --- s3secrets-helper/s3/s3.go | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/s3secrets-helper/s3/s3.go b/s3secrets-helper/s3/s3.go index fca34d9..4608ee6 100644 --- a/s3secrets-helper/s3/s3.go +++ b/s3secrets-helper/s3/s3.go @@ -24,8 +24,28 @@ func New(log *log.Logger, bucket string) (*Client, error) { ctx := context.Background() // Using the current region (or a guess) find where the bucket lives + + /* + There are three region resolvers: + - resolveRegion + - resolveEC2IMDSRegion + - resolveDefaultRegion + + There are also three config providers: + - LoadOptions (programatic provided below) + - EnvConfig (reads environment variables) + - SharedConfig (reads ~/.aws files) + + The resolvers are run sequentially until a region is found, not all + config providers support each resolver. The absolute order is: + + - resolveRegion LoadOptions => config.WithRegion() if given + - resolveRegion EnvConfig => first of AWS_REGION, AWS_DEFAULT_REGION + - resolveRegion SharedConfig => default profile on disk + - resolveEC2IMDSRegion LoadOptions => config.WithEC2IMDSRegion() if given + - resolveDefaultRegion LoadOptions => config.WithDefaultRegion() if given + */ config, err := config.LoadDefaultConfig(ctx, - config.WithRegion(os.Getenv("AWS_DEFAULT_REGION")), config.WithEC2IMDSRegion(), config.WithDefaultRegion("us-east-1"), ) From 5e763ead310605dca3577d45a1e533e2ea142c32 Mon Sep 17 00:00:00 2001 From: Keith Duncan Date: Thu, 21 Oct 2021 16:14:16 +1000 Subject: [PATCH 08/12] Implement the local-region discovery algorithm ourselves --- s3secrets-helper/go.mod | 1 + s3secrets-helper/s3/s3.go | 45 ++++++++++++++++++++------------------- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/s3secrets-helper/go.mod b/s3secrets-helper/go.mod index 5cd2dda..be3badd 100644 --- a/s3secrets-helper/go.mod +++ b/s3secrets-helper/go.mod @@ -5,6 +5,7 @@ go 1.15 require ( github.com/aws/aws-sdk-go-v2 v1.9.2 // indirect github.com/aws/aws-sdk-go-v2/config v1.8.3 + github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.6.0 // indirect github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.5.4 github.com/aws/aws-sdk-go-v2/service/s3 v1.16.1 github.com/aws/smithy-go v1.8.0 diff --git a/s3secrets-helper/s3/s3.go b/s3secrets-helper/s3/s3.go index 4608ee6..a9b1c45 100644 --- a/s3secrets-helper/s3/s3.go +++ b/s3secrets-helper/s3/s3.go @@ -8,6 +8,7 @@ import ( "os" "github.com/aws/aws-sdk-go-v2/config" + "github.com/aws/aws-sdk-go-v2/feature/ec2/imds" "github.com/aws/aws-sdk-go-v2/feature/s3/manager" "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go-v2/service/s3/types" @@ -20,34 +21,34 @@ type Client struct { bucket string } +func getRegion(ctx context.Context) (string, error) { + if region := os.Getenv("AWS_DEFAULT_REGION"); len(region) > 0 { + return region, nil + } + + imdsClient := imds.New(imds.Options{}) + if result, err := imdsClient.GetRegion(ctx, nil); err == nil { + if len(result.Region) > 0 { + return result.Region, nil + } + } + + return "", errors.New("Unknown current region") +} + func New(log *log.Logger, bucket string) (*Client, error) { ctx := context.Background() // Using the current region (or a guess) find where the bucket lives - /* - There are three region resolvers: - - resolveRegion - - resolveEC2IMDSRegion - - resolveDefaultRegion - - There are also three config providers: - - LoadOptions (programatic provided below) - - EnvConfig (reads environment variables) - - SharedConfig (reads ~/.aws files) - - The resolvers are run sequentially until a region is found, not all - config providers support each resolver. The absolute order is: - - - resolveRegion LoadOptions => config.WithRegion() if given - - resolveRegion EnvConfig => first of AWS_REGION, AWS_DEFAULT_REGION - - resolveRegion SharedConfig => default profile on disk - - resolveEC2IMDSRegion LoadOptions => config.WithEC2IMDSRegion() if given - - resolveDefaultRegion LoadOptions => config.WithDefaultRegion() if given - */ + region, err := getRegion(ctx) + if err != nil { + // Ignore error and fallback to us-east-1 for bucket lookup + region = "us-east-1" + } + config, err := config.LoadDefaultConfig(ctx, - config.WithEC2IMDSRegion(), - config.WithDefaultRegion("us-east-1"), + config.WithRegion(region), ) if err != nil { return nil, err From 2ee64f00c90dbd0bb814201ba703eb1c4dd69d99 Mon Sep 17 00:00:00 2001 From: Keith Duncan Date: Wed, 27 Oct 2021 08:21:21 +1000 Subject: [PATCH 09/12] Apply suggestions from code review --- s3secrets-helper/s3/s3.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/s3secrets-helper/s3/s3.go b/s3secrets-helper/s3/s3.go index a9b1c45..faa205b 100644 --- a/s3secrets-helper/s3/s3.go +++ b/s3secrets-helper/s3/s3.go @@ -80,7 +80,7 @@ func (c *Client) Bucket() (string) { // sentinel.ErrNotFound and sentinel.ErrForbidden are returned for those cases. // Other errors are returned verbatim. func (c *Client) Get(key string) ([]byte, error) { - out, err := c.s3.GetObject(context.Background(), &s3.GetObjectInput{ + out, err := c.s3.GetObject(context.TODO(), &s3.GetObjectInput{ Bucket: &c.bucket, Key: &key, }) @@ -112,7 +112,7 @@ func (c *Client) Get(key string) ([]byte, error) { // 404 Not Found and 403 Forbidden return false without error. // Other errors result in false with an error. func (c *Client) BucketExists() (bool, error) { - if _, err := c.s3.HeadBucket(context.Background(), &s3.HeadBucketInput{Bucket: &c.bucket}); err != nil { + if _, err := c.s3.HeadBucket(context.TODO(), &s3.HeadBucketInput{Bucket: &c.bucket}); err != nil { return false, err } return true, nil From c9efc39afb0e5f2fdfb81667fd3e7a4f2bd1bc47 Mon Sep 17 00:00:00 2001 From: Keith Duncan Date: Wed, 27 Oct 2021 08:43:46 +1000 Subject: [PATCH 10/12] Replace Forbidden with AccessDenied and link to S3 API error docs --- s3secrets-helper/s3/s3.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/s3secrets-helper/s3/s3.go b/s3secrets-helper/s3/s3.go index faa205b..c5066e2 100644 --- a/s3secrets-helper/s3/s3.go +++ b/s3secrets-helper/s3/s3.go @@ -90,11 +90,11 @@ func (c *Client) Get(key string) ([]byte, error) { return nil, sentinel.ErrNotFound } + // Possible values can be found at https://docs.aws.amazon.com/AmazonS3/latest/API/API_Error.html var apiErr smithy.APIError if errors.As(err, &apiErr) { code := apiErr.ErrorCode() - // TODO confirm "Forbidden" is a member of the set of values this can return - if code == "Forbidden" { + if code == "AccessDenied" { return nil, sentinel.ErrForbidden } } From df507f3147d322d100b988e5b4e5172fa9617330 Mon Sep 17 00:00:00 2001 From: Keith Duncan Date: Thu, 28 Oct 2021 11:27:21 +1000 Subject: [PATCH 11/12] Add improved error messages --- s3secrets-helper/s3/s3.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/s3secrets-helper/s3/s3.go b/s3secrets-helper/s3/s3.go index c5066e2..76d5879 100644 --- a/s3secrets-helper/s3/s3.go +++ b/s3secrets-helper/s3/s3.go @@ -3,6 +3,7 @@ package s3 import ( "context" "errors" + "fmt" "log" "io/ioutil" "os" @@ -51,14 +52,14 @@ func New(log *log.Logger, bucket string) (*Client, error) { config.WithRegion(region), ) if err != nil { - return nil, err + return nil, fmt.Errorf("Could not load the AWS SDK config (%v)", err) } log.Printf("Discovered current region as %q\n", config.Region) bucketRegion, err := manager.GetBucketRegion(ctx, s3.NewFromConfig(config), bucket) if err != nil { - return nil, err + return nil, fmt.Errorf("Could not discover the region for bucket %q: (%v)", bucket, err) } log.Printf("Discovered bucket region as %q\n", bucketRegion) @@ -99,7 +100,7 @@ func (c *Client) Get(key string) ([]byte, error) { } } - return nil, err + return nil, fmt.Errorf("Could not GetObject (%s) in bucket (%s): (%v)", key, c.bucket, err) } defer out.Body.Close() // we probably should return io.Reader or io.ReadCloser rather than []byte, @@ -113,7 +114,7 @@ func (c *Client) Get(key string) ([]byte, error) { // Other errors result in false with an error. func (c *Client) BucketExists() (bool, error) { if _, err := c.s3.HeadBucket(context.TODO(), &s3.HeadBucketInput{Bucket: &c.bucket}); err != nil { - return false, err + return false, fmt.Errorf("Could not HeadBucket (%s): (%v)", c.bucket, err) } return true, nil } From 60bbda3d44fc061bc89c27a170b7483843053c98 Mon Sep 17 00:00:00 2001 From: Keith Duncan Date: Thu, 28 Oct 2021 11:30:51 +1000 Subject: [PATCH 12/12] More error details --- s3secrets-helper/s3/s3.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/s3secrets-helper/s3/s3.go b/s3secrets-helper/s3/s3.go index 76d5879..a201e54 100644 --- a/s3secrets-helper/s3/s3.go +++ b/s3secrets-helper/s3/s3.go @@ -100,7 +100,7 @@ func (c *Client) Get(key string) ([]byte, error) { } } - return nil, fmt.Errorf("Could not GetObject (%s) in bucket (%s): (%v)", key, c.bucket, err) + return nil, fmt.Errorf("Could not GetObject (%s) in bucket (%s). Ensure your IAM Identity has s3:GetObject permission for this key and bucket. (%v)", key, c.bucket, err) } defer out.Body.Close() // we probably should return io.Reader or io.ReadCloser rather than []byte, @@ -114,7 +114,7 @@ func (c *Client) Get(key string) ([]byte, error) { // Other errors result in false with an error. func (c *Client) BucketExists() (bool, error) { if _, err := c.s3.HeadBucket(context.TODO(), &s3.HeadBucketInput{Bucket: &c.bucket}); err != nil { - return false, fmt.Errorf("Could not HeadBucket (%s): (%v)", c.bucket, err) + return false, fmt.Errorf("Could not HeadBucket (%s). Ensure your IAM Identity has s3:ListBucket permission for this bucket. (%v)", c.bucket, err) } return true, nil }