From b1cd4724833367af82ce273703c06e2a7d93eee0 Mon Sep 17 00:00:00 2001 From: Vaibhav Kamra Date: Fri, 2 Sep 2022 11:38:01 -0700 Subject: [PATCH] Remove AWS credentials initialization (#610) ## Description Instead of initializing static AWS credentials, we rely on the credential provider chain in [Kopia](https://github.com/kopia/kopia/pull/2213) to discover and initialize credentials. This currently supports the following in this order: - Static credentials - Environment variables (what Corso used to implement) - IAM Going forward, this will also allow us to support shared credentials (cred file) once that is added to the credential provider chain. ## Type of change Please check the type of change your PR introduces: - [x] :sunflower: Feature - [ ] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Test - [ ] :hamster: Trivial/Minor ## Issue(s) - #384 ## Test Plan - [x] :muscle: Manual (see test output below) - [ ] :zap: Unit test - [x] :green_heart: E2E --- src/cli/config/config_test.go | 8 ------- src/cli/config/storage.go | 14 +++++-------- src/cli/repo/s3.go | 4 ---- src/go.mod | 2 ++ src/go.sum | 4 ++++ src/internal/kopia/s3.go | 9 +++----- src/internal/tester/storage.go | 1 - src/pkg/storage/s3.go | 30 +++++++-------------------- src/pkg/storage/s3_test.go | 38 ---------------------------------- 9 files changed, 21 insertions(+), 89 deletions(-) diff --git a/src/cli/config/config_test.go b/src/cli/config/config_test.go index 4e7eb637f..01dbdb8b5 100644 --- a/src/cli/config/config_test.go +++ b/src/cli/config/config_test.go @@ -235,10 +235,6 @@ func (suite *ConfigIntegrationSuite) TestGetStorageAndAccount() { assert.Equal(t, readS3Cfg.Endpoint, s3Cfg.Endpoint) assert.Equal(t, readS3Cfg.Prefix, s3Cfg.Prefix) - assert.Equal(t, readS3Cfg.AccessKey, os.Getenv(credentials.AWSAccessKeyID)) - assert.Equal(t, readS3Cfg.SecretKey, os.Getenv(credentials.AWSSecretAccessKey)) - assert.Equal(t, readS3Cfg.SessionToken, os.Getenv(credentials.AWSSessionToken)) - common, err := st.CommonConfig() require.NoError(t, err, "reading common config from storage") assert.Equal(t, common.CorsoPassword, os.Getenv(credentials.CorsoPassword)) @@ -287,10 +283,6 @@ func (suite *ConfigIntegrationSuite) TestGetStorageAndAccount_noFileOnlyOverride assert.Equal(t, readS3Cfg.Endpoint, s3Cfg.Endpoint) assert.Equal(t, readS3Cfg.Prefix, s3Cfg.Prefix) - assert.Equal(t, readS3Cfg.AccessKey, os.Getenv(credentials.AWSAccessKeyID)) - assert.Equal(t, readS3Cfg.SecretKey, os.Getenv(credentials.AWSSecretAccessKey)) - assert.Equal(t, readS3Cfg.SessionToken, os.Getenv(credentials.AWSSessionToken)) - common, err := st.CommonConfig() require.NoError(t, err, "reading common config from storage") assert.Equal(t, common.CorsoPassword, os.Getenv(credentials.CorsoPassword)) diff --git a/src/cli/config/storage.go b/src/cli/config/storage.go index 3504dd647..40d20f0cf 100644 --- a/src/cli/config/storage.go +++ b/src/cli/config/storage.go @@ -4,6 +4,7 @@ import ( "os" "path/filepath" + "github.com/aws/aws-sdk-go/aws/defaults" "github.com/pkg/errors" "github.com/spf13/viper" @@ -61,14 +62,12 @@ func configureStorage( } } - // compose the s3 storage config and credentials - aws := credentials.GetAWS(overrides) - if err := aws.Validate(); err != nil { + _, err = defaults.CredChain(defaults.Config().WithCredentialsChainVerboseErrors(true), defaults.Handlers()).Get() + if err != nil { return store, errors.Wrap(err, "validating aws credentials") } s3Cfg = storage.S3Config{ - AWS: aws, Bucket: common.First(overrides[storage.Bucket], s3Cfg.Bucket, os.Getenv(storage.BucketKey)), Endpoint: common.First(overrides[storage.Endpoint], s3Cfg.Endpoint, os.Getenv(storage.EndpointKey)), Prefix: common.First(overrides[storage.Prefix], s3Cfg.Prefix, os.Getenv(storage.PrefixKey)), @@ -93,11 +92,8 @@ func configureStorage( // ensure required properties are present if err := utils.RequireProps(map[string]string{ - credentials.AWSAccessKeyID: aws.AccessKey, - storage.Bucket: s3Cfg.Bucket, - credentials.AWSSecretAccessKey: aws.SecretKey, - credentials.AWSSessionToken: aws.SessionToken, - credentials.CorsoPassword: corso.CorsoPassword, + storage.Bucket: s3Cfg.Bucket, + credentials.CorsoPassword: corso.CorsoPassword, }); err != nil { return storage.Storage{}, err } diff --git a/src/cli/repo/s3.go b/src/cli/repo/s3.go index 069921baf..0fd03ef1e 100644 --- a/src/cli/repo/s3.go +++ b/src/cli/repo/s3.go @@ -10,14 +10,12 @@ import ( "github.com/alcionai/corso/cli/utils" "github.com/alcionai/corso/internal/kopia" "github.com/alcionai/corso/pkg/account" - "github.com/alcionai/corso/pkg/credentials" "github.com/alcionai/corso/pkg/repository" "github.com/alcionai/corso/pkg/storage" ) // s3 bucket info from flags var ( - accessKey string bucket string endpoint string prefix string @@ -38,7 +36,6 @@ func addS3Commands(parent *cobra.Command) *cobra.Command { c, fs = utils.AddCommand(parent, s3ConnectCmd()) } - fs.StringVar(&accessKey, "access-key", "", "Access key ID (replaces the AWS_ACCESS_KEY_ID env variable).") fs.StringVar(&bucket, "bucket", "", "Name of the S3 bucket (required).") cobra.CheckErr(c.MarkFlagRequired("bucket")) fs.StringVar(&endpoint, "endpoint", "s3.amazonaws.com", "Server endpoint for S3 communication.") @@ -169,7 +166,6 @@ func s3Overrides() map[string]string { return map[string]string{ config.AccountProviderTypeKey: account.ProviderM365.String(), config.StorageProviderTypeKey: storage.ProviderS3.String(), - credentials.AWSAccessKeyID: accessKey, storage.Bucket: bucket, storage.Endpoint: endpoint, storage.Prefix: prefix, diff --git a/src/go.mod b/src/go.mod index 3edcb7b36..41e7e223a 100644 --- a/src/go.mod +++ b/src/go.mod @@ -6,6 +6,7 @@ replace github.com/kopia/kopia => github.com/kopia/kopia v0.11.4-0.2022082219422 require ( github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.0.0 + github.com/aws/aws-sdk-go v1.44.81 github.com/google/uuid v1.3.0 github.com/hashicorp/go-multierror v1.1.1 github.com/kopia/kopia v0.11.1 @@ -58,6 +59,7 @@ require ( github.com/hashicorp/errwrap v1.0.0 // indirect github.com/hashicorp/golang-lru v0.5.4 // indirect github.com/inconshreveable/mousetrap v1.0.0 // indirect + github.com/jmespath/go-jmespath v0.4.0 // indirect github.com/json-iterator/go v1.1.12 // indirect github.com/klauspost/compress v1.15.9 // indirect github.com/klauspost/cpuid/v2 v2.1.0 // indirect diff --git a/src/go.sum b/src/go.sum index ee6317600..d1d4be955 100644 --- a/src/go.sum +++ b/src/go.sum @@ -53,6 +53,7 @@ github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4/go.mod h1:ybxpYRF github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d/go.mod h1:rBZYJk541a8SKzHPHnH3zbiI+7dagKZ0cgpgrD7Fyho= github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY= github.com/aws/aws-sdk-go v1.44.81 h1:C8oBZ+a+ka0qk3Q24MohQIFq0tkbO8IAu5tfpAMKVWE= +github.com/aws/aws-sdk-go v1.44.81/go.mod h1:y4AeaBuwd2Lk+GepC1E9v0qOiTws0MIWAX4oIKwKHZo= github.com/benbjohnson/clock v1.1.0 h1:Q92kusRqC1XV2MjkWETPvjJVqKetz1OzxZB7mHJLju8= github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA= github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q= @@ -200,6 +201,9 @@ github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1: github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM= github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8= 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/jpillora/backoff v1.0.0/go.mod h1:J/6gKK9jxlEcS3zixgDgUAsiuZ7yrSoa/FX5e0EB2j4= github.com/json-iterator/go v1.1.6/go.mod h1:+SdeFBvtyEkXs7REEP0seUULqWtbJapLOCVDaaPEHmU= github.com/json-iterator/go v1.1.10/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4= diff --git a/src/internal/kopia/s3.go b/src/internal/kopia/s3.go index 31190895e..e029b5b87 100644 --- a/src/internal/kopia/s3.go +++ b/src/internal/kopia/s3.go @@ -25,12 +25,9 @@ func s3BlobStorage(ctx context.Context, s storage.Storage) (blob.Storage, error) } opts := s3.Options{ - AccessKeyID: cfg.AccessKey, - BucketName: cfg.Bucket, - Endpoint: endpoint, - Prefix: cfg.Prefix, - SecretAccessKey: cfg.SecretKey, - SessionToken: cfg.SessionToken, + BucketName: cfg.Bucket, + Endpoint: endpoint, + Prefix: cfg.Prefix, } return s3.New(ctx, &opts) diff --git a/src/internal/tester/storage.go b/src/internal/tester/storage.go index fb134cc15..4ab2b16c8 100644 --- a/src/internal/tester/storage.go +++ b/src/internal/tester/storage.go @@ -30,7 +30,6 @@ func NewPrefixedS3Storage(t *testing.T) storage.Storage { st, err := storage.NewStorage( storage.ProviderS3, storage.S3Config{ - AWS: credentials.GetAWS(nil), Bucket: cfg[TestCfgBucket], Prefix: t.Name() + "-" + now, }, diff --git a/src/pkg/storage/s3.go b/src/pkg/storage/s3.go index 2c96a0fd7..d63637334 100644 --- a/src/pkg/storage/s3.go +++ b/src/pkg/storage/s3.go @@ -2,13 +2,9 @@ package storage import ( "github.com/pkg/errors" - - "github.com/alcionai/corso/pkg/credentials" ) type S3Config struct { - credentials.AWS // requires: AccessKey, SecretKey, SessionToken - Bucket string // required Endpoint string Prefix string @@ -16,12 +12,9 @@ type S3Config struct { // config key consts const ( - keyS3AccessKey = "s3_accessKey" - keyS3Bucket = "s3_bucket" - keyS3Endpoint = "s3_endpoint" - keyS3Prefix = "s3_prefix" - keyS3SecretKey = "s3_secretKey" - keyS3SessionToken = "s3_sessionToken" + keyS3Bucket = "s3_bucket" + keyS3Endpoint = "s3_endpoint" + keyS3Prefix = "s3_prefix" ) // config exported name consts @@ -36,12 +29,9 @@ const ( // serialize into the map are expected to be strings. func (c S3Config) StringConfig() (map[string]string, error) { cfg := map[string]string{ - keyS3AccessKey: c.AccessKey, - keyS3Bucket: c.Bucket, - keyS3Endpoint: c.Endpoint, - keyS3Prefix: c.Prefix, - keyS3SecretKey: c.SecretKey, - keyS3SessionToken: c.SessionToken, + keyS3Bucket: c.Bucket, + keyS3Endpoint: c.Endpoint, + keyS3Prefix: c.Prefix, } return cfg, c.validate() @@ -52,12 +42,9 @@ func (s Storage) S3Config() (S3Config, error) { c := S3Config{} if len(s.Config) > 0 { - c.AccessKey = orEmptyString(s.Config[keyS3AccessKey]) c.Bucket = orEmptyString(s.Config[keyS3Bucket]) c.Endpoint = orEmptyString(s.Config[keyS3Endpoint]) c.Prefix = orEmptyString(s.Config[keyS3Prefix]) - c.SecretKey = orEmptyString(s.Config[keyS3SecretKey]) - c.SessionToken = orEmptyString(s.Config[keyS3SessionToken]) } return c, c.validate() @@ -65,10 +52,7 @@ func (s Storage) S3Config() (S3Config, error) { func (c S3Config) validate() error { check := map[string]string{ - credentials.AWSAccessKeyID: c.AccessKey, - credentials.AWSSecretAccessKey: c.SecretKey, - credentials.AWSSessionToken: c.SessionToken, - Bucket: c.Bucket, + Bucket: c.Bucket, } for k, v := range check { if len(v) == 0 { diff --git a/src/pkg/storage/s3_test.go b/src/pkg/storage/s3_test.go index 08e2e19bc..08f8ddc10 100644 --- a/src/pkg/storage/s3_test.go +++ b/src/pkg/storage/s3_test.go @@ -6,7 +6,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" - "github.com/alcionai/corso/pkg/credentials" "github.com/alcionai/corso/pkg/storage" ) @@ -19,11 +18,6 @@ func TestS3CfgSuite(t *testing.T) { } var goodS3Config = storage.S3Config{ - AWS: credentials.AWS{ - AccessKey: "ak", - SecretKey: "sk", - SessionToken: "tkn", - }, Bucket: "bkt", Endpoint: "end", Prefix: "pre", @@ -39,11 +33,8 @@ func (suite *S3CfgSuite) TestS3Config_Config() { expect string }{ {"s3_bucket", s3.Bucket}, - {"s3_accessKey", s3.AccessKey}, {"s3_endpoint", s3.Endpoint}, {"s3_prefix", s3.Prefix}, - {"s3_secretKey", s3.SecretKey}, - {"s3_sessionToken", s3.SessionToken}, } for _, test := range table { assert.Equal(suite.T(), test.expect, c[test.key]) @@ -60,20 +51,12 @@ func (suite *S3CfgSuite) TestStorage_S3Config() { assert.NoError(t, err) assert.Equal(t, in.Bucket, out.Bucket) - assert.Equal(t, in.AccessKey, out.AccessKey) assert.Equal(t, in.Endpoint, out.Endpoint) assert.Equal(t, in.Prefix, out.Prefix) - assert.Equal(t, in.SecretKey, out.SecretKey) - assert.Equal(t, in.SessionToken, out.SessionToken) } func makeTestS3Cfg(ak, bkt, end, pre, sk, tkn string) storage.S3Config { return storage.S3Config{ - AWS: credentials.AWS{ - AccessKey: ak, - SecretKey: sk, - SessionToken: tkn, - }, Bucket: bkt, Endpoint: end, Prefix: pre, @@ -86,10 +69,7 @@ func (suite *S3CfgSuite) TestStorage_S3Config_InvalidCases() { name string cfg storage.S3Config }{ - {"missing access key", makeTestS3Cfg("", "bkt", "end", "pre", "sk", "tkn")}, {"missing bucket", makeTestS3Cfg("ak", "", "end", "pre", "sk", "tkn")}, - {"missing secret key", makeTestS3Cfg("ak", "bkt", "end", "pre", "", "tkn")}, - {"missing session token", makeTestS3Cfg("ak", "bkt", "end", "pre", "sk", "")}, } for _, test := range table { suite.T().Run(test.name, func(t *testing.T) { @@ -103,30 +83,12 @@ func (suite *S3CfgSuite) TestStorage_S3Config_InvalidCases() { name string amend func(storage.Storage) }{ - { - "missing access key", - func(s storage.Storage) { - s.Config["s3_accessKey"] = "" - }, - }, { "missing bucket", func(s storage.Storage) { s.Config["s3_bucket"] = "" }, }, - { - "missing secret key", - func(s storage.Storage) { - s.Config["s3_secretKey"] = "" - }, - }, - { - "missing session token", - func(s storage.Storage) { - s.Config["s3_sessionToken"] = "" - }, - }, } for _, test := range table2 { suite.T().Run(test.name, func(t *testing.T) {