chore: avoid depending on rbac in slim builds

ethanndickson · ethanndickson · commit 52d65eeb3ce5 · 2025-05-21T04:11:26.000Z
diff --git a/cli/testdata/coder_users_edit-roles_--help.golden b/cli/testdata/coder_users_edit-roles_--help.golden
@@ -8,8 +8,7 @@ USAGE:
 OPTIONS:
       --roles string-array
           A list of roles to give to the user. This removes any existing roles
-          the user may have. The available roles are: auditor, member, owner,
-          template-admin, user-admin.
+          the user may have.
 
   -y, --yes bool
           Bypass prompts.
diff --git a/cli/usereditroles.go b/cli/usereditroles.go
@@ -1,40 +1,27 @@
 package cli
 
 import (
-	"fmt"
 	"slices"
-	"sort"
 	"strings"
 
 	"golang.org/x/xerrors"
 
 	"github.com/coder/coder/v2/cli/cliui"
-	"github.com/coder/coder/v2/coderd/rbac"
 	"github.com/coder/coder/v2/codersdk"
 	"github.com/coder/serpent"
 )
 
 func (r *RootCmd) userEditRoles() *serpent.Command {
 	client := new(codersdk.Client)
-
-	roles := rbac.SiteRoles()
-
-	siteRoles := make([]string, 0)
-	for _, role := range roles {
-		siteRoles = append(siteRoles, role.Identifier.Name)
-	}
-	sort.Strings(siteRoles)
-
 	var givenRoles []string
-
 	cmd := &serpent.Command{
 		Use:   "edit-roles <username|user_id>",
 		Short: "Edit a user's roles by username or id",
 		Options: []serpent.Option{
 			cliui.SkipPromptOption(),
 			{
 				Name:        "roles",
-				Description: fmt.Sprintf("A list of roles to give to the user. This removes any existing roles the user may have. The available roles are: %s.", strings.Join(siteRoles, ", ")),
+				Description: "A list of roles to give to the user. This removes any existing roles the user may have.",
 				Flag:        "roles",
 				Value:       serpent.StringArrayOf(&givenRoles),
 			},
@@ -52,13 +39,21 @@ func (r *RootCmd) userEditRoles() *serpent.Command {
 			if err != nil {
 				return xerrors.Errorf("fetch user roles: %w", err)
 			}
+			siteRoles, err := client.ListSiteRoles(ctx)
+			if err != nil {
+				return xerrors.Errorf("fetch site roles: %w", err)
+			}
+			siteRoleNames := make([]string, 0, len(siteRoles))
+			for _, role := range siteRoles {
+				siteRoleNames = append(siteRoleNames, role.Name)
+			}
 
 			var selectedRoles []string
 			if len(givenRoles) > 0 {
 				// Make sure all of the given roles are valid site roles
 				for _, givenRole := range givenRoles {
-					if !slices.Contains(siteRoles, givenRole) {
-						siteRolesPretty := strings.Join(siteRoles, ", ")
+					if !slices.Contains(siteRoleNames, givenRole) {
+						siteRolesPretty := strings.Join(siteRoleNames, ", ")
 						return xerrors.Errorf("The role %s is not valid. Please use one or more of the following roles: %s\n", givenRole, siteRolesPretty)
 					}
 				}
@@ -67,7 +62,7 @@ func (r *RootCmd) userEditRoles() *serpent.Command {
 			} else {
 				selectedRoles, err = cliui.MultiSelect(inv, cliui.MultiSelectOptions{
 					Message:  "Select the roles you'd like to assign to the user",
-					Options:  siteRoles,
+					Options:  siteRoleNames,
 					Defaults: userRoles.Roles,
 				})
 				if err != nil {
diff --git a/coderd/httpapi/authz.go b/coderd/httpapi/authz.go
@@ -0,0 +1,28 @@
+//go:build !slim
+
+package httpapi
+
+import (
+	"context"
+	"net/http"
+
+	"github.com/coder/coder/v2/coderd/rbac"
+)
+
+// This is defined separately in slim builds to avoid importing the rbac
+// package, which is a large dependency.
+func SetAuthzCheckRecorderHeader(ctx context.Context, rw http.ResponseWriter) {
+	if rec, ok := rbac.GetAuthzCheckRecorder(ctx); ok {
+		// If you're here because you saw this header in a response, and you're
+		// trying to investigate the code, here are a couple of notable things
+		// for you to know:
+		// - If any of the checks are `false`, they might not represent the whole
+		//   picture. There could be additional checks that weren't performed,
+		//   because processing stopped after the failure.
+		// - The checks are recorded by the `authzRecorder` type, which is
+		//   configured on server startup for development and testing builds.
+		// - If this header is missing from a response, make sure the response is
+		//   being written by calling `httpapi.Write`!
+		rw.Header().Set("x-authz-checks", rec.String())
+	}
+}
diff --git a/coderd/httpapi/authz_slim.go b/coderd/httpapi/authz_slim.go
@@ -0,0 +1,13 @@
+//go:build slim
+
+package httpapi
+
+import (
+	"context"
+	"net/http"
+)
+
+func SetAuthzCheckRecorderHeader(ctx context.Context, rw http.ResponseWriter) {
+	// There's no RBAC on the agent API, so this is separately defined to
+	// avoid importing the RBAC package, which is a large dependency.
+}
diff --git a/coderd/httpapi/httpapi.go b/coderd/httpapi/httpapi.go
@@ -20,7 +20,6 @@ import (
 	"github.com/coder/websocket/wsjson"
 
 	"github.com/coder/coder/v2/coderd/httpapi/httpapiconstraints"
-	"github.com/coder/coder/v2/coderd/rbac"
 	"github.com/coder/coder/v2/coderd/tracing"
 	"github.com/coder/coder/v2/codersdk"
 )
@@ -199,19 +198,7 @@ func Write(ctx context.Context, rw http.ResponseWriter, status int, response int
 	_, span := tracing.StartSpan(ctx)
 	defer span.End()
 
-	if rec, ok := rbac.GetAuthzCheckRecorder(ctx); ok {
-		// If you're here because you saw this header in a response, and you're
-		// trying to investigate the code, here are a couple of notable things
-		// for you to know:
-		// - If any of the checks are `false`, they might not represent the whole
-		//   picture. There could be additional checks that weren't performed,
-		//   because processing stopped after the failure.
-		// - The checks are recorded by the `authzRecorder` type, which is
-		//   configured on server startup for development and testing builds.
-		// - If this header is missing from a response, make sure the response is
-		//   being written by calling `httpapi.Write`!
-		rw.Header().Set("x-authz-checks", rec.String())
-	}
+	SetAuthzCheckRecorderHeader(ctx, rw)
 
 	rw.Header().Set("Content-Type", "application/json; charset=utf-8")
 	rw.WriteHeader(status)
@@ -228,9 +215,7 @@ func WriteIndent(ctx context.Context, rw http.ResponseWriter, status int, respon
 	_, span := tracing.StartSpan(ctx)
 	defer span.End()
 
-	if rec, ok := rbac.GetAuthzCheckRecorder(ctx); ok {
-		rw.Header().Set("x-authz-checks", rec.String())
-	}
+	SetAuthzCheckRecorderHeader(ctx, rw)
 
 	rw.Header().Set("Content-Type", "application/json; charset=utf-8")
 	rw.WriteHeader(status)
diff --git a/coderd/httpmw/authz.go b/coderd/httpmw/authz.go
@@ -1,3 +1,5 @@
+//go:build !slim
+
 package httpmw
 
 import (
diff --git a/coderd/rbac/no_slim.go b/coderd/rbac/no_slim.go
@@ -0,0 +1,8 @@
+package rbac
+
+const (
+	// This declaration protects against imports in slim builds, see
+	// no_slim_slim.go.
+	//nolint:revive,unused
+	_DO_NOT_IMPORT_THIS_PACKAGE_IN_SLIM_BUILDS = "DO_NOT_IMPORT_THIS_PACKAGE_IN_SLIM_BUILDS"
+)
diff --git a/coderd/rbac/no_slim_slim.go b/coderd/rbac/no_slim_slim.go
@@ -0,0 +1,14 @@
+//go:build slim
+
+package rbac
+
+const (
+	// This re-declaration will result in a compilation error and is present to
+	// prevent increasing the slim binary size by importing this package,
+	// directly or indirectly.
+	//
+	// no_slim_slim.go:7:2: _DO_NOT_IMPORT_THIS_PACKAGE_IN_SLIM_BUILDS redeclared in this block
+	// 	no_slim.go:4:2: other declaration of _DO_NOT_IMPORT_THIS_PACKAGE_IN_SLIM_BUILDS
+	//nolint:revive,unused
+	_DO_NOT_IMPORT_THIS_PACKAGE_IN_SLIM_BUILDS = "DO_NOT_IMPORT_THIS_PACKAGE_IN_SLIM_BUILDS"
+)
diff --git a/docs/reference/cli/users_edit-roles.md b/docs/reference/cli/users_edit-roles.md

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+//go:build !slim`
	`2`	`+`
`1`	`3`	`package httpmw`
`2`	`4`
`3`	`5`	`import (`