Merge pull request #1236 from stgraber/main

Various bugfixes

cmd/incus-user/server.go

@@ -3,6 +3,7 @@ package main
 import (
 	"encoding/base64"
 	"fmt"
+	"net/http"
 	"os"
 	"path/filepath"
 	"slices"
@@ -238,7 +239,7 @@ func serverSetupUser(uid uint32) error {
 		network.Description = fmt.Sprintf("Network for user restricted project %s", projectName)
 
 		err = client.CreateNetwork(network)
-		if err != nil {
+		if err != nil && !api.StatusErrorCheck(err, http.StatusConflict) {
 			return fmt.Errorf("Failed to create network: %w", err)
 		}
 

cmd/incusd/api_project.go

@@ -862,7 +862,7 @@ func projectPost(d *Daemon, r *http.Request) response.Response {
 
 		err = s.Authorizer.RenameProject(s.ShutdownCtx, id, name, req.Name)
 		if err != nil {
-			return err
+			logger.Error("Failed to rename project in authorizer", logger.Ctx{"name": name, "new_name": req.Name, "err": err})
 		}
 
 		requestor := request.CreateRequestor(r)
@@ -1196,7 +1196,7 @@ func projectDelete(d *Daemon, r *http.Request) response.Response {
 
 	err = s.Authorizer.DeleteProject(r.Context(), id, name)
 	if err != nil {
-		return response.SmartError(err)
+		logger.Error("Failed to remove project from authorizer", logger.Ctx{"name": name, "err": err})
 	}
 
 	requestor := request.CreateRequestor(r)

cmd/incusd/networks.go

@@ -248,6 +248,11 @@ func networksGet(d *Daemon, r *http.Request) response.Response {
 			}
 
 			if !recursion {
+				// Check if project allows access to network.
+				if !project.NetworkAllowed(reqProject.Config, networkName, true) {
+					continue
+				}
+
 				resultString = append(resultString, fmt.Sprintf("/%s/networks/%s", version.APIVersion, networkName))
 			} else {
 				netInfo, err := doNetworkGet(s, r, s.ServerClustered, projectName, reqProject.Config, networkName)
@@ -438,7 +443,7 @@ func networksPost(d *Daemon, r *http.Request) response.Response {
 		})
 		if err != nil {
 			if err == db.ErrAlreadyDefined {
-				return response.BadRequest(fmt.Errorf("The network is already defined on member %q", targetNode))
+				return response.Conflict(fmt.Errorf("Network %q is already defined on member %q", req.Name, targetNode))
 			}
 
 			return response.SmartError(err)
@@ -532,7 +537,7 @@ func networksPost(d *Daemon, r *http.Request) response.Response {
 
 	// Non-clustered network creation.
 	if netInfo != nil {
-		return response.BadRequest(fmt.Errorf("The network already exists"))
+		return response.Conflict(fmt.Errorf("Network %q already exists", req.Name))
 	}
 
 	revert := revert.New()

cmd/incusd/operations.go

@@ -262,11 +262,19 @@ func operationDelete(d *Daemon, r *http.Request) response.Response {
 		if objectType != "" {
 			for _, v := range op.Resources() {
 				for _, u := range v {
-					_, _, _, pathArgs, err := dbCluster.URLToEntityType(u.String())
-					if err != nil {
-						return response.InternalError(fmt.Errorf("Unable to parse operation resource URL: %w", err))
+					// When dealing with specific objects, get the arguments from the URL.
+					var pathArgs []string
+
+					if objectType != auth.ObjectTypeProject {
+						var err error
+
+						_, _, _, pathArgs, err = dbCluster.URLToEntityType(u.String())
+						if err != nil {
+							return response.InternalError(fmt.Errorf("Unable to parse operation resource URL: %w", err))
+						}
 					}
 
+					// Check that the access is allowed.
 					object, err := auth.NewObject(objectType, projectName, pathArgs...)
 					if err != nil {
 						return response.InternalError(fmt.Errorf("Unable to create authorization object for operation: %w", err))

cmd/incusd/storage_volumes_utils.go

@@ -29,7 +29,10 @@ func storagePoolVolumeUpdateUsers(ctx context.Context, s *state.State, projectNa
 			_, exists := localDevices[devName]
 			if exists {
 				localDevices[devName]["pool"] = newPoolName
-				localDevices[devName]["source"] = newVol.Name
+
+				volFields := strings.SplitN(localDevices[devName]["source"], "/", 2)
+				volFields[0] = newVol.Name
+				localDevices[devName]["source"] = strings.Join(volFields, "/")
 			}
 		}
 
@@ -61,7 +64,10 @@ func storagePoolVolumeUpdateUsers(ctx context.Context, s *state.State, projectNa
 		for name, dev := range profile.Devices {
 			if slices.Contains(usedByDevices, name) {
 				dev["pool"] = newPoolName
-				dev["source"] = newVol.Name
+
+				volFields := strings.SplitN(dev["source"], "/", 2)
+				volFields[0] = newVol.Name
+				dev["source"] = strings.Join(volFields, "/")
 			}
 		}
 

doc/config_options.txt

@@ -1180,7 +1180,9 @@ See {ref}`instance-options-snapshots-names` for more information.
 :liveupdate: "no"
 :shortdesc: "Schedule for automatic instance snapshots"
 :type: "string"
-Specify either a cron expression (`<minute> <hour> <dom> <month> <dow>`), a comma-separated list of schedule aliases (`@hourly`, `@daily`, `@midnight`, `@weekly`, `@monthly`, `@annually`, `@yearly`), or leave empty to disable automatic snapshots.
+Specify either a cron expression (`<minute> <hour> <dom> <month> <dow>`), a comma-and-space-separated list of schedule aliases (`@startup`, `@hourly`, `@daily`, `@midnight`, `@weekly`, `@monthly`, `@annually`, `@yearly`), or leave empty to disable automatic snapshots.
+
+Note that unlike most other configuration keys, this one must be comma-and-space-separated and not just comma-separated as cron expression can themselves contain commas.
 
 ```
 

internal/instance/config.go

@@ -295,7 +295,9 @@ var InstanceConfigKeysAny = map[string]func(value string) error{
 	"security.protection.delete": validate.Optional(validate.IsBool),
 
 	// gendoc:generate(entity=instance, group=snapshots, key=snapshots.schedule)
-	// Specify either a cron expression (`<minute> <hour> <dom> <month> <dow>`), a comma-separated list of schedule aliases (`@hourly`, `@daily`, `@midnight`, `@weekly`, `@monthly`, `@annually`, `@yearly`), or leave empty to disable automatic snapshots.
+	// Specify either a cron expression (`<minute> <hour> <dom> <month> <dow>`), a comma-and-space-separated list of schedule aliases (`@startup`, `@hourly`, `@daily`, `@midnight`, `@weekly`, `@monthly`, `@annually`, `@yearly`), or leave empty to disable automatic snapshots.
+	//
+	// Note that unlike most other configuration keys, this one must be comma-and-space-separated and not just comma-separated as cron expression can themselves contain commas.
 	//
 	// ---
 	//  type: string

internal/server/auth/driver_tls.go

@@ -83,11 +83,16 @@ func (t *tls) CheckPermission(ctx context.Context, r *http.Request, object Objec
 
 	// Check project level permissions against the certificates project list.
 	projectName := object.Project()
-	if !slices.Contains(projectNames, projectName) {
-		return api.StatusErrorf(http.StatusForbidden, "User does not have permission for project %q", projectName)
+	if slices.Contains(projectNames, projectName) {
+		return nil
 	}
 
-	return nil
+	// Also allow read-only access to inherited resources.
+	if object.Project() == api.ProjectDefaultName && entitlement == EntitlementCanView && slices.Contains([]ObjectType{ObjectTypeImage, ObjectTypeProfile, ObjectTypeStorageVolume, ObjectTypeStorageBucket, ObjectTypeNetwork, ObjectTypeNetworkZone}, object.Type()) {
+		return nil
+	}
+
+	return api.StatusErrorf(http.StatusForbidden, "User does not have permission for project %q", projectName)
 }
 
 // GetPermissionChecker returns a function that can be used to check whether a user has the required entitlement on an authorization object.
@@ -154,7 +159,25 @@ func (t *tls) GetPermissionChecker(ctx context.Context, r *http.Request, entitle
 
 	// Filter objects by project.
 	return func(object Object) bool {
-		return slices.Contains(projectNames, object.Project())
+		// Allow if the project is in the allowed set.
+		if slices.Contains(projectNames, object.Project()) {
+			return true
+		}
+
+		// Also allow read-only access to inherited resources.
+		if object.Project() != api.ProjectDefaultName {
+			return false
+		}
+
+		if entitlement != EntitlementCanView {
+			return false
+		}
+
+		if !slices.Contains([]ObjectType{ObjectTypeImage, ObjectTypeProfile, ObjectTypeStorageVolume, ObjectTypeStorageBucket, ObjectTypeNetwork, ObjectTypeNetworkZone}, objectType) {
+			return false
+		}
+
+		return true
 	}, nil
 }
 

internal/server/metadata/configuration.json

@@ -1271,7 +1271,7 @@
 						"snapshots.schedule": {
 							"defaultdesc": "empty",
 							"liveupdate": "no",
-							"longdesc": "Specify either a cron expression (`\u003cminute\u003e \u003chour\u003e \u003cdom\u003e \u003cmonth\u003e \u003cdow\u003e`), a comma-separated list of schedule aliases (`@hourly`, `@daily`, `@midnight`, `@weekly`, `@monthly`, `@annually`, `@yearly`), or leave empty to disable automatic snapshots.\n",
+							"longdesc": "Specify either a cron expression (`\u003cminute\u003e \u003chour\u003e \u003cdom\u003e \u003cmonth\u003e \u003cdow\u003e`), a comma-and-space-separated list of schedule aliases (`@startup`, `@hourly`, `@daily`, `@midnight`, `@weekly`, `@monthly`, `@annually`, `@yearly`), or leave empty to disable automatic snapshots.\n\nNote that unlike most other configuration keys, this one must be comma-and-space-separated and not just comma-separated as cron expression can themselves contain commas.\n",
 							"shortdesc": "Schedule for automatic instance snapshots",
 							"type": "string"
 						}

internal/server/storage/drivers/utils.go

@@ -514,21 +514,20 @@ func growFileSystem(fsType string, devPath string, vol Volume) error {
 	}
 
 	return vol.MountTask(func(mountPath string, op *operations.Operation) error {
-		var msg string
 		var err error
 		switch fsType {
 		case "ext4":
-			msg, err = subprocess.TryRunCommand("resize2fs", devPath)
+			_, err = subprocess.TryRunCommand("resize2fs", devPath)
 		case "xfs":
-			msg, err = subprocess.TryRunCommand("xfs_growfs", mountPath)
+			_, err = subprocess.TryRunCommand("xfs_growfs", mountPath)
 		case "btrfs":
-			msg, err = subprocess.TryRunCommand("btrfs", "filesystem", "resize", "max", mountPath)
+			_, err = subprocess.TryRunCommand("btrfs", "filesystem", "resize", "max", mountPath)
 		default:
 			return fmt.Errorf("Unrecognised filesystem type %q", fsType)
 		}
 
 		if err != nil {
-			return fmt.Errorf("Could not grow underlying %q filesystem for %q: %s", fsType, devPath, msg)
+			return fmt.Errorf("Could not grow underlying %q filesystem for %q: %w", fsType, devPath, err)
 		}
 
 		return nil

shared/validate/validate.go

@@ -803,6 +803,17 @@ func IsValidCPUSet(value string) error {
 		return fmt.Errorf("Invalid CPU limit syntax")
 	}
 
+	// Validate single values.
+	cpu, err := strconv.ParseInt(value, 10, 64)
+	if err == nil {
+		if cpu < 1 {
+			return fmt.Errorf("Invalid cpuset value: %s", value)
+		}
+
+		return nil
+	}
+
+	// Handle complex values.
 	cpus := make(map[int64]int)
 	chunks := strings.Split(value, ",")