fix: an odd crash when deleting null DEL markers (#18727)

fixes #18724

A regression was introduced in #18547, that attempted
to file adding a missing `null` marker however we
should not skip returning based on versionID instead
it must be based on if we are being asked to create
a DEL marker or not.

The PR also has a side-affect for replicating `null`
marker permanent delete, as it may end up adding a
`null` marker while removing one.

This PR should address both scenarios.

.github/workflows/replication.yaml

@@ -54,3 +54,8 @@ jobs:
           sudo sysctl net.ipv6.conf.default.disable_ipv6=0
           make test-site-replication-minio
 
+      - name: Test Versioning
+        run: |
+          sudo sysctl net.ipv6.conf.all.disable_ipv6=0
+          sudo sysctl net.ipv6.conf.default.disable_ipv6=0
+          make test-versioning

Makefile

@@ -59,6 +59,10 @@ test-decom: install-race
 	@env bash $(PWD)/docs/distributed/decom-encrypted-sse-s3.sh
 	@env bash $(PWD)/docs/distributed/decom-compressed-sse-s3.sh
 
+test-versioning: install-race
+	@echo "Running minio versioning tests"
+	@env bash $(PWD)/docs/bucket/versioning/versioning-tests.sh
+
 test-configfile: install-race
 	@env bash $(PWD)/docs/distributed/distributed-from-config-file.sh
 

cmd/xl-storage-format-v2.go

@@ -1410,15 +1410,13 @@ func (x *xlMetaV2) DeleteVersion(fi FileInfo) (string, error) {
 				err = x.setIdx(i, *ver)
 				return "", err
 			}
-			var err error
 			x.versions = append(x.versions[:i], x.versions[i+1:]...)
 			if fi.MarkDeleted && (fi.VersionPurgeStatus().Empty() || (fi.VersionPurgeStatus() != Complete)) {
 				err = x.addVersion(ventry)
+			} else if fi.Deleted && uv.String() == emptyUUID {
+				return "", x.addVersion(ventry)
 			}
-			// if we remove null version. we should try to add null version to top layer.
-			if uv.String() != emptyUUID {
-				return "", err
-			}
+			return "", err
 		case ObjectType:
 			if updateVersion && !fi.Deleted {
 				ver, err := x.getIdx(i)

docs/bucket/versioning/versioning-tests.sh

@@ -0,0 +1,81 @@
+#!/usr/bin/env bash
+
+if [ -n "$TEST_DEBUG" ]; then
+	set -x
+fi
+
+trap 'catch $LINENO' ERR
+
+# shellcheck disable=SC2120
+catch() {
+	if [ $# -ne 0 ]; then
+		echo "error on line $1"
+		echo "$site server logs ========="
+		cat "/tmp/${site}_1.log"
+		echo "==========================="
+		cat "/tmp/${site}_2.log"
+	fi
+
+	echo "Cleaning up instances of MinIO"
+	pkill minio
+	pkill -9 minio
+	rm -rf /tmp/multisitea
+}
+
+catch
+
+set -e
+export MINIO_CI_CD=1
+export MINIO_BROWSER=off
+export MINIO_ROOT_USER="minio"
+export MINIO_ROOT_PASSWORD="minio123"
+export MINIO_KMS_AUTO_ENCRYPTION=off
+export MINIO_PROMETHEUS_AUTH_TYPE=public
+export MINIO_KMS_SECRET_KEY=my-minio-key:OSMM+vkKUTCvQs9YL/CVMIMt43HFhkUpqJxTmGl6rYw=
+unset MINIO_KMS_KES_CERT_FILE
+unset MINIO_KMS_KES_KEY_FILE
+unset MINIO_KMS_KES_ENDPOINT
+unset MINIO_KMS_KES_KEY_NAME
+
+if [ ! -f ./mc ]; then
+	wget -O mc https://dl.minio.io/client/mc/release/linux-amd64/mc &&
+		chmod +x mc
+fi
+
+minio server --address 127.0.0.1:9001 "http://127.0.0.1:9001/tmp/multisitea/data/disterasure/xl{1...4}" \
+	"http://127.0.0.1:9002/tmp/multisitea/data/disterasure/xl{5...8}" >/tmp/sitea_1.log 2>&1 &
+minio server --address 127.0.0.1:9002 "http://127.0.0.1:9001/tmp/multisitea/data/disterasure/xl{1...4}" \
+	"http://127.0.0.1:9002/tmp/multisitea/data/disterasure/xl{5...8}" >/tmp/sitea_2.log 2>&1 &
+
+export MC_HOST_sitea=http://minio:minio123@127.0.0.1:9001
+
+./mc mb sitea/delissue
+
+./mc version enable sitea/delissue
+
+echo hello | ./mc pipe sitea/delissue/hello
+
+./mc version suspend sitea/delissue
+
+./mc rm sitea/delissue/hello
+
+./mc version enable sitea/delissue
+
+echo hello | ./mc pipe sitea/delissue/hello
+
+./mc version suspend sitea/delissue
+
+./mc rm sitea/delissue/hello
+
+count=$(./mc ls --versions sitea/delissue | wc -l)
+
+if [ ${count} -ne 3 ]; then
+	echo "BUG: expected number of versions to be '3' found ${count}"
+	echo "===== DEBUG ====="
+	./mc ls --versions sitea/delissue
+fi
+
+echo "SUCCESS:"
+./mc ls --versions sitea/delissue
+
+catch