Backports (stable-5.0) (#13801)

.shellcheckrc

@@ -0,0 +1,2 @@
+#
+shell=bash

Makefile

@@ -263,8 +263,7 @@ ifeq ($(shell command -v flake8),)
 	exit 1
 endif
 	flake8 test/deps/import-busybox
-	shellcheck --shell bash test/*.sh test/includes/*.sh test/suites/*.sh test/backends/*.sh test/lint/*.sh
-	shellcheck test/extras/*.sh
+	shellcheck test/*.sh test/includes/*.sh test/suites/*.sh test/backends/*.sh test/lint/*.sh test/extras/*.sh
 	NOT_EXEC="$(shell find test/lint -type f -not -executable)"; \
 	if [ -n "$$NOT_EXEC" ]; then \
         echo "lint scripts not executable: $$NOT_EXEC"; \

lxd-agent/server.go

@@ -96,7 +96,7 @@ func createCmd(restAPI *mux.Router, version string, c APIEndpoint, cert *x509.Ce
 		if err != nil {
 			writeErr := response.InternalError(err).Render(w)
 			if writeErr != nil {
-				logger.Error("Failed writing error for HTTP response", logger.Ctx{"url": uri, "error": err, "writeErr": writeErr})
+				logger.Error("Failed writing error for HTTP response", logger.Ctx{"url": uri, "err": err, "writeErr": writeErr})
 			}
 		}
 	})

lxd-migrate/main_migrate.go

@@ -582,7 +582,7 @@ func (c *cmdMigrate) run(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	err = transferRootfs(ctx, server, op, fullPath, c.flagRsyncArgs, config.InstanceArgs.Type)
+	err = transferRootDiskForMigration(ctx, op, fullPath, c.flagRsyncArgs, config.InstanceArgs.Type)
 	if err != nil {
 		return err
 	}

lxd-migrate/transfer.go

@@ -131,6 +131,28 @@ func rsyncSendSetup(ctx context.Context, path string, rsyncArgs string, instance
 	return cmd, conn, stderr, nil
 }
 
+func sendBlockVol(ctx context.Context, conn io.WriteCloser, path string) error {
+	f, err := os.Open(path)
+	if err != nil {
+		return err
+	}
+
+	defer func() { _ = f.Close() }()
+
+	go func() {
+		<-ctx.Done()
+		_ = conn.Close()
+		_ = f.Close()
+	}()
+
+	_, err = io.Copy(conn, f)
+	if err != nil {
+		return err
+	}
+
+	return conn.Close()
+}
+
 func protoSendError(ws *websocket.Conn, err error) {
 	migration.ProtoSendControl(ws, err)
 

lxd-migrate/utils.go

@@ -6,7 +6,6 @@ import (
 	"crypto/x509"
 	"encoding/pem"
 	"fmt"
-	"io"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -28,7 +27,7 @@ import (
 	"github.com/canonical/lxd/shared/ws"
 )
 
-func transferRootfs(ctx context.Context, dst lxd.InstanceServer, op lxd.Operation, rootfs string, rsyncArgs string, instanceType api.InstanceType) error {
+func transferRootDiskForMigration(ctx context.Context, op lxd.Operation, rootfs string, rsyncArgs string, instanceType api.InstanceType) error {
 	opAPI := op.Get()
 
 	// Connect to the websockets
@@ -105,27 +104,7 @@ func transferRootfs(ctx context.Context, dst lxd.InstanceServer, op lxd.Operatio
 
 	// Send block volume
 	if instanceType == api.InstanceTypeVM {
-		f, err := os.Open(filepath.Join(rootfs, "root.img"))
-		if err != nil {
-			return abort(err)
-		}
-
-		defer func() { _ = f.Close() }()
-
-		conn := ws.NewWrapper(wsFs)
-
-		go func() {
-			<-ctx.Done()
-			_ = conn.Close()
-			_ = f.Close()
-		}()
-
-		_, err = io.Copy(conn, f)
-		if err != nil {
-			return abort(fmt.Errorf("Failed sending block volume: %w", err))
-		}
-
-		err = conn.Close()
+		err := sendBlockVol(ctx, ws.NewWrapper(wsFs), filepath.Join(rootfs, "root.img"))
 		if err != nil {
 			return abort(err)
 		}

lxd/api_project.go

@@ -632,14 +632,14 @@ func projectPatch(d *Daemon, r *http.Request) response.Response {
 		req.Description = project.Description
 	}
 
-	config, err := reqRaw.GetMap("config")
-	if err != nil {
-		req.Config = project.Config
-	} else {
-		for k, v := range project.Config {
-			_, ok := config[k]
-			if !ok {
-				config[k] = v
+	// Perform config patch
+	req.Config = util.CopyConfig(project.Config)
+	patches, err := reqRaw.GetMap("config")
+	if err == nil {
+		for k, v := range patches {
+			strVal, ok := v.(string)
+			if ok {
+				req.Config[k] = strVal
 			}
 		}
 	}

lxd/apparmor/archive.go

@@ -122,7 +122,7 @@ func archiveProfile(outputPath string, allowedCommandPaths []string) (string, er
 	}
 
 	// Render the profile.
-	var sb *strings.Builder = &strings.Builder{}
+	sb := &strings.Builder{}
 	err = archiveProfileTpl.Execute(sb, map[string]any{
 		"name":                ArchiveProfileName(outputPath), // Use non-deferenced outputPath for name.
 		"outputPath":          outputPathFull,                 // Use deferenced path in AppArmor profile.

lxd/apparmor/instance.go

@@ -154,7 +154,7 @@ func instanceProfile(sysOS *sys.OS, inst instance) (string, error) {
 	}
 
 	// Render the profile.
-	var sb *strings.Builder = &strings.Builder{}
+	sb := &strings.Builder{}
 	if inst.Type() == instancetype.Container {
 		err = lxcProfileTpl.Execute(sb, map[string]any{
 			"feature_cgns":     sysOS.CGInfo.Namespacing,

lxd/apparmor/rsync.go

@@ -175,7 +175,7 @@ func rsyncProfile(sysOS *sys.OS, name string, sourcePath string, dstPath string)
 		execPath = fullPath
 	}
 
-	var sb *strings.Builder = &strings.Builder{}
+	sb := &strings.Builder{}
 	err = rsyncProfileTpl.Execute(sb, map[string]any{
 		"name":        name,
 		"execPath":    execPath,

lxd/certificates.go

@@ -341,9 +341,20 @@ func clusterMemberJoinTokenValid(s *state.State, r *http.Request, projectName st
 
 			// Depending on whether it's a local operation or not, expiry will either be a time.Time or a string.
 			if s.ServerName == foundOp.Location {
-				expiry, _ = expiresAt.(time.Time)
+				expiry, ok = expiresAt.(time.Time)
+				if !ok {
+					return nil, fmt.Errorf("Unexpected expiry type in cluster join token operation: %T (%v)", expiresAt, expiresAt)
+				}
 			} else {
-				expiry, _ = time.Parse(time.RFC3339Nano, expiresAt.(string))
+				expiryStr, ok := expiresAt.(string)
+				if !ok {
+					return nil, fmt.Errorf("Unexpected expiry type in cluster join token operation: %T (%v)", expiresAt, expiresAt)
+				}
+
+				expiry, err = time.Parse(time.RFC3339Nano, expiryStr)
+				if err != nil {
+					return nil, fmt.Errorf("Invalid expiry format in cluster join token operation: %w (%q)", err, expiryStr)
+				}
 			}
 
 			// Check if token has expired.

lxd/cluster/membership.go

@@ -405,7 +405,7 @@ func Join(state *state.State, gateway *Gateway, networkCert *shared.CertInfo, se
 			return nil
 		})
 		if err != nil {
-			logger.Error("Failed to unlock global database after cluster join error", logger.Ctx{"error": err})
+			logger.Error("Failed to unlock global database after cluster join error", logger.Ctx{"err": err})
 		}
 	})
 
@@ -437,32 +437,32 @@ func Join(state *state.State, gateway *Gateway, networkCert *shared.CertInfo, se
 			return tx.ReplaceRaftNodes([]db.RaftNode{})
 		})
 		if err != nil {
-			logger.Error("Failed to clear local raft node records after cluster join error", logger.Ctx{"error": err})
+			logger.Error("Failed to clear local raft node records after cluster join error", logger.Ctx{"err": err})
 			return
 		}
 
 		err = gateway.Shutdown()
 		if err != nil {
-			logger.Error("Failed to shutdown gateway after cluster join error", logger.Ctx{"error": err})
+			logger.Error("Failed to shutdown gateway after cluster join error", logger.Ctx{"err": err})
 			return
 		}
 
 		err = os.RemoveAll(state.OS.GlobalDatabaseDir())
 		if err != nil {
-			logger.Error("Failed to remove raft data after cluster join error", logger.Ctx{"error": err})
+			logger.Error("Failed to remove raft data after cluster join error", logger.Ctx{"err": err})
 			return
 		}
 
 		gateway.networkCert = oldCert
 		err = gateway.init(false)
 		if err != nil {
-			logger.Error("Failed to re-initialize gateway after cluster join error", logger.Ctx{"error": err})
+			logger.Error("Failed to re-initialize gateway after cluster join error", logger.Ctx{"err": err})
 			return
 		}
 
 		_, err = cluster.EnsureSchema(state.DB.Cluster.DB(), localClusterAddress, state.OS.GlobalDatabaseDir())
 		if err != nil {
-			logger.Error("Failed to reload schema after cluster join error", logger.Ctx{"error": err})
+			logger.Error("Failed to reload schema after cluster join error", logger.Ctx{"err": err})
 			return
 		}
 	})

lxd/device/config/devices.go

@@ -97,7 +97,7 @@ func (list Devices) Contains(k string, d Device) bool {
 // Update returns the difference between two device sets (removed, added, updated devices) and a list of all
 // changed keys across all devices. Accepts a function to return which keys can be live updated, which prevents
 // them being removed and re-added if the device supports live updates of certain keys.
-func (list Devices) Update(newlist Devices, updateFields func(Device, Device) []string) (map[string]Device, map[string]Device, map[string]Device, []string) {
+func (list Devices) Update(newlist Devices, updateFields func(Device, Device) []string) (removedList Devices, addedList Devices, updatedList Devices, changedKeys []string) {
 	rmlist := map[string]Device{}
 	addlist := map[string]Device{}
 	updatelist := map[string]Device{}

lxd/device/device_utils_disk.go

@@ -21,6 +21,7 @@ import (
 	"github.com/canonical/lxd/shared"
 	"github.com/canonical/lxd/shared/osarch"
 	"github.com/canonical/lxd/shared/revert"
+	"github.com/canonical/lxd/shared/version"
 )
 
 // RBDFormatPrefix is the prefix used in disk paths to identify RBD.
@@ -424,7 +425,7 @@ func DiskVMVirtfsProxyStop(pidPath string) error {
 // Returns UnsupportedError error if the host system or instance does not support virtiosfd, returns normal error
 // type if process cannot be started for other reasons.
 // Returns revert function and listener file handle on success.
-func DiskVMVirtiofsdStart(execPath string, inst instance.Instance, socketPath string, pidPath string, logPath string, sharePath string, idmaps []idmap.IdmapEntry) (func(), net.Listener, error) {
+func DiskVMVirtiofsdStart(kernelVersion version.DottedVersion, inst instance.Instance, socketPath string, pidPath string, logPath string, sharePath string, idmaps []idmap.IdmapEntry) (func(), net.Listener, error) {
 	revert := revert.New()
 	defer revert.Fail()
 
@@ -498,7 +499,18 @@ func DiskVMVirtiofsdStart(execPath string, inst instance.Instance, socketPath st
 	defer func() { _ = unixFile.Close() }()
 
 	// Start the virtiofsd process in non-daemon mode.
-	args := []string{"--fd=3", "-o", fmt.Sprintf("source=%s", sharePath)}
+	args := []string{
+		"--fd=3",
+		"-o", fmt.Sprintf("source=%s", sharePath),
+	}
+
+	// Virtiofsd defaults to namespace sandbox mode which requires pidfd_open support.
+	// This was added in Linux 5.3, so if running an earlier kernel fallback to chroot sandbox mode.
+	minVer, _ := version.NewDottedVersion("5.3.0")
+	if kernelVersion.Compare(minVer) < 0 {
+		args = append(args, "--sandbox=chroot")
+	}
+
 	proc, err := subprocess.NewProcess(cmd, args, logPath, logPath)
 	if err != nil {
 		return nil, nil, err

lxd/device/disk.go

@@ -210,7 +210,7 @@ func (d *disk) validateConfig(instConf instance.ConfigReader) error {
 	}
 
 	if d.config["source"] == "" && d.config["path"] != "/" {
-		return fmt.Errorf(`Disk entry is missing the required "source" or "path" property`)
+		return fmt.Errorf(`Non root disk devices require the "source" property`)
 	}
 
 	if d.config["path"] == "/" && d.config["source"] != "" {
@@ -898,7 +898,7 @@ func (d *disk) startVM() (*deviceConfig.RunConfig, error) {
 					logPath := filepath.Join(d.inst.LogPath(), fmt.Sprintf("disk.%s.log", d.name))
 					_ = os.Remove(logPath) // Remove old log if needed.
 
-					revertFunc, unixListener, err := DiskVMVirtiofsdStart(d.state.OS.ExecPath, d.inst, sockPath, pidPath, logPath, mount.DevPath, rawIDMaps)
+					revertFunc, unixListener, err := DiskVMVirtiofsdStart(d.state.OS.KernelVersion, d.inst, sockPath, pidPath, logPath, mount.DevPath, rawIDMaps)
 					if err != nil {
 						var errUnsupported UnsupportedError
 						if errors.As(err, &errUnsupported) {

lxd/devlxd.go

@@ -294,18 +294,27 @@ func devLxdAPI(d *Daemon, f hoistFunc) http.Handler {
  */
 var pidMapper = ConnPidMapper{m: map[*net.UnixConn]*unix.Ucred{}}
 
+// ConnPidMapper is threadsafe cache of unix connections to process IDs. We use this in hoistReq to determine
+// the instance that the connection has been made from.
 type ConnPidMapper struct {
 	m     map[*net.UnixConn]*unix.Ucred
 	mLock sync.Mutex
 }
 
+// ConnStateHandler is used in the `ConnState` field of the devlxd http.Server so that we can cache the process ID of the
+// caller when a new connection is made and delete it when the connection is closed.
 func (m *ConnPidMapper) ConnStateHandler(conn net.Conn, state http.ConnState) {
-	unixConn := conn.(*net.UnixConn)
+	unixConn, _ := conn.(*net.UnixConn)
+	if unixConn == nil {
+		logger.Error("Invalid type for devlxd connection", logger.Ctx{"conn_type": fmt.Sprintf("%T", conn)})
+		return
+	}
+
 	switch state {
 	case http.StateNew:
 		cred, err := ucred.GetCred(unixConn)
 		if err != nil {
-			logger.Debugf("Error getting ucred for conn %s", err)
+			logger.Debug("Error getting ucred for devlxd connection", logger.Ctx{"err": err})
 		} else {
 			m.mLock.Lock()
 			m.m[unixConn] = cred
@@ -333,7 +342,7 @@ func (m *ConnPidMapper) ConnStateHandler(conn net.Conn, state http.ConnState) {
 		delete(m.m, unixConn)
 		m.mLock.Unlock()
 	default:
-		logger.Debugf("Unknown state for connection %s", state)
+		logger.Debug("Unknown state for devlxd connection", logger.Ctx{"state": state.String()})
 	}
 }
 
@@ -386,7 +395,9 @@ func findContainerForPid(pid int32, s *state.State) (instance.Container, error)
 				return nil, fmt.Errorf("Instance is not container type")
 			}
 
-			return inst.(instance.Container), nil
+			// Explicitly ignore type assertion check. We've just checked that it's a container.
+			c, _ := inst.(instance.Container)
+			return c, nil
 		}
 
 		status, err := os.ReadFile(fmt.Sprintf("/proc/%d/status", pid))
@@ -439,7 +450,9 @@ func findContainerForPid(pid int32, s *state.State) (instance.Container, error)
 		}
 
 		if origPidNs == pidNs {
-			return inst.(instance.Container), nil
+			// Explicitly ignore type assertion check. The instance must be a container if we've found it via the process ID.
+			c, _ := inst.(instance.Container)
+			return c, nil
 		}
 	}
 

lxd/firewall/drivers/drivers_xtables.go

@@ -1548,7 +1548,7 @@ func (d Xtables) NetworkApplyForwards(networkName string, rules []AddressForward
 	reverter.Add(func() {
 		err := clearNetworkForwards()
 		if err != nil {
-			logger.Error("Failed to clear firewall rules after failing to apply network forwards", logger.Ctx{"network_name": networkName, "error": err})
+			logger.Error("Failed to clear firewall rules after failing to apply network forwards", logger.Ctx{"network_name": networkName, "err": err})
 		}
 	})
 

lxd/instance/drivers/driver_qemu.go

@@ -1357,7 +1357,7 @@ func (d *qemu) start(stateful bool, op *operationlock.InstanceOperation) error {
 	// This is used by the lxd-agent in preference to 9p (due to its improved performance) and in scenarios
 	// where 9p isn't available in the VM guest OS.
 	configSockPath, configPIDPath := d.configVirtiofsdPaths()
-	revertFunc, unixListener, err := device.DiskVMVirtiofsdStart(d.state.OS.ExecPath, d, configSockPath, configPIDPath, "", configMntPath, nil)
+	revertFunc, unixListener, err := device.DiskVMVirtiofsdStart(d.state.OS.KernelVersion, d, configSockPath, configPIDPath, "", configMntPath, nil)
 	if err != nil {
 		var errUnsupported device.UnsupportedError
 		if !errors.As(err, &errUnsupported) {

lxd/migrate.go

@@ -144,6 +144,8 @@ type migrationSourceWs struct {
 	pushSecrets      map[string]string
 }
 
+// Metadata returns a map where each key is a connection name and each value is
+// the secret of the corresponding websocket connection.
 func (s *migrationSourceWs) Metadata() any {
 	secrets := make(shared.Jmap, len(s.conns))
 	for connName, conn := range s.conns {
@@ -153,6 +155,9 @@ func (s *migrationSourceWs) Metadata() any {
 	return secrets
 }
 
+// Connect handles an incoming HTTP request to establish a websocket connection for migration.
+// It verifies the provided secret and matches it to the appropriate connection. If the secret
+// is valid, it accepts the incoming connection. Otherwise, it returns an error.
 func (s *migrationSourceWs) Connect(op *operations.Operation, r *http.Request, w http.ResponseWriter) error {
 	incomingSecret := r.FormValue("secret")
 	if incomingSecret == "" {
@@ -186,7 +191,7 @@ type migrationSink struct {
 	refresh               bool
 }
 
-// MigrationSinkArgs arguments to configure migration sink.
+// migrationSinkArgs arguments to configure migration sink.
 type migrationSinkArgs struct {
 	// General migration fields
 	Dialer  *websocket.Dialer