Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug list size overload #37

Merged
merged 28 commits into from
Jun 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
690336a
add a new GetList with call back
Akopti8 Jan 26, 2024
9144518
fix get size to use new get list
Akopti8 Jan 26, 2024
3bd4cce
fix delete prefix to use new list functionality
Akopti8 Jan 27, 2024
72f052a
refactor page processor in delete prefix
Akopti8 Jan 29, 2024
1539cf5
make move prefix use teh get list with call back
Akopti8 Jan 29, 2024
4fafd40
Change download script presigned url to use get list with call back
Akopti8 Jan 30, 2024
c987ad8
check if prefix is empty
Akopti8 Jan 30, 2024
36d1be2
add prefix not found for prefix move
Akopti8 Jan 30, 2024
37d5c8f
fix prefix exists checker
Akopti8 Jan 30, 2024
ae80715
code clean up
Akopti8 Jan 30, 2024
b1623f4
make error handeling uniform
Akopti8 Jan 30, 2024
c8b5988
add new endpoints to e2e test
Akopti8 Jan 30, 2024
a13d4de
change status code back to 401 for e2e test
Akopti8 Jan 30, 2024
ae72cb6
correct file path for e2e
Akopti8 Jan 30, 2024
1891541
remove unnecessary postgress env vars from e2e
Akopti8 Jan 30, 2024
921f31f
delete unused test file
Akopti8 Jan 31, 2024
be3b547
fix list with details to return the same thing
Akopti8 Jan 31, 2024
a6d0d8c
Merge remote-tracking branch 'origin/main' into bug-list-size-overload
Akopti8 Jun 12, 2024
b6afe78
remove unnecessary s3Mock declaration
Akopti8 Jun 12, 2024
ec737bc
rename copyPrefix to MovePrefix
Akopti8 Jun 12, 2024
78f1101
change all occurences of errors.New to fmt.errorf
Akopti8 Jun 12, 2024
a14f5e6
refactor prefix checks
Akopti8 Jun 12, 2024
5111774
consistent error language
Akopti8 Jun 12, 2024
286bfe7
change no prefix found check to test file count and not file size
Akopti8 Jun 12, 2024
3efcb65
test without temp minio client
Akopti8 Jun 12, 2024
92f4771
revert temp minio client
Akopti8 Jun 12, 2024
36b1c3b
add explanatory comments
Akopti8 Jun 12, 2024
0c976c9
Add comment, reorder download script
ShaneMPutnam Jun 12, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions .github/workflows/e2e-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,6 @@ jobs:
echo AWS_S3_BUCKET='test-bucket' >> .env
echo S3API_SERVICE_PORT='5005' >> .env
echo AUTH_LEVEL=0 >> .env
echo POSTGRES_CONN_STRING='postgres://user:password@postgres:5432/db?sslmode=disable' >> .env
echo POSTGRES_PASSWORD='password' >> .env
echo POSTGRES_USER='user' >> .env
echo POSTGRES_DB='db' >> .env
echo PG_LOG_CHECKPOINTS='off' >> .env

- name: Substitute secret variables in JSON
env:
Expand Down
6 changes: 3 additions & 3 deletions blobstore/blobhandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ type S3Controller struct {
Sess *session.Session
S3Svc *s3.S3
Buckets []string
S3Mock bool
}

// Config holds the configuration settings for the REST API server.
Expand Down Expand Up @@ -92,13 +93,12 @@ func NewBlobHandler(envJson string, authLvl int) (*BlobHandler, error) {
}

// Configure the BlobHandler with MinIO session and bucket information
config.S3Controllers = []S3Controller{{Sess: sess, S3Svc: s3SVC, Buckets: []string{creds.Bucket}}}
config.S3Controllers = []S3Controller{{Sess: sess, S3Svc: s3SVC, Buckets: []string{creds.Bucket}, S3Mock: true}}
// Return the configured BlobHandler
return &config, nil
}

// Using AWS S3

// Load AWS credentials from the provided .env.json file
log.Debug("looking for .env.json")
awsConfig, err := newAWSConfig(envJson)
Expand Down Expand Up @@ -153,7 +153,7 @@ func NewBlobHandler(envJson string, authLvl int) (*BlobHandler, error) {
}

if len(bucketNames) > 0 {
config.S3Controllers = append(config.S3Controllers, S3Controller{Sess: sess, S3Svc: s3SVC, Buckets: bucketNames})
config.S3Controllers = append(config.S3Controllers, S3Controller{Sess: sess, S3Svc: s3SVC, Buckets: bucketNames, S3Mock: false})
}
}

Expand Down
10 changes: 4 additions & 6 deletions blobstore/buckets.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,17 +112,15 @@ func (bh *BlobHandler) HandleListBuckets(c echo.Context) error {
}
}
bh.Mu.Unlock()

log.Info("HandleListBuckets: Successfully retrieved list of buckets")

log.Info("Successfully retrieved list of buckets")
return c.JSON(http.StatusOK, allBuckets)
}

// func (bh *BlobHandler) HandleCreateBucket(c echo.Context) error {
// bucketName := c.QueryParam("name")

// if bucketName == "" {
// err := errors.New("request must include a `name` parameter")
// err := fmt.Errorf("request must include a `name` parameter")
// log.Info("HandleCreateBucket: " + err.Error())
// return c.JSON(http.StatusBadRequest, err.Error())
// }
Expand Down Expand Up @@ -157,7 +155,7 @@ func (bh *BlobHandler) HandleListBuckets(c echo.Context) error {
// bucketName := c.QueryParam("name")

// if bucketName == "" {
// err := errors.New("request must include a `name` parameter")
// err := fmt.Errorf("request must include a `name` parameter")
// log.Info("HandleDeleteBucket: " + err.Error())
// return c.JSON(http.StatusBadRequest, err.Error())
// }
Expand All @@ -177,7 +175,7 @@ func (bh *BlobHandler) HandleListBuckets(c echo.Context) error {
// bucketName := c.QueryParam("name")

// if bucketName == "" {
// err := errors.New("request must include a `name` parameter")
// err := fmt.Errorf("request must include a `name` parameter")
// log.Info("HandleGetBucketACL: " + err.Error())
// return c.JSON(http.StatusBadRequest, err.Error())
// }
Expand Down
158 changes: 78 additions & 80 deletions blobstore/delete.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package blobstore

import (
"errors"
"fmt"
"net/http"
"strings"
Expand All @@ -12,39 +11,45 @@ import (
log "github.com/sirupsen/logrus"
)

func (s3Ctrl *S3Controller) RecursivelyDeleteObjects(bucket, prefix string) error {
prefixPath := strings.Trim(prefix, "/") + "/"
query := &s3.ListObjectsV2Input{
Bucket: aws.String(bucket),
Prefix: aws.String(prefixPath),
func (s3Ctrl *S3Controller) DeleteList(page *s3.ListObjectsV2Output, bucket string) error {
if len(page.Contents) == 0 {
return nil // No objects to delete in this page
}
resp, err := s3Ctrl.S3Svc.ListObjectsV2(query)

var objectsToDelete []*s3.ObjectIdentifier
for _, obj := range page.Contents {
objectsToDelete = append(objectsToDelete, &s3.ObjectIdentifier{Key: obj.Key})
}

// Perform the delete operation for the current page
_, err := s3Ctrl.S3Svc.DeleteObjects(&s3.DeleteObjectsInput{
Bucket: aws.String(bucket),
Delete: &s3.Delete{
Objects: objectsToDelete,
Quiet: aws.Bool(true),
},
})
if err != nil {
return fmt.Errorf("recursivelyDeleteObjects: error listing objects: %s", err)
return fmt.Errorf("error deleting objects: %v", err)
}
if len(resp.Contents) > 0 {
var objectsToDelete []*s3.ObjectIdentifier

for _, obj := range resp.Contents {
objectsToDelete = append(objectsToDelete, &s3.ObjectIdentifier{
Key: obj.Key,
})
}
return nil
}

if len(objectsToDelete) > 0 {
_, err = s3Ctrl.S3Svc.DeleteObjects(&s3.DeleteObjectsInput{
Bucket: aws.String(bucket),
Delete: &s3.Delete{
Objects: objectsToDelete,
},
})

if err != nil {
return fmt.Errorf("recursivelyDeleteObjects: error Deleting objects %v: %s", objectsToDelete, err)
}
func (s3Ctrl *S3Controller) RecursivelyDeleteObjects(bucket, prefix string) error {
var objectsFound bool
err := s3Ctrl.GetListWithCallBack(bucket, prefix, false, func(page *s3.ListObjectsV2Output) error {
if len(page.Contents) > 0 {
objectsFound = true
}
} else {
return fmt.Errorf("recursivelyDeleteObjects: object %s not found and no objects were deleted", prefixPath)
return s3Ctrl.DeleteList(page, bucket)
})
if err != nil {
return fmt.Errorf("error processing objects for deletion: %v", err)
}

if !objectsFound {
return fmt.Errorf("prefix not found")
}
return nil
}
Expand All @@ -56,28 +61,29 @@ func (bh *BlobHandler) HandleDeleteObject(c echo.Context) error {
bucket := c.QueryParam("bucket")
s3Ctrl, err := bh.GetController(bucket)
if err != nil {
errMsg := fmt.Errorf("bucket %s is not available, %s", bucket, err.Error())
errMsg := fmt.Errorf("parameter `bucket` %s is not available, %s", bucket, err.Error())
log.Error(errMsg.Error())
return c.JSON(http.StatusUnprocessableEntity, errMsg.Error())
}

key := c.QueryParam("key")
if key == "" {
err := errors.New("parameter 'key' is required")
log.Errorf("HandleDeleteObjects: %s", err.Error())
return c.JSON(http.StatusUnprocessableEntity, err.Error())
errMsg := fmt.Errorf("parameter `key` is required")
log.Error(errMsg.Error())
return c.JSON(http.StatusUnprocessableEntity, errMsg.Error())
}

// If the key is not a folder, proceed with deleting a single object
keyExist, err := s3Ctrl.KeyExists(bucket, key)
if err != nil {
log.Errorf("HandleDeleteObjects: Error checking if key exists: %s", err.Error())
return c.JSON(http.StatusInternalServerError, err)
errMsg := fmt.Errorf("error checking if object exists: %s", err.Error())
log.Error(errMsg.Error())
return c.JSON(http.StatusInternalServerError, errMsg.Error())
}
if !keyExist {
err := fmt.Errorf("object %s not found", key)
log.Errorf("HandleDeleteObjects: %s", err.Error())
return c.JSON(http.StatusNotFound, err.Error())
errMsg := fmt.Errorf("object %s not found", key)
log.Error(errMsg.Error())
return c.JSON(http.StatusNotFound, errMsg.Error())
}

deleteInput := &s3.DeleteObjectInput{
Expand All @@ -87,54 +93,45 @@ func (bh *BlobHandler) HandleDeleteObject(c echo.Context) error {

_, err = s3Ctrl.S3Svc.DeleteObject(deleteInput)
if err != nil {
msg := fmt.Sprintf("error deleting object. %s", err.Error())
log.Errorf("HandleDeleteObjects: %s", err.Error())
return c.JSON(http.StatusInternalServerError, msg)
errMsg := fmt.Errorf("error deleting object. %s", err.Error())
log.Error(errMsg.Error())
return c.JSON(http.StatusInternalServerError, errMsg.Error())
}

log.Info("HandleDeleteObjects: Successfully deleted file with key:", key)
log.Infof("successfully deleted file with key: %s", key)
return c.JSON(http.StatusOK, fmt.Sprintf("Successfully deleted object: %s", key))
}

func (bh *BlobHandler) HandleDeletePrefix(c echo.Context) error {
bucket := c.QueryParam("bucket")
s3Ctrl, err := bh.GetController(bucket)
if err != nil {
errMsg := fmt.Errorf("bucket %s is not available, %s", bucket, err.Error())
errMsg := fmt.Errorf("parameter `bucket` %s is not available, %s", bucket, err.Error())
log.Error(errMsg.Error())
return c.JSON(http.StatusUnprocessableEntity, errMsg.Error())
}

prefix := c.QueryParam("prefix")
if prefix == "" {
err = errors.New("parameter 'prefix' is required")
log.Errorf("HandleDeleteObjects: %s", err.Error())
return c.JSON(http.StatusUnprocessableEntity, err.Error())
errMsg := fmt.Errorf("parameter `prefix` is required")
log.Error(errMsg.Error())
return c.JSON(http.StatusUnprocessableEntity, errMsg.Error())
}
if !strings.HasSuffix(prefix, "/") {
prefix = prefix + "/"
}
response, err := s3Ctrl.GetList(bucket, prefix, false)
if err != nil {
log.Errorf("HandleDeleteObjects: Error getting list: %s", err.Error())
return c.JSON(http.StatusInternalServerError, err)
}
if *response.KeyCount == 0 {
err := fmt.Errorf("the specified prefix %s does not exist in S3", prefix)
log.Errorf("HandleDeleteObjects: %s", err.Error())
return c.JSON(http.StatusNotFound, err.Error())
}
// This will recursively delete all objects with the specified prefix
err = s3Ctrl.RecursivelyDeleteObjects(bucket, prefix)
if err != nil {
msg := fmt.Sprintf("error deleting objects. %s", err.Error())
log.Errorf("HandleDeleteObjects: %s", msg)
return c.JSON(http.StatusInternalServerError, msg)
if strings.Contains(err.Error(), "prefix not found") {
errMsg := fmt.Errorf("no objects found with prefix: %s", prefix)
log.Error(errMsg.Error())
return c.JSON(http.StatusNotFound, errMsg.Error())
}
errMsg := fmt.Errorf("error deleting objects: %s", err.Error())
log.Error(errMsg.Error())
return c.JSON(http.StatusInternalServerError, errMsg.Error())
}

log.Info("HandleDeleteObjects: Successfully deleted prefix and its contents for prefix:", prefix)
log.Info("Successfully deleted prefix and its contents for prefix:", prefix)
return c.JSON(http.StatusOK, "Successfully deleted prefix and its contents")

}

func (s3Ctrl *S3Controller) DeleteKeys(bucket string, key []string) error {
Expand All @@ -157,7 +154,7 @@ func (s3Ctrl *S3Controller) DeleteKeys(bucket string, key []string) error {

_, err := s3Ctrl.S3Svc.DeleteObjects(input)
if err != nil {
return fmt.Errorf("deleteKeys: error Deleting objects: %s", err.Error())
return fmt.Errorf("error deleting objects: %s", err.Error())
}
return nil
}
Expand All @@ -169,21 +166,22 @@ func (bh *BlobHandler) HandleDeleteObjectsByList(c echo.Context) error {
}
var deleteRequest DeleteRequest
if err := c.Bind(&deleteRequest); err != nil {
log.Errorf("HandleDeleteObjectsByList: Error parsing request body: %s" + err.Error())
return c.JSON(http.StatusBadRequest, "Invalid request body")
errMsg := fmt.Errorf("error parsing request body: %s" + err.Error())
log.Error(errMsg.Error())
return c.JSON(http.StatusBadRequest, errMsg.Error())
}

// Ensure there are keys to delete
if len(deleteRequest.Keys) == 0 {
errMsg := "No keys to delete. Please provide 'keys' in the request body."
log.Errorf("HandleDeleteObjectsByList: %s", errMsg)
return c.JSON(http.StatusUnprocessableEntity, errMsg)
errMsg := fmt.Errorf("no keys to delete. Please provide 'keys' in the request body")
log.Error(errMsg.Error())
return c.JSON(http.StatusUnprocessableEntity, errMsg.Error())
}

bucket := c.QueryParam("bucket")
s3Ctrl, err := bh.GetController(bucket)
if err != nil {
errMsg := fmt.Errorf("bucket %s is not available, %s", bucket, err.Error())
errMsg := fmt.Errorf("`bucket` %s is not available, %s", bucket, err.Error())
log.Error(errMsg.Error())
return c.JSON(http.StatusUnprocessableEntity, errMsg.Error())
}
Expand All @@ -197,14 +195,14 @@ func (bh *BlobHandler) HandleDeleteObjectsByList(c echo.Context) error {
// Check if the key exists before appending it to the keys list
keyExists, err := s3Ctrl.KeyExists(bucket, s3Path)
if err != nil {
msg := fmt.Errorf("error checking if key exists. %s", err.Error())
log.Errorf("HandleDeleteObjectsByList: %s", msg)
return c.JSON(http.StatusInternalServerError, msg)
errMsg := fmt.Errorf("error checking if object exists. %s", err.Error())
log.Error(errMsg.Error())
return c.JSON(http.StatusInternalServerError, errMsg)
}
if !keyExists {
errMsg := fmt.Sprintf("object %s not found", s3Path)
log.Errorf("HandleDeleteObjectsByList: %s", errMsg)
return c.JSON(http.StatusNotFound, errMsg)
errMsg := fmt.Errorf("object %s not found", s3Path)
log.Error(errMsg.Error())
return c.JSON(http.StatusNotFound, errMsg.Error())
}

keys = append(keys, *key)
Expand All @@ -213,11 +211,11 @@ func (bh *BlobHandler) HandleDeleteObjectsByList(c echo.Context) error {
// Delete the objects using the deleteKeys function
err = s3Ctrl.DeleteKeys(bucket, keys)
if err != nil {
msg := fmt.Sprintf("error deleting objects. %s", err.Error())
log.Errorf("HandleDeleteObjectsByList: %s", msg)
return c.JSON(http.StatusInternalServerError, msg)
errMsg := fmt.Errorf("error deleting objects. %s", err.Error())
log.Error(errMsg.Error())
return c.JSON(http.StatusInternalServerError, errMsg)
}

log.Info("HandleDeleteObjectsByList: Successfully deleted objects:", deleteRequest.Keys)
log.Info("Successfully deleted objects:", deleteRequest.Keys)
return c.JSON(http.StatusOK, "Successfully deleted objects")
}
Loading
Loading