config: copy go's file lock and use it instead of client-go's (#5982)

* config: copy go's file lock and use it instead of client-go's

Fixes #4814.

Signed-off-by: Nick Sieger <[email protected]>

* config: attempt to complete windows filelock implementation

Signed-off-by: Nick Sieger <[email protected]>

Signed-off-by: Nick Sieger <[email protected]>

internal/cli/client/config.go

@@ -4,7 +4,9 @@ import (
 	"fmt"
 
 	"k8s.io/client-go/tools/clientcmd"
+	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
 
+	"github.com/tilt-dev/tilt/internal/filelock"
 	"github.com/tilt-dev/tilt/pkg/model"
 )
 
@@ -13,7 +15,12 @@ type TiltClientConfig clientcmd.ClientConfig
 // Uses the kubernetes config-loading library to create a client config
 // for the given server name.
 func ProvideClientConfig(apiServerName model.APIServerName, configAccess clientcmd.ConfigAccess) (TiltClientConfig, error) {
-	config, err := configAccess.GetStartingConfig()
+	var config *clientcmdapi.Config
+	err := filelock.WithRLock(configAccess, func() error {
+		var e error
+		config, e = configAccess.GetStartingConfig()
+		return e
+	})
 	if err != nil {
 		return nil, err
 	}

internal/filelock/filelock.go

@@ -0,0 +1,102 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// TODO(Tilt team): Remove this copy of Go's internal filelock module when
+// https://github.com/golang/go/issues/33974 makes a public file locking api.
+
+// Package filelock provides a platform-independent API for advisory file
+// locking. Calls to functions in this package on platforms that do not support
+// advisory locks will return errors for which IsNotSupported returns true.
+package filelock
+
+import (
+	"errors"
+	"io/fs"
+	"os"
+)
+
+// A File provides the minimal set of methods required to lock an open file.
+// File implementations must be usable as map keys.
+// The usual implementation is *os.File.
+type File interface {
+	// Name returns the name of the file.
+	Name() string
+
+	// Fd returns a valid file descriptor.
+	// (If the File is an *os.File, it must not be closed.)
+	Fd() uintptr
+
+	// Stat returns the FileInfo structure describing file.
+	Stat() (fs.FileInfo, error)
+}
+
+// Lock places an advisory write lock on the file, blocking until it can be
+// locked.
+//
+// If Lock returns nil, no other process will be able to place a read or write
+// lock on the file until this process exits, closes f, or calls Unlock on it.
+//
+// If f's descriptor is already read- or write-locked, the behavior of Lock is
+// unspecified.
+//
+// Closing the file may or may not release the lock promptly. Callers should
+// ensure that Unlock is always called when Lock succeeds.
+func Lock(f File) error {
+	return lock(f, writeLock)
+}
+
+// RLock places an advisory read lock on the file, blocking until it can be locked.
+//
+// If RLock returns nil, no other process will be able to place a write lock on
+// the file until this process exits, closes f, or calls Unlock on it.
+//
+// If f is already read- or write-locked, the behavior of RLock is unspecified.
+//
+// Closing the file may or may not release the lock promptly. Callers should
+// ensure that Unlock is always called if RLock succeeds.
+func RLock(f File) error {
+	return lock(f, readLock)
+}
+
+// Unlock removes an advisory lock placed on f by this process.
+//
+// The caller must not attempt to unlock a file that is not locked.
+func Unlock(f File) error {
+	return unlock(f)
+}
+
+// String returns the name of the function corresponding to lt
+// (Lock, RLock, or Unlock).
+func (lt lockType) String() string {
+	switch lt {
+	case readLock:
+		return "RLock"
+	case writeLock:
+		return "Lock"
+	default:
+		return "Unlock"
+	}
+}
+
+// IsNotSupported returns a boolean indicating whether the error is known to
+// report that a function is not supported (possibly for a specific input).
+// It is satisfied by ErrNotSupported as well as some syscall errors.
+func IsNotSupported(err error) bool {
+	return isNotSupported(underlyingError(err))
+}
+
+var ErrNotSupported = errors.New("operation not supported")
+
+// underlyingError returns the underlying error for known os error types.
+func underlyingError(err error) error {
+	switch err := err.(type) {
+	case *fs.PathError:
+		return err.Err
+	case *os.LinkError:
+		return err.Err
+	case *os.SyscallError:
+		return err.Err
+	}
+	return err
+}

internal/filelock/filelock_unix.go

@@ -0,0 +1,44 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build darwin || dragonfly || freebsd || illumos || linux || netbsd || openbsd
+
+package filelock
+
+import (
+	"io/fs"
+	"syscall"
+)
+
+type lockType int16
+
+const (
+	readLock  lockType = syscall.LOCK_SH
+	writeLock lockType = syscall.LOCK_EX
+)
+
+func lock(f File, lt lockType) (err error) {
+	for {
+		err = syscall.Flock(int(f.Fd()), int(lt))
+		if err != syscall.EINTR {
+			break
+		}
+	}
+	if err != nil {
+		return &fs.PathError{
+			Op:   lt.String(),
+			Path: f.Name(),
+			Err:  err,
+		}
+	}
+	return nil
+}
+
+func unlock(f File) error {
+	return lock(f, syscall.LOCK_UN)
+}
+
+func isNotSupported(err error) bool {
+	return err == syscall.ENOSYS || err == syscall.ENOTSUP || err == syscall.EOPNOTSUPP || err == ErrNotSupported
+}

internal/filelock/filelock_windows.go

@@ -0,0 +1,124 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build windows
+
+package filelock
+
+import (
+	"io/fs"
+	"syscall"
+	"unsafe"
+)
+
+type lockType uint32
+
+const (
+	LOCKFILE_EXCLUSIVE_LOCK = 0x00000002 // from internal/syscall/windows
+
+	readLock  lockType = 0
+	writeLock lockType = LOCKFILE_EXCLUSIVE_LOCK
+)
+
+const (
+	reserved = 0
+	allBytes = ^uint32(0)
+)
+
+func lock(f File, lt lockType) error {
+	// Per https://golang.org/issue/19098, “Programs currently expect the Fd
+	// method to return a handle that uses ordinary synchronous I/O.”
+	// However, LockFileEx still requires an OVERLAPPED structure,
+	// which contains the file offset of the beginning of the lock range.
+	// We want to lock the entire file, so we leave the offset as zero.
+	ol := new(syscall.Overlapped)
+
+	err := LockFileEx(syscall.Handle(f.Fd()), uint32(lt), reserved, allBytes, allBytes, ol)
+	if err != nil {
+		return &fs.PathError{
+			Op:   lt.String(),
+			Path: f.Name(),
+			Err:  err,
+		}
+	}
+	return nil
+}
+
+func unlock(f File) error {
+	ol := new(syscall.Overlapped)
+	err := UnlockFileEx(syscall.Handle(f.Fd()), reserved, allBytes, allBytes, ol)
+	if err != nil {
+		return &fs.PathError{
+			Op:   "Unlock",
+			Path: f.Name(),
+			Err:  err,
+		}
+	}
+	return nil
+}
+
+func isNotSupported(err error) bool {
+	switch err {
+	case ERROR_NOT_SUPPORTED, ERROR_CALL_NOT_IMPLEMENTED, ErrNotSupported:
+		return true
+	default:
+		return false
+	}
+}
+
+// portions of internal/syscall/windows below here for LockFileEx/UnlockFileEx
+var _ unsafe.Pointer
+
+// Do the interface allocations only once for common
+// Errno values.
+const (
+	errnoERROR_IO_PENDING = 997
+)
+
+var (
+	errERROR_IO_PENDING error = syscall.Errno(errnoERROR_IO_PENDING)
+	errERROR_EINVAL     error = syscall.EINVAL
+)
+
+// errnoErr returns common boxed Errno values, to prevent
+// allocations at runtime.
+func errnoErr(e syscall.Errno) error {
+	switch e {
+	case 0:
+		return errERROR_EINVAL
+	case errnoERROR_IO_PENDING:
+		return errERROR_IO_PENDING
+	}
+	// TODO: add more here, after collecting data on the common
+	// error values see on Windows. (perhaps when running
+	// all.bat?)
+	return e
+}
+
+const (
+	ERROR_NOT_SUPPORTED        syscall.Errno = 50
+	ERROR_CALL_NOT_IMPLEMENTED syscall.Errno = 120
+)
+
+var (
+	modkernel32      = syscall.NewLazyDLL("kernel32.dll")
+	procLockFileEx   = modkernel32.NewProc("LockFileEx")
+	procUnlockFileEx = modkernel32.NewProc("UnlockFileEx")
+)
+
+func LockFileEx(file syscall.Handle, flags uint32, reserved uint32, bytesLow uint32, bytesHigh uint32, overlapped *syscall.Overlapped) (err error) {
+	r1, _, e1 := syscall.Syscall6(procLockFileEx.Addr(), 6, uintptr(file), uintptr(flags), uintptr(reserved), uintptr(bytesLow), uintptr(bytesHigh), uintptr(unsafe.Pointer(overlapped)))
+	if r1 == 0 {
+		err = errnoErr(e1)
+	}
+	return
+}
+
+func UnlockFileEx(file syscall.Handle, reserved uint32, bytesLow uint32, bytesHigh uint32, overlapped *syscall.Overlapped) (err error) {
+	r1, _, e1 := syscall.Syscall6(procUnlockFileEx.Addr(), 5, uintptr(file), uintptr(reserved), uintptr(bytesLow), uintptr(bytesHigh), uintptr(unsafe.Pointer(overlapped)), 0)
+	if r1 == 0 {
+		err = errnoErr(e1)
+	}
+	return
+}

internal/filelock/with_lock.go

@@ -0,0 +1,33 @@
+package filelock
+
+import (
+	"os"
+
+	"k8s.io/client-go/tools/clientcmd"
+)
+
+func init() {
+	// We're using our own file locking mechanism
+	clientcmd.UseModifyConfigLock = false
+}
+
+func WithLock(configAccess clientcmd.ConfigAccess, action func() error) error {
+	return withLock(configAccess.GetDefaultFilename()+".lock", writeLock, action)
+}
+
+func WithRLock(configAccess clientcmd.ConfigAccess, action func() error) error {
+	return withLock(configAccess.GetDefaultFilename()+".lock", readLock, action)
+}
+
+func withLock(filename string, lt lockType, action func() error) error {
+	lockfile, err := os.Create(filename)
+	if err != nil {
+		return err
+	}
+	err = lock(lockfile, lt)
+	if err != nil {
+		return err
+	}
+	defer func() { _ = Unlock(lockfile) }()
+	return action()
+}

internal/hud/server/controller.go

@@ -17,6 +17,7 @@ import (
 	"k8s.io/kubectl/pkg/proxy"
 
 	"github.com/tilt-dev/tilt-apiserver/pkg/server/start"
+	"github.com/tilt-dev/tilt/internal/filelock"
 	"github.com/tilt-dev/tilt/internal/store"
 	"github.com/tilt-dev/tilt/pkg/assets"
 	"github.com/tilt-dev/tilt/pkg/model"
@@ -202,7 +203,12 @@ func (s *HeadsUpServerController) addToAPIServerConfig() error {
 		return nil
 	}
 
-	newConfig, err := s.configAccess.GetStartingConfig()
+	var newConfig *clientcmdapi.Config
+	err := filelock.WithRLock(s.configAccess, func() error {
+		var e error
+		newConfig, e = s.configAccess.GetStartingConfig()
+		return e
+	})
 	if err != nil {
 		return err
 	}
@@ -227,7 +233,7 @@ func (s *HeadsUpServerController) addToAPIServerConfig() error {
 		CertificateAuthorityData: clientConfig.TLSClientConfig.CAData,
 	}
 
-	return clientcmd.ModifyConfig(s.configAccess, *newConfig, true)
+	return s.modifyConfig(*newConfig)
 }
 
 // Remove this API server's configs into the user settings directory.
@@ -238,7 +244,12 @@ func (s *HeadsUpServerController) removeFromAPIServerConfig() error {
 		return nil
 	}
 
-	newConfig, err := s.configAccess.GetStartingConfig()
+	var newConfig *clientcmdapi.Config
+	err := filelock.WithRLock(s.configAccess, func() error {
+		var e error
+		newConfig, e = s.configAccess.GetStartingConfig()
+		return e
+	})
 	if err != nil {
 		return err
 	}
@@ -252,7 +263,13 @@ func (s *HeadsUpServerController) removeFromAPIServerConfig() error {
 	delete(newConfig.AuthInfos, name)
 	delete(newConfig.Clusters, name)
 
-	return clientcmd.ModifyConfig(s.configAccess, *newConfig, true)
+	return s.modifyConfig(*newConfig)
+}
+
+func (s *HeadsUpServerController) modifyConfig(config clientcmdapi.Config) error {
+	return filelock.WithLock(s.configAccess, func() error {
+		return clientcmd.ModifyConfig(s.configAccess, config, true)
+	})
 }
 
 func newAPIServerProxyHandler(config *rest.Config) (http.Handler, error) {