feat: Added integration tests for bucket object versioning. Made a co…

…uple of bug fixes in the versioning implementation

backend/walk.go

@@ -263,7 +263,7 @@ type ObjVersionFuncResult struct {
 	Truncated           bool
 }
 
-type GetVersionsFunc func(path, versionIdMarker string, availableObjCount int, d fs.DirEntry) (*ObjVersionFuncResult, error)
+type GetVersionsFunc func(path, versionIdMarker string, pastVersionIdMarker *bool, availableObjCount int, d fs.DirEntry) (*ObjVersionFuncResult, error)
 
 // WalkVersions walks the supplied fs.FS and returns results compatible with
 // ListObjectVersions action response
@@ -280,6 +280,8 @@ func WalkVersions(ctx context.Context, fileSystem fs.FS, prefix, delimiter, keyM
 	var nextVersionIdMarker string
 	var truncated bool
 
+	pastVersionIdMarker := versionIdMarker == ""
+
 	err := fs.WalkDir(fileSystem, ".", func(path string, d fs.DirEntry, err error) error {
 		if err != nil {
 			return err
@@ -295,6 +297,15 @@ func WalkVersions(ctx context.Context, fileSystem fs.FS, prefix, delimiter, keyM
 			return fs.SkipDir
 		}
 
+		if !pastMarker {
+			if path == keyMarker {
+				pastMarker = true
+			}
+			if path < keyMarker {
+				return nil
+			}
+		}
+
 		if d.IsDir() {
 			// If prefix is defined and the directory does not match prefix,
 			// do not descend into the directory because nothing will
@@ -309,18 +320,23 @@ func WalkVersions(ctx context.Context, fileSystem fs.FS, prefix, delimiter, keyM
 				return fs.SkipDir
 			}
 
-			// skip directory objects, as they can't have versions
-			return nil
-		}
-
-		if !pastMarker {
-			if path == keyMarker {
-				pastMarker = true
+			res, err := getObj(path, versionIdMarker, &pastVersionIdMarker, max-len(objects)-len(delMarkers)-len(cpmap), d)
+			if err == ErrSkipObj {
 				return nil
 			}
-			if path < keyMarker {
-				return nil
+			if err != nil {
+				return fmt.Errorf("directory to object %q: %w", path, err)
 			}
+			objects = append(objects, res.ObjectVersions...)
+			delMarkers = append(delMarkers, res.DelMarkers...)
+			if res.Truncated {
+				truncated = true
+				nextMarker = path
+				nextVersionIdMarker = res.NextVersionIdMarker
+				return fs.SkipAll
+			}
+
+			return nil
 		}
 
 		// If object doesn't have prefix, don't include in results.
@@ -331,7 +347,7 @@ func WalkVersions(ctx context.Context, fileSystem fs.FS, prefix, delimiter, keyM
 		if delimiter == "" {
 			// If no delimiter specified, then all files with matching
 			// prefix are included in results
-			res, err := getObj(path, versionIdMarker, max-len(objects)-len(delMarkers)-len(cpmap), d)
+			res, err := getObj(path, versionIdMarker, &pastVersionIdMarker, max-len(objects)-len(delMarkers)-len(cpmap), d)
 			if err == ErrSkipObj {
 				return nil
 			}
@@ -374,7 +390,7 @@ func WalkVersions(ctx context.Context, fileSystem fs.FS, prefix, delimiter, keyM
 		suffix := strings.TrimPrefix(path, prefix)
 		before, _, found := strings.Cut(suffix, delimiter)
 		if !found {
-			res, err := getObj(path, versionIdMarker, max-len(objects)-len(delMarkers)-len(cpmap), d)
+			res, err := getObj(path, versionIdMarker, &pastVersionIdMarker, max-len(objects)-len(delMarkers)-len(cpmap), d)
 			if err == ErrSkipObj {
 				return nil
 			}

cmd/versitygw/test.go

@@ -22,20 +22,21 @@ import (
 )
 
 var (
-	awsID           string
-	awsSecret       string
-	endpoint        string
-	prefix          string
-	dstBucket       string
-	partSize        int64
-	objSize         int64
-	concurrency     int
-	files           int
-	totalReqs       int
-	upload          bool
-	download        bool
-	pathStyle       bool
-	checksumDisable bool
+	awsID             string
+	awsSecret         string
+	endpoint          string
+	prefix            string
+	dstBucket         string
+	partSize          int64
+	objSize           int64
+	concurrency       int
+	files             int
+	totalReqs         int
+	upload            bool
+	download          bool
+	pathStyle         bool
+	checksumDisable   bool
+	versioningEnabled bool
 )
 
 func testCommand() *cli.Command {
@@ -87,6 +88,14 @@ func initTestCommands() []*cli.Command {
 			Usage:       "Tests the full flow of gateway.",
 			Description: `Runs all the available tests to test the full flow of the gateway.`,
 			Action:      getAction(integration.TestFullFlow),
+			Flags: []cli.Flag{
+				&cli.BoolFlag{
+					Name:        "versioning-enabled",
+					Usage:       "Test the bucket object versioning, if the versioning is enabled",
+					Destination: &versioningEnabled,
+					Aliases:     []string{"vs"},
+				},
+			},
 		},
 		{
 			Name:   "posix",
@@ -276,6 +285,9 @@ func getAction(tf testFunc) func(*cli.Context) error {
 		if debug {
 			opts = append(opts, integration.WithDebug())
 		}
+		if versioningEnabled {
+			opts = append(opts, integration.WithVersioningEnabled())
+		}
 
 		s := integration.NewS3Conf(opts...)
 		tf(s)

go.mod

@@ -16,6 +16,7 @@ require (
 	github.com/google/uuid v1.6.0
 	github.com/hashicorp/vault-client-go v0.4.3
 	github.com/nats-io/nats.go v1.37.0
+	github.com/oklog/ulid/v2 v2.1.0
 	github.com/pkg/xattr v0.4.10
 	github.com/segmentio/kafka-go v0.4.47
 	github.com/smira/go-statsd v1.3.3
@@ -45,7 +46,6 @@ require (
 	github.com/mitchellh/go-homedir v1.1.0 // indirect
 	github.com/nats-io/nkeys v0.4.7 // indirect
 	github.com/nats-io/nuid v1.0.1 // indirect
-	github.com/oklog/ulid/v2 v2.1.0 // indirect
 	github.com/pierrec/lz4/v4 v4.1.21 // indirect
 	github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect
 	github.com/ryanuber/go-glob v1.0.0 // indirect

runtests.sh

@@ -5,9 +5,11 @@ rm -rf /tmp/gw
 mkdir /tmp/gw
 rm -rf /tmp/covdata
 mkdir /tmp/covdata
+rm -rf /tmp/versioningdir
+mkdir /tmp/versioningdir
 
 # run server in background
-GOCOVERDIR=/tmp/covdata ./versitygw -a user -s pass --iam-dir /tmp/gw posix /tmp/gw &
+GOCOVERDIR=/tmp/covdata ./versitygw -a user -s pass --iam-dir /tmp/gw posix --versioning-dir /tmp/versioningdir /tmp/gw &
 GW_PID=$!
 
 # wait a second for server to start up
@@ -21,7 +23,7 @@ fi
 
 # run tests
 # full flow tests
-if ! ./versitygw test -a user -s pass -e http://127.0.0.1:7070 full-flow; then
+if ! ./versitygw test -a user -s pass -e http://127.0.0.1:7070 full-flow -vs; then
 	echo "full flow tests failed"
 	kill $GW_PID
 	exit 1

s3api/controllers/base.go

@@ -495,6 +495,15 @@ func (c S3ApiController) GetActions(ctx *fiber.Ctx) error {
 	utils.SetMetaHeaders(ctx, res.Metadata)
 	// Set other response headers
 	utils.SetResponseHeaders(ctx, hdrs)
+	// Set version id header
+	if getstring(res.VersionId) != "" {
+		utils.SetResponseHeaders(ctx, []utils.CustomHeader{
+			{
+				Key:   "x-amz-version-id",
+				Value: getstring(res.VersionId),
+			},
+		})
+	}
 
 	status := http.StatusOK
 	if acceptRange != "" {
@@ -2945,6 +2954,7 @@ func (c S3ApiController) HeadObject(ctx *fiber.Ctx) error {
 			Value: getstring(res.VersionId),
 		})
 	}
+
 	utils.SetResponseHeaders(ctx, headers)
 
 	return SendResponse(ctx, nil,

s3err/s3err.go

@@ -133,6 +133,7 @@ const (
 	ErrInvalidMetadataDirective
 	ErrKeyTooLong
 	ErrInvalidVersionId
+	ErrNoSuchVersion
 
 	// Non-AWS errors
 	ErrExistingObjectIsDirectory
@@ -531,6 +532,11 @@ var errorCodeResponse = map[ErrorCode]APIError{
 		Description:    "Your key is too long.",
 		HTTPStatusCode: http.StatusBadRequest,
 	},
+	ErrNoSuchVersion: {
+		Code:           "NoSuchVersion",
+		Description:    "The specified version does not exist.",
+		HTTPStatusCode: http.StatusNotFound,
+	},
 
 	// non aws errors
 	ErrExistingObjectIsDirectory: {

tests/integration/group-tests.go

@@ -469,6 +469,9 @@ func TestFullFlow(s *S3Conf) {
 	TestGetObjectLegalHold(s)
 	TestWORMProtection(s)
 	TestAccessControl(s)
+	if s.versioningEnabled {
+		TestVersioning(s)
+	}
 }
 
 func TestPosix(s *S3Conf) {
@@ -503,6 +506,34 @@ func TestAccessControl(s *S3Conf) {
 	AccessControl_copy_object_with_starting_slash_for_user(s)
 }
 
+func TestVersioning(s *S3Conf) {
+	PutBucketVersioning_non_existing_bucket(s)
+	PutBucketVersioning_invalid_status(s)
+	PutBucketVersioning_success(s)
+	GetBucketVersioning_non_existing_bucket(s)
+	GetBucketVersioning_success(s)
+	Versioning_PutObject_success(s)
+	Versioning_CopyObject_success(s)
+	Versioning_CopyObject_non_existing_version_id(s)
+	Versioning_CopyObject_from_an_object_version(s)
+	Versioning_HeadObject_invalid_versionId(s)
+	Versioning_HeadObject_success(s)
+	Versioning_HeadObject_delete_marker(s)
+	Versioning_GetObject_invalid_versionId(s)
+	Versioning_GetObject_success(s)
+	Versioning_GetObject_delete_marker(s)
+	Versioning_DeleteObject_delete_object_version(s)
+	Versioning_DeleteObject_delete_a_delete_marker(s)
+	Versioning_DeleteObjects_success(s)
+	Versioning_DeleteObjects_delete_deleteMarkers(s)
+	// ListObjectVersions
+	ListObjectVersions_non_existing_bucket(s)
+	ListObjectVersions_list_single_object_versions(s)
+	ListObjectVersions_list_multiple_object_versions(s)
+	ListObjectVersions_multiple_object_versions_truncated(s)
+	ListObjectVersions_with_delete_markers(s)
+}
+
 type IntTests map[string]func(s *S3Conf) error
 
 func GetIntTests() IntTests {
@@ -812,5 +843,29 @@ func GetIntTests() IntTests {
 		"AccessControl_root_PutBucketAcl":                                     AccessControl_root_PutBucketAcl,
 		"AccessControl_user_PutBucketAcl_with_policy_access":                  AccessControl_user_PutBucketAcl_with_policy_access,
 		"AccessControl_copy_object_with_starting_slash_for_user":              AccessControl_copy_object_with_starting_slash_for_user,
+		"PutBucketVersioning_non_existing_bucket":                             PutBucketVersioning_non_existing_bucket,
+		"PutBucketVersioning_invalid_status":                                  PutBucketVersioning_invalid_status,
+		"PutBucketVersioning_success":                                         PutBucketVersioning_success,
+		"GetBucketVersioning_non_existing_bucket":                             GetBucketVersioning_non_existing_bucket,
+		"GetBucketVersioning_success":                                         GetBucketVersioning_success,
+		"Versioning_PutObject_success":                                        Versioning_PutObject_success,
+		"Versioning_CopyObject_success":                                       Versioning_CopyObject_success,
+		"Versioning_CopyObject_non_existing_version_id":                       Versioning_CopyObject_non_existing_version_id,
+		"Versioning_CopyObject_from_an_object_version":                        Versioning_CopyObject_from_an_object_version,
+		"Versioning_HeadObject_invalid_versionId":                             Versioning_HeadObject_invalid_versionId,
+		"Versioning_HeadObject_success":                                       Versioning_HeadObject_success,
+		"Versioning_HeadObject_delete_marker":                                 Versioning_HeadObject_delete_marker,
+		"Versioning_GetObject_invalid_versionId":                              Versioning_GetObject_invalid_versionId,
+		"Versioning_GetObject_success":                                        Versioning_GetObject_success,
+		"Versioning_GetObject_delete_marker":                                  Versioning_GetObject_delete_marker,
+		"Versioning_DeleteObject_delete_object_version":                       Versioning_DeleteObject_delete_object_version,
+		"Versioning_DeleteObject_delete_a_delete_marker":                      Versioning_DeleteObject_delete_a_delete_marker,
+		"Versioning_DeleteObjects_success":                                    Versioning_DeleteObjects_success,
+		"Versioning_DeleteObjects_delete_deleteMarkers":                       Versioning_DeleteObjects_delete_deleteMarkers,
+		"ListObjectVersions_non_existing_bucket":                              ListObjectVersions_non_existing_bucket,
+		"ListObjectVersions_list_single_object_versions":                      ListObjectVersions_list_single_object_versions,
+		"ListObjectVersions_list_multiple_object_versions":                    ListObjectVersions_list_multiple_object_versions,
+		"ListObjectVersions_multiple_object_versions_truncated":               ListObjectVersions_multiple_object_versions_truncated,
+		"ListObjectVersions_with_delete_markers":                              ListObjectVersions_with_delete_markers,
 	}
 }

tests/integration/s3conf.go

@@ -31,15 +31,16 @@ import (
 )
 
 type S3Conf struct {
-	awsID           string
-	awsSecret       string
-	awsRegion       string
-	endpoint        string
-	checksumDisable bool
-	pathStyle       bool
-	PartSize        int64
-	Concurrency     int
-	debug           bool
+	awsID             string
+	awsSecret         string
+	awsRegion         string
+	endpoint          string
+	checksumDisable   bool
+	pathStyle         bool
+	PartSize          int64
+	Concurrency       int
+	debug             bool
+	versioningEnabled bool
 }
 
 func NewS3Conf(opts ...Option) *S3Conf {
@@ -80,6 +81,9 @@ func WithConcurrency(c int) Option {
 func WithDebug() Option {
 	return func(s *S3Conf) { s.debug = true }
 }
+func WithVersioningEnabled() Option {
+	return func(s *S3Conf) { s.versioningEnabled = true }
+}
 
 func (c *S3Conf) getCreds() credentials.StaticCredentialsProvider {
 	// TODO support token/IAM