From 88ad75eaada65874a5851c649f7dd98d7ba99be3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Sat, 21 Sep 2024 14:34:33 +0100 Subject: [PATCH 01/12] incusd/project: Don't fail project deletion on authorizer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We automatically re-sync authorizer entries in the background so a failure to update the authorizer isn't critical. Signed-off-by: Stéphane Graber --- cmd/incusd/api_project.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/incusd/api_project.go b/cmd/incusd/api_project.go index 6b7d740f38..81c80a0868 100644 --- a/cmd/incusd/api_project.go +++ b/cmd/incusd/api_project.go @@ -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) From d092279529d478962de4eed90b7cd60642fa4b87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Sat, 21 Sep 2024 14:37:24 +0100 Subject: [PATCH 02/12] incusd/project: Don't fail project rename on authorizer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stéphane Graber --- cmd/incusd/api_project.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/incusd/api_project.go b/cmd/incusd/api_project.go index 81c80a0868..250739dae0 100644 --- a/cmd/incusd/api_project.go +++ b/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) From fda9ad59c14551a93b51c0928cc3f6ff3fb12a3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Sat, 21 Sep 2024 14:41:22 +0100 Subject: [PATCH 03/12] incus-user: Handle existing network MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes https://github.com/zabbly/incus/issues/56 Signed-off-by: Stéphane Graber --- cmd/incus-user/server.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/incus-user/server.go b/cmd/incus-user/server.go index 4ac5fe98e8..1274da20ff 100644 --- a/cmd/incus-user/server.go +++ b/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) } From 5f11dc3cac8f79eb3af85249f106eb81b880d7f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Sat, 21 Sep 2024 14:45:09 +0100 Subject: [PATCH 04/12] incusd/networks: Return HTTP Conflict on existing network MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stéphane Graber --- cmd/incusd/networks.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/incusd/networks.go b/cmd/incusd/networks.go index 1e69cd962e..8e0c9637f6 100644 --- a/cmd/incusd/networks.go +++ b/cmd/incusd/networks.go @@ -438,7 +438,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 +532,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() From d29c0d61f7d6a4477b313b1e9f11b00946b164ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Sat, 21 Sep 2024 15:24:12 +0100 Subject: [PATCH 05/12] incusd/networks: Apply project restrictions to list of network names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stéphane Graber --- cmd/incusd/networks.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cmd/incusd/networks.go b/cmd/incusd/networks.go index 8e0c9637f6..8608635919 100644 --- a/cmd/incusd/networks.go +++ b/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) From c93ff45b825b5ff77fc82b835bc0c8e231d1f331 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Sat, 21 Sep 2024 15:24:27 +0100 Subject: [PATCH 06/12] incusd/auth/tls: Allow access to inherited resources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stéphane Graber --- internal/server/auth/driver_tls.go | 31 ++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/internal/server/auth/driver_tls.go b/internal/server/auth/driver_tls.go index fca64cd179..8d21f4847f 100644 --- a/internal/server/auth/driver_tls.go +++ b/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 } From 3998864d7b756e2309e7ee2037728f2dde13794a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Sat, 21 Sep 2024 15:30:48 +0100 Subject: [PATCH 07/12] instance/config: Add @startup to documentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stéphane Graber --- internal/instance/config.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/instance/config.go b/internal/instance/config.go index 6c6e79d119..b3dfde78b0 100644 --- a/internal/instance/config.go +++ b/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 (` `), 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 (` `), 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 From 10f3edbc29f0a85f8cc3cb8a1bb41540813022ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Sat, 21 Sep 2024 15:31:19 +0100 Subject: [PATCH 08/12] doc: Update metadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stéphane Graber --- doc/config_options.txt | 4 +++- internal/server/metadata/configuration.json | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/doc/config_options.txt b/doc/config_options.txt index 72ec63351b..d208db2071 100644 --- a/doc/config_options.txt +++ b/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 (` `), 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 (` `), 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. ``` diff --git a/internal/server/metadata/configuration.json b/internal/server/metadata/configuration.json index 4a909fd648..cdf06cd3c5 100644 --- a/internal/server/metadata/configuration.json +++ b/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" } From b6da683926c00ee79f0d52c762b5fa8e7ab9e3e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Sat, 21 Sep 2024 15:43:50 +0100 Subject: [PATCH 09/12] shared/validate: Better validate simple CPU limits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds a short path for simple CPU limits (integer) and also confirms that negative and zero values aren't considered valid. Signed-off-by: Stéphane Graber --- shared/validate/validate.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/shared/validate/validate.go b/shared/validate/validate.go index 4c5392c641..7c3d04c2ff 100644 --- a/shared/validate/validate.go +++ b/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, ",") From 0ef41bb110036ccef8dcb7ae8758167050209c14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Sat, 21 Sep 2024 16:05:34 +0100 Subject: [PATCH 10/12] incusd/operations: Fix operation cancelation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When canceling the creation of an instance, the object type for the operation is the target project. As a project authorizer only needs the project name, feeding it information about the instance causes an error and prevents the cancelation. This changes the logic to only compute the arguments when dealing with a specific object (not a whole project). Signed-off-by: Stéphane Graber --- cmd/incusd/operations.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/cmd/incusd/operations.go b/cmd/incusd/operations.go index 956938fb4a..8b45e41fbb 100644 --- a/cmd/incusd/operations.go +++ b/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)) From 150d68513f3e7526f67f30ae9b7ec28256670e4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Sat, 21 Sep 2024 16:12:25 +0100 Subject: [PATCH 11/12] incusd/storage_volumes: Handle rename of volumes with sub-paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stéphane Graber --- cmd/incusd/storage_volumes_utils.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/cmd/incusd/storage_volumes_utils.go b/cmd/incusd/storage_volumes_utils.go index db3191561e..2393d88520 100644 --- a/cmd/incusd/storage_volumes_utils.go +++ b/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, "/") } } From d09778d8757363cfbab01f8896da5195ea12cb73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Sat, 21 Sep 2024 06:14:21 +0200 Subject: [PATCH 12/12] incusd/storage/utils: Only show actual errors in growFileSystem MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was masking an actual resize2fs error in our tests. Signed-off-by: Stéphane Graber --- internal/server/storage/drivers/utils.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/internal/server/storage/drivers/utils.go b/internal/server/storage/drivers/utils.go index 6c1a6b3e93..f319d705e0 100644 --- a/internal/server/storage/drivers/utils.go +++ b/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