From 5099b30ac61315530bae84950341c27b6a5a1def Mon Sep 17 00:00:00 2001
From: Zhongpeng Lin <zplin@uber.com>
Date: Fri, 3 Mar 2023 19:46:09 -0800
Subject: [PATCH] Removing Exists() check from S3 getters (#1842)

* Atomic access to S3

* fixing linting errors

* make delete return KindNotFound

* restore checker and deleter
---
 pkg/storage/s3/getter.go  | 39 ++++++++++++++++++---------------------
 pkg/storage/s3/s3_test.go |  1 -
 2 files changed, 18 insertions(+), 22 deletions(-)

diff --git a/pkg/storage/s3/getter.go b/pkg/storage/s3/getter.go
index 8420864b..8bc04fe8 100644
--- a/pkg/storage/s3/getter.go
+++ b/pkg/storage/s3/getter.go
@@ -2,10 +2,12 @@ package s3
 
 import (
 	"context"
+	errs "errors"
 	"fmt"
 	"io"
 
 	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/service/s3"
 	"github.com/gomods/athens/pkg/config"
 	"github.com/gomods/athens/pkg/errors"
@@ -18,16 +20,13 @@ func (s *Storage) Info(ctx context.Context, module, version string) ([]byte, err
 	const op errors.Op = "s3.Info"
 	ctx, span := observ.StartSpan(ctx, op.String())
 	defer span.End()
-	exists, err := s.Exists(ctx, module, version)
-	if err != nil {
-		return nil, errors.E(op, err, errors.M(module), errors.V(version))
-	}
-	if !exists {
-		return nil, errors.E(op, errors.M(module), errors.V(version), errors.KindNotFound)
-	}
 
 	infoReader, err := s.open(ctx, config.PackageVersionedName(module, version, "info"))
 	if err != nil {
+		var aerr awserr.Error
+		if errs.As(err, &aerr) && aerr.Code() == s3.ErrCodeNoSuchKey {
+			return nil, errors.E(op, errors.M(module), errors.V(version), errors.KindNotFound)
+		}
 		return nil, errors.E(op, err, errors.M(module), errors.V(version))
 	}
 	defer func() { _ = infoReader.Close() }()
@@ -44,16 +43,13 @@ func (s *Storage) GoMod(ctx context.Context, module, version string) ([]byte, er
 	const op errors.Op = "s3.GoMod"
 	ctx, span := observ.StartSpan(ctx, op.String())
 	defer span.End()
-	exists, err := s.Exists(ctx, module, version)
-	if err != nil {
-		return nil, errors.E(op, err, errors.M(module), errors.V(version))
-	}
-	if !exists {
-		return nil, errors.E(op, errors.M(module), errors.V(version), errors.KindNotFound)
-	}
 
 	modReader, err := s.open(ctx, config.PackageVersionedName(module, version, "mod"))
 	if err != nil {
+		var aerr awserr.Error
+		if errs.As(err, &aerr) && aerr.Code() == s3.ErrCodeNoSuchKey {
+			return nil, errors.E(op, errors.M(module), errors.V(version), errors.KindNotFound)
+		}
 		return nil, errors.E(op, err, errors.M(module), errors.V(version))
 	}
 	defer func() { _ = modReader.Close() }()
@@ -71,16 +67,13 @@ func (s *Storage) Zip(ctx context.Context, module, version string) (storage.Size
 	const op errors.Op = "s3.Zip"
 	ctx, span := observ.StartSpan(ctx, op.String())
 	defer span.End()
-	exists, err := s.Exists(ctx, module, version)
-	if err != nil {
-		return nil, errors.E(op, err, errors.M(module), errors.V(version))
-	}
-	if !exists {
-		return nil, errors.E(op, errors.M(module), errors.V(version), errors.KindNotFound)
-	}
 
 	zipReader, err := s.open(ctx, config.PackageVersionedName(module, version, "zip"))
 	if err != nil {
+		var aerr awserr.Error
+		if errs.As(err, &aerr) && aerr.Code() == s3.ErrCodeNoSuchKey {
+			return nil, errors.E(op, errors.M(module), errors.V(version), errors.KindNotFound)
+		}
 		return nil, errors.E(op, err, errors.M(module), errors.V(version))
 	}
 
@@ -98,6 +91,10 @@ func (s *Storage) open(ctx context.Context, path string) (storage.SizeReadCloser
 
 	goo, err := s.s3API.GetObjectWithContext(ctx, getParams)
 	if err != nil {
+		var aerr awserr.Error
+		if errs.As(err, &aerr) && aerr.Code() == s3.ErrCodeNoSuchKey {
+			return nil, errors.E(op, errors.KindNotFound)
+		}
 		return nil, errors.E(op, err)
 	}
 	var size int64
diff --git a/pkg/storage/s3/s3_test.go b/pkg/storage/s3/s3_test.go
index 6cc54e6a..08c7edd2 100644
--- a/pkg/storage/s3/s3_test.go
+++ b/pkg/storage/s3/s3_test.go
@@ -89,7 +89,6 @@ func getStorage(t testing.TB) *Storage {
 		config.GetTimeoutDuration(300),
 		options,
 	)
-
 	if err != nil {
 		t.Fatal(err)
 	}