From d856ee06354c4c3e47a832f59a19c82d49c2ddf7 Mon Sep 17 00:00:00 2001 From: bestmike007 Date: Mon, 15 Jan 2024 10:19:06 -0600 Subject: [PATCH 1/9] feat: presign url using blob storage Signed-off-by: bestmike007 --- internal/config/config.go | 5 +- internal/server/handler.go | 13 +---- .../storage/blobstorage/gcs/blob_storage.go | 51 +++++++++++++------ .../gcs/blob_storage_integration_test.go | 3 +- .../blobstorage/internal/blobstorage.go | 1 + internal/storage/blobstorage/mocks/mocks.go | 15 ++++++ .../storage/blobstorage/s3/blob_storage.go | 18 +++++++ 7 files changed, 76 insertions(+), 30 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index d8325d77..b05e0bbe 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -126,8 +126,9 @@ type ( } GcpConfig struct { - Project string `mapstructure:"project" validate:"required"` - Bucket string `mapstructure:"bucket"` + Project string `mapstructure:"project" validate:"required"` + Bucket string `mapstructure:"bucket"` + PresignedUrlExpiration *time.Duration `mapstructure:"presigned_url_expiration"` } DynamoDBConfig struct { diff --git a/internal/server/handler.go b/internal/server/handler.go index 5681f654..307893cf 100644 --- a/internal/server/handler.go +++ b/internal/server/handler.go @@ -14,8 +14,6 @@ import ( grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware" "google.golang.org/grpc/reflection" - "github.com/aws/aws-sdk-go/aws" - awss3 "github.com/aws/aws-sdk-go/service/s3" "github.com/cenkalti/backoff" "github.com/uber-go/tally/v4" "go.uber.org/fx" @@ -55,7 +53,6 @@ type ( metaStorage metastorage.MetaStorage blobStorage blobstorage.BlobStorage transactionStorage metastorage.TransactionStorage - s3Client s3.Client blockchainClient client.Client parser parser.Parser metrics *serverMetrics @@ -204,7 +201,6 @@ func NewServer(params ServerParams) *Server { metaStorage: params.MetaStorage, blobStorage: params.BlobStorage, transactionStorage: params.TransactionStorage, - s3Client: params.S3Client, blockchainClient: params.BlockchainClient, parser: params.Parser, metrics: newServerMetrics(params.Metrics), @@ -717,14 +713,9 @@ func (s *Server) newBlockFile(block *api.BlockMetadata) (*api.BlockFile, error) key := block.GetObjectKeyMain() compression := storage_utils.GetCompressionType(key) - getObjectReq, _ := s.s3Client.GetObjectRequest(&awss3.GetObjectInput{ - Bucket: aws.String(s.config.AWS.Bucket), - Key: aws.String(key), - }) - fileUrl, err := getObjectReq.Presign(s.config.AWS.PresignedUrlExpiration) + fileUrl, err := s.blobStorage.PreSign(context.Background(), key) if err != nil { - s.logger.Error("block file s3 presign error", zap.Reflect("block", block), zap.Error(err)) - return nil, status.Errorf(codes.Internal, "internal block file url generation error: %+v", err) + return nil, err } return &api.BlockFile{ diff --git a/internal/storage/blobstorage/gcs/blob_storage.go b/internal/storage/blobstorage/gcs/blob_storage.go index dfc3fe64..20b258a7 100644 --- a/internal/storage/blobstorage/gcs/blob_storage.go +++ b/internal/storage/blobstorage/gcs/blob_storage.go @@ -9,10 +9,12 @@ import ( "time" "cloud.google.com/go/storage" + "github.com/gogo/status" "github.com/uber-go/tally/v4" "go.uber.org/fx" "go.uber.org/zap" "golang.org/x/xerrors" + "google.golang.org/grpc/codes" "google.golang.org/protobuf/proto" "github.com/coinbase/chainstorage/internal/config" @@ -36,14 +38,15 @@ type ( } blobStorageImpl struct { - logger *zap.Logger - config *config.Config - project string - bucket string - client *storage.Client - blobStorageMetrics *blobStorageMetrics - instrumentUpload instrument.InstrumentWithResult[string] - instrumentDownload instrument.InstrumentWithResult[*api.Block] + logger *zap.Logger + config *config.Config + project string + bucket string + client *storage.Client + presignedUrlExpiration time.Duration + blobStorageMetrics *blobStorageMetrics + instrumentUpload instrument.InstrumentWithResult[string] + instrumentDownload instrument.InstrumentWithResult[*api.Block] } blobStorageMetrics struct { @@ -79,6 +82,9 @@ func New(params BlobStorageParams) (internal.BlobStorage, error) { if len(params.Config.GCP.Bucket) == 0 { return nil, xerrors.Errorf("GCP bucket not configure for blob storage") } + if params.Config.GCP.PresignedUrlExpiration == nil { + return nil, xerrors.Errorf("GCP presign url expiration not configure for blob storage") + } ctx := context.Background() client, err := storage.NewClient(ctx) if err != nil { @@ -89,14 +95,15 @@ func New(params BlobStorageParams) (internal.BlobStorage, error) { blobUploadedSize: metrics.SubScope(blobUploaderScopeName).Timer(blobSizeMetricName), } return &blobStorageImpl{ - logger: log.WithPackage(params.Logger), - config: params.Config, - project: params.Config.GCP.Project, - bucket: params.Config.GCP.Bucket, - client: client, - blobStorageMetrics: blobStorageMetrics, - instrumentUpload: instrument.NewWithResult[string](metrics, "upload"), - instrumentDownload: instrument.NewWithResult[*api.Block](metrics, "download"), + logger: log.WithPackage(params.Logger), + config: params.Config, + project: params.Config.GCP.Project, + bucket: params.Config.GCP.Bucket, + client: client, + presignedUrlExpiration: *params.Config.GCP.PresignedUrlExpiration, + blobStorageMetrics: blobStorageMetrics, + instrumentUpload: instrument.NewWithResult[string](metrics, "upload"), + instrumentDownload: instrument.NewWithResult[*api.Block](metrics, "download"), }, nil } @@ -226,6 +233,18 @@ func (s *blobStorageImpl) Download(ctx context.Context, metadata *api.BlockMetad }) } +// PreSign implements internal.BlobStorage. +func (s *blobStorageImpl) PreSign(ctx context.Context, objectKey string) (string, error) { + fileUrl, err := s.client.Bucket(s.bucket).SignedURL(objectKey, &storage.SignedURLOptions{ + Expires: time.Now().Add(s.presignedUrlExpiration), + }) + if err != nil { + s.logger.Error("block file gcs presign error", zap.Reflect("key", objectKey), zap.Error(err)) + return "", status.Errorf(codes.Internal, "internal block file url generation error: %+v", err) + } + return fileUrl, nil +} + func (s *blobStorageImpl) logDuration(method string, start time.Time) { s.logger.Debug( "blob_storage", diff --git a/internal/storage/blobstorage/gcs/blob_storage_integration_test.go b/internal/storage/blobstorage/gcs/blob_storage_integration_test.go index 80c20ed9..288b2290 100644 --- a/internal/storage/blobstorage/gcs/blob_storage_integration_test.go +++ b/internal/storage/blobstorage/gcs/blob_storage_integration_test.go @@ -7,13 +7,14 @@ import ( "go.uber.org/fx" "google.golang.org/protobuf/proto" + "github.com/stretchr/testify/suite" + "github.com/coinbase/chainstorage/internal/config" "github.com/coinbase/chainstorage/internal/storage/blobstorage/internal" "github.com/coinbase/chainstorage/internal/utils/testapp" "github.com/coinbase/chainstorage/internal/utils/testutil" "github.com/coinbase/chainstorage/protos/coinbase/c3/common" api "github.com/coinbase/chainstorage/protos/coinbase/chainstorage" - "github.com/stretchr/testify/suite" ) type gcpBlobStorageTestSuite struct { diff --git a/internal/storage/blobstorage/internal/blobstorage.go b/internal/storage/blobstorage/internal/blobstorage.go index 8e215180..a2dd6fb2 100644 --- a/internal/storage/blobstorage/internal/blobstorage.go +++ b/internal/storage/blobstorage/internal/blobstorage.go @@ -15,6 +15,7 @@ type ( BlobStorage interface { Upload(ctx context.Context, block *api.Block, compression api.Compression) (string, error) Download(ctx context.Context, metadata *api.BlockMetadata) (*api.Block, error) + PreSign(ctx context.Context, objectKey string) (string, error) } BlobStorageFactory interface { diff --git a/internal/storage/blobstorage/mocks/mocks.go b/internal/storage/blobstorage/mocks/mocks.go index 68a244b1..6a20177e 100644 --- a/internal/storage/blobstorage/mocks/mocks.go +++ b/internal/storage/blobstorage/mocks/mocks.go @@ -55,6 +55,21 @@ func (mr *MockBlobStorageMockRecorder) Download(arg0, arg1 any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Download", reflect.TypeOf((*MockBlobStorage)(nil).Download), arg0, arg1) } +// PreSign mocks base method. +func (m *MockBlobStorage) PreSign(arg0 context.Context, arg1 string) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "PreSign", arg0, arg1) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// PreSign indicates an expected call of PreSign. +func (mr *MockBlobStorageMockRecorder) PreSign(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PreSign", reflect.TypeOf((*MockBlobStorage)(nil).PreSign), arg0, arg1) +} + // Upload mocks base method. func (m *MockBlobStorage) Upload(arg0 context.Context, arg1 *chainstorage.Block, arg2 chainstorage.Compression) (string, error) { m.ctrl.T.Helper() diff --git a/internal/storage/blobstorage/s3/blob_storage.go b/internal/storage/blobstorage/s3/blob_storage.go index e1206d98..4a8d7e6e 100644 --- a/internal/storage/blobstorage/s3/blob_storage.go +++ b/internal/storage/blobstorage/s3/blob_storage.go @@ -13,10 +13,12 @@ import ( "github.com/aws/aws-sdk-go/aws/request" awss3 "github.com/aws/aws-sdk-go/service/s3" "github.com/aws/aws-sdk-go/service/s3/s3manager" + "github.com/gogo/status" "github.com/uber-go/tally/v4" "go.uber.org/fx" "go.uber.org/zap" "golang.org/x/xerrors" + "google.golang.org/grpc/codes" "google.golang.org/protobuf/proto" "github.com/coinbase/chainstorage/internal/config" @@ -34,6 +36,7 @@ type ( BlobStorageParams struct { fx.In fxparams.Params + Client s3.Client Downloader s3.Downloader Uploader s3.Uploader } @@ -46,6 +49,7 @@ type ( logger *zap.Logger config *config.Config bucket string + client s3.Client downloader s3.Downloader uploader s3.Uploader blobStorageMetrics *blobStorageMetrics @@ -85,6 +89,7 @@ func New(params BlobStorageParams) (internal.BlobStorage, error) { logger: log.WithPackage(params.Logger), config: params.Config, bucket: params.Config.AWS.Bucket, + client: params.Client, downloader: params.Downloader, uploader: params.Uploader, blobStorageMetrics: newBlobStorageMetrics(metrics), @@ -214,6 +219,19 @@ func (s *blobStorageImpl) Download(ctx context.Context, metadata *api.BlockMetad }) } +func (s *blobStorageImpl) PreSign(ctx context.Context, objectKey string) (string, error) { + getObjectReq, _ := s.client.GetObjectRequest(&awss3.GetObjectInput{ + Bucket: aws.String(s.config.AWS.Bucket), + Key: aws.String(objectKey), + }) + fileUrl, err := getObjectReq.Presign(s.config.AWS.PresignedUrlExpiration) + if err != nil { + s.logger.Error("block file s3 presign error", zap.Reflect("key", objectKey), zap.Error(err)) + return "", status.Errorf(codes.Internal, "internal block file url generation error: %+v", err) + } + return fileUrl, nil +} + func (s *blobStorageImpl) logDuration(method string, start time.Time) { s.logger.Debug( "blob_storage", From c48eb638988f38d35b5c08e605f8a7e06a25659f Mon Sep 17 00:00:00 2001 From: bestmike007 Date: Mon, 15 Jan 2024 11:28:59 -0600 Subject: [PATCH 2/9] chore: fix test Signed-off-by: bestmike007 --- internal/server/handler_test.go | 74 ++++++------------- .../blobstorage/s3/blob_storage_test.go | 5 ++ 2 files changed, 29 insertions(+), 50 deletions(-) diff --git a/internal/server/handler_test.go b/internal/server/handler_test.go index 7fa3b431..4b0c0b0e 100644 --- a/internal/server/handler_test.go +++ b/internal/server/handler_test.go @@ -12,10 +12,8 @@ import ( "github.com/aws/aws-sdk-go/aws" awsClient "github.com/aws/aws-sdk-go/aws/client" - "github.com/aws/aws-sdk-go/aws/request" "github.com/aws/aws-sdk-go/awstesting" "github.com/aws/aws-sdk-go/awstesting/unit" - awss3 "github.com/aws/aws-sdk-go/service/s3" geth "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" "github.com/stretchr/testify/suite" @@ -242,11 +240,10 @@ func (s *handlerTestSuite) TestGetBlockFile() { }, nil }, ), - s.s3Client.EXPECT().GetObjectRequest(gomock.Any()).Times(1).DoAndReturn( - func(req *awss3.GetObjectInput) (*request.Request, *awss3.GetObjectOutput) { - require.Equal("example-chainstorage-ethereum-mainnet-dev", *req.Bucket) - require.Equal(objectKeyMain, *req.Key) - return s.newAwsPresignRequest("name", "GET", objectKeyMain), nil + s.blobStorage.EXPECT().PreSign(gomock.Any(), gomock.Any()).Times(1).DoAndReturn( + func(_ context.Context, key string) (string, error) { + require.Equal(objectKeyMain, key) + return "http://endpoint/foo/bar", nil }, ), ) @@ -293,11 +290,10 @@ func (s *handlerTestSuite) TestGetBlockFile_Gzip() { }, nil }, ), - s.s3Client.EXPECT().GetObjectRequest(gomock.Any()).Times(1).DoAndReturn( - func(req *awss3.GetObjectInput) (*request.Request, *awss3.GetObjectOutput) { - require.Equal("example-chainstorage-ethereum-mainnet-dev", *req.Bucket) - require.Equal(objectKeyMain, *req.Key) - return s.newAwsPresignRequest("name", "GET", objectKeyMain), nil + s.blobStorage.EXPECT().PreSign(gomock.Any(), gomock.Any()).Times(1).DoAndReturn( + func(_ context.Context, key string) (string, error) { + require.Equal(objectKeyMain, key) + return "http://endpoint/foo/bar.gzip", nil }, ), ) @@ -434,22 +430,16 @@ func (s *handlerTestSuite) TestGetBlockFilesByRange_PresignErr() { }, nil }, ), - s.s3Client.EXPECT().GetObjectRequest(gomock.Any()).Times(1).DoAndReturn( - func(req *awss3.GetObjectInput) (*request.Request, *awss3.GetObjectOutput) { - require.Equal("example-chainstorage-ethereum-mainnet-dev", *req.Bucket) - require.Equal("foo", *req.Key) - return s.awsClient.NewRequest(&request.Operation{ - Name: "name", - HTTPMethod: "GET", - HTTPPath: "/foo", - }, &struct{}{}, &struct{}{}), nil + s.blobStorage.EXPECT().PreSign(gomock.Any(), gomock.Any()).Times(1).DoAndReturn( + func(_ context.Context, key string) (string, error) { + require.Equal("foo", key) + return "http://endpoint/foo", nil }, ), - s.s3Client.EXPECT().GetObjectRequest(gomock.Any()).Times(1).DoAndReturn( - func(req *awss3.GetObjectInput) (*request.Request, *awss3.GetObjectOutput) { - require.Equal("example-chainstorage-ethereum-mainnet-dev", *req.Bucket) - require.Equal("bar", *req.Key) - return s.newAwsPresignFailedRequest(), nil + s.blobStorage.EXPECT().PreSign(gomock.Any(), gomock.Any()).Times(1).DoAndReturn( + func(_ context.Context, key string) (string, error) { + require.Equal("bar", key) + return "", fmt.Errorf("failed") }, ), ) @@ -489,18 +479,16 @@ func (s *handlerTestSuite) TestGetBlockFilesByRange() { }, nil }, ), - s.s3Client.EXPECT().GetObjectRequest(gomock.Any()).Times(1).DoAndReturn( - func(req *awss3.GetObjectInput) (*request.Request, *awss3.GetObjectOutput) { - require.Equal("example-chainstorage-ethereum-mainnet-dev", *req.Bucket) - require.Equal("foo", *req.Key) - return s.newAwsPresignRequest("name", "GET", "/foo"), nil + s.blobStorage.EXPECT().PreSign(gomock.Any(), gomock.Any()).Times(1).DoAndReturn( + func(_ context.Context, key string) (string, error) { + require.Equal("foo", key) + return "http://endpoint/foo", nil }, ), - s.s3Client.EXPECT().GetObjectRequest(gomock.Any()).Times(1).DoAndReturn( - func(req *awss3.GetObjectInput) (*request.Request, *awss3.GetObjectOutput) { - require.Equal("example-chainstorage-ethereum-mainnet-dev", *req.Bucket) - require.Equal("bar", *req.Key) - return s.newAwsPresignRequest("name", "GET", "/bar"), nil + s.blobStorage.EXPECT().PreSign(gomock.Any(), gomock.Any()).Times(1).DoAndReturn( + func(_ context.Context, key string) (string, error) { + require.Equal("bar", key) + return "http://endpoint/bar", nil }, ), ) @@ -1161,20 +1149,6 @@ func (s *handlerTestSuite) TestGetRosettaBlocksByRange_NotImplemented() { s.verifyStatusCode(codes.Unimplemented, err) } -func (s *handlerTestSuite) newAwsPresignFailedRequest() *request.Request { - return s.awsClient.NewRequest(&request.Operation{ - BeforePresignFn: func(r *request.Request) error { return fmt.Errorf("fail") }, - }, &struct{}{}, &struct{}{}) -} - -func (s *handlerTestSuite) newAwsPresignRequest(name, method, path string) *request.Request { - return s.awsClient.NewRequest(&request.Operation{ - Name: name, - HTTPMethod: method, - HTTPPath: path, - }, &struct{}{}, &struct{}{}) -} - func (s *handlerTestSuite) TestStreamChainEvents_WithSequence() { require := testutil.Require(s.T()) const ( diff --git a/internal/storage/blobstorage/s3/blob_storage_test.go b/internal/storage/blobstorage/s3/blob_storage_test.go index dc4d3ff5..7375035e 100644 --- a/internal/storage/blobstorage/s3/blob_storage_test.go +++ b/internal/storage/blobstorage/s3/blob_storage_test.go @@ -56,6 +56,7 @@ func TestBlobStorage_NoCompression(t *testing.T) { return &s3manager.UploadOutput{}, nil }) + client := s3mocks.NewMockClient(ctrl) var storage internal.BlobStorage app := testapp.New( @@ -63,6 +64,7 @@ func TestBlobStorage_NoCompression(t *testing.T) { fx.Provide(New), fx.Provide(func() s3.Downloader { return downloader }), fx.Provide(func() s3.Uploader { return uploader }), + fx.Provide(func() s3.Client { return client }), fx.Populate(&storage), ) defer app.Close() @@ -102,6 +104,7 @@ func TestBlobStorage_NoCompression_SkippedBlock(t *testing.T) { fx.Provide(New), fx.Provide(func() s3.Downloader { return nil }), fx.Provide(func() s3.Uploader { return nil }), + fx.Provide(func() s3.Client { return nil }), fx.Populate(&storage), ) defer app.Close() @@ -139,6 +142,7 @@ func TestBlobStorage_DownloadErrRequestCanceled(t *testing.T) { uploader := s3mocks.NewMockUploader(ctrl) downloader := s3manager.NewDownloader(unit.Session) + client := s3mocks.NewMockClient(ctrl) var blobStorage internal.BlobStorage app := testapp.New( @@ -146,6 +150,7 @@ func TestBlobStorage_DownloadErrRequestCanceled(t *testing.T) { fx.Provide(New), fx.Provide(func() s3.Downloader { return downloader }), fx.Provide(func() s3.Uploader { return uploader }), + fx.Provide(func() s3.Client { return client }), fx.Populate(&blobStorage), ) defer app.Close() From b8a3b02f4e413e39a9e20771f9656d6bd15bad27 Mon Sep 17 00:00:00 2001 From: Yuanhai He Date: Tue, 23 Jan 2024 09:55:31 +0800 Subject: [PATCH 3/9] Update internal/storage/blobstorage/gcs/blob_storage.go Co-authored-by: Jie Zhang --- internal/storage/blobstorage/gcs/blob_storage.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/storage/blobstorage/gcs/blob_storage.go b/internal/storage/blobstorage/gcs/blob_storage.go index 20b258a7..7af98dbd 100644 --- a/internal/storage/blobstorage/gcs/blob_storage.go +++ b/internal/storage/blobstorage/gcs/blob_storage.go @@ -239,7 +239,7 @@ func (s *blobStorageImpl) PreSign(ctx context.Context, objectKey string) (string Expires: time.Now().Add(s.presignedUrlExpiration), }) if err != nil { - s.logger.Error("block file gcs presign error", zap.Reflect("key", objectKey), zap.Error(err)) + s.logger.Error("block file gcs presign error", zap.String("key", objectKey), zap.Error(err)) return "", status.Errorf(codes.Internal, "internal block file url generation error: %+v", err) } return fileUrl, nil From e6b7b246d6f3a811d5cfaae01a9bce0810ec73ab Mon Sep 17 00:00:00 2001 From: bestmike007 Date: Mon, 22 Jan 2024 20:26:14 -0600 Subject: [PATCH 4/9] chore: quick fix import Signed-off-by: bestmike007 --- .../storage/blobstorage/gcs/blob_storage_integration_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/storage/blobstorage/gcs/blob_storage_integration_test.go b/internal/storage/blobstorage/gcs/blob_storage_integration_test.go index 8fe90af5..3c7dae2d 100644 --- a/internal/storage/blobstorage/gcs/blob_storage_integration_test.go +++ b/internal/storage/blobstorage/gcs/blob_storage_integration_test.go @@ -8,8 +8,6 @@ import ( "go.uber.org/fx" "google.golang.org/protobuf/proto" - "github.com/stretchr/testify/suite" - "github.com/coinbase/chainstorage/internal/config" "github.com/coinbase/chainstorage/internal/storage/blobstorage/internal" "github.com/coinbase/chainstorage/internal/utils/testapp" From 42d88264500da6bbc7c6ce547f95ad7c9b7a5d0a Mon Sep 17 00:00:00 2001 From: bestmike007 Date: Mon, 22 Jan 2024 20:49:15 -0600 Subject: [PATCH 5/9] chore: avoid grpc error in storage layer Signed-off-by: bestmike007 --- internal/server/handler.go | 3 ++- internal/storage/blobstorage/gcs/blob_storage.go | 5 +---- internal/storage/blobstorage/s3/blob_storage.go | 5 +---- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/internal/server/handler.go b/internal/server/handler.go index 307893cf..f8d046de 100644 --- a/internal/server/handler.go +++ b/internal/server/handler.go @@ -715,7 +715,8 @@ func (s *Server) newBlockFile(block *api.BlockMetadata) (*api.BlockFile, error) compression := storage_utils.GetCompressionType(key) fileUrl, err := s.blobStorage.PreSign(context.Background(), key) if err != nil { - return nil, err + s.logger.Error("block file s3 presign error", zap.String("key", key), zap.Error(err)) + return nil, status.Errorf(codes.Internal, "internal block file url generation error: %+v", err) } return &api.BlockFile{ diff --git a/internal/storage/blobstorage/gcs/blob_storage.go b/internal/storage/blobstorage/gcs/blob_storage.go index 7af98dbd..d9b1972c 100644 --- a/internal/storage/blobstorage/gcs/blob_storage.go +++ b/internal/storage/blobstorage/gcs/blob_storage.go @@ -9,12 +9,10 @@ import ( "time" "cloud.google.com/go/storage" - "github.com/gogo/status" "github.com/uber-go/tally/v4" "go.uber.org/fx" "go.uber.org/zap" "golang.org/x/xerrors" - "google.golang.org/grpc/codes" "google.golang.org/protobuf/proto" "github.com/coinbase/chainstorage/internal/config" @@ -239,8 +237,7 @@ func (s *blobStorageImpl) PreSign(ctx context.Context, objectKey string) (string Expires: time.Now().Add(s.presignedUrlExpiration), }) if err != nil { - s.logger.Error("block file gcs presign error", zap.String("key", objectKey), zap.Error(err)) - return "", status.Errorf(codes.Internal, "internal block file url generation error: %+v", err) + return "", xerrors.Errorf("failed to generate presigned url: %w", err) } return fileUrl, nil } diff --git a/internal/storage/blobstorage/s3/blob_storage.go b/internal/storage/blobstorage/s3/blob_storage.go index b11c9cc8..17376f3c 100644 --- a/internal/storage/blobstorage/s3/blob_storage.go +++ b/internal/storage/blobstorage/s3/blob_storage.go @@ -13,12 +13,10 @@ import ( "github.com/aws/aws-sdk-go/aws/request" awss3 "github.com/aws/aws-sdk-go/service/s3" "github.com/aws/aws-sdk-go/service/s3/s3manager" - "github.com/gogo/status" "github.com/uber-go/tally/v4" "go.uber.org/fx" "go.uber.org/zap" "golang.org/x/xerrors" - "google.golang.org/grpc/codes" "google.golang.org/protobuf/proto" "github.com/coinbase/chainstorage/internal/config" @@ -228,8 +226,7 @@ func (s *blobStorageImpl) PreSign(ctx context.Context, objectKey string) (string }) fileUrl, err := getObjectReq.Presign(s.config.AWS.PresignedUrlExpiration) if err != nil { - s.logger.Error("block file s3 presign error", zap.Reflect("key", objectKey), zap.Error(err)) - return "", status.Errorf(codes.Internal, "internal block file url generation error: %+v", err) + return "", xerrors.Errorf("failed to generate presigned url: %w", err) } return fileUrl, nil } From d2ea75a29c444a3b4a38f6275b03bc14d1a9d826 Mon Sep 17 00:00:00 2001 From: bestmike007 Date: Mon, 22 Jan 2024 20:52:57 -0600 Subject: [PATCH 6/9] chore: make presigned_url_expiration required Signed-off-by: bestmike007 --- internal/config/config.go | 6 +++--- internal/storage/blobstorage/gcs/blob_storage.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 0c2b3b5b..2512f4ff 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -126,9 +126,9 @@ type ( } GcpConfig struct { - Project string `mapstructure:"project" validate:"required"` - Bucket string `mapstructure:"bucket"` - PresignedUrlExpiration *time.Duration `mapstructure:"presigned_url_expiration"` + Project string `mapstructure:"project" validate:"required"` + Bucket string `mapstructure:"bucket"` + PresignedUrlExpiration time.Duration `mapstructure:"presigned_url_expiration" validate:"required"` } DynamoDBConfig struct { diff --git a/internal/storage/blobstorage/gcs/blob_storage.go b/internal/storage/blobstorage/gcs/blob_storage.go index d9b1972c..a7c0c6aa 100644 --- a/internal/storage/blobstorage/gcs/blob_storage.go +++ b/internal/storage/blobstorage/gcs/blob_storage.go @@ -80,7 +80,7 @@ func New(params BlobStorageParams) (internal.BlobStorage, error) { if len(params.Config.GCP.Bucket) == 0 { return nil, xerrors.Errorf("GCP bucket not configure for blob storage") } - if params.Config.GCP.PresignedUrlExpiration == nil { + if params.Config.GCP.PresignedUrlExpiration == 0 { return nil, xerrors.Errorf("GCP presign url expiration not configure for blob storage") } ctx := context.Background() @@ -98,7 +98,7 @@ func New(params BlobStorageParams) (internal.BlobStorage, error) { project: params.Config.GCP.Project, bucket: params.Config.GCP.Bucket, client: client, - presignedUrlExpiration: *params.Config.GCP.PresignedUrlExpiration, + presignedUrlExpiration: params.Config.GCP.PresignedUrlExpiration, blobStorageMetrics: blobStorageMetrics, instrumentUpload: instrument.NewWithResult[string](metrics, "upload"), instrumentDownload: instrument.NewWithResult[*api.Block](metrics, "download"), From b3faa94c08905511e96c03d7445d7dfd8a4115f9 Mon Sep 17 00:00:00 2001 From: bestmike007 Date: Mon, 22 Jan 2024 20:56:32 -0600 Subject: [PATCH 7/9] chore: add presigned_url_expiration default value for gcp config Signed-off-by: bestmike007 --- config/chainstorage/aptos/mainnet/base.yml | 3 +++ config/chainstorage/arbitrum/mainnet/base.yml | 3 +++ config/chainstorage/avacchain/mainnet/base.yml | 3 +++ config/chainstorage/base/goerli/base.yml | 3 +++ config/chainstorage/base/mainnet/base.yml | 3 +++ config/chainstorage/bitcoin/mainnet/base.yml | 3 +++ config/chainstorage/bsc/mainnet/base.yml | 3 +++ config/chainstorage/dogecoin/mainnet/base.yml | 3 +++ config/chainstorage/ethereum/goerli/base.yml | 3 +++ config/chainstorage/ethereum/holesky/base.yml | 3 +++ config/chainstorage/ethereum/mainnet/base.yml | 3 +++ config/chainstorage/fantom/mainnet/base.yml | 3 +++ config/chainstorage/optimism/mainnet/base.yml | 3 +++ config/chainstorage/polygon/mainnet/base.yml | 3 +++ config/chainstorage/polygon/testnet/base.yml | 3 +++ config/chainstorage/solana/mainnet/base.yml | 3 +++ config_templates/config/base.template.yml | 3 +++ 17 files changed, 51 insertions(+) diff --git a/config/chainstorage/aptos/mainnet/base.yml b/config/chainstorage/aptos/mainnet/base.yml index 08ac38ed..d22e6017 100644 --- a/config/chainstorage/aptos/mainnet/base.yml +++ b/config/chainstorage/aptos/mainnet/base.yml @@ -62,6 +62,9 @@ config_name: aptos_mainnet cron: block_range_size: 4 functional_test: "" +gcp: + presigned_url_expiration: 30m + project: development sdk: auth_header: "" auth_token: "" diff --git a/config/chainstorage/arbitrum/mainnet/base.yml b/config/chainstorage/arbitrum/mainnet/base.yml index 7882297a..e74f86c5 100644 --- a/config/chainstorage/arbitrum/mainnet/base.yml +++ b/config/chainstorage/arbitrum/mainnet/base.yml @@ -62,6 +62,9 @@ config_name: arbitrum_mainnet cron: block_range_size: 4 functional_test: "" +gcp: + presigned_url_expiration: 30m + project: development sdk: auth_header: "" auth_token: "" diff --git a/config/chainstorage/avacchain/mainnet/base.yml b/config/chainstorage/avacchain/mainnet/base.yml index 7d50b407..bc8ff5a3 100644 --- a/config/chainstorage/avacchain/mainnet/base.yml +++ b/config/chainstorage/avacchain/mainnet/base.yml @@ -62,6 +62,9 @@ config_name: avacchain_mainnet cron: block_range_size: 4 functional_test: "" +gcp: + presigned_url_expiration: 30m + project: development sdk: auth_header: "" auth_token: "" diff --git a/config/chainstorage/base/goerli/base.yml b/config/chainstorage/base/goerli/base.yml index 7b8162a4..4077fbc0 100644 --- a/config/chainstorage/base/goerli/base.yml +++ b/config/chainstorage/base/goerli/base.yml @@ -64,6 +64,9 @@ config_name: base_goerli cron: block_range_size: 4 functional_test: "" +gcp: + presigned_url_expiration: 30m + project: development sdk: auth_header: "" auth_token: "" diff --git a/config/chainstorage/base/mainnet/base.yml b/config/chainstorage/base/mainnet/base.yml index b23fecd2..2e8c7ce9 100644 --- a/config/chainstorage/base/mainnet/base.yml +++ b/config/chainstorage/base/mainnet/base.yml @@ -64,6 +64,9 @@ config_name: base_mainnet cron: block_range_size: 4 functional_test: "" +gcp: + presigned_url_expiration: 30m + project: development sdk: auth_header: "" auth_token: "" diff --git a/config/chainstorage/bitcoin/mainnet/base.yml b/config/chainstorage/bitcoin/mainnet/base.yml index c4f1ca50..111aef2a 100644 --- a/config/chainstorage/bitcoin/mainnet/base.yml +++ b/config/chainstorage/bitcoin/mainnet/base.yml @@ -65,6 +65,9 @@ cron: block_range_size: 2 disable_dlq_processor: true functional_test: "" +gcp: + presigned_url_expiration: 30m + project: development sdk: auth_header: "" auth_token: "" diff --git a/config/chainstorage/bsc/mainnet/base.yml b/config/chainstorage/bsc/mainnet/base.yml index ee982575..a487de52 100644 --- a/config/chainstorage/bsc/mainnet/base.yml +++ b/config/chainstorage/bsc/mainnet/base.yml @@ -65,6 +65,9 @@ cron: disable_polling_canary: false disable_streaming_canary: false functional_test: "" +gcp: + presigned_url_expiration: 30m + project: development sdk: auth_header: "" auth_token: "" diff --git a/config/chainstorage/dogecoin/mainnet/base.yml b/config/chainstorage/dogecoin/mainnet/base.yml index 9edd17e3..012b24ad 100644 --- a/config/chainstorage/dogecoin/mainnet/base.yml +++ b/config/chainstorage/dogecoin/mainnet/base.yml @@ -69,6 +69,9 @@ cron: block_range_size: 4 disable_dlq_processor: true functional_test: "" +gcp: + presigned_url_expiration: 30m + project: development sdk: auth_header: "" auth_token: "" diff --git a/config/chainstorage/ethereum/goerli/base.yml b/config/chainstorage/ethereum/goerli/base.yml index a29e2514..b4a111e6 100644 --- a/config/chainstorage/ethereum/goerli/base.yml +++ b/config/chainstorage/ethereum/goerli/base.yml @@ -66,6 +66,9 @@ config_name: ethereum_goerli cron: block_range_size: 4 functional_test: "" +gcp: + presigned_url_expiration: 30m + project: development sdk: auth_header: "" auth_token: "" diff --git a/config/chainstorage/ethereum/holesky/base.yml b/config/chainstorage/ethereum/holesky/base.yml index b91dbd96..619c3dc6 100644 --- a/config/chainstorage/ethereum/holesky/base.yml +++ b/config/chainstorage/ethereum/holesky/base.yml @@ -62,6 +62,9 @@ config_name: ethereum_holesky cron: block_range_size: 4 functional_test: "" +gcp: + presigned_url_expiration: 30m + project: development sdk: auth_header: "" auth_token: "" diff --git a/config/chainstorage/ethereum/mainnet/base.yml b/config/chainstorage/ethereum/mainnet/base.yml index 17fe05bf..7e7e3a18 100644 --- a/config/chainstorage/ethereum/mainnet/base.yml +++ b/config/chainstorage/ethereum/mainnet/base.yml @@ -66,6 +66,9 @@ config_name: ethereum_mainnet cron: block_range_size: 4 functional_test: "" +gcp: + presigned_url_expiration: 30m + project: development sdk: auth_header: "" auth_token: "" diff --git a/config/chainstorage/fantom/mainnet/base.yml b/config/chainstorage/fantom/mainnet/base.yml index 8301eefc..97d5eb42 100644 --- a/config/chainstorage/fantom/mainnet/base.yml +++ b/config/chainstorage/fantom/mainnet/base.yml @@ -62,6 +62,9 @@ config_name: fantom_mainnet cron: block_range_size: 4 functional_test: "" +gcp: + presigned_url_expiration: 30m + project: development sdk: auth_header: "" auth_token: "" diff --git a/config/chainstorage/optimism/mainnet/base.yml b/config/chainstorage/optimism/mainnet/base.yml index 48feb180..1452f3f1 100644 --- a/config/chainstorage/optimism/mainnet/base.yml +++ b/config/chainstorage/optimism/mainnet/base.yml @@ -62,6 +62,9 @@ config_name: optimism_mainnet cron: block_range_size: 4 functional_test: "" +gcp: + presigned_url_expiration: 30m + project: development sdk: auth_header: "" auth_token: "" diff --git a/config/chainstorage/polygon/mainnet/base.yml b/config/chainstorage/polygon/mainnet/base.yml index f060959a..e3575b99 100644 --- a/config/chainstorage/polygon/mainnet/base.yml +++ b/config/chainstorage/polygon/mainnet/base.yml @@ -68,6 +68,9 @@ config_name: polygon_mainnet cron: block_range_size: 4 functional_test: "" +gcp: + presigned_url_expiration: 30m + project: development sdk: auth_header: "" auth_token: "" diff --git a/config/chainstorage/polygon/testnet/base.yml b/config/chainstorage/polygon/testnet/base.yml index 00b73bbb..d46095be 100644 --- a/config/chainstorage/polygon/testnet/base.yml +++ b/config/chainstorage/polygon/testnet/base.yml @@ -64,6 +64,9 @@ config_name: polygon_testnet cron: block_range_size: 4 functional_test: "" +gcp: + presigned_url_expiration: 30m + project: development sdk: auth_header: "" auth_token: "" diff --git a/config/chainstorage/solana/mainnet/base.yml b/config/chainstorage/solana/mainnet/base.yml index f386c3ed..cb10dbd9 100644 --- a/config/chainstorage/solana/mainnet/base.yml +++ b/config/chainstorage/solana/mainnet/base.yml @@ -65,6 +65,9 @@ cron: block_range_size: 4 disable_dlq_processor: true functional_test: "" +gcp: + presigned_url_expiration: 30m + project: development sdk: auth_header: "" auth_token: "" diff --git a/config_templates/config/base.template.yml b/config_templates/config/base.template.yml index 8e13b5a3..3dd0e5c7 100644 --- a/config_templates/config/base.template.yml +++ b/config_templates/config/base.template.yml @@ -25,6 +25,9 @@ aws: region: us-east-1 storage: data_compression: GZIP +gcp: + project: "development" + presigned_url_expiration: 30m cadence: address: "" retention_period: 7 From 821aa211329a7a4ae90a23da38fe36619af6a761 Mon Sep 17 00:00:00 2001 From: bestmike007 Date: Mon, 22 Jan 2024 21:11:30 -0600 Subject: [PATCH 8/9] chore: revert logging Signed-off-by: bestmike007 --- internal/server/handler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/server/handler.go b/internal/server/handler.go index f8d046de..421941c2 100644 --- a/internal/server/handler.go +++ b/internal/server/handler.go @@ -715,7 +715,7 @@ func (s *Server) newBlockFile(block *api.BlockMetadata) (*api.BlockFile, error) compression := storage_utils.GetCompressionType(key) fileUrl, err := s.blobStorage.PreSign(context.Background(), key) if err != nil { - s.logger.Error("block file s3 presign error", zap.String("key", key), zap.Error(err)) + s.logger.Error("block file s3 presign error", zap.Reflect("block", block), zap.Error(err)) return nil, status.Errorf(codes.Internal, "internal block file url generation error: %+v", err) } From c0a829804f7382d4d0a63af88d6b86c526d81bbd Mon Sep 17 00:00:00 2001 From: bestmike007 Date: Thu, 25 Jan 2024 15:31:26 -0600 Subject: [PATCH 9/9] chore: fix error logging Signed-off-by: bestmike007 --- internal/server/handler.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/server/handler.go b/internal/server/handler.go index 421941c2..62e99eaf 100644 --- a/internal/server/handler.go +++ b/internal/server/handler.go @@ -715,8 +715,7 @@ func (s *Server) newBlockFile(block *api.BlockMetadata) (*api.BlockFile, error) compression := storage_utils.GetCompressionType(key) fileUrl, err := s.blobStorage.PreSign(context.Background(), key) if err != nil { - s.logger.Error("block file s3 presign error", zap.Reflect("block", block), zap.Error(err)) - return nil, status.Errorf(codes.Internal, "internal block file url generation error: %+v", err) + return nil, xerrors.Errorf("failed to generate presigned url: %w", err) } return &api.BlockFile{