Fix OpenFile confusion of flags and permissions

OpenFile used to accept `fs.FileMode` twice, as in:

```go
func (f *PosixFsys) OpenFile(path string, mode FileMode, perm FileMode)
  (*PosixFile, error)
```

The correct (or stdlib equivalent) signature is to accept an integer for
the status flags and fs.FileMode for the permissions, as in:

```go
func (f *PosixFsys) OpenFile(path string, flags int, perm fs.FileMode)
  (*PosixFile, error)
```

The OpenFile status flags are:

File access:
- O_RDONLY - open the file read-only.
- O_WRONLY - open the file write-only.
- O_RDWR - open the file read-write.

File creation:
- O_APPEND - append data to the file when writing.
- O_CREATE - create a new file if none exists.
- O_TRUNC  - truncate the file if it exists
- O_EXCL   - used in conjunction with O_CREATE, file must not already exist.

There's also O_SYNC but it is not implemented in rig, it's always sync.

See https://pkg.go.dev/os#pkg-constants

*Usage:*

    // Explanation of the flags:
    // - Create a new file if one doesn't exist
    // - Truncate an existing file if one exist (keeps
    //   existing permissions)
    // - Open the file for writing
    fsys.OpenFile("/tmp/example", os.O_CREATE|os.O_TRUNC|os.O_WRONLY,
        fs.FileMode(0644))

The `rigfs.FileMode` is now retired, you're now expected to use fs.FileMode
from `"io/fs"`.

Signed-off-by: Kimmo Lehto <[email protected]>

cmd/rigtest/rigtest.go

@@ -302,7 +302,7 @@ func main() {
 				shasum := sha256.New()
 				reader := io.TeeReader(origin, shasum)
 
-				destf, err := fsys.OpenFile(fn, rigfs.ModeCreate, 0644)
+				destf, err := fsys.OpenFile(fn, goos.O_CREATE|goos.O_WRONLY, 0644)
 				require.NoError(t, err, "open file")
 
 				n, err := io.Copy(destf, reader)
@@ -320,7 +320,7 @@ func main() {
 
 				require.Equal(t, fmt.Sprintf("%x", shasum.Sum(nil)), destSum, "sha256 mismatch after io.copy from local to remote")
 
-				destf, err = fsys.OpenFile(fn, rigfs.ModeRead, 0644)
+				destf, err = fsys.OpenFile(fn, goos.O_RDONLY, 0)
 				require.NoError(t, err, "open file for read")
 
 				readSha := sha256.New()

connection.go

@@ -372,7 +372,7 @@ func (c *Connection) Upload(src, dst string, _ ...exec.Option) error {
 	shasum := sha256.New()
 
 	fsys := c.Fsys()
-	remote, err := fsys.OpenFile(dst, rigfs.ModeCreate, rigfs.FileMode(stat.Mode()))
+	remote, err := fsys.OpenFile(dst, os.O_CREATE|os.O_WRONLY, stat.Mode())
 	if err != nil {
 		return fmt.Errorf("%w: open remote file %s for writing: %w", ErrInvalidPath, dst, err)
 	}

pkg/rigfs/fileinfo.go

@@ -36,29 +36,6 @@ func (f *FileInfo) UnmarshalJSON(b []byte) error {
 	}
 	f.FModTime = time.Unix(f.ModtimeS, 0)
 	f.FName = strings.ReplaceAll(f.FName, "\\", "/")
-
-	var newmode fs.FileMode
-
-	if f.FMode&1 != 0 { // "Readonly"
-		newmode = 0o444
-	}
-	if f.FMode&16 != 0 { // "Directory"
-		newmode |= fs.ModeDir | 0o544
-	}
-	if f.FMode&64 != 0 { // "Device"
-		newmode |= fs.ModeCharDevice
-	}
-	if f.FMode&4096 != 0 { // "Offline"
-		newmode |= fs.ModeIrregular
-	}
-	if f.FMode&1024 != 0 { // "ReparsePoint"
-		newmode |= fs.ModeIrregular
-		newmode |= fs.ModeSymlink
-	}
-	if f.FMode&256 != 0 { // "Temporary"
-		newmode |= fs.ModeTemporary
-	}
-	f.FMode = newmode
 	return nil
 }
 

pkg/rigfs/posixfsys.go

@@ -4,17 +4,18 @@ import (
 	"bytes"
 	_ "embed"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"io/fs"
 	"os"
 	"path/filepath"
 	"strconv"
 	"strings"
-	"time"
 
 	"github.com/alessio/shellescape"
 	"github.com/k0sproject/rig/exec"
+	"github.com/k0sproject/rig/log"
 )
 
 // rigHelper is a helper script to avoid having to write complex bash oneliners in Go
@@ -50,7 +51,8 @@ type PosixFile struct {
 	isEOF  bool
 	pos    int64
 	size   int64
-	mode   FileMode
+	mode   fs.FileMode
+	flags  int
 
 	blockSize int
 }
@@ -106,11 +108,11 @@ func (f *PosixFile) fsBlockSize() int {
 }
 
 func (f *PosixFile) isReadable() bool {
-	return f.mode&ModeRead != 0
+	return f.isOpen && (f.flags&os.O_WRONLY != os.O_WRONLY || f.flags&os.O_RDWR == os.O_RDWR)
 }
 
 func (f *PosixFile) isWritable() bool {
-	return f.mode&ModeWrite != 0
+	return f.isOpen && f.flags&os.O_WRONLY != 0
 }
 
 func (f *PosixFile) ddParams(offset int64, numBytes int) (int, int64, int) {
@@ -357,50 +359,87 @@ func (fsys *PosixFsys) Sha256(name string) (string, error) {
 
 // Open opens the named file for reading.
 func (fsys *PosixFsys) Open(name string) (fs.File, error) {
-	info, err := fsys.Stat(name)
-	if err != nil {
-		return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist}
+	return fsys.OpenFile(name, os.O_RDONLY, 0)
+}
+
+func (fsys *PosixFsys) createFile(name string, perm fs.FileMode) error {
+	if _, err := fsys.helper("touch", name); err != nil {
+		return err
 	}
-	file := PosixFile{fsys: fsys, path: name, isOpen: true, size: info.Size(), mode: ModeRead}
-	if info.IsDir() {
-		return &PosixDir{PosixFile: file}, nil
+
+	if _, err := fsys.helper("chmod", name, fmt.Sprintf("%#o", perm)); err != nil {
+		return err
 	}
-	return &file, nil
+	return nil
 }
 
-// OpenFile is the generalized open call; most users will use Open instead.. The perms are ignored if the file exists.
-func (fsys *PosixFsys) OpenFile(name string, mode FileMode, perm FileMode) (File, error) {
+// OpenFile is used to open a file with access/creation flags for reading or writing. For info on flags,
+// see https://pkg.go.dev/os#pkg-constants
+func (fsys *PosixFsys) OpenFile(name string, flags int, perm fs.FileMode) (File, error) { //nolint:cyclop
+	log.Debugf("openfile: %s flags: %d perm: %v", name, flags, perm)
 	var pos int64
 	info, err := fsys.Stat(name)
-	if err != nil {
-		// file exists
-		switch {
-		case mode&ModeRead == ModeRead:
+	log.Debugf("openfile: stat %s: %v err: %v", name, info, err)
+	if err != nil && !errors.Is(err, fs.ErrNotExist) {
+		return nil, err
+	}
+	fileExists := err == nil
+
+	switch fileExists {
+	case false:
+		switch flags & os.O_CREATE {
+		case 0:
 			return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist}
-		case mode&ModeCreate == ModeCreate:
-			if _, err := fsys.helper("touch", name); err != nil {
+		default:
+			log.Debugf("openfile: creating file %s", name)
+			if err := fsys.createFile(name, perm); err != nil {
 				return nil, err
 			}
-			if _, err := fsys.helper("chmod", name, fmt.Sprintf("%#o", perm)); err != nil {
+
+			// re-stat to ensure file is now there and get the correct bits if there's a umask
+			i, err := fsys.Stat(name)
+			log.Debugf("openfile: re-stat %s: %v err: %v", name, info, err)
+			if err != nil {
 				return nil, err
 			}
+			info = i
 		}
-		info = &FileInfo{FName: name, FMode: fs.FileMode(perm), FSize: 0, FIsDir: false, FModTime: time.Now(), fsys: fsys}
-	}
-	if info.IsDir() {
-		return nil, &fs.PathError{Op: "open", Path: name, Err: fmt.Errorf("%w: is a directory", fs.ErrPermission)}
-	}
-	switch {
-	case mode&ModeAppend == ModeAppend:
-		pos = info.Size()
-	case mode&ModeCreate == ModeCreate:
-		if info.Size() > 0 {
+	case true:
+		switch {
+		case info.IsDir():
+			log.Debugf("openfile: is dir: %s", name)
+			return nil, &fs.PathError{Op: "open", Path: name, Err: fmt.Errorf("%w: is a directory", fs.ErrInvalid)}
+		case flags&(os.O_CREATE|os.O_EXCL) == (os.O_CREATE | os.O_EXCL):
+			log.Debugf("openfile: create exclusive but exists: %s", name)
+			return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrExist}
+		case flags&os.O_TRUNC != 0:
+			log.Debugf("openfile: truncating: %s", name)
 			if _, err := fsys.helper("truncate", name, "0"); err != nil {
 				return nil, err
 			}
+			i, err := fsys.Stat(name)
+			log.Debugf("openfile: re-stat %s: %v err: %v", name, info, err)
+			if err != nil {
+				return nil, err
+			}
+			info = i
+		case flags&os.O_APPEND != 0:
+			log.Debugf("openfile: appending to: %s", name)
+			pos = info.Size()
 		}
 	}
-	return &PosixFile{fsys: fsys, path: name, isOpen: true, size: info.Size(), pos: pos, mode: mode}, nil
+
+	f := &PosixFile{
+		fsys:   fsys,
+		path:   name,
+		isOpen: true,
+		size:   info.Size(),
+		pos:    pos,
+		mode:   info.Mode(),
+		flags:  flags,
+	}
+	log.Debugf("openfile: returning file: %s: %+v", name, f)
+	return f, nil
 }
 
 // ReadDir reads the directory named by dirname and returns a list of directory entries
@@ -440,7 +479,7 @@ func (fsys *PosixFsys) RemoveAll(name string) error {
 
 // MkDirAll creates a new directory structure with the specified name and permission bits.
 // If the directory already exists, MkDirAll does nothing and returns nil.
-func (fsys *PosixFsys) MkDirAll(name string, perm FileMode) error {
+func (fsys *PosixFsys) MkDirAll(name string, perm fs.FileMode) error {
 	dir := shellescape.Quote(name)
 	if existing, err := fsys.Stat(name); err == nil {
 		if existing.IsDir() {
@@ -449,11 +488,11 @@ func (fsys *PosixFsys) MkDirAll(name string, perm FileMode) error {
 		return fmt.Errorf("%w: mkdir %s: %w", ErrCommandFailed, name, fs.ErrExist)
 	}
 
-	if err := fsys.conn.Exec(fmt.Sprintf("mkdir -p %s", dir), fsys.opts...); err != nil {
+	if err := fsys.conn.Exec(fmt.Sprintf("mkdir -p %s", shellescape.Quote(dir)), fsys.opts...); err != nil {
 		return fmt.Errorf("%w: mkdir %s: %w", ErrCommandFailed, name, err)
 	}
 
-	if err := fsys.conn.Exec(fmt.Sprintf("chmod %#o %s", os.FileMode(perm).Perm(), dir), fsys.opts...); err != nil {
+	if err := fsys.conn.Exec(fmt.Sprintf("chmod %#o %s", perm, shellescape.Quote(dir)), fsys.opts...); err != nil {
 		return fmt.Errorf("%w: chmod (mkdir) %s: %w", ErrCommandFailed, name, err)
 	}
 

pkg/rigfs/types.go

@@ -35,21 +35,10 @@ type File interface {
 // Fsys is a filesystem on the remote host
 type Fsys interface {
 	fs.FS
-	OpenFile(path string, mode FileMode, perm FileMode) (File, error)
+	OpenFile(path string, flag int, perm fs.FileMode) (File, error)
 	Sha256(path string) (string, error)
 	Stat(path string) (fs.FileInfo, error)
 	Remove(path string) error
 	RemoveAll(path string) error
-	MkDirAll(path string, perm FileMode) error
+	MkDirAll(path string, perm fs.FileMode) error
 }
-
-// FileMode is used to set the type of allowed operations when opening remote files
-type FileMode int
-
-const (
-	ModeRead      FileMode = 1                    // ModeRead = Read only
-	ModeWrite     FileMode = 2                    // ModeWrite = Write only
-	ModeReadWrite FileMode = ModeRead | ModeWrite // ModeReadWrite = Read and Write
-	ModeCreate    FileMode = 4 | ModeWrite        // ModeCreate = Create a new file or truncate an existing one. Includes write permission.
-	ModeAppend    FileMode = 8 | ModeCreate       // ModeAppend = Append to an existing file. Includes create and write permissions.
-)

pkg/rigfs/winfsys.go

@@ -352,29 +352,30 @@ func (f *winFile) Close() error {
 // Use OpenFile to get a file that can be written to or if you need any of the methods not
 // available on fs.File interface without type assertion.
 func (fsys *WinFsys) Open(name string) (fs.File, error) {
-	f, err := fsys.OpenFile(name, ModeRead, 0o644)
+	f, err := fsys.OpenFile(name, os.O_RDONLY, 0)
 	if err != nil {
 		return nil, err
 	}
 	return f, nil
 }
 
-// OpenFile opens the named remote file with the specified FileMode. Permission bits are ignored on Windows.
-func (fsys *WinFsys) OpenFile(name string, mode FileMode, _ FileMode) (File, error) {
+// OpenFile opens the named remote file with the specified flags. os.O_EXCL and permission bits are ignored on Windows.
+// For a description of the flags, see https://pkg.go.dev/os#pkg-constants
+func (fsys *WinFsys) OpenFile(name string, flags int, _ fs.FileMode) (File, error) {
 	var modeStr string
-	switch mode {
-	case ModeRead:
+	switch {
+	case flags&os.O_RDONLY == os.O_RDONLY:
 		modeStr = "ro"
-	case ModeWrite:
+	case flags&os.O_WRONLY == os.O_WRONLY:
 		modeStr = "w"
-	case ModeReadWrite:
+	case flags&os.O_RDWR == os.O_RDWR:
 		modeStr = "rw"
-	case ModeAppend:
+	case flags&os.O_APPEND == os.O_APPEND:
 		modeStr = "a"
-	case ModeCreate:
+	case flags&os.O_CREATE == os.O_CREATE:
 		modeStr = "c"
 	default:
-		return nil, &fs.PathError{Op: "open", Path: name, Err: fmt.Errorf("%w: invalid mode: %d", ErrRcpCommandFailed, mode)}
+		return nil, &fs.PathError{Op: "open", Path: name, Err: fmt.Errorf("%w: invalid mode: %d", ErrRcpCommandFailed, flags)}
 	}
 
 	log.Debugf("opening remote file %s (mode %s)", name, modeStr)
@@ -463,8 +464,8 @@ func (fsys *WinFsys) removeDirAll(name string) error {
 	return nil
 }
 
-// MkDirAll creates a directory named path, along with any necessary parents. The permission bits perm are ignored on Windows.
-func (fsys *WinFsys) MkDirAll(name string, _ FileMode) error {
+// MkDirAll creates a directory named path, along with any necessary parents. The permission bits are ignored on Windows.
+func (fsys *WinFsys) MkDirAll(name string, _ fs.FileMode) error {
 	if err := fsys.conn.Exec(fmt.Sprintf("mkdir -p %s", ps.DoubleQuote(name))); err != nil {
 		return fmt.Errorf("%w: mkdir %s: %w", ErrCommandFailed, name, err)
 	}