PWX-40172: makes chattr ignore 'inappropriate ioctl' errors

Signed-off-by: Adam Krpan <[email protected]>

pkg/chattr/chattr.go

@@ -16,71 +16,57 @@ const (
 	lsattrCmd = "lsattr"
 )
 
-func AddImmutable(path string) error {
-	return ForceAddImmutable(path, false)
+func isInappropriateIoctl(err string) bool {
+	lowerErr := strings.ToLower(err)
+	// "inappropriate ioctl for device" returned inside PX container
+	// "invalid argument while setting flags" returned on host
+	return strings.Contains(lowerErr, "inappropriate ioctl for device") || strings.Contains(lowerErr, "invalid argument while setting flags")
 }
 
-func ForceAddImmutable(path string, force bool) error {
+func runChattr(path string, arg string, force bool) error {
 	chattrBin := which(chattrCmd)
 	if _, err := os.Stat(path); err == nil {
-		var cmd *exec.Cmd
+		args := []string{arg}
 		if force {
-			cmd = exec.Command(chattrBin, "+i", "-f", path)
-		} else {
-			cmd = exec.Command(chattrBin, "+i", path)
+			args = append(args, "-f")
 		}
+		args = append(args, path)
+		cmd := exec.Command(chattrBin, args...)
 		var stderr bytes.Buffer
 		cmd.Stderr = &stderr
 
-		err = cmd.Run()
-		if err != nil {
+		if err = cmd.Run(); err != nil {
 			// chattr exits with status 1 for files that are not permitted without force flag
 			// this will happen for files that throws the error
 			//  "Operation not supported while reading flags"
 			if force && cmd.ProcessState.ExitCode() == 1 {
 				return nil
 			}
-			if force {
-				return fmt.Errorf("%s +i -f failed: %s. Err: %v", chattrBin, stderr.String(), err)
+
+			stderrStr := stderr.String()
+			if isInappropriateIoctl(stderrStr) { // Returned in case of filesystem that does not support chattr
+				return nil
 			}
-			return fmt.Errorf("%s +i failed: %s. Err: %v", chattrBin, stderr.String(), err)
+			return fmt.Errorf("%s %s failed: %s. Err: %v", chattrBin, strings.Join(args, " "), stderrStr, err)
 		}
 	}
 
 	return nil
 }
 
+func AddImmutable(path string) error {
+	return ForceAddImmutable(path, false)
+}
+
+func ForceAddImmutable(path string, force bool) error {
+	return runChattr(path, "+i", force)
+}
+
 func RemoveImmutable(path string) error {
 	return ForceRemoveImmutable(path, false)
 }
 func ForceRemoveImmutable(path string, force bool) error {
-	chattrBin := which(chattrCmd)
-	if _, err := os.Stat(path); err == nil {
-		var cmd *exec.Cmd
-		if force {
-			cmd = exec.Command(chattrBin, "-i", "-f", path)
-		} else {
-			cmd = exec.Command(chattrBin, "-i", path)
-		}
-		var stderr bytes.Buffer
-		cmd.Stderr = &stderr
-
-		err = cmd.Run()
-		if err != nil {
-			// chattr exits with status 1 for files that are not permitted without force flag
-			// this will happen for files that throws the error
-			//  "Operation not supported while reading flags"
-			if force && cmd.ProcessState.ExitCode() == 1 {
-				return nil
-			}
-			if force {
-				return fmt.Errorf("%s -i -f failed: %s. Err: %v", chattrBin, stderr.String(), err)
-			}
-			return fmt.Errorf("%s -i failed: %s. Err: %v", chattrBin, stderr.String(), err)
-		}
-	}
-
-	return nil
+	return runChattr(path, "-i", force)
 }
 
 func IsImmutable(path string) bool {
@@ -91,6 +77,9 @@ func IsImmutable(path string) bool {
 	}
 	op, err := exec.Command(lsattrBin, "-d", path).CombinedOutput()
 	if err != nil {
+		if isInappropriateIoctl(string(op)) { // Returned in case of filesystem that does not support chattr
+			return false
+		}
 		// Cannot get path status, return true so that immutable bit is not reverted
 		logrus.Errorf("Error listing attrs for %v err:%v", path, string(op))
 		return true

pkg/chattr/chattr_test.go

@@ -1,7 +1,9 @@
 package chattr
 
 import (
+	"fmt"
 	"os"
+	"os/exec"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -48,5 +50,56 @@ func TestImmutable(t *testing.T) {
 	_, err = os.Stat(testFile)
 	require.Error(t, err, "Expected the file to be removed")
 	require.True(t, os.IsNotExist(err), "Unexpected error on remove")
+}
+
+func TestImmutable_UnsupportedFilesystem(t *testing.T) {
+
+	const (
+		imgFile   = "/tmp/chattr-test.img"
+		ntfsMount = "/tmp/chattr-test-mount"
+		fsType    = "ntfs"
+	)
+	mkfsCommand := "mkfs." + fsType
+	if which(mkfsCommand) == mkfsCommand {
+		t.Skip("Skipping test: " + mkfsCommand + " not found")
+	}
+
+	// Create a file to serve as a disk image
+	err := exec.Command("dd", "if=/dev/zero", "of="+imgFile, "bs=1M", "count=100").Run()
+	require.NoError(t, err, "Failed to create NTFS image file")
+
+	defer func() {
+		err = os.Remove(imgFile)
+		require.NoError(t, err, "Failed to remove NTFS image file")
+	}()
+
+	// Format the file
+	err = exec.Command(mkfsCommand, "-F", imgFile).Run()
+	require.NoError(t, err, fmt.Sprintf("Failed to format %s image", fsType))
+
+	// Create mount point
+	err = os.MkdirAll(ntfsMount, 0755)
+	require.NoError(t, err, "Failed to create mount point")
+
+	defer func() {
+		err = os.RemoveAll(ntfsMount)
+		require.NoError(t, err, "Failed to remove mount point")
+	}()
+
+	// Mount the image
+	err = exec.Command("mount", "-o", "loop", "-t", fsType, imgFile, ntfsMount).Run()
+	require.NoError(t, err, "Failed to mount image")
+
+	defer func() {
+		err = exec.Command("umount", ntfsMount).Run()
+		require.NoError(t, err, "Failed to unmount image")
+	}()
+
+	// Attempt to add immutable attribute to the mount point: should not error even though filesystem does not support it
+	err = AddImmutable(ntfsMount)
+	require.NoError(t, err, "Unexpected error on AddImmutable")
 
+	// Check if the mount point is immutable
+	isImmutable := IsImmutable(ntfsMount)
+	require.False(t, isImmutable, "Mount point should not be immutable")
 }