From 84bf4624a4586ffae4fabf67c237597eb524b429 Mon Sep 17 00:00:00 2001
From: Harshavardhana <harsha@minio.io>
Date: Fri, 18 Sep 2020 00:16:16 -0700
Subject: [PATCH] fix: make sure to preserve metadata during overwrite in FS
 mode (#10512)

This bug was introduced in 14f0047295930fbaadc88438889270956689b6fe
almost 3yrs ago, as a side affect of removing stale `fs.json`
but we in-fact end up removing existing good `fs.json` for an
existing object, leading to some form of a data loss.

fixes #10496
---
 cmd/fs-v1-multipart.go | 31 +++++++++++++++++++++++++++----
 cmd/fs-v1.go           | 24 ++++++++++++++++++------
 2 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/cmd/fs-v1-multipart.go b/cmd/fs-v1-multipart.go
index dbe5afb9e..00d1617fa 100644
--- a/cmd/fs-v1-multipart.go
+++ b/cmd/fs-v1-multipart.go
@@ -19,6 +19,7 @@ package cmd
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -708,13 +709,35 @@ func (fs *FSObjects) CompleteMultipartUpload(ctx context.Context, bucket string,
 		return oi, err
 	}
 	defer destLock.Unlock()
-	fsMetaPath := pathJoin(fs.fsPath, minioMetaBucket, bucketMetaPrefix, bucket, object, fs.metaJSONFile)
-	metaFile, err := fs.rwPool.Create(fsMetaPath)
+
+	bucketMetaDir := pathJoin(fs.fsPath, minioMetaBucket, bucketMetaPrefix)
+	fsMetaPath := pathJoin(bucketMetaDir, bucket, object, fs.metaJSONFile)
+	metaFile, err := fs.rwPool.Write(fsMetaPath)
+	var freshFile bool
 	if err != nil {
-		logger.LogIf(ctx, err)
-		return oi, toObjectErr(err, bucket, object)
+		if !errors.Is(err, errFileNotFound) {
+			logger.LogIf(ctx, err)
+			return oi, toObjectErr(err, bucket, object)
+		}
+		metaFile, err = fs.rwPool.Create(fsMetaPath)
+		if err != nil {
+			logger.LogIf(ctx, err)
+			return oi, toObjectErr(err, bucket, object)
+		}
+		freshFile = true
 	}
 	defer metaFile.Close()
+	defer func() {
+		// Remove meta file when CompleteMultipart encounters
+		// any error and it is a fresh file.
+		//
+		// We should preserve the `fs.json` of any
+		// existing object
+		if e != nil && freshFile {
+			tmpDir := pathJoin(fs.fsPath, minioMetaTmpBucket, fs.fsUUID)
+			fsRemoveMeta(ctx, bucketMetaDir, fsMetaPath, tmpDir)
+		}
+	}()
 
 	// Read saved fs metadata for ongoing multipart.
 	fsMetaBuf, err := ioutil.ReadFile(pathJoin(uploadIDDir, fs.metaJSONFile))
diff --git a/cmd/fs-v1.go b/cmd/fs-v1.go
index 82d6fbe34..394772d68 100644
--- a/cmd/fs-v1.go
+++ b/cmd/fs-v1.go
@@ -1173,18 +1173,30 @@ func (fs *FSObjects) putObject(ctx context.Context, bucket string, object string
 	var wlk *lock.LockedFile
 	if bucket != minioMetaBucket {
 		bucketMetaDir := pathJoin(fs.fsPath, minioMetaBucket, bucketMetaPrefix)
-
 		fsMetaPath := pathJoin(bucketMetaDir, bucket, object, fs.metaJSONFile)
-		wlk, err = fs.rwPool.Create(fsMetaPath)
+		wlk, err = fs.rwPool.Write(fsMetaPath)
+		var freshFile bool
 		if err != nil {
-			logger.LogIf(ctx, err)
-			return ObjectInfo{}, toObjectErr(err, bucket, object)
+			if !errors.Is(err, errFileNotFound) {
+				logger.LogIf(ctx, err)
+				return ObjectInfo{}, toObjectErr(err, bucket, object)
+			}
+			wlk, err = fs.rwPool.Create(fsMetaPath)
+			if err != nil {
+				logger.LogIf(ctx, err)
+				return ObjectInfo{}, toObjectErr(err, bucket, object)
+			}
+			freshFile = true
 		}
 		// This close will allow for locks to be synchronized on `fs.json`.
 		defer wlk.Close()
 		defer func() {
-			// Remove meta file when PutObject encounters any error
-			if retErr != nil {
+			// Remove meta file when PutObject encounters
+			// any error and it is a fresh file.
+			//
+			// We should preserve the `fs.json` of any
+			// existing object
+			if retErr != nil && freshFile {
 				tmpDir := pathJoin(fs.fsPath, minioMetaTmpBucket, fs.fsUUID)
 				fsRemoveMeta(ctx, bucketMetaDir, fsMetaPath, tmpDir)
 			}