From 003719cc74a6e11fb92d8398634d0a30acaeeb8e Mon Sep 17 00:00:00 2001
From: Ben McClelland <ben.mcclelland@versity.com>
Date: Sat, 1 Mar 2025 10:50:30 -0800
Subject: [PATCH] feat: return method not allowed when uploading to read only
 fs

Instead of an internal server error, we should be returning
method not allowed when trying to upload to a read only
filesystem or make other modifications that are expected to
fail. This will give clearer feedback to the clients
that this is not expected to work.

Fixes #1062
---
 backend/meta/xattr.go             | 16 ++++++++++++++--
 backend/posix/posix.go            |  3 +++
 backend/posix/with_otmpfile.go    |  5 +++++
 backend/posix/without_otmpfile.go |  8 ++++++++
 4 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/backend/meta/xattr.go b/backend/meta/xattr.go
index 89277b1..692b3c1 100644
--- a/backend/meta/xattr.go
+++ b/backend/meta/xattr.go
@@ -23,6 +23,7 @@ import (
 	"syscall"
 
 	"github.com/pkg/xattr"
+	"github.com/versity/versitygw/s3err"
 )
 
 const (
@@ -56,10 +57,18 @@ func (x XattrMeta) RetrieveAttribute(f *os.File, bucket, object, attribute strin
 // StoreAttribute stores the value of a specific attribute for an object in a bucket.
 func (x XattrMeta) StoreAttribute(f *os.File, bucket, object, attribute string, value []byte) error {
 	if f != nil {
-		return xattr.FSet(f, xattrPrefix+attribute, value)
+		err := xattr.FSet(f, xattrPrefix+attribute, value)
+		if errors.Is(err, syscall.EROFS) {
+			return s3err.GetAPIError(s3err.ErrMethodNotAllowed)
+		}
+		return err
 	}
 
-	return xattr.Set(filepath.Join(bucket, object), xattrPrefix+attribute, value)
+	err := xattr.Set(filepath.Join(bucket, object), xattrPrefix+attribute, value)
+	if errors.Is(err, syscall.EROFS) {
+		return s3err.GetAPIError(s3err.ErrMethodNotAllowed)
+	}
+	return err
 }
 
 // DeleteAttribute removes the value of a specific attribute for an object in a bucket.
@@ -68,6 +77,9 @@ func (x XattrMeta) DeleteAttribute(bucket, object, attribute string) error {
 	if errors.Is(err, xattr.ENOATTR) {
 		return ErrNoSuchKey
 	}
+	if errors.Is(err, syscall.EROFS) {
+		return s3err.GetAPIError(s3err.ErrMethodNotAllowed)
+	}
 	return err
 }
 
diff --git a/backend/posix/posix.go b/backend/posix/posix.go
index 94a4909..591dff1 100644
--- a/backend/posix/posix.go
+++ b/backend/posix/posix.go
@@ -362,6 +362,9 @@ func (p *Posix) CreateBucket(ctx context.Context, input *s3.CreateBucketInput, a
 		return s3err.GetAPIError(s3err.ErrBucketAlreadyExists)
 	}
 	if err != nil {
+		if errors.Is(err, syscall.EROFS) {
+			return s3err.GetAPIError(s3err.ErrMethodNotAllowed)
+		}
 		return fmt.Errorf("mkdir bucket: %w", err)
 	}
 
diff --git a/backend/posix/with_otmpfile.go b/backend/posix/with_otmpfile.go
index ac0281e..beb626c 100644
--- a/backend/posix/with_otmpfile.go
+++ b/backend/posix/with_otmpfile.go
@@ -29,6 +29,7 @@ import (
 
 	"github.com/versity/versitygw/auth"
 	"github.com/versity/versitygw/backend"
+	"github.com/versity/versitygw/s3err"
 	"golang.org/x/sys/unix"
 )
 
@@ -62,6 +63,10 @@ func (p *Posix) openTmpFile(dir, bucket, obj string, size int64, acct auth.Accou
 	// this is not supported.
 	fd, err := unix.Open(dir, unix.O_RDWR|unix.O_TMPFILE|unix.O_CLOEXEC, defaultFilePerm)
 	if err != nil {
+		if errors.Is(err, syscall.EROFS) {
+			return nil, s3err.GetAPIError(s3err.ErrMethodNotAllowed)
+		}
+
 		// O_TMPFILE not supported, try fallback
 		err = backend.MkdirAll(dir, uid, gid, doChown, p.newDirPerm)
 		if err != nil {
diff --git a/backend/posix/without_otmpfile.go b/backend/posix/without_otmpfile.go
index ed74237..2310394 100644
--- a/backend/posix/without_otmpfile.go
+++ b/backend/posix/without_otmpfile.go
@@ -24,9 +24,11 @@ import (
 	"io/fs"
 	"os"
 	"path/filepath"
+	"syscall"
 
 	"github.com/versity/versitygw/auth"
 	"github.com/versity/versitygw/backend"
+	"github.com/versity/versitygw/s3err"
 )
 
 type tmpfile struct {
@@ -43,11 +45,17 @@ func (p *Posix) openTmpFile(dir, bucket, obj string, size int64, acct auth.Accou
 	var err error
 	err = backend.MkdirAll(dir, uid, gid, doChown, p.newDirPerm)
 	if err != nil {
+		if errors.Is(err, syscall.EROFS) {
+			return nil, s3err.GetAPIError(s3err.ErrMethodNotAllowed)
+		}
 		return nil, fmt.Errorf("make temp dir: %w", err)
 	}
 	f, err := os.CreateTemp(dir,
 		fmt.Sprintf("%x.", sha256.Sum256([]byte(obj))))
 	if err != nil {
+		if errors.Is(err, syscall.EROFS) {
+			return nil, s3err.GetAPIError(s3err.ErrMethodNotAllowed)
+		}
 		return nil, fmt.Errorf("create temp file: %w", err)
 	}