Repository: minio/minio (language: go)

Issue:
Panic Error: slice bounds out of range, in DeleteVersion
Hello MinIO Community, 

I experience a potential bug/missed error handling in the Way Versions are handled/deleted. 

After upgrading to Version 2023-12-23T07:19:11Z from 2023-11-20T22:40:07Z, my cluster crashes every 1-2 minutes, as soon as some specific versioned Files are to be deleted. The Error Message is: 


```
panic: runtime error: slice bounds out of range [3:1]
goroutine 57207 [running]:
github.com/minio/minio/cmd.(*xlMetaV2).DeleteVersion(_, {{0x0, 0x0}, {0xc0137d0000, 0x3a}, {0x0, 0x0}, 0x0, 0x0, {0x0, ...}, ...})
#011github.com/minio/minio/cmd/xl-storage-format-v2.go:1414 +0x2e30
github.com/minio/minio/cmd.(*xlStorage).DeleteVersion(_, {_, _}, {_, _}, {_, _}, {{0x0, 0x0}, {0xc0137d0000, ...}, ...}, ...)
#011github.com/minio/minio/cmd/xl-storage.go:1212 +0x768
github.com/minio/minio/cmd.(*xlStorageDiskIDCheck).DeleteVersion.func1()
#011github.com/minio/minio/cmd/xl-storage-disk-id-check.go:593 +0x66
github.com/minio/minio/internal/ioutil.(*DeadlineWorker).Run.func1()
#011github.com/minio/minio/internal/ioutil/ioutil.go:141 +0x22
created by github.com/minio/minio/internal/ioutil.(*DeadlineWorker).Run in goroutine 57256
#011github.com/minio/minio/internal/ioutil/ioutil.go:140 +0xbe

panic: runtime error: slice bounds out of range [2:1]
goroutine 4223965 [running]:
github.com/minio/minio/cmd.(*xlMetaV2).DeleteVersion(_, {{0x0, 0x0}, {0xc019e5e280, 0x45}, {0x0, 0x0}, 0x0, 0x0, {0x0, ...}, ...})
#011github.com/minio/minio/cmd/xl-storage-format-v2.go:1414 +0x2e30
github.com/minio/minio/cmd.(*xlStorage).DeleteVersion(_, {_, _}, {_, _}, {_, _}, {{0x0, 0x0}, {0xc019e5e280, ...}, ...}, ...)
#011github.com/minio/minio/cmd/xl-storage.go:1212 +0x768
github.com/minio/minio/cmd.(*xlStorageDiskIDCheck).DeleteVersion.func1()
#011github.com/minio/minio/cmd/xl-storage-disk-id-check.go:593 +0x66
github.com/minio/minio/internal/ioutil.(*DeadlineWorker).Run.func1()
#011github.com/minio/minio/internal/ioutil/ioutil.go:141 +0x22
created by github.com/minio/minio/internal/ioutil.(*DeadlineWorker).Run in goroutine 4226206
#011github.com/minio/minio/internal/ioutil/ioutil.go:140 +0xbe
```
 
A specific File for Example looks like this: 
```
mc ls minio/loki-chunks/index/loki_index_19343/loki-write-0-1671277734359685422-1671317692.gz --versions
[2023-12-25 17:02:15 CET]     0B STANDARD null v1 DEL loki-write-0-1671277734359685422-1671317692.gz
```
 
If I try to delete it: 
```
mc rm minio/loki-chunks/index/loki_index_19343/loki-write-0-1671277734359685422-1671317692.gz --versions --force
Removed `minio/loki-chunks/index/loki_index_19343/loki-write-0-1671277734359685422-1671317692.gz` (versionId=null).
```
the file is still there after Server crashed and restarted.
the Trace for this request end up failing:
```
mc admin trace -v minio --path loki-chunks/index                                                                
mc: <ERROR> Unable to listen to http trace: unexpected EOF.
```
 
This happens buckets that have been versioned for quite some time. 

## Expected Behavior
A version of a file, the file itself etc. Is deleted. 

## Current Behavior
When trying to delete the file it is suggested by cli that it is working, but the server is crashing with a panic error. After the Crash, the Cluster keeps restarting for a few minutes repeatedly trying to delete the dangling files. 

## Possible Solution
Just give an error message, delete the file that is broken instead of panic error. 
Even better would be, if such orphanage files would be removed automatically. 

## Steps to Reproduce (for bugs)
No steps to reproduce on a new system possible as it requires older files. 

## Context
Old Versions need to be pruned from Buckets. 

## Regression
There are multiple Issues that had the context of Old Files making Problems, but i cannot name a Version specific

## Your Environment
* Version used (`minio --version`):
```
mc admin info minio-hq
●  minio-1.example.org:9000
   Uptime: 17 minutes 
   Version: 2023-12-23T07:19:11Z
   Network: 4/4 OK 
   Drives: 8/8 OK 
   Pool: 1

●  minio-2.example.org:9000
   Uptime: 17 minutes 
   Version: 2023-12-23T07:19:11Z
   Network: 4/4 OK 
   Drives: 8/8 OK 
   Pool: 1

●  minio-3.example.org:9000
   Uptime: 17 minutes 
   Version: 2023-12-23T07:19:11Z
   Network: 4/4 OK 
   Drives: 8/8 OK 
   Pool: 1

●  minio-4.example.org:9000
   Uptime: 17 minutes 
   Version: 2023-12-23T07:19:11Z
   Network: 4/4 OK 
   Drives: 8/8 OK 
   Pool: 1

Pools:
   1st, Erasure sets: 2, Drives per erasure set: 16

3.6 TiB Used, 44 Buckets, 402,534 Objects, 494,349 Versions, 89,961 Delete Markers
32 drives online, 0 drives offline
```

* Server setup and configuration:
4 baremetal Linux Server
- 8 Drives each 
- filesystem: xfs
- HBA mode
* Operating System and version (`uname -a`):
```
uname -a
Linux minio-3 5.4.0-164-generic #181-Ubuntu SMP Fri Sep 1 13:41:22 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
```



Discussion / hints:
@jiuker Could you check if this is a regression from #18547 ?


I can't reproduce it - it seems to me there is some inconsistency in the xl.meta versions that is causing this, we need the exact file here to reproduce this problem @HannesDampft 

Can you share the file with us, capturing it directly from the backend disks?
Hello @harshavardhana 
sure i can provide you with the files. you need the xl.meta of each drive from each server from one exemplar defekt one (or is there a documented way to get files from the backend?)
You have to manually look for it and capture from all nodes.
I got all files concerning one exemplar File (the one mentioned in the Issue).
where shall i sent the archive to?

files are only on disk 1-4, on 5-8 they were missing.
```
.
├── minio-1
│   └── mnt
│       ├── disk1
│       │   └── data
│       │       └── loki-mt-chunks
│       │           └── index
│       │               └── loki_index_19343
│       │                   └── loki-write-0-1671277734359685422-1671317692.gz
│       │                       └── xl.meta
│       ├── disk2
│       │   └── data
│       │       └── loki-mt-chunks
│       │           └── index
│       │               └── loki_index_19343
│       │                   └── loki-write-0-1671277734359685422-1671317692.gz
│       │                       └── xl.meta
│       ├── disk3
│       │   └── data
│       │       └── loki-mt-chunks
│       │           └── index
│       │               └── loki_index_19343
│       │                   └── loki-write-0-1671277734359685422-1671317692.gz
│       │                       └── xl.meta
│       └── disk4
│           └── data
│               └── loki-mt-chunks
│                   └── index
│                       └── loki_index_19343
│                           └── loki-write-0-1671277734359685422-1671317692.gz
│                               └── xl.meta
├── minio-2
│   └── mnt
│       ├── disk1
│       │   └── data
│       │       └── loki-mt-chunks
│       │           └── index
│       │               └── loki_index_19343
│       │                   └── loki-write-0-1671277734359685422-1671317692.gz
│       │                       └── xl.meta
│       ├── disk2
│       │   └── data
│       │       └── loki-mt-chunks
│       │           └── index
│       │               └── loki_index_19343
│       │                   └── loki-write-0-1671277734359685422-1671317692.gz
│       │                       └── xl.meta
│       ├── disk3
│       │   └── data
│       │       └── loki-mt-chunks
│       │           └── index
│       │               └── loki_index_19343
│       │                   └── loki-write-0-1671277734359685422-1671317692.gz
│       │                       └── xl.meta
│       └── disk4
│           └── data
│               └── loki-mt-chunks
│                   └── index
│                       └── loki_index_19343
│                           └── loki-write-0-1671277734359685422-1671317692.gz
│                               └── xl.meta
├── minio-3
│   └── mnt
│       ├── disk1
│       │   └── data
│       │       └── loki-mt-chunks
│       │           └── index
│       │               └── loki_index_19343
│       │                   └── loki-write-0-1671277734359685422-1671317692.gz
│       │                       └── xl.meta
│       ├── disk2
│       │   └── data
│       │       └── loki-mt-chunks
│       │           └── index
│       │               └── loki_index_19343
│       │                   └── loki-write-0-1671277734359685422-1671317692.gz
│       │                       └── xl.meta
│       ├── disk3
│       │   └── data
│       │       └── loki-mt-chunks
│       │           └── index
│       │               └── loki_index_19343
│       │                   └── loki-write-0-1671277734359685422-1671317692.gz
│       │                       └── xl.meta
│       └── disk4
│           └── data
│               └── loki-mt-chunks
│                   └── index
│                       └── loki_index_19343
│                           └── loki-write-0-1671277734359685422-1671317692.gz
│                               └── xl.meta
└── minio-4
    └── mnt
        ├── disk1
        │   └── data
        │       └── loki-mt-chunks
        │           └── index
        │               └── loki_index_19343
        │                   └── loki-write-0-1671277734359685422-1671317692.gz
        │                       └── xl.meta
        ├── disk2
        │   └── data
        │       └── loki-mt-chunks
        │           └── index
        │               └── loki_index_19343
        │                   └── loki-write-0-1671277734359685422-1671317692.gz
        │                       └── xl.meta
        ├── disk3
        │   └── data
        │       └── loki-mt-chunks
        │           └── index
        │               └── loki_index_19343
        │                   └── loki-write-0-1671277734359685422-1671317692.gz
        │                       └── xl.meta
        └── disk4
            └── data
                └── loki-mt-chunks
                    └── index
                        └── loki_index_19343
                            └── loki-write-0-1671277734359685422-1671317692.gz
                                └── xl.meta
```
Just upload it here as a ZIP file.
here you go!
[issue18724.zip](https://github.com/minio/minio/files/13800635/issue18724.zip)


@HannesDampft can you also capture your `.minio.sys/format.json` from all the disks? 
Nevermind reproduced it 

```
mc rm --versions --force myminio/loki-mt-chunks/index/loki_index_19343/loki-write-0-1671277734359685422-1671317692.gz
panic: runtime error: slice bounds out of range [4:2]

goroutine 4247 [running]:
github.com/minio/minio/cmd.(*xlMetaV2).DeleteVersion(_, {{0x0, 0x0}, {0xc000055720, 0x45}, {0x0, 0x0}, 0x0, 0x0, {0x0, ...}, ...})
	github.com/minio/minio/cmd/xl-storage-format-v2.go:1414 +0x2e30
github.com/minio/minio/cmd.(*xlStorage).deleteVersions(0xc000f0b600, {0x4c1c780, 0xc0136234a0}, {0xc00184d446, 0xe}, {0xc000055720, 0x45}, {0xc01056cf00, 0x1, 0x1})
	github.com/minio/minio/cmd/xl-storage.go:1081 +0x5c9
github.com/minio/minio/cmd.(*xlStorage).DeleteVersions.func1()
	github.com/minio/minio/cmd/xl-storage.go:1141 +0x4d
github.com/minio/minio/internal/ioutil.(*DeadlineWorker).Run.func1()
	github.com/minio/minio/internal/ioutil/ioutil.go:141 +0x22
created by github.com/minio/minio/internal/ioutil.(*DeadlineWorker).Run in goroutine 4246
	github.com/minio/minio/internal/ioutil/ioutil.go:140 +0xbe
```

Your files have a lot of history - 

- You had replication in the past and there was an incessant DEL marker creations in the past
- Then you forcibly removed replication and also suspended versioning on the bucket
- Then you recreated the `null` DEL marker

:-) 

That sounds historically right. (amazing what some metadata can tell about the history of the whole cluster)
Point 1-2 were an old test to create Backup via Site-Replication as adviced by MinIO, but after Problems i had to remove it again and suspended/removed Versioning from the bucket. (that was about a Year ago)
Point 3 would be the DEL Marker i added now?

Do you still need the `.minio.sys/format.json`?

Fix patch:
diff --git a/.github/workflows/replication.yaml b/.github/workflows/replication.yaml
index 46b5434b647ad..e6a674f5a3534 100644
--- a/.github/workflows/replication.yaml
+++ b/.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
diff --git a/Makefile b/Makefile
index 22a05d488b1f0..099ed4db1ecc5 100644
--- a/Makefile
+++ b/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
 
diff --git a/cmd/xl-storage-format-v2.go b/cmd/xl-storage-format-v2.go
index b9b84068ec524..a84f829fd1e63 100644
--- a/cmd/xl-storage-format-v2.go
+++ b/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)


Test patch:
diff --git a/docs/bucket/versioning/versioning-tests.sh b/docs/bucket/versioning/versioning-tests.sh
new file mode 100755
index 0000000000000..584734b6f93ee
--- /dev/null
+++ b/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