Prevent cluster groups from being deleted when referenced by a projects' configuration (#15119)

Adds an API extension `clustering_groups_used_by` and adds a `UsedBy`
field to `api.ClusterGroup`. On GET requests, the `UsedBy` URLs are
projects that contain the cluster group in `restricted.cluster.groups`.
DELETE requests will fail if the cluster group is used by a project.

Closes #15118

Author: tomponline

doc/api-extensions.md

@@ -2625,3 +2625,8 @@ Adds flags --network and --storage. The --network flag adds a network device con
 ## `client_cert_presence`
 
 Adds the field `client_certificate` to `GET /1.0` to indicate if the current request has a client certificate in it. This is for informational purposes only and does not affect the behavior of the API.
+
+## `clustering_groups_used_by`
+
+This API extension adds a `used_by` field to the API response for a {ref}`cluster group <cluster-groups>`.
+Deletion of a cluster group is disallowed if the cluster group is referenced by project configuration (see {config:option}`project-restricted:restricted.cluster.groups`).

doc/rest-api.yaml

@@ -327,6 +327,15 @@ definitions:
                 example: group1
                 type: string
                 x-go-name: Name
+            used_by:
+                description: |-
+                    UsedBy is a list or LXD entity URLs that reference the cluster group.
+
+                    API extension: clustering_groups_used_by
+                items:
+                    type: string
+                type: array
+                x-go-name: UsedBy
         title: ClusterGroup represents a cluster group.
         type: object
         x-go-package: github.com/canonical/lxd/shared/api

lxd/api_cluster.go

@@ -16,6 +16,7 @@ import (
 	"sync"
 	"time"
 
+	dqlite "github.com/canonical/go-dqlite/v3/client"
 	"github.com/gorilla/mux"
 
 	"github.com/canonical/lxd/client"
@@ -774,11 +775,15 @@ func clusterPutJoin(d *Daemon, r *http.Request, req api.ClusterPut) response.Res
 		s.UpdateIdentityCache()
 
 		// Update local setup and possibly join the raft dqlite cluster.
-		nodes := make([]db.RaftNode, len(info.RaftNodes))
-		for i, node := range info.RaftNodes {
-			nodes[i].ID = node.ID
-			nodes[i].Address = node.Address
-			nodes[i].Role = db.RaftRole(node.Role)
+		nodes := make([]db.RaftNode, 0, len(info.RaftNodes))
+		for _, node := range info.RaftNodes {
+			nodes = append(nodes, db.RaftNode{
+				NodeInfo: dqlite.NodeInfo{
+					ID:      node.ID,
+					Address: node.Address,
+					Role:    db.RaftRole(node.Role),
+				},
+			})
 		}
 
 		err = cluster.Join(s, d.gateway, networkCert, serverCert, req.ServerName, nodes)
@@ -1378,6 +1383,7 @@ func clusterNodesPost(d *Daemon, r *http.Request) response.Response {
 		}
 
 		// Filter to online members.
+		onlineNodeAddresses = make([]any, 0, len(members))
 		for _, member := range members {
 			if member.State == db.ClusterMemberStateEvacuated || member.IsOffline(s.GlobalConfig.OfflineThreshold()) {
 				continue
@@ -2481,14 +2487,16 @@ func internalClusterPostAccept(d *Daemon, r *http.Request) response.Response {
 	}
 
 	accepted := internalClusterPostAcceptResponse{
-		RaftNodes:  make([]internalRaftNode, len(nodes)),
+		RaftNodes:  make([]internalRaftNode, 0, len(nodes)),
 		PrivateKey: s.Endpoints.NetworkPrivateKey(),
 	}
 
-	for i, node := range nodes {
-		accepted.RaftNodes[i].ID = node.ID
-		accepted.RaftNodes[i].Address = node.Address
-		accepted.RaftNodes[i].Role = int(node.Role)
+	for _, node := range nodes {
+		accepted.RaftNodes = append(accepted.RaftNodes, internalRaftNode{
+			ID:      node.ID,
+			Address: node.Address,
+			Role:    int(node.Role),
+		})
 	}
 
 	return response.SyncResponse(true, accepted)
@@ -2718,12 +2726,16 @@ func internalClusterPostAssign(d *Daemon, r *http.Request) response.Response {
 		return response.BadRequest(fmt.Errorf("No raft members provided"))
 	}
 
-	nodes := make([]db.RaftNode, len(req.RaftNodes))
-	for i, node := range req.RaftNodes {
-		nodes[i].ID = node.ID
-		nodes[i].Address = node.Address
-		nodes[i].Role = db.RaftRole(node.Role)
-		nodes[i].Name = node.Name
+	nodes := make([]db.RaftNode, 0, len(req.RaftNodes))
+	for _, node := range req.RaftNodes {
+		nodes = append(nodes, db.RaftNode{
+			NodeInfo: dqlite.NodeInfo{
+				ID:      node.ID,
+				Address: node.Address,
+				Role:    db.RaftRole(node.Role),
+			},
+			Name: node.Name,
+		})
 	}
 
 	err = cluster.Assign(s, d.gateway, nodes)
@@ -3249,15 +3261,15 @@ func evacuateClusterMember(s *state.State, gateway *cluster.Gateway, r *http.Req
 		return response.SmartError(err)
 	}
 
-	instances := make([]instance.Instance, len(dbInstances))
+	instances := make([]instance.Instance, 0, len(dbInstances))
 
-	for i, dbInst := range dbInstances {
+	for _, dbInst := range dbInstances {
 		inst, err := instance.LoadByProjectAndName(s, dbInst.Project, dbInst.Name)
 		if err != nil {
 			return response.SmartError(fmt.Errorf("Failed to load instance: %w", err))
 		}
 
-		instances[i] = inst
+		instances = append(instances, inst)
 	}
 
 	run := func(op *operations.Operation) error {
@@ -3426,8 +3438,8 @@ func restoreClusterMember(d *Daemon, r *http.Request) response.Response {
 		return response.SmartError(err)
 	}
 
-	instances := make([]instance.Instance, 0)
-	localInstances := make([]instance.Instance, 0)
+	instances := make([]instance.Instance, 0, len(dbInstances))
+	localInstances := make([]instance.Instance, 0, len(dbInstances))
 
 	for _, dbInst := range dbInstances {
 		inst, err := instance.LoadByProjectAndName(s, dbInst.Project, dbInst.Name)
@@ -3817,26 +3829,24 @@ func clusterGroupsGet(d *Daemon, r *http.Request) response.Response {
 				return err
 			}
 
-			for i := range clusterGroups {
-				nodeClusterGroups, err := dbCluster.GetNodeClusterGroups(ctx, tx.Tx(), dbCluster.NodeClusterGroupFilter{GroupID: &clusterGroups[i].ID})
+			apiClusterGroups := make([]*api.ClusterGroup, 0, len(clusterGroups))
+			for _, clusterGroup := range clusterGroups {
+				nodeClusterGroups, err := dbCluster.GetNodeClusterGroups(ctx, tx.Tx(), dbCluster.NodeClusterGroupFilter{GroupID: &clusterGroup.ID})
 				if err != nil {
 					return err
 				}
 
-				clusterGroups[i].Nodes = make([]string, 0, len(nodeClusterGroups))
+				clusterGroup.Nodes = make([]string, 0, len(nodeClusterGroups))
 				for _, node := range nodeClusterGroups {
-					clusterGroups[i].Nodes = append(clusterGroups[i].Nodes, node.Node)
+					clusterGroup.Nodes = append(clusterGroup.Nodes, node.Node)
 				}
-			}
 
-			apiClusterGroups := make([]*api.ClusterGroup, len(clusterGroups))
-			for i, clusterGroup := range clusterGroups {
-				members, err := tx.GetClusterGroupNodes(ctx, clusterGroup.Name)
+				apiClusterGroup, err := clusterGroup.ToAPI(ctx, tx.Tx())
 				if err != nil {
 					return err
 				}
 
-				apiClusterGroups[i] = db.ClusterGroupToAPI(&clusterGroup, members)
+				apiClusterGroups = append(apiClusterGroups, apiClusterGroup)
 			}
 
 			result = apiClusterGroups
@@ -3900,7 +3910,7 @@ func clusterGroupGet(d *Daemon, r *http.Request) response.Response {
 	}
 
 	var group *dbCluster.ClusterGroup
-
+	var apiGroup *api.ClusterGroup
 	err = s.DB.Cluster.Transaction(context.TODO(), func(ctx context.Context, tx *db.ClusterTx) error {
 		// Get the cluster group.
 		group, err = dbCluster.GetClusterGroup(ctx, tx.Tx(), name)
@@ -3918,13 +3928,17 @@ func clusterGroupGet(d *Daemon, r *http.Request) response.Response {
 			group.Nodes = append(group.Nodes, node.Node)
 		}
 
+		apiGroup, err = group.ToAPI(ctx, tx.Tx())
+		if err != nil {
+			return err
+		}
+
 		return nil
 	})
 	if err != nil {
 		return response.SmartError(err)
 	}
 
-	apiGroup, err := group.ToAPI()
 	if err != nil {
 		return response.InternalError(err)
 	}
@@ -4193,17 +4207,17 @@ func clusterGroupPatch(d *Daemon, r *http.Request) response.Response {
 			dbClusterGroup.Nodes = append(dbClusterGroup.Nodes, node.Node)
 		}
 
+		clusterGroup, err = dbClusterGroup.ToAPI(ctx, tx.Tx())
+		if err != nil {
+			return err
+		}
+
 		return nil
 	})
 	if err != nil {
 		return response.SmartError(err)
 	}
 
-	clusterGroup, err = dbClusterGroup.ToAPI()
-	if err != nil {
-		return response.SmartError(err)
-	}
-
 	req := clusterGroup.Writable()
 
 	// Validate the ETag.
@@ -4345,7 +4359,16 @@ func clusterGroupDelete(d *Daemon, r *http.Request) response.Response {
 		}
 
 		if len(members) > 0 {
-			return fmt.Errorf("Only empty cluster groups can be removed")
+			return api.StatusErrorf(http.StatusBadRequest, "Only empty cluster groups can be removed")
+		}
+
+		usedBy, err := dbCluster.GetClusterGroupUsedBy(ctx, tx.Tx(), name)
+		if err != nil {
+			return err
+		}
+
+		if len(usedBy) > 0 {
+			return api.StatusErrorf(http.StatusBadRequest, "Cluster group is currently in use")
 		}
 
 		return dbCluster.DeleteClusterGroup(ctx, tx.Tx(), name)

lxd/db/cluster.go

@@ -12,18 +12,6 @@ import (
 	"github.com/canonical/lxd/shared/version"
 )
 
-// ClusterGroupToAPI is a convenience to convert a ClusterGroup db struct into
-// an API cluster group struct.
-func ClusterGroupToAPI(clusterGroup *cluster.ClusterGroup, nodes []string) *api.ClusterGroup {
-	c := &api.ClusterGroup{
-		Name:        clusterGroup.Name,
-		Description: clusterGroup.Description,
-		Members:     nodes,
-	}
-
-	return c
-}
-
 // GetClusterGroupNodes returns a list of nodes of the given cluster group.
 func (c *ClusterTx) GetClusterGroupNodes(ctx context.Context, groupName string) ([]string, error) {
 	q := `SELECT nodes.name FROM nodes_cluster_groups

lxd/db/cluster/cluster_groups.go

@@ -1,7 +1,13 @@
 package cluster
 
 import (
+	"context"
+	"database/sql"
+
+	"github.com/canonical/lxd/lxd/db/query"
+	"github.com/canonical/lxd/shared"
 	"github.com/canonical/lxd/shared/api"
+	"github.com/canonical/lxd/shared/entity"
 )
 
 //go:generate -command mapper lxd-generate db mapper -t cluster_groups.mapper.go
@@ -42,12 +48,49 @@ type ClusterGroupFilter struct {
 }
 
 // ToAPI returns a LXD API entry.
-func (c *ClusterGroup) ToAPI() (*api.ClusterGroup, error) {
+func (c *ClusterGroup) ToAPI(ctx context.Context, tx *sql.Tx) (*api.ClusterGroup, error) {
+	usedBy, err := GetClusterGroupUsedBy(ctx, tx, c.Name)
+	if err != nil {
+		return nil, err
+	}
+
 	result := api.ClusterGroup{
 		Name:        c.Name,
 		Description: c.Description,
 		Members:     c.Nodes,
+		UsedBy:      usedBy,
 	}
 
 	return &result, nil
 }
+
+// GetClusterGroupUsedBy collates references to the cluster group with the given name.
+// This currently only returns the URLs of projects whose `restricted.cluster.groups` configuration
+// contains the cluster group.
+func GetClusterGroupUsedBy(ctx context.Context, tx *sql.Tx, groupName string) ([]string, error) {
+	q := `
+SELECT projects.name, projects_config.value FROM projects 
+JOIN projects_config ON projects.id = projects_config.project_id 
+WHERE projects_config.key = 'restricted.cluster.groups'`
+
+	var projectURLs []string
+	err := query.Scan(ctx, tx, q, func(scan func(dest ...any) error) error {
+		var projectName string
+		var configValue string
+		err := scan(&projectName, &configValue)
+		if err != nil {
+			return err
+		}
+
+		if shared.ValueInSlice(groupName, shared.SplitNTrimSpace(configValue, ",", -1, false)) {
+			projectURLs = append(projectURLs, entity.ProjectURL(projectName).String())
+		}
+
+		return nil
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	return projectURLs, nil
+}

shared/api/cluster.go

@@ -321,6 +321,11 @@ type ClusterGroup struct {
 	// List of members in this group
 	// Example: ["node1", "node3"]
 	Members []string `json:"members" yaml:"members"`
+
+	// UsedBy is a list or LXD entity URLs that reference the cluster group.
+	//
+	// API extension: clustering_groups_used_by
+	UsedBy []string `json:"used_by" yaml:"used_by"`
 }
 
 // ClusterGroupPost represents the fields required to rename a cluster group.

shared/version/api.go

@@ -441,6 +441,7 @@ var APIExtensions = []string{
 	"oidc_scopes",
 	"project_default_network_and_storage",
 	"client_cert_presence",
+	"clustering_groups_used_by",
 }
 
 // APIExtensionsCount returns the number of available API extensions.

test/suites/clustering.sh

@@ -3810,7 +3810,22 @@ EOF
   # Clean up
   lxc rm -f c1 c2 c3 c4 c5
 
-  # Restricted project tests
+  ## Restricted project tests
+
+  # Create an empty cluster group and reference it from project config
+  lxc cluster group create cluster:fizz
+  lxc project create cluster:buzz -c restricted=true -c restricted.cluster.groups=fizz
+
+  # Cannot launch an instance because fizz has no members
+  ! lxc init testimage cluster:c1 --project buzz || false
+
+  # Group fizz has no members, but it cannot be deleted because it is referenced by project buzz.
+  ! lxc cluster group delete cluster:fizz || false
+
+  # Clean up.
+  lxc project delete cluster:buzz
+  lxc cluster group delete cluster:fizz
+
   lxc project create foo -c features.images=false -c restricted=true -c restricted.cluster.groups=blah
   lxc profile show default | lxc profile edit default --project foo