APPS-1054 Fix files deletion on S3 (#86)

* add storage validation

* update s3 context

* disable tests on github

* return immediately

pkg/model/storage.go

@@ -1,6 +1,9 @@
 package model
 
-import "errors"
+import (
+	"errors"
+	"fmt"
+)
 
 // Storage represents the configuration for a backup storage details.
 type Storage struct {
@@ -28,5 +31,22 @@ func (s *Storage) Validate() error {
 	if s.Path == nil {
 		return errors.New("storage path is not specified")
 	}
+	if s.Type == S3 {
+		if s.S3Region == nil {
+			return errors.New("s3 region is not specified")
+		}
+	}
+	if err := s.validateType(); err != nil {
+		return err
+	}
 	return nil
 }
+
+func (s *Storage) validateType() error {
+	switch s.Type {
+	case Local, S3:
+		return nil
+	default:
+		return fmt.Errorf("invalid storage type: %v", s.Type)
+	}
+}

pkg/service/s3_context.go

@@ -33,11 +33,7 @@ func NewS3Context(storage *model.Storage) *S3Context {
 	// Load the SDK's configuration from environment and shared config, and
 	// create the client with this.
 	ctx := context.TODO()
-	cfg, err := config.LoadDefaultConfig(
-		ctx,
-		config.WithSharedConfigProfile(*storage.S3Profile),
-		config.WithRegion(*storage.S3Region),
-	)
+	cfg, err := createConfig(ctx, storage)
 	if err != nil {
 		panic(fmt.Sprintf("Failed to load S3 SDK configuration: %v", err))
 	}
@@ -76,6 +72,16 @@ func NewS3Context(storage *model.Storage) *S3Context {
 	return s
 }
 
+func createConfig(ctx context.Context, storage *model.Storage) (aws.Config, error) {
+	cfgOptions := []func(*config.LoadOptions) error{
+		config.WithRegion(*storage.S3Region),
+	}
+	if storage.S3Profile != nil {
+		cfgOptions = append(cfgOptions, config.WithSharedConfigProfile(*storage.S3Profile))
+	}
+	return config.LoadDefaultConfig(ctx, cfgOptions...)
+}
+
 // readFile reads and decodes the YAML content from the given filePath into v.
 func (s *S3Context) readFile(filePath string, v any) {
 	result, err := s.client.GetObject(s.ctx, &s3.GetObjectInput{
@@ -223,7 +229,7 @@ func (s *S3Context) DeleteFile(path string) error {
 
 	input := &s3.DeleteObjectInput{
 		Bucket: aws.String(s.bucket),
-		Key:    aws.String(parsed.Path),
+		Key:    aws.String(removeLeadingSlash(parsed.Path)),
 	}
 
 	// Execute the delete operation

pkg/service/s3_context_test.go

@@ -9,25 +9,65 @@ import (
 	"testing"
 )
 
-func TestS3Context_DeleteFile(t *testing.T) {
-	context := NewS3Context(&model.Storage{
+var contexts = []S3Context{
+	*NewS3Context(&model.Storage{
 		Type:               model.S3,
-		Path:               ptr.String("s3://as-backup-bucket/storage1"),
+		Path:               ptr.String("s3://as-backup-bucket/storageMinio"),
 		S3Profile:          ptr.String("minio"),
 		S3Region:           ptr.String("eu-central-1"),
 		S3EndpointOverride: ptr.String("http://localhost:9000"),
-	})
+	}),
+	*NewS3Context(&model.Storage{
+		Type:     model.S3,
+		Path:     ptr.String("s3://as-backup-integration-test/storageAws"),
+		S3Region: ptr.String("eu-central-1"),
+	}),
+}
+
+func TestS3Context_CleanDir(t *testing.T) {
+	for _, context := range contexts {
+		t.Run(context.Path, func(t *testing.T) {
+			runCleanDirTest(t, context)
+		})
+	}
+}
 
-	context.writeFile("storage1/incremental/file.txt", "data")
-	context.writeFile("storage1/incremental/file2.txt", "data")
+func runCleanDirTest(t *testing.T, context S3Context) {
+	context.writeFile(context.Path+"/incremental/file.txt", "data")
+	context.writeFile(context.Path+"/incremental/file2.txt", "data")
 
-	if files, _ := context.listFiles("storage1/incremental"); len(files) != 2 {
+	if files, _ := context.listFiles(context.Path + "/incremental"); len(files) != 2 {
 		t.Error("files not created")
 	}
 
 	context.CleanDir("incremental") // clean is public function, so "storage1" is appended inside
 
-	if files, _ := context.listFiles("storage1/incremental"); len(files) > 0 {
+	if files, _ := context.listFiles(context.Path + "/incremental"); len(files) > 0 {
+		t.Error("files not deleted")
+	}
+}
+
+func TestS3Context_DeleteFile(t *testing.T) {
+	for _, context := range contexts {
+		t.Run(context.Path, func(t *testing.T) {
+			runDeleteFileTest(t, context)
+		})
+	}
+}
+
+func runDeleteFileTest(t *testing.T, context S3Context) {
+	context.writeFile(context.Path+"/incremental/file.txt", "data")
+	context.writeFile(context.Path+"/incremental/file2.txt", "data")
+
+	if files, _ := context.listFiles(context.Path + "/incremental"); len(files) != 2 {
+		t.Error("files not created")
+	}
+
+	// DeleteFile require full path
+	context.DeleteFile("s3://" + context.bucket + context.Path + "/incremental/file.txt")
+	context.DeleteFile("s3://" + context.bucket + context.Path + "/incremental/file2.txt")
+
+	if files, _ := context.listFiles(context.Path + "/incremental"); len(files) > 0 {
 		t.Error("files not deleted")
 	}
 }