From 43af738edd688ea064ec1920a2f4a0e600051a79 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Fri, 4 Dec 2020 16:10:21 -0800 Subject: [PATCH 01/50] Add s3-disable-ssl-verify to deploy.cfg.example --- deploy.cfg.example | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/deploy.cfg.example b/deploy.cfg.example index 30b5272..a948793 100644 --- a/deploy.cfg.example +++ b/deploy.cfg.example @@ -21,6 +21,7 @@ s3-access-key = [access key goes here] s3-access-secret = [access secret goes here] s3-region = us-west-1 #s3-disable-ssl = false +#s3-disable-ssl-verify = false # KBase auth server parameters. # The root url of the auth server. @@ -31,4 +32,4 @@ kbase-auth-admin-roles = KBASE_ADMIN, BLOBSTORE_ADMIN # If "true", make the server ignore the X-Forwarded-For and X-Real-IP headers. Otherwise # (the default behavior), the logged IP address for a request, in order of precedence, is # 1) the first address in X-Forwarded-For, 2) X-Real-IP, and 3) the address of the client. -dont-trust-x-ip-headers = false \ No newline at end of file +dont-trust-x-ip-headers = false From c8db28281b931631ca2435978314239365e4da77 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Fri, 4 Dec 2020 16:13:13 -0800 Subject: [PATCH 02/50] Update deploy.cfg.example --- deploy.cfg.example | 2 ++ 1 file changed, 2 insertions(+) diff --git a/deploy.cfg.example b/deploy.cfg.example index a948793..5901b9f 100644 --- a/deploy.cfg.example +++ b/deploy.cfg.example @@ -20,7 +20,9 @@ s3-bucket = blobstore s3-access-key = [access key goes here] s3-access-secret = [access secret goes here] s3-region = us-west-1 +# Use plaintext to talk to destination S3. Default false. #s3-disable-ssl = false +# Disable verifying the destination S3 SSL cert (e.g. for self-signed certs). Default false. #s3-disable-ssl-verify = false # KBase auth server parameters. From 3c0abc14aa9d966747cf686af30d82154175a470 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Fri, 4 Dec 2020 16:55:58 -0800 Subject: [PATCH 03/50] Update deploy.cfg.example --- deploy.cfg.example | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deploy.cfg.example b/deploy.cfg.example index 5901b9f..5a6aeea 100644 --- a/deploy.cfg.example +++ b/deploy.cfg.example @@ -20,7 +20,7 @@ s3-bucket = blobstore s3-access-key = [access key goes here] s3-access-secret = [access secret goes here] s3-region = us-west-1 -# Use plaintext to talk to destination S3. Default false. +# Use plaintext to talk to destination S3. Default false. (false is not tested) #s3-disable-ssl = false # Disable verifying the destination S3 SSL cert (e.g. for self-signed certs). Default false. #s3-disable-ssl-verify = false From 3d5d0f8db589f1b2f30eada0baad817148efb2fe Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Tue, 8 Dec 2020 12:19:15 -0800 Subject: [PATCH 04/50] Add code for s3-disable-ssl-verify key --- config/config.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/config/config.go b/config/config.go index 4a44901..7a51ac4 100644 --- a/config/config.go +++ b/config/config.go @@ -35,6 +35,10 @@ const ( // KeyS3DisableSSL is the configuration key that determines whether SSL is to be used. // any value other than 'true' is treated as false. KeyS3DisableSSL = "s3-disable-ssl" + // KeyS3DisableSSLVerify is the configuration key that determines whether to verify the + // SSL certificate of the remote servers (in case a self-signed cert is used). + // any value other than 'true' is treated as false. + KeyS3DisableSSLVerify = "s3-disable-ssl-verify" // KeyS3Region is the configuration key where the value is the S3 region KeyS3Region = "s3-region" // KeyAuthURL is the configuration key where the value is the KBase auth server URL @@ -69,6 +73,8 @@ type Config struct { S3AccessSecret string // S3DisableSSL determines whether SSL should be used S3DisableSSL bool + // S3DisableSSLVerify determines whether SSL certs should be verified + S3DisableSSLVerify bool // S3Region is the S3 region S3Region string // AuthURL is the KBase auth server URL. It is never nil. @@ -102,6 +108,7 @@ func New(configFilePath string) (*Config, error) { s3key, err := getString(err, configFilePath, sec, KeyS3AccessKey, true) s3secret, err := getString(err, configFilePath, sec, KeyS3AccessSecret, true) s3disableSSL, err := getString(err, configFilePath, sec, KeyS3DisableSSL, false) + s3disableSSLVerify, err := getString(err, configFilePath, sec, KeyS3DisableSSLVerify, false) s3region, err := getString(err, configFilePath, sec, KeyS3Region, true) authurl, err := getURL(err, configFilePath, sec, KeyAuthURL) roles, err := getStringList(err, configFilePath, sec, KeyAuthAdminRoles) @@ -126,6 +133,7 @@ func New(configFilePath string) (*Config, error) { S3AccessKey: s3key, S3AccessSecret: s3secret, S3DisableSSL: "true" == s3disableSSL, + S3DisableSSLVerify: "true" == s3disableSSLVerify, S3Region: s3region, AuthURL: authurl, AuthAdminRoles: roles, From 498c49ba184f8c17f19d6649e3815b7994a48332 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Tue, 8 Dec 2020 12:34:19 -0800 Subject: [PATCH 05/50] Support for not verifying cert of minio client --- service/build.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/service/build.go b/service/build.go index c7a2a7c..c4acc57 100644 --- a/service/build.go +++ b/service/build.go @@ -2,6 +2,7 @@ package service import ( "context" + "net/http" "github.com/aws/aws-sdk-go/service/s3" @@ -22,6 +23,8 @@ import ( authcache "github.com/kbase/blobstore/auth/cache" "github.com/kbase/blobstore/config" "github.com/minio/minio-go" + "github.com/minio/minio-go/pkg/credentials" + "go.mongodb.org/mongo-driver/mongo" ) @@ -58,6 +61,12 @@ func buildFileStore(cfg *config.Config) (filestore.FileStore, error) { sess := session.Must(session.NewSession()) creds := credentials.NewStaticCredentials(cfg.S3AccessKey, cfg.S3AccessSecret, "") + + // need a custom transport to support not verifying SSL cert + customTransport := &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: &cfg.S3DisableSSLVerify} + } + awscli := s3.New(sess, &aws.Config{ Credentials: creds, Endpoint: &cfg.S3Host, @@ -66,7 +75,14 @@ func buildFileStore(cfg *config.Config) (filestore.FileStore, error) { S3ForcePathStyle: &trueref}) // minio pukes otherwise minioClient, err := minio.NewWithRegion( - cfg.S3Host, cfg.S3AccessKey, cfg.S3AccessSecret, !cfg.S3DisableSSL, cfg.S3Region) + cfg.S3Host, + &minio.Options{ + Creds: credentialsNewStaticV4(cfg.S3AccessKey, cfg.S3AccessSecret, ""), + Secure: !cfg.S3DisableSSL, + Region: cfg.S3Region, + Transport: customTransport + } ) + if err != nil { return nil, err } From c88336f47a6b4c3d604569798f8a96126f07df81 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Tue, 8 Dec 2020 12:37:59 -0800 Subject: [PATCH 06/50] Update build.go --- service/build.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/service/build.go b/service/build.go index c4acc57..be491bd 100644 --- a/service/build.go +++ b/service/build.go @@ -23,7 +23,6 @@ import ( authcache "github.com/kbase/blobstore/auth/cache" "github.com/kbase/blobstore/config" "github.com/minio/minio-go" - "github.com/minio/minio-go/pkg/credentials" "go.mongodb.org/mongo-driver/mongo" ) @@ -64,7 +63,7 @@ func buildFileStore(cfg *config.Config) (filestore.FileStore, error) { // need a custom transport to support not verifying SSL cert customTransport := &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: &cfg.S3DisableSSLVerify} + TLSClientConfig: &tls.Config{InsecureSkipVerify: &cfg.S3DisableSSLVerify}, } awscli := s3.New(sess, &aws.Config{ @@ -80,7 +79,7 @@ func buildFileStore(cfg *config.Config) (filestore.FileStore, error) { Creds: credentialsNewStaticV4(cfg.S3AccessKey, cfg.S3AccessSecret, ""), Secure: !cfg.S3DisableSSL, Region: cfg.S3Region, - Transport: customTransport + Transport: customTransport, } ) if err != nil { From 32811ed18525ce7f8ad831bf9db4753be19729ab Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Tue, 8 Dec 2020 12:39:49 -0800 Subject: [PATCH 07/50] Use same creds for minio as for aws ...and hope minio creds can use the AWS creds object --- service/build.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/build.go b/service/build.go index be491bd..0eef1ba 100644 --- a/service/build.go +++ b/service/build.go @@ -76,7 +76,7 @@ func buildFileStore(cfg *config.Config) (filestore.FileStore, error) { minioClient, err := minio.NewWithRegion( cfg.S3Host, &minio.Options{ - Creds: credentialsNewStaticV4(cfg.S3AccessKey, cfg.S3AccessSecret, ""), + Creds: creds, Secure: !cfg.S3DisableSSL, Region: cfg.S3Region, Transport: customTransport, From 90038ac0ac2825d52bd4fa8fd94b4f180e6df8be Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Tue, 8 Dec 2020 12:53:19 -0800 Subject: [PATCH 08/50] Use old style minio.NewWithRegion call NewWithRegion doesn't seem to like the (url, &minio.Options) call. Try using the older (documented) style with this for setting a custom transport: https://github.com/minio/minio-go/issues/1019 --- service/build.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/service/build.go b/service/build.go index 0eef1ba..8b42f09 100644 --- a/service/build.go +++ b/service/build.go @@ -74,13 +74,8 @@ func buildFileStore(cfg *config.Config) (filestore.FileStore, error) { S3ForcePathStyle: &trueref}) // minio pukes otherwise minioClient, err := minio.NewWithRegion( - cfg.S3Host, - &minio.Options{ - Creds: creds, - Secure: !cfg.S3DisableSSL, - Region: cfg.S3Region, - Transport: customTransport, - } ) + cfg.S3Host, cfg.S3AccessKey, cfg.S3AccessSecret, !cfg.S3DisableSSL, cfg.S3Region) + minioClient.SetCustomTransport(customTransport) if err != nil { return nil, err From 3a1b1618120ad1b775ac17f71c6b8f05642e5e9a Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Tue, 8 Dec 2020 12:56:36 -0800 Subject: [PATCH 09/50] Add tls Forgot import --- service/build.go | 1 + 1 file changed, 1 insertion(+) diff --git a/service/build.go b/service/build.go index 8b42f09..766500e 100644 --- a/service/build.go +++ b/service/build.go @@ -3,6 +3,7 @@ package service import ( "context" "net/http" + "crypto/tls" "github.com/aws/aws-sdk-go/service/s3" From 291f871d6c71a885931ebeefa81c8dda5166111b Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Tue, 8 Dec 2020 12:59:39 -0800 Subject: [PATCH 10/50] Update build.go --- service/build.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/build.go b/service/build.go index 766500e..5e9f0b1 100644 --- a/service/build.go +++ b/service/build.go @@ -64,7 +64,7 @@ func buildFileStore(cfg *config.Config) (filestore.FileStore, error) { // need a custom transport to support not verifying SSL cert customTransport := &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: &cfg.S3DisableSSLVerify}, + TLSClientConfig: &tls.Config{InsecureSkipVerify: cfg.S3DisableSSLVerify}, } awscli := s3.New(sess, &aws.Config{ From 0027c0149df65c1f1f022363e8c83cf8208e1a72 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Tue, 8 Dec 2020 14:59:21 -0800 Subject: [PATCH 11/50] Add custom client to awscli Adapted from https://github.com/aws/aws-sdk-go/issues/2404 --- service/build.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/service/build.go b/service/build.go index 5e9f0b1..ce99703 100644 --- a/service/build.go +++ b/service/build.go @@ -66,12 +66,14 @@ func buildFileStore(cfg *config.Config) (filestore.FileStore, error) { customTransport := &http.Transport{ TLSClientConfig: &tls.Config{InsecureSkipVerify: cfg.S3DisableSSLVerify}, } + customHTTPClient := &http.Client{Transport: customTransport} awscli := s3.New(sess, &aws.Config{ Credentials: creds, Endpoint: &cfg.S3Host, Region: &cfg.S3Region, DisableSSL: &cfg.S3DisableSSL, + HTTPClient: customHTTPClient, S3ForcePathStyle: &trueref}) // minio pukes otherwise minioClient, err := minio.NewWithRegion( From 3c3c114242c7c62b9184f15429f0e671fbbab5df Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Tue, 8 Dec 2020 15:00:42 -0800 Subject: [PATCH 12/50] fix spacing --- service/build.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/build.go b/service/build.go index ce99703..92ddd76 100644 --- a/service/build.go +++ b/service/build.go @@ -73,7 +73,7 @@ func buildFileStore(cfg *config.Config) (filestore.FileStore, error) { Endpoint: &cfg.S3Host, Region: &cfg.S3Region, DisableSSL: &cfg.S3DisableSSL, - HTTPClient: customHTTPClient, + HTTPClient: customHTTPClient, S3ForcePathStyle: &trueref}) // minio pukes otherwise minioClient, err := minio.NewWithRegion( From d3842057f59700f6ebab846e917fe25f7bfaa514 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Tue, 8 Dec 2020 15:57:11 -0800 Subject: [PATCH 13/50] Add disableSSLverify boolean to NewS3FileStore Still need to add flag to actual http.NewRequest call in StoreFile() --- filestore/s3.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/filestore/s3.go b/filestore/s3.go index 6eb6167..90b69c5 100644 --- a/filestore/s3.go +++ b/filestore/s3.go @@ -33,6 +33,7 @@ type S3FileStore struct { s3client *s3.S3 minioClient *minio.Client bucket string + disableSSLverify bool } // NewS3FileStore creates a new S3 based file store. Files will be stored in the provided @@ -44,6 +45,7 @@ func NewS3FileStore( s3client *s3.S3, minioClient *minio.Client, bucket string, + disableSSLverify bool, ) (*S3FileStore, error) { if s3client == nil { @@ -62,7 +64,7 @@ func NewS3FileStore( // Ignore for now. return nil, err } - return &S3FileStore{s3client: s3client, minioClient: minioClient, bucket: bucket}, nil + return &S3FileStore{s3client: s3client, minioClient: minioClient, bucket: bucket, disableSSLverify: disableSSLverify}, nil } func checkBucketName(bucket string) (string, error) { From 266b1203ee944115be30d334293db7eb388b2f55 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Tue, 8 Dec 2020 16:02:18 -0800 Subject: [PATCH 14/50] Add S3DisableSSLVerify to NewS3FileStore call --- service/build.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/build.go b/service/build.go index 92ddd76..22888b3 100644 --- a/service/build.go +++ b/service/build.go @@ -83,7 +83,7 @@ func buildFileStore(cfg *config.Config) (filestore.FileStore, error) { if err != nil { return nil, err } - return filestore.NewS3FileStore(awscli, minioClient, cfg.S3Bucket) + return filestore.NewS3FileStore(awscli, minioClient, cfg.S3Bucket, cfg.S3DisableSSLVerify) } func buildNodeStore(cfg *config.Config) (nodestore.NodeStore, error) { From 4ac2e7010d73924f8dd2c85c37072d312652be9e Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Tue, 8 Dec 2020 16:13:40 -0800 Subject: [PATCH 15/50] add disableSSLVerify false to calls to NewS3FileStore NewS3FileStore() takes an additional boolean argument for disableSSLVerify. Hardcode `false` to each of those calls in the test suite. --- filestore/s3_test.go | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/filestore/s3_test.go b/filestore/s3_test.go index ebbdc13..abbb3a5 100644 --- a/filestore/s3_test.go +++ b/filestore/s3_test.go @@ -83,7 +83,7 @@ func (t *TestSuite) TestConstructWithGoodBucketNames() { ls := b.String() t.Equal(63, len(ls), "incorrect string length") for _, bucket := range []string{"foo", ls} { - fstore, err := NewS3FileStore(cli, min, bucket) + fstore, err := NewS3FileStore(cli, min, bucket, false) t.NotNil(fstore, "expected filestore client") t.Nil(err, "unexpected error") } @@ -123,7 +123,7 @@ func (t *TestSuite) TestConstructFailBadBucketName() { } func constructFail(t *TestSuite, client *s3.S3, min *minio.Client, bucket string, expected error) { - fstore, err := NewS3FileStore(client, min, bucket) + fstore, err := NewS3FileStore(client, min, bucket, false) if err == nil { t.FailNow("expected error") } @@ -142,7 +142,7 @@ func (t *TestSuite) TestConstructWithExistingBucket() { if err != nil { t.FailNow(err.Error()) } - fstore, err := NewS3FileStore(s3client, mclient, bucket) + fstore, err := NewS3FileStore(s3client, mclient, bucket, false) if err != nil { t.FailNow(err.Error()) } @@ -192,7 +192,7 @@ func (t *TestSuite) storeAndGet( ) { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket") + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) p, _ := NewStoreFileParams( "myid", 12, @@ -241,7 +241,7 @@ func (t *TestSuite) storeAndGet( func (t *TestSuite) TestStoreWithNilInput() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket") + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) res, err := fstore.StoreFile(nil, &StoreFileParams{}) // DO NOT init SFP like this t.Nil(res, "expected error") @@ -256,7 +256,7 @@ func (t *TestSuite) TestStoreWithNilInput() { func (t *TestSuite) TestStoreWithIncorrectSize() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket") + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) p, _ := NewStoreFileParams( "myid", 11, @@ -278,7 +278,7 @@ func (t *TestSuite) TestStoreWithIncorrectSize() { func (t *TestSuite) TestStoreFailNoBucket() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket") + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) t.minio.Clear(false) @@ -308,7 +308,7 @@ func (t *TestSuite) TestStoreFailNoBucket() { func (t *TestSuite) TestGetWithBlankID() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket") + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) res, err := fstore.GetFile("", 0, 0) if res != nil { @@ -320,7 +320,7 @@ func (t *TestSuite) TestGetWithBlankID() { func (t *TestSuite) TestGetWithNonexistentID() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket") + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) p, _ := NewStoreFileParams( "myid", 12, @@ -344,7 +344,7 @@ func (t *TestSuite) assertNoFile(fstore FileStore, id string) { func (t *TestSuite) TestGetWithExcessSeek() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket") + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) p, _ := NewStoreFileParams( "myid", 12, @@ -372,7 +372,7 @@ func (t *TestSuite) TestGetWithoutMetaData() { id := "myid" s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, bkt) + fstore, _ := NewS3FileStore(s3client, mclient, bkt, false) _, err := s3client.PutObject(&s3.PutObjectInput{ Bucket: &bkt, @@ -414,7 +414,7 @@ func (t *TestSuite) TestGetWithoutMetaData() { func (t *TestSuite) TestDeleteObject() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket") + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) p, _ := NewStoreFileParams( "myid", 12, @@ -434,7 +434,7 @@ func (t *TestSuite) TestDeleteObject() { func (t *TestSuite) TestDeleteObjectWrongID() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket") + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) p, _ := NewStoreFileParams( "myid", 12, @@ -460,7 +460,7 @@ func (t *TestSuite) TestDeleteObjectWrongID() { func (t *TestSuite) TestDeleteWithBlankID() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket") + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) err := fstore.DeleteFile("") t.Equal(errors.New("id cannot be empty or whitespace only"), err, "incorrect err") @@ -469,7 +469,7 @@ func (t *TestSuite) TestDeleteWithBlankID() { func (t *TestSuite) TestDeleteFailNoBucket() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket") + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) t.minio.Clear(false) @@ -493,7 +493,7 @@ func (t *TestSuite) copy( format string) { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket") + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) p, _ := NewStoreFileParams( srcobj, 12, @@ -544,7 +544,7 @@ func (t *TestSuite) copy( func (t *TestSuite) TestCopyBadInput() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket") + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) t.copyFail(fstore, " \t \n ", "bar", errors.New("sourceID cannot be empty or whitespace only")) @@ -566,7 +566,7 @@ func (t *TestSuite) copyFail(fstore FileStore, src string, dst string, expected func (t *TestSuite) TestCopyNonExistentFile() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket") + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) p, _ := NewStoreFileParams( "myid", 12, @@ -594,7 +594,7 @@ func (t *TestSuite) testCopyLargeObject() { } s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket") + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) p, _ := NewStoreFileParams("myid", fi.Size(), reader) start := time.Now() obj, err := fstore.StoreFile(logrus.WithField("a", "b"), p) From 2202920a0a7bccedf5a98c00238aed29073e0f6c Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Tue, 8 Dec 2020 17:05:59 -0800 Subject: [PATCH 16/50] Disable ssl verify in PUT For example, when using a self-signed certificate, disable verifying the cert in the PUT of a new object. --- filestore/s3.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/filestore/s3.go b/filestore/s3.go index 90b69c5..5a94d28 100644 --- a/filestore/s3.go +++ b/filestore/s3.go @@ -131,7 +131,16 @@ func (fs *S3FileStore) StoreFile(le *logrus.Entry, p *StoreFileParams) (out *Fil req.Header.Set("x-amz-meta-Filename", p.filename) req.Header.Set("x-amz-meta-Format", p.format) - resp, err := http.DefaultClient.Do(req) + // disable SSL verify if necessary + customTransport := &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: fs.disableSSLverify}, + } +// Timeout: time.Second * 10, + httpClient := &http.DefaultClient{ + Transport: customTransport, + } + + resp, err := httpClient.Do(req) if err != nil { // don't expose the presigned url in the returned error errstr := err.(*url.Error).Err.Error() From 7a0bed70cd6df1b69d70305ea57e4e62728b2d68 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Tue, 8 Dec 2020 17:09:13 -0800 Subject: [PATCH 17/50] Add import Need crypto/tls to specify custom transport config. --- filestore/s3.go | 1 + 1 file changed, 1 insertion(+) diff --git a/filestore/s3.go b/filestore/s3.go index 5a94d28..972649a 100644 --- a/filestore/s3.go +++ b/filestore/s3.go @@ -6,6 +6,7 @@ import ( "io" "net/http" "net/url" + "crypto/tls" "strconv" "strings" "time" From 3d4ff6f411390536006a9d92e9561dc6d34916f7 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Tue, 8 Dec 2020 17:12:54 -0800 Subject: [PATCH 18/50] Use Client instead of DefaultClient (unsure what the real difference is if http.Client() just starts with a DefaultClient) --- filestore/s3.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filestore/s3.go b/filestore/s3.go index 972649a..4592e10 100644 --- a/filestore/s3.go +++ b/filestore/s3.go @@ -137,7 +137,7 @@ func (fs *S3FileStore) StoreFile(le *logrus.Entry, p *StoreFileParams) (out *Fil TLSClientConfig: &tls.Config{InsecureSkipVerify: fs.disableSSLverify}, } // Timeout: time.Second * 10, - httpClient := &http.DefaultClient{ + httpClient := &http.Client{ Transport: customTransport, } From 266aa7b4b4c2406fce1b87e9b113b4380461d101 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Thu, 10 Dec 2020 14:31:28 -0800 Subject: [PATCH 19/50] Add s3-disable-ssl-verify to config template --- deployment/conf/deployment.cfg.templ | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/deployment/conf/deployment.cfg.templ b/deployment/conf/deployment.cfg.templ index 59286ae..bfccc16 100644 --- a/deployment/conf/deployment.cfg.templ +++ b/deployment/conf/deployment.cfg.templ @@ -21,6 +21,7 @@ s3-access-key = {{ default .Env.s3_access_key "" }} s3-access-secret = {{ default .Env.s3_access_secret "" }} s3-region = {{ default .Env.s3_region "us-west-1" }} s3-disable-ssl = {{ default .Env.s3_disable_ssl "false" }} +s3-disable-ssl-verify = {{ default .Env.s3_disable_ssl_verify "false" }} # KBase auth server parameters. # The root url of the auth server. @@ -31,4 +32,4 @@ kbase-auth-admin-roles = {{ default .Env.kbase_auth_admin_roles "KBASE_ADMIN, BL # If "true", make the server ignore the X-Forwarded-For and X-Real-IP headers. Otherwise # (the default behavior), the logged IP address for a request, in order of precedence, is # 1) the first address in X-Forwarded-For, 2) X-Real-IP, and 3) the address of the client. -dont-trust-x-ip-headers = {{ default .Env.dont_trust_x_ip_headers "false" }} \ No newline at end of file +dont-trust-x-ip-headers = {{ default .Env.dont_trust_x_ip_headers "false" }} From 59ca9b731fe379222d532186b3e95bac2c43d815 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Thu, 10 Dec 2020 15:38:33 -0800 Subject: [PATCH 20/50] Update deploy.cfg.example --- deploy.cfg.example | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/deploy.cfg.example b/deploy.cfg.example index 5a6aeea..04af651 100644 --- a/deploy.cfg.example +++ b/deploy.cfg.example @@ -11,7 +11,7 @@ mongodb-database = blobstore #mongodb-user = [username] #mongodb-pwd = [password] -# S3 API parameters. All are required other than disable-ssl. +# S3 API parameters. All are required other than s3-disable-ssl and s3-disable-ssl-verify. # disable-ssl treats any value other than 'true' as false. s3-host = localhost:9000 # The bucket name must obey https://docs.aws.amazon.com/AmazonS3/latest/dev/BucketRestrictions.html @@ -20,7 +20,7 @@ s3-bucket = blobstore s3-access-key = [access key goes here] s3-access-secret = [access secret goes here] s3-region = us-west-1 -# Use plaintext to talk to destination S3. Default false. (false is not tested) +# Use plaintext to talk to destination S3. Default false. #s3-disable-ssl = false # Disable verifying the destination S3 SSL cert (e.g. for self-signed certs). Default false. #s3-disable-ssl-verify = false From d4936ce1ec45b0a3cf70fa85e50d8c6c302e3300 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Thu, 10 Dec 2020 15:40:41 -0800 Subject: [PATCH 21/50] Update build.go Fixed indentation --- service/build.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/build.go b/service/build.go index 22888b3..2fed937 100644 --- a/service/build.go +++ b/service/build.go @@ -78,7 +78,7 @@ func buildFileStore(cfg *config.Config) (filestore.FileStore, error) { minioClient, err := minio.NewWithRegion( cfg.S3Host, cfg.S3AccessKey, cfg.S3AccessSecret, !cfg.S3DisableSSL, cfg.S3Region) - minioClient.SetCustomTransport(customTransport) + minioClient.SetCustomTransport(customTransport) if err != nil { return nil, err From 709a65b50b97f193d767454019496082aca0b913 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Fri, 11 Dec 2020 14:47:47 -0800 Subject: [PATCH 22/50] Update build.go Put a custom http.Client in NewS3Filestore instead of the disableS3verify boolean, so that the Client can be shared across goroutines. --- service/build.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/service/build.go b/service/build.go index 2fed937..236e4d2 100644 --- a/service/build.go +++ b/service/build.go @@ -4,6 +4,7 @@ import ( "context" "net/http" "crypto/tls" + "time" "github.com/aws/aws-sdk-go/service/s3" @@ -66,7 +67,9 @@ func buildFileStore(cfg *config.Config) (filestore.FileStore, error) { customTransport := &http.Transport{ TLSClientConfig: &tls.Config{InsecureSkipVerify: cfg.S3DisableSSLVerify}, } - customHTTPClient := &http.Client{Transport: customTransport} + customHTTPClient := &http.Client{ + Transport: customTransport, + Timmeout: 24 * time.Hour } awscli := s3.New(sess, &aws.Config{ Credentials: creds, @@ -83,7 +86,7 @@ func buildFileStore(cfg *config.Config) (filestore.FileStore, error) { if err != nil { return nil, err } - return filestore.NewS3FileStore(awscli, minioClient, cfg.S3Bucket, cfg.S3DisableSSLVerify) + return filestore.NewS3FileStore(awscli, minioClient, cfg.S3Bucket, customHTTPClient) } func buildNodeStore(cfg *config.Config) (nodestore.NodeStore, error) { From 101d4bae20f69a31965d4a2c900807b52be39ce2 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Fri, 11 Dec 2020 14:52:00 -0800 Subject: [PATCH 23/50] Update s3.go Use constructor's shared customHTTPClient. --- filestore/s3.go | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/filestore/s3.go b/filestore/s3.go index 4592e10..5215627 100644 --- a/filestore/s3.go +++ b/filestore/s3.go @@ -46,7 +46,7 @@ func NewS3FileStore( s3client *s3.S3, minioClient *minio.Client, bucket string, - disableSSLverify bool, + customHTTPClient *http.Client, ) (*S3FileStore, error) { if s3client == nil { @@ -65,7 +65,7 @@ func NewS3FileStore( // Ignore for now. return nil, err } - return &S3FileStore{s3client: s3client, minioClient: minioClient, bucket: bucket, disableSSLverify: disableSSLverify}, nil + return &S3FileStore{s3client: s3client, minioClient: minioClient, bucket: bucket, customHTTPClient: customHTTPClient}, nil } func checkBucketName(bucket string) (string, error) { @@ -132,16 +132,7 @@ func (fs *S3FileStore) StoreFile(le *logrus.Entry, p *StoreFileParams) (out *Fil req.Header.Set("x-amz-meta-Filename", p.filename) req.Header.Set("x-amz-meta-Format", p.format) - // disable SSL verify if necessary - customTransport := &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: fs.disableSSLverify}, - } -// Timeout: time.Second * 10, - httpClient := &http.Client{ - Transport: customTransport, - } - - resp, err := httpClient.Do(req) + resp, err := fs.customHTTPClient.Do(req) if err != nil { // don't expose the presigned url in the returned error errstr := err.(*url.Error).Err.Error() From 87d1ec0a215ea8af11db190fb3eb0cc1d1a6adfb Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Fri, 11 Dec 2020 14:59:34 -0800 Subject: [PATCH 24/50] Update s3.go Fix some naming --- filestore/s3.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/filestore/s3.go b/filestore/s3.go index 5215627..05c6b5c 100644 --- a/filestore/s3.go +++ b/filestore/s3.go @@ -6,7 +6,6 @@ import ( "io" "net/http" "net/url" - "crypto/tls" "strconv" "strings" "time" @@ -34,7 +33,7 @@ type S3FileStore struct { s3client *s3.S3 minioClient *minio.Client bucket string - disableSSLverify bool + httpClient *http.Client } // NewS3FileStore creates a new S3 based file store. Files will be stored in the provided @@ -46,7 +45,7 @@ func NewS3FileStore( s3client *s3.S3, minioClient *minio.Client, bucket string, - customHTTPClient *http.Client, + httpClient *http.Client, ) (*S3FileStore, error) { if s3client == nil { @@ -65,7 +64,7 @@ func NewS3FileStore( // Ignore for now. return nil, err } - return &S3FileStore{s3client: s3client, minioClient: minioClient, bucket: bucket, customHTTPClient: customHTTPClient}, nil + return &S3FileStore{s3client: s3client, minioClient: minioClient, bucket: bucket, httpClient: httpClient}, nil } func checkBucketName(bucket string) (string, error) { @@ -132,7 +131,7 @@ func (fs *S3FileStore) StoreFile(le *logrus.Entry, p *StoreFileParams) (out *Fil req.Header.Set("x-amz-meta-Filename", p.filename) req.Header.Set("x-amz-meta-Format", p.format) - resp, err := fs.customHTTPClient.Do(req) + resp, err := fs.httpClient.Do(req) if err != nil { // don't expose the presigned url in the returned error errstr := err.(*url.Error).Err.Error() From cc425c619b5ae54fd1c80a906affa01d1f14a175 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Fri, 11 Dec 2020 15:02:41 -0800 Subject: [PATCH 25/50] Update build.go Fix typo --- service/build.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/build.go b/service/build.go index 236e4d2..9e1e5b9 100644 --- a/service/build.go +++ b/service/build.go @@ -69,7 +69,7 @@ func buildFileStore(cfg *config.Config) (filestore.FileStore, error) { } customHTTPClient := &http.Client{ Transport: customTransport, - Timmeout: 24 * time.Hour } + Timeout: 24 * time.Hour } awscli := s3.New(sess, &aws.Config{ Credentials: creds, From 02e7a821fe83cc74f5eac3f53beeadd35e2faa26 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Fri, 11 Dec 2020 15:17:58 -0800 Subject: [PATCH 26/50] Update blobstore.go --- app/blobstore.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/blobstore.go b/app/blobstore.go index 8a5b38e..79b4e2d 100644 --- a/app/blobstore.go +++ b/app/blobstore.go @@ -19,7 +19,7 @@ import ( const ( name = "blobstore" - version = "0.1.1" + version = "0.1.2" shockname = "Shock" shockver = "0.9.6" // do not increment deprecation = "The id and version fields are deprecated." From 004608bce6fc020540492a90e944c1047c68d83f Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Fri, 11 Dec 2020 15:24:16 -0800 Subject: [PATCH 27/50] Update s3_test.go Update tests for NewS3FileStore structure. --- filestore/s3_test.go | 49 +++++++++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/filestore/s3_test.go b/filestore/s3_test.go index abbb3a5..5e82c35 100644 --- a/filestore/s3_test.go +++ b/filestore/s3_test.go @@ -9,6 +9,7 @@ import ( "strings" "testing" "time" + "net/http" "github.com/kbase/blobstore/core/values" "github.com/sirupsen/logrus" @@ -28,6 +29,7 @@ type TestSuite struct { minio *miniocontroller.Controller loggerhook *logrust.Hook deleteTempDir bool + httpClient *http.Client } func (t *TestSuite) SetupSuite() { @@ -49,6 +51,15 @@ func (t *TestSuite) SetupSuite() { t.minio = minio t.deleteTempDir = tcfg.DeleteTempDir t.loggerhook = logrust.NewGlobal() + + customTransport := &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: false}, + } + customHTTPClient := &http.Client{ + Transport: customTransport, + Timeout: 24 * time.Hour } + t.httpClient = customHTTPClient + logrus.SetOutput(ioutil.Discard) } @@ -83,7 +94,7 @@ func (t *TestSuite) TestConstructWithGoodBucketNames() { ls := b.String() t.Equal(63, len(ls), "incorrect string length") for _, bucket := range []string{"foo", ls} { - fstore, err := NewS3FileStore(cli, min, bucket, false) + fstore, err := NewS3FileStore(cli, min, bucket, t.httpClient) t.NotNil(fstore, "expected filestore client") t.Nil(err, "unexpected error") } @@ -123,7 +134,7 @@ func (t *TestSuite) TestConstructFailBadBucketName() { } func constructFail(t *TestSuite, client *s3.S3, min *minio.Client, bucket string, expected error) { - fstore, err := NewS3FileStore(client, min, bucket, false) + fstore, err := NewS3FileStore(client, min, bucket, t.httpClient) if err == nil { t.FailNow("expected error") } @@ -142,7 +153,7 @@ func (t *TestSuite) TestConstructWithExistingBucket() { if err != nil { t.FailNow(err.Error()) } - fstore, err := NewS3FileStore(s3client, mclient, bucket, false) + fstore, err := NewS3FileStore(s3client, mclient, bucket, t.httpClient) if err != nil { t.FailNow(err.Error()) } @@ -192,7 +203,7 @@ func (t *TestSuite) storeAndGet( ) { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) p, _ := NewStoreFileParams( "myid", 12, @@ -241,7 +252,7 @@ func (t *TestSuite) storeAndGet( func (t *TestSuite) TestStoreWithNilInput() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) res, err := fstore.StoreFile(nil, &StoreFileParams{}) // DO NOT init SFP like this t.Nil(res, "expected error") @@ -256,7 +267,7 @@ func (t *TestSuite) TestStoreWithNilInput() { func (t *TestSuite) TestStoreWithIncorrectSize() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) p, _ := NewStoreFileParams( "myid", 11, @@ -278,7 +289,7 @@ func (t *TestSuite) TestStoreWithIncorrectSize() { func (t *TestSuite) TestStoreFailNoBucket() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) t.minio.Clear(false) @@ -308,7 +319,7 @@ func (t *TestSuite) TestStoreFailNoBucket() { func (t *TestSuite) TestGetWithBlankID() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) res, err := fstore.GetFile("", 0, 0) if res != nil { @@ -320,7 +331,7 @@ func (t *TestSuite) TestGetWithBlankID() { func (t *TestSuite) TestGetWithNonexistentID() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) p, _ := NewStoreFileParams( "myid", 12, @@ -344,7 +355,7 @@ func (t *TestSuite) assertNoFile(fstore FileStore, id string) { func (t *TestSuite) TestGetWithExcessSeek() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) p, _ := NewStoreFileParams( "myid", 12, @@ -372,7 +383,7 @@ func (t *TestSuite) TestGetWithoutMetaData() { id := "myid" s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, bkt, false) + fstore, _ := NewS3FileStore(s3client, mclient, bkt, t.httpClient) _, err := s3client.PutObject(&s3.PutObjectInput{ Bucket: &bkt, @@ -414,7 +425,7 @@ func (t *TestSuite) TestGetWithoutMetaData() { func (t *TestSuite) TestDeleteObject() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) p, _ := NewStoreFileParams( "myid", 12, @@ -434,7 +445,7 @@ func (t *TestSuite) TestDeleteObject() { func (t *TestSuite) TestDeleteObjectWrongID() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) p, _ := NewStoreFileParams( "myid", 12, @@ -460,7 +471,7 @@ func (t *TestSuite) TestDeleteObjectWrongID() { func (t *TestSuite) TestDeleteWithBlankID() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) err := fstore.DeleteFile("") t.Equal(errors.New("id cannot be empty or whitespace only"), err, "incorrect err") @@ -469,7 +480,7 @@ func (t *TestSuite) TestDeleteWithBlankID() { func (t *TestSuite) TestDeleteFailNoBucket() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) t.minio.Clear(false) @@ -493,7 +504,7 @@ func (t *TestSuite) copy( format string) { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) p, _ := NewStoreFileParams( srcobj, 12, @@ -544,7 +555,7 @@ func (t *TestSuite) copy( func (t *TestSuite) TestCopyBadInput() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) t.copyFail(fstore, " \t \n ", "bar", errors.New("sourceID cannot be empty or whitespace only")) @@ -566,7 +577,7 @@ func (t *TestSuite) copyFail(fstore FileStore, src string, dst string, expected func (t *TestSuite) TestCopyNonExistentFile() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) p, _ := NewStoreFileParams( "myid", 12, @@ -594,7 +605,7 @@ func (t *TestSuite) testCopyLargeObject() { } s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", false) + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) p, _ := NewStoreFileParams("myid", fi.Size(), reader) start := time.Now() obj, err := fstore.StoreFile(logrus.WithField("a", "b"), p) From 5b039b7c8b7a5485d2a138529b844a5c1fd691d2 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Fri, 11 Dec 2020 15:28:37 -0800 Subject: [PATCH 28/50] Update s3_test.go Need crypto/tls when creating custom client in the test. --- filestore/s3_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/filestore/s3_test.go b/filestore/s3_test.go index 5e82c35..e6242ed 100644 --- a/filestore/s3_test.go +++ b/filestore/s3_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" "net/http" + "crypto/tls" "github.com/kbase/blobstore/core/values" "github.com/sirupsen/logrus" From 00d8d149ed1f4d36b151c35885ba919c5aef5ee6 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Fri, 11 Dec 2020 15:45:45 -0800 Subject: [PATCH 29/50] Update RELEASE_NOTES.md --- RELEASE_NOTES.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 5b3bab4..21f6934 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,7 +1,11 @@ +# 0.1.2 + +- Support for disabling SSL verification of remote S3 certificates (default false) with the s3-disable-ssl-verify option in the configuration file. + # 0.1.1 - Added seek & length parameters to file download requests # 0.1.0 -- Initial release \ No newline at end of file +- Initial release From 3714831521cce8282809a4f1039941a411d92339 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Fri, 11 Dec 2020 15:51:27 -0800 Subject: [PATCH 30/50] Update build.go Added comments and links for using custom transports/clients with AWS and Minio clients. --- service/build.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/service/build.go b/service/build.go index 9e1e5b9..7ffe378 100644 --- a/service/build.go +++ b/service/build.go @@ -71,6 +71,9 @@ func buildFileStore(cfg *config.Config) (filestore.FileStore, error) { Transport: customTransport, Timeout: 24 * time.Hour } + // use our http.Client with the aws client + // this is encouraged, see https://docs.aws.amazon.com/sdk-for-go/api/aws/ + // (search for "SDK Default HTTP Client") awscli := s3.New(sess, &aws.Config{ Credentials: creds, Endpoint: &cfg.S3Host, @@ -81,6 +84,8 @@ func buildFileStore(cfg *config.Config) (filestore.FileStore, error) { minioClient, err := minio.NewWithRegion( cfg.S3Host, cfg.S3AccessKey, cfg.S3AccessSecret, !cfg.S3DisableSSL, cfg.S3Region) + // use our http.Transport with the minio client + // this is typical, see https://godoc.org/gopkg.in/minio/minio-go.v1#Client.SetCustomTransport minioClient.SetCustomTransport(customTransport) if err != nil { From 2b5fb1cc0539f736e23592ed13d2b6f8c6a63721 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Fri, 11 Dec 2020 16:48:29 -0800 Subject: [PATCH 31/50] Update blobstore.go Use shared constant to set the HTTP server timeouts (and pass to service constructor). --- app/blobstore.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/blobstore.go b/app/blobstore.go index 79b4e2d..6575ce2 100644 --- a/app/blobstore.go +++ b/app/blobstore.go @@ -23,6 +23,7 @@ const ( shockname = "Shock" shockver = "0.9.6" // do not increment deprecation = "The id and version fields are deprecated." + httpTimeout = 24 * time.Hour ) // expect initialization via go build -ldflags "-X main.gitCommit=$GIT_COMMIT" @@ -53,6 +54,7 @@ func main() { ServerVersionCompat: shockver, DeprecationWarning: deprecation, GitCommit: gitCommit, + HTTPTimeout: httpTimeout, }, ) if err != nil { @@ -62,8 +64,8 @@ func main() { server := &http.Server{ Addr: cfg.Host, Handler: serv, - ReadTimeout: 24 * time.Hour, - WriteTimeout: 24 * time.Hour, + ReadTimeout: httpTimeout, + WriteTimeout: httpTimeout, } // TODO BUGNASTY figure out how to abort when no more data is being sent https://groups.google.com/forum/#!topic/golang-nuts/Hmjf5Ws8g5w From 521cda94c991eb3236590b3cfd70c27352dca47e Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Fri, 11 Dec 2020 16:54:30 -0800 Subject: [PATCH 32/50] Update server.go Add HTTPTimeout to ServerStaticConf and constructDependencies() --- service/server.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/service/server.go b/service/server.go index bfb4a2e..beaaee5 100644 --- a/service/server.go +++ b/service/server.go @@ -55,6 +55,8 @@ type ServerStaticConf struct { DeprecationWarning string // GitCommit is the git commit from which the server was built. GitCommit string + // HTTPTimeout is the timeout of the S3 clients' http.Client + HTTPTimeout int } // Server the blobstore server @@ -73,7 +75,7 @@ func New(cfg *config.Config, sconf ServerStaticConf) (*Server, error) { if cfg.AuthURL.Scheme != "https" { logrus.Warnf("Insecure auth url " + cfg.AuthURL.String()) } - deps, err := constructDependencies(cfg) + deps, err := constructDependencies(cfg, sconf.HTTPTimeout) if err != nil { return nil, err // this is a pain to test } From 593b18aac0fbc7ff3afe7685425996d362671c10 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Fri, 11 Dec 2020 16:54:59 -0800 Subject: [PATCH 33/50] Update build.go Add HTTPTimeout to constructDependencies and buildFileStore --- service/build.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/service/build.go b/service/build.go index 7ffe378..d320f41 100644 --- a/service/build.go +++ b/service/build.go @@ -38,7 +38,7 @@ type Dependencies struct { } // ConstructDependencies builds the blobstore dependencies from a configuration. -func constructDependencies(cfg *config.Config) (*Dependencies, error) { +func constructDependencies(cfg *config.Config, HTTPTimeout int) (*Dependencies, error) { d := Dependencies{} auth, err := buildAuth(cfg) if err != nil { @@ -49,7 +49,7 @@ func constructDependencies(cfg *config.Config) (*Dependencies, error) { if err != nil { return nil, err } - fs, err := buildFileStore(cfg) + fs, err := buildFileStore(cfg, HTTPTimeout) if err != nil { return nil, err } @@ -57,7 +57,7 @@ func constructDependencies(cfg *config.Config) (*Dependencies, error) { return &d, nil } -func buildFileStore(cfg *config.Config) (filestore.FileStore, error) { +func buildFileStore(cfg *config.Config, HTTPTimeout int) (filestore.FileStore, error) { trueref := true sess := session.Must(session.NewSession()) @@ -69,7 +69,7 @@ func buildFileStore(cfg *config.Config) (filestore.FileStore, error) { } customHTTPClient := &http.Client{ Transport: customTransport, - Timeout: 24 * time.Hour } + Timeout: HTTPTimeout } // use our http.Client with the aws client // this is encouraged, see https://docs.aws.amazon.com/sdk-for-go/api/aws/ From 5453c9a2e5e5947a14690cffbb7027d95c8ca00e Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Fri, 11 Dec 2020 16:56:45 -0800 Subject: [PATCH 34/50] Update server.go Change type to time.Duration --- service/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/server.go b/service/server.go index beaaee5..32e50f3 100644 --- a/service/server.go +++ b/service/server.go @@ -56,7 +56,7 @@ type ServerStaticConf struct { // GitCommit is the git commit from which the server was built. GitCommit string // HTTPTimeout is the timeout of the S3 clients' http.Client - HTTPTimeout int + HTTPTimeout time.Duration } // Server the blobstore server From 08209e824ca3c9c2732572be89800fc745518001 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Fri, 11 Dec 2020 16:57:14 -0800 Subject: [PATCH 35/50] Update build.go change HTTPTimeout type to time.Duration --- service/build.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/service/build.go b/service/build.go index d320f41..ff3ba0e 100644 --- a/service/build.go +++ b/service/build.go @@ -38,7 +38,7 @@ type Dependencies struct { } // ConstructDependencies builds the blobstore dependencies from a configuration. -func constructDependencies(cfg *config.Config, HTTPTimeout int) (*Dependencies, error) { +func constructDependencies(cfg *config.Config, HTTPTimeout *time.Duration) (*Dependencies, error) { d := Dependencies{} auth, err := buildAuth(cfg) if err != nil { @@ -57,7 +57,7 @@ func constructDependencies(cfg *config.Config, HTTPTimeout int) (*Dependencies, return &d, nil } -func buildFileStore(cfg *config.Config, HTTPTimeout int) (filestore.FileStore, error) { +func buildFileStore(cfg *config.Config, HTTPTimeout *time.Duration) (filestore.FileStore, error) { trueref := true sess := session.Must(session.NewSession()) From 2d73cd9049171d1f4a4ff016bf6f3aba203e1fce Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Fri, 11 Dec 2020 16:57:53 -0800 Subject: [PATCH 36/50] Update build.go --- service/build.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/build.go b/service/build.go index ff3ba0e..b3273b8 100644 --- a/service/build.go +++ b/service/build.go @@ -57,7 +57,7 @@ func constructDependencies(cfg *config.Config, HTTPTimeout *time.Duration) (*Dep return &d, nil } -func buildFileStore(cfg *config.Config, HTTPTimeout *time.Duration) (filestore.FileStore, error) { +func buildFileStore(cfg *config.Config, HTTPTimeout time.Duration) (filestore.FileStore, error) { trueref := true sess := session.Must(session.NewSession()) From 5bad3627ea72c7371cebbcc2f289ebf23a57a498 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Fri, 11 Dec 2020 16:58:57 -0800 Subject: [PATCH 37/50] Update build.go --- service/build.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/build.go b/service/build.go index b3273b8..2e352ae 100644 --- a/service/build.go +++ b/service/build.go @@ -38,7 +38,7 @@ type Dependencies struct { } // ConstructDependencies builds the blobstore dependencies from a configuration. -func constructDependencies(cfg *config.Config, HTTPTimeout *time.Duration) (*Dependencies, error) { +func constructDependencies(cfg *config.Config, HTTPTimeout time.Duration) (*Dependencies, error) { d := Dependencies{} auth, err := buildAuth(cfg) if err != nil { From d96f739a17eb659a07780d00a7c7c4d5bef2e954 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Fri, 11 Dec 2020 17:03:21 -0800 Subject: [PATCH 38/50] Update config_test.go Add s3-disable-ssl-verify to config checks --- config/config_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/config/config_test.go b/config/config_test.go index dcbdbff..032143d 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -90,6 +90,7 @@ func (t *TestSuite) TestMinimalConfig() { S3AccessSecret: "sooporsekrit", S3Region: "us-west-1", S3DisableSSL: false, + S3DisableSSLVerify: false, AuthURL: u, AuthAdminRoles: &[]string{}, DontTrustXIPHeaders: false, @@ -109,6 +110,7 @@ func (t *TestSuite) TestMinimalConfigWhitespaceFields() { "s3-access-key = akey", "s3-access-secret = sooporsekrit", "s3-disable-ssl = \t tru ", + "s3-disable-ssl-verify = \t tru ", "s3-region = us-west-1 \t ", "kbase-auth-url = https://kbase.us/authyauth", "kbase-auth-admin-roles = \t ", @@ -127,6 +129,7 @@ func (t *TestSuite) TestMinimalConfigWhitespaceFields() { S3AccessSecret: "sooporsekrit", S3Region: "us-west-1", S3DisableSSL: false, + S3DisableSSLVerify: false, AuthURL: u, AuthAdminRoles: &[]string{}, DontTrustXIPHeaders: false, @@ -147,6 +150,7 @@ func (t *TestSuite) TestMaximalConfig() { "s3-access-secret = sooporsekrit", "s3-region = us-west-1", "s3-disable-ssl= true ", + "s3-disable-ssl-verify= true ", "kbase-auth-url = https://kbase.us/authyauth", "kbase-auth-admin-roles = \t , foo , \tbar\t , , baz ,,", "dont-trust-x-ip-headers = true \t ", @@ -165,6 +169,7 @@ func (t *TestSuite) TestMaximalConfig() { S3AccessKey: "akey", S3AccessSecret: "sooporsekrit", S3DisableSSL: true, + S3DisableSSLVerify: true, S3Region: "us-west-1", AuthURL: u, AuthAdminRoles: &[]string{"foo", "bar", "baz"}, From 9498065fa08d174ab651b3a33888629757d3919e Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Tue, 15 Dec 2020 14:49:40 -0800 Subject: [PATCH 39/50] Update s3_test.go Made helper httpClient() function to be called by each test that needs one. --- filestore/s3_test.go | 76 ++++++++++++++++++++++++++++---------------- 1 file changed, 49 insertions(+), 27 deletions(-) diff --git a/filestore/s3_test.go b/filestore/s3_test.go index e6242ed..e542b52 100644 --- a/filestore/s3_test.go +++ b/filestore/s3_test.go @@ -33,6 +33,17 @@ type TestSuite struct { httpClient *http.Client } +func httpClient() *http.Client { + + customTransport := &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: false}, + } + customHTTPClient := &http.Client{ + Transport: customTransport, + Timeout: 24 * time.Hour } + return customHTTPClient +} + func (t *TestSuite) SetupSuite() { tcfg, err := testhelpers.GetConfig() if err != nil { @@ -52,14 +63,6 @@ func (t *TestSuite) SetupSuite() { t.minio = minio t.deleteTempDir = tcfg.DeleteTempDir t.loggerhook = logrust.NewGlobal() - - customTransport := &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: false}, - } - customHTTPClient := &http.Client{ - Transport: customTransport, - Timeout: 24 * time.Hour } - t.httpClient = customHTTPClient logrus.SetOutput(ioutil.Discard) } @@ -94,8 +97,9 @@ func (t *TestSuite) TestConstructWithGoodBucketNames() { } ls := b.String() t.Equal(63, len(ls), "incorrect string length") + httpClient := httpClient() for _, bucket := range []string{"foo", ls} { - fstore, err := NewS3FileStore(cli, min, bucket, t.httpClient) + fstore, err := NewS3FileStore(cli, min, bucket, httpClient) t.NotNil(fstore, "expected filestore client") t.Nil(err, "unexpected error") } @@ -135,7 +139,8 @@ func (t *TestSuite) TestConstructFailBadBucketName() { } func constructFail(t *TestSuite, client *s3.S3, min *minio.Client, bucket string, expected error) { - fstore, err := NewS3FileStore(client, min, bucket, t.httpClient) + httpClient := httpClient() + fstore, err := NewS3FileStore(client, min, bucket, httpClient) if err == nil { t.FailNow("expected error") } @@ -154,7 +159,8 @@ func (t *TestSuite) TestConstructWithExistingBucket() { if err != nil { t.FailNow(err.Error()) } - fstore, err := NewS3FileStore(s3client, mclient, bucket, t.httpClient) + httpClient := httpClient() + fstore, err := NewS3FileStore(s3client, mclient, bucket, httpClient) if err != nil { t.FailNow(err.Error()) } @@ -204,7 +210,8 @@ func (t *TestSuite) storeAndGet( ) { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) + httpClient := httpClient() + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) p, _ := NewStoreFileParams( "myid", 12, @@ -253,7 +260,8 @@ func (t *TestSuite) storeAndGet( func (t *TestSuite) TestStoreWithNilInput() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) + httpClient := httpClient() + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) res, err := fstore.StoreFile(nil, &StoreFileParams{}) // DO NOT init SFP like this t.Nil(res, "expected error") @@ -268,7 +276,8 @@ func (t *TestSuite) TestStoreWithNilInput() { func (t *TestSuite) TestStoreWithIncorrectSize() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) + httpClient := httpClient() + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) p, _ := NewStoreFileParams( "myid", 11, @@ -290,7 +299,8 @@ func (t *TestSuite) TestStoreWithIncorrectSize() { func (t *TestSuite) TestStoreFailNoBucket() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) + httpClient := httpClient() + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) t.minio.Clear(false) @@ -320,7 +330,8 @@ func (t *TestSuite) TestStoreFailNoBucket() { func (t *TestSuite) TestGetWithBlankID() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) + httpClient := httpClient() + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) res, err := fstore.GetFile("", 0, 0) if res != nil { @@ -332,7 +343,8 @@ func (t *TestSuite) TestGetWithBlankID() { func (t *TestSuite) TestGetWithNonexistentID() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) + httpClient := httpClient() + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) p, _ := NewStoreFileParams( "myid", 12, @@ -356,7 +368,8 @@ func (t *TestSuite) assertNoFile(fstore FileStore, id string) { func (t *TestSuite) TestGetWithExcessSeek() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) + httpClient := httpClient() + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) p, _ := NewStoreFileParams( "myid", 12, @@ -384,7 +397,8 @@ func (t *TestSuite) TestGetWithoutMetaData() { id := "myid" s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, bkt, t.httpClient) + httpClient := httpClient() + fstore, _ := NewS3FileStore(s3client, mclient, bkt, httpClient) _, err := s3client.PutObject(&s3.PutObjectInput{ Bucket: &bkt, @@ -426,7 +440,8 @@ func (t *TestSuite) TestGetWithoutMetaData() { func (t *TestSuite) TestDeleteObject() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) + httpClient := httpClient() + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) p, _ := NewStoreFileParams( "myid", 12, @@ -446,7 +461,8 @@ func (t *TestSuite) TestDeleteObject() { func (t *TestSuite) TestDeleteObjectWrongID() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) + httpClient := httpClient() + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) p, _ := NewStoreFileParams( "myid", 12, @@ -472,7 +488,8 @@ func (t *TestSuite) TestDeleteObjectWrongID() { func (t *TestSuite) TestDeleteWithBlankID() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) + httpClient := httpClient() + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) err := fstore.DeleteFile("") t.Equal(errors.New("id cannot be empty or whitespace only"), err, "incorrect err") @@ -481,7 +498,8 @@ func (t *TestSuite) TestDeleteWithBlankID() { func (t *TestSuite) TestDeleteFailNoBucket() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) + httpClient := httpClient() + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) t.minio.Clear(false) @@ -505,7 +523,8 @@ func (t *TestSuite) copy( format string) { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) + httpClient := httpClient() + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) p, _ := NewStoreFileParams( srcobj, 12, @@ -556,7 +575,8 @@ func (t *TestSuite) copy( func (t *TestSuite) TestCopyBadInput() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) + httpClient := httpClient() + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) t.copyFail(fstore, " \t \n ", "bar", errors.New("sourceID cannot be empty or whitespace only")) @@ -578,7 +598,8 @@ func (t *TestSuite) copyFail(fstore FileStore, src string, dst string, expected func (t *TestSuite) TestCopyNonExistentFile() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) + httpClient := httpClient() + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) p, _ := NewStoreFileParams( "myid", 12, @@ -606,7 +627,8 @@ func (t *TestSuite) testCopyLargeObject() { } s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", t.httpClient) + httpClient := httpClient() + fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) p, _ := NewStoreFileParams("myid", fi.Size(), reader) start := time.Now() obj, err := fstore.StoreFile(logrus.WithField("a", "b"), p) From 047c78b4acb93812f5b7c0c25f36f78d6122bddc Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Tue, 15 Dec 2020 14:56:46 -0800 Subject: [PATCH 40/50] Update s3_test.go Remove http.Client from TestSuite struct. --- filestore/s3_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/filestore/s3_test.go b/filestore/s3_test.go index e542b52..1a8cf63 100644 --- a/filestore/s3_test.go +++ b/filestore/s3_test.go @@ -30,7 +30,6 @@ type TestSuite struct { minio *miniocontroller.Controller loggerhook *logrust.Hook deleteTempDir bool - httpClient *http.Client } func httpClient() *http.Client { From 4737526a5b217ac880f7cb9c3da850b04886276b Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Thu, 17 Dec 2020 15:01:50 -0800 Subject: [PATCH 41/50] Update build.go try to fix indentation --- service/build.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/service/build.go b/service/build.go index 2e352ae..5ddb34a 100644 --- a/service/build.go +++ b/service/build.go @@ -68,8 +68,8 @@ func buildFileStore(cfg *config.Config, HTTPTimeout time.Duration) (filestore.Fi TLSClientConfig: &tls.Config{InsecureSkipVerify: cfg.S3DisableSSLVerify}, } customHTTPClient := &http.Client{ - Transport: customTransport, - Timeout: HTTPTimeout } + Transport: customTransport, + Timeout: HTTPTimeout } // use our http.Client with the aws client // this is encouraged, see https://docs.aws.amazon.com/sdk-for-go/api/aws/ From 153870274d4aaacfa49b86c92290f9d4455a9be4 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Thu, 17 Dec 2020 15:04:30 -0800 Subject: [PATCH 42/50] Update server.go Clarified the HTTPTimeout variable --- service/server.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/service/server.go b/service/server.go index 32e50f3..e331aef 100644 --- a/service/server.go +++ b/service/server.go @@ -55,7 +55,9 @@ type ServerStaticConf struct { DeprecationWarning string // GitCommit is the git commit from which the server was built. GitCommit string - // HTTPTimeout is the timeout of the S3 clients' http.Client + // HTTPTimeout is the timeout of the blobstore http.Server, + // the minio http.Client, the AWS http.Client, + // and the custom http.Client that pushes to S3 HTTPTimeout time.Duration } From f2a570d951e619a4083a7eab85029484dacf3b0a Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Thu, 17 Dec 2020 15:05:46 -0800 Subject: [PATCH 43/50] Update s3_test.go try to fix indentation --- filestore/s3_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/filestore/s3_test.go b/filestore/s3_test.go index 1a8cf63..c14828a 100644 --- a/filestore/s3_test.go +++ b/filestore/s3_test.go @@ -38,8 +38,8 @@ func httpClient() *http.Client { TLSClientConfig: &tls.Config{InsecureSkipVerify: false}, } customHTTPClient := &http.Client{ - Transport: customTransport, - Timeout: 24 * time.Hour } + Transport: customTransport, + Timeout: 24 * time.Hour } return customHTTPClient } From dcd87cb3db32eca2c1bdff764fdb479f83278c60 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Thu, 17 Dec 2020 15:43:49 -0800 Subject: [PATCH 44/50] Update s3.go Make sure httpClient is not nil --- filestore/s3.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/filestore/s3.go b/filestore/s3.go index 05c6b5c..a081637 100644 --- a/filestore/s3.go +++ b/filestore/s3.go @@ -54,6 +54,9 @@ func NewS3FileStore( if minioClient == nil { return nil, errors.New("minioClient cannot be nil") } + if httpClient == nil { + return nil, errors.New("httpClient cannot be nil") + } bucket, err := checkBucketName(bucket) if err != nil { return nil, err From d22bc0bbacf464781290b72232da44545602fed2 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Thu, 17 Dec 2020 15:48:42 -0800 Subject: [PATCH 45/50] Update s3_test.go Added test for nil httpClient --- filestore/s3_test.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/filestore/s3_test.go b/filestore/s3_test.go index c14828a..b00f127 100644 --- a/filestore/s3_test.go +++ b/filestore/s3_test.go @@ -107,13 +107,16 @@ func (t *TestSuite) TestConstructWithGoodBucketNames() { func (t *TestSuite) TestConstructFail() { min, _ := t.minio.CreateMinioClient() cli := t.minio.CreateS3Client() - constructFail(t, nil, min, "s", errors.New("s3client cannot be nil")) - constructFail(t, cli, nil, "s", errors.New("minioClient cannot be nil")) + httpClient := httpClient() + constructFail(t, nil, min, httpClient, "s", errors.New("s3client cannot be nil")) + constructFail(t, cli, nil, httpClient, "s", errors.New("minioClient cannot be nil")) + constructFail(t, cli, min, nil, "s", errors.New("httpClient cannot be nil")) } func (t *TestSuite) TestConstructFailBadBucketName() { min, _ := t.minio.CreateMinioClient() cli := t.minio.CreateS3Client() + httpClient := httpClient() b := strings.Builder{} for i := 0; i < 6; i++ { @@ -133,12 +136,11 @@ func (t *TestSuite) TestConstructFailBadBucketName() { } for bucket, er := range testcases { - constructFail(t, cli, min, bucket, errors.New(er)) + constructFail(t, cli, min, httpClient, bucket, errors.New(er)) } } -func constructFail(t *TestSuite, client *s3.S3, min *minio.Client, bucket string, expected error) { - httpClient := httpClient() +func constructFail(t *TestSuite, client *s3.S3, min *minio.Client, httpClient *http.Client, bucket string, expected error) { fstore, err := NewS3FileStore(client, min, bucket, httpClient) if err == nil { t.FailNow("expected error") @@ -152,13 +154,13 @@ func constructFail(t *TestSuite, client *s3.S3, min *minio.Client, bucket string func (t *TestSuite) TestConstructWithExistingBucket() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() + httpClient := httpClient() bucket := "somebucket" input := &s3.CreateBucketInput{Bucket: aws.String(bucket)} _, err := s3client.CreateBucket(input) if err != nil { t.FailNow(err.Error()) } - httpClient := httpClient() fstore, err := NewS3FileStore(s3client, mclient, bucket, httpClient) if err != nil { t.FailNow(err.Error()) From 9d623e1529fe05bb7c76122753a4555b95edf524 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Thu, 17 Dec 2020 17:15:00 -0800 Subject: [PATCH 46/50] Test insecure skip verify Refactor storeAndGet in order to test when insecure skip verify is true. --- filestore/s3_test.go | 66 ++++++++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 30 deletions(-) diff --git a/filestore/s3_test.go b/filestore/s3_test.go index b00f127..5930f71 100644 --- a/filestore/s3_test.go +++ b/filestore/s3_test.go @@ -32,10 +32,10 @@ type TestSuite struct { deleteTempDir bool } -func httpClient() *http.Client { +func httpClient(insecureSkipVerify bool) *http.Client { customTransport := &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: false}, + TLSClientConfig: &tls.Config{InsecureSkipVerify: insecureSkipVerify}, } customHTTPClient := &http.Client{ Transport: customTransport, @@ -96,7 +96,7 @@ func (t *TestSuite) TestConstructWithGoodBucketNames() { } ls := b.String() t.Equal(63, len(ls), "incorrect string length") - httpClient := httpClient() + httpClient := httpClient(false) for _, bucket := range []string{"foo", ls} { fstore, err := NewS3FileStore(cli, min, bucket, httpClient) t.NotNil(fstore, "expected filestore client") @@ -107,7 +107,7 @@ func (t *TestSuite) TestConstructWithGoodBucketNames() { func (t *TestSuite) TestConstructFail() { min, _ := t.minio.CreateMinioClient() cli := t.minio.CreateS3Client() - httpClient := httpClient() + httpClient := httpClient(false) constructFail(t, nil, min, httpClient, "s", errors.New("s3client cannot be nil")) constructFail(t, cli, nil, httpClient, "s", errors.New("minioClient cannot be nil")) constructFail(t, cli, min, nil, "s", errors.New("httpClient cannot be nil")) @@ -116,7 +116,7 @@ func (t *TestSuite) TestConstructFail() { func (t *TestSuite) TestConstructFailBadBucketName() { min, _ := t.minio.CreateMinioClient() cli := t.minio.CreateS3Client() - httpClient := httpClient() + httpClient := httpClient(false) b := strings.Builder{} for i := 0; i < 6; i++ { @@ -154,7 +154,7 @@ func constructFail(t *TestSuite, client *s3.S3, min *minio.Client, httpClient *h func (t *TestSuite) TestConstructWithExistingBucket() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - httpClient := httpClient() + httpClient := httpClient(false) bucket := "somebucket" input := &s3.CreateBucketInput{Bucket: aws.String(bucket)} _, err := s3client.CreateBucket(input) @@ -172,34 +172,39 @@ func (t *TestSuite) TestConstructWithExistingBucket() { } func (t *TestSuite) TestStoreAndGet() { - t.storeAndGet("", "", 0, 0, "012345678910") + t.storeAndGet("", "", 0, 0, false, "012345678910") } + +func (t *TestSuite) TestStoreAndGetInsecureVerify() { + t.storeAndGet("", "", 0, 0, true, "012345678910") +} + func (t *TestSuite) TestStoreAndGetWithMeta() { - t.storeAndGet("fn", "json", 0, 0, "012345678910") + t.storeAndGet("fn", "json", 0, 0, false, "012345678910") } func (t *TestSuite) TestStoreAndGetWithSeek() { - t.storeAndGet("", "", 3, 0, "345678910") + t.storeAndGet("", "", 3, 0, false, "345678910") } func (t *TestSuite) TestStoreAndGetWithExactSeek() { - t.storeAndGet("", "", 11, 0, "0") + t.storeAndGet("", "", 11, 0, false, "0") } func (t *TestSuite) TestStoreAndGetWithLength() { - t.storeAndGet("", "", 0, 8, "01234567") + t.storeAndGet("", "", 0, 8, false, "01234567") } func (t *TestSuite) TestStoreAndGetWithSeekAndLength() { - t.storeAndGet("", "", 1, 5, "12345") + t.storeAndGet("", "", 1, 5, false, "12345") } func (t *TestSuite) TestStoreAndGetWithSeekAndExactLength() { - t.storeAndGet("", "", 1, 11, "12345678910") + t.storeAndGet("", "", 1, 11, false, "12345678910") } func (t *TestSuite) TestStoreAndGetWithSeekAndExcessLength() { - t.storeAndGet("", "", 1, 15, "12345678910") + t.storeAndGet("", "", 1, 15, false, "12345678910") } func (t *TestSuite) storeAndGet( @@ -207,11 +212,12 @@ func (t *TestSuite) storeAndGet( format string, seek uint64, length uint64, + insecureSkipVerify bool, expectedfile string, ) { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - httpClient := httpClient() + httpClient := httpClient(insecureSkipVerify) fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) p, _ := NewStoreFileParams( "myid", @@ -261,7 +267,7 @@ func (t *TestSuite) storeAndGet( func (t *TestSuite) TestStoreWithNilInput() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - httpClient := httpClient() + httpClient := httpClient(false) fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) res, err := fstore.StoreFile(nil, &StoreFileParams{}) // DO NOT init SFP like this @@ -277,7 +283,7 @@ func (t *TestSuite) TestStoreWithNilInput() { func (t *TestSuite) TestStoreWithIncorrectSize() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - httpClient := httpClient() + httpClient := httpClient(false) fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) p, _ := NewStoreFileParams( "myid", @@ -300,7 +306,7 @@ func (t *TestSuite) TestStoreWithIncorrectSize() { func (t *TestSuite) TestStoreFailNoBucket() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - httpClient := httpClient() + httpClient := httpClient(false) fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) t.minio.Clear(false) @@ -331,7 +337,7 @@ func (t *TestSuite) TestStoreFailNoBucket() { func (t *TestSuite) TestGetWithBlankID() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - httpClient := httpClient() + httpClient := httpClient(false) fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) res, err := fstore.GetFile("", 0, 0) @@ -344,7 +350,7 @@ func (t *TestSuite) TestGetWithBlankID() { func (t *TestSuite) TestGetWithNonexistentID() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - httpClient := httpClient() + httpClient := httpClient(false) fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) p, _ := NewStoreFileParams( "myid", @@ -369,7 +375,7 @@ func (t *TestSuite) assertNoFile(fstore FileStore, id string) { func (t *TestSuite) TestGetWithExcessSeek() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - httpClient := httpClient() + httpClient := httpClient(false) fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) p, _ := NewStoreFileParams( "myid", @@ -398,7 +404,7 @@ func (t *TestSuite) TestGetWithoutMetaData() { id := "myid" s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - httpClient := httpClient() + httpClient := httpClient(false) fstore, _ := NewS3FileStore(s3client, mclient, bkt, httpClient) _, err := s3client.PutObject(&s3.PutObjectInput{ @@ -441,7 +447,7 @@ func (t *TestSuite) TestGetWithoutMetaData() { func (t *TestSuite) TestDeleteObject() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - httpClient := httpClient() + httpClient := httpClient(false) fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) p, _ := NewStoreFileParams( "myid", @@ -462,7 +468,7 @@ func (t *TestSuite) TestDeleteObject() { func (t *TestSuite) TestDeleteObjectWrongID() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - httpClient := httpClient() + httpClient := httpClient(false) fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) p, _ := NewStoreFileParams( "myid", @@ -489,7 +495,7 @@ func (t *TestSuite) TestDeleteObjectWrongID() { func (t *TestSuite) TestDeleteWithBlankID() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - httpClient := httpClient() + httpClient := httpClient(false) fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) err := fstore.DeleteFile("") @@ -499,7 +505,7 @@ func (t *TestSuite) TestDeleteWithBlankID() { func (t *TestSuite) TestDeleteFailNoBucket() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - httpClient := httpClient() + httpClient := httpClient(false) fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) t.minio.Clear(false) @@ -524,7 +530,7 @@ func (t *TestSuite) copy( format string) { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - httpClient := httpClient() + httpClient := httpClient(false) fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) p, _ := NewStoreFileParams( srcobj, @@ -576,7 +582,7 @@ func (t *TestSuite) copy( func (t *TestSuite) TestCopyBadInput() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - httpClient := httpClient() + httpClient := httpClient(false) fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) t.copyFail(fstore, " \t \n ", "bar", @@ -599,7 +605,7 @@ func (t *TestSuite) copyFail(fstore FileStore, src string, dst string, expected func (t *TestSuite) TestCopyNonExistentFile() { s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - httpClient := httpClient() + httpClient := httpClient(false) fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) p, _ := NewStoreFileParams( "myid", @@ -628,7 +634,7 @@ func (t *TestSuite) testCopyLargeObject() { } s3client := t.minio.CreateS3Client() mclient, _ := t.minio.CreateMinioClient() - httpClient := httpClient() + httpClient := httpClient(false) fstore, _ := NewS3FileStore(s3client, mclient, "mybucket", httpClient) p, _ := NewStoreFileParams("myid", fi.Size(), reader) start := time.Now() From bb01f19dac112bb4e435f7e2f62e9ca07eb34d1f Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Fri, 18 Dec 2020 11:34:28 -0800 Subject: [PATCH 47/50] Update build.go Added a comment to test against a minio with a self-signed SSL cert if changing the http.Client code. --- service/build.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/service/build.go b/service/build.go index 5ddb34a..8424760 100644 --- a/service/build.go +++ b/service/build.go @@ -64,6 +64,8 @@ func buildFileStore(cfg *config.Config, HTTPTimeout time.Duration) (filestore.Fi creds := credentials.NewStaticCredentials(cfg.S3AccessKey, cfg.S3AccessSecret, "") // need a custom transport to support not verifying SSL cert + // if you modify the SSL code, be sure to manually test against + // a minio instance with a self-signed certificate customTransport := &http.Transport{ TLSClientConfig: &tls.Config{InsecureSkipVerify: cfg.S3DisableSSLVerify}, } From cf1a80ff47901fb6c864a22dc9c8a07ba488f5bd Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Fri, 18 Dec 2020 15:28:45 -0800 Subject: [PATCH 48/50] Update build.go Another attempt to fix indentation --- service/build.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/service/build.go b/service/build.go index 8424760..4296afd 100644 --- a/service/build.go +++ b/service/build.go @@ -67,10 +67,10 @@ func buildFileStore(cfg *config.Config, HTTPTimeout time.Duration) (filestore.Fi // if you modify the SSL code, be sure to manually test against // a minio instance with a self-signed certificate customTransport := &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: cfg.S3DisableSSLVerify}, + TLSClientConfig: &tls.Config{InsecureSkipVerify: cfg.S3DisableSSLVerify}, } customHTTPClient := &http.Client{ - Transport: customTransport, + Transport: customTransport, Timeout: HTTPTimeout } // use our http.Client with the aws client From 854214eea9f8ec2108593dd35e2283b100a62d8d Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Fri, 18 Dec 2020 15:32:16 -0800 Subject: [PATCH 49/50] Update s3.go Added comments about the various clients used by the constructor. --- filestore/s3.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/filestore/s3.go b/filestore/s3.go index a081637..4cad268 100644 --- a/filestore/s3.go +++ b/filestore/s3.go @@ -39,7 +39,10 @@ type S3FileStore struct { // NewS3FileStore creates a new S3 based file store. Files will be stored in the provided // bucket, which will be created if it doesn't exist. The provided clients must have write // privileges for the bucket. -// Two clients are currently required because they are better at different operations. +// Three clients are currently required because they are better at different operations: +// s3client: an aws-sdk s3 client +// minioClient: a minio-go client +// httpClient: an http.Client client (used directly for faster PUTs) // This may change in a future version if one client provides all the necessary operations. func NewS3FileStore( s3client *s3.S3, From b56d8326e66aea8c04894afc6c77677813cc2b64 Mon Sep 17 00:00:00 2001 From: kkellerlbl Date: Fri, 18 Dec 2020 15:34:09 -0800 Subject: [PATCH 50/50] Update build.go Added comment about arguments for constructDependencies --- service/build.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/service/build.go b/service/build.go index 4296afd..77bcec8 100644 --- a/service/build.go +++ b/service/build.go @@ -38,6 +38,8 @@ type Dependencies struct { } // ConstructDependencies builds the blobstore dependencies from a configuration. +// cfg is a configuration structure populated from the config file +// HTTPTimeout is a timeout for the various clients connecting to the S3 backend (s3, minio, http) func constructDependencies(cfg *config.Config, HTTPTimeout time.Duration) (*Dependencies, error) { d := Dependencies{} auth, err := buildAuth(cfg)