Skip to content

Commit 34494fb

Browse files
ethanndicksonethanndickson
authored
chore: avoid depending on rbac in slim builds (#17959)
I noticed the `coder-vpn.dylib` (of course alongside the Agent/CLI binaries) had grown substantially (from 29MB to 37MB for the dylib), and discovered that importing RBAC in slim builds was the issue This PR removes the dependency on RBAC in slim builds, and adds a compile-time check to ensure it can't be imported in the future: ``` $ make build # github.com/coder/coder/v2/coderd/rbac coderd/rbac/no_slim.go:8:2: initialization cycle: _DO_NOT_IMPORT_THIS_PACKAGE_IN_SLIM_BUILDS refers to itself make: *** [Makefile:224: build/coder-slim_2.22.1-devel+7e46d24b4_linux_amd64] Error 1 ``` Before and after for `coder-slim_darwin_arm64`: ``` $ gsa before after ┌───────────────────────────────────────────────────────────────────────────────────┐ │ Diff between before and after │ ├─────────┬─────────────────────────────────────────┬──────────┬──────────┬─────────┤ │ PERCENT │ NAME │ OLD SIZE │ NEW SIZE │ DIFF │ ├─────────┼─────────────────────────────────────────┼──────────┼──────────┼─────────┤ │ -100% │ github.com/gorilla/mux │ │ │ +0 B │ │ -100% │ github.com/ammario/tlru │ │ │ +0 B │ │ -100% │ github.com/armon/go-radix │ │ │ +0 B │ │ -0.00% │ gvisor.dev/gvisor │ 2.4 MB │ 2.4 MB │ -4 B │ │ -0.21% │ os │ 155 kB │ 155 kB │ -328 B │ │ -0.23% │ regexp │ 152 kB │ 152 kB │ -346 B │ │ -0.04% │ runtime │ 876 kB │ 876 kB │ -372 B │ │ -100% │ github.com/rcrowley/go-metrics │ 675 B │ │ -675 B │ │ -23.79% │ github.com/cespare/xxhash/v2 │ 3.0 kB │ 2.3 kB │ -715 B │ │ -100% │ github.com/agnivade/levenshtein │ 1.4 kB │ │ -1.4 kB │ │ -100% │ github.com/go-ini/ini │ 1.5 kB │ │ -1.5 kB │ │ -100% │ github.com/xeipuuv/gojsonreference │ 2.4 kB │ │ -2.4 kB │ │ -100% │ github.com/xeipuuv/gojsonpointer │ 5.2 kB │ │ -5.2 kB │ │ -2.43% │ go.opentelemetry.io/otel │ 316 kB │ 309 kB │ -7.7 kB │ │ -2.40% │ slices │ 381 kB │ 372 kB │ -9.2 kB │ │ -0.68% │ crypto │ 1.4 MB │ 1.4 MB │ -9.5 kB │ │ -100% │ github.com/tchap/go-patricia/v2 │ 23 kB │ │ -23 kB │ │ -100% │ github.com/yashtewari/glob-intersection │ 28 kB │ │ -28 kB │ │ -4.35% │ <autogenerated> │ 754 kB │ 721 kB │ -33 kB │ │ -100% │ github.com/sirupsen/logrus │ 72 kB │ │ -72 kB │ │ -2.56% │ github.com/coder/coder/v2 │ 3.3 MB │ 3.2 MB │ -84 kB │ │ -100% │ github.com/gobwas/glob │ 107 kB │ │ -107 kB │ │ -100% │ sigs.k8s.io/yaml │ 244 kB │ │ -244 kB │ │ -100% │ github.com/open-policy-agent/opa │ 2.2 MB │ │ -2.2 MB │ ├─────────┼─────────────────────────────────────────┼──────────┼──────────┼─────────┤ │ -7.79% │ __go_buildinfo __DATA │ 18 kB │ 17 kB │ -1.4 kB │ │ -6.81% │ __itablink __DATA_CONST │ 23 kB │ 22 kB │ -1.6 kB │ │ -6.61% │ __typelink __DATA_CONST │ 71 kB │ 66 kB │ -4.7 kB │ │ -2.86% │ __noptrdata __DATA │ 1.0 MB │ 993 kB │ -29 kB │ │ -21.49% │ __data __DATA │ 320 kB │ 251 kB │ -69 kB │ │ -6.19% │ __rodata __DATA_CONST │ 6.0 MB │ 5.6 MB │ -372 kB │ │ -47.19% │ __rodata __TEXT │ 7.6 MB │ 4.0 MB │ -3.6 MB │ ├─────────┼─────────────────────────────────────────┼──────────┼──────────┼─────────┤ │ -14.02% │ before │ 50 MB │ 43 MB │ -7.0 MB │ │ │ after │ │ │ │ └─────────┴─────────────────────────────────────────┴──────────┴──────────┴─────────┘ ```
1 parent 1e1e6f3 commit 34494fb

File tree

13 files changed

+78
-60
lines changed

13 files changed

+78
-60
lines changed

‎cli/testdata/coder_users_edit-roles_--help.golden‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@ USAGE:
88
OPTIONS:
99
--roles string-array
1010
A list of roles to give to the user. This removes any existing roles
11-
the user may have. The available roles are: auditor, member, owner,
12-
template-admin, user-admin.
11+
the user may have.
1312

1413
-y, --yes bool
1514
Bypass prompts.

‎cli/usereditroles.go‎

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,27 @@
11
package cli
22

33
import (
4-
"fmt"
54
"slices"
6-
"sort"
75
"strings"
86

97
"golang.org/x/xerrors"
108

119
"github.com/coder/coder/v2/cli/cliui"
12-
"github.com/coder/coder/v2/coderd/rbac"
1310
"github.com/coder/coder/v2/codersdk"
1411
"github.com/coder/serpent"
1512
)
1613

1714
func (r *RootCmd) userEditRoles() *serpent.Command {
1815
client := new(codersdk.Client)
19-
20-
roles := rbac.SiteRoles()
21-
22-
siteRoles := make([]string, 0)
23-
for _, role := range roles {
24-
siteRoles = append(siteRoles, role.Identifier.Name)
25-
}
26-
sort.Strings(siteRoles)
27-
2816
var givenRoles []string
29-
3017
cmd := &serpent.Command{
3118
Use: "edit-roles <username|user_id>",
3219
Short: "Edit a user's roles by username or id",
3320
Options: []serpent.Option{
3421
cliui.SkipPromptOption(),
3522
{
3623
Name: "roles",
37-
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, ", ")),
24+
Description: "A list of roles to give to the user. This removes any existing roles the user may have.",
3825
Flag: "roles",
3926
Value: serpent.StringArrayOf(&givenRoles),
4027
},
@@ -52,13 +39,21 @@ func (r *RootCmd) userEditRoles() *serpent.Command {
5239
if err != nil {
5340
return xerrors.Errorf("fetch user roles: %w", err)
5441
}
42+
siteRoles, err := client.ListSiteRoles(ctx)
43+
if err != nil {
44+
return xerrors.Errorf("fetch site roles: %w", err)
45+
}
46+
siteRoleNames := make([]string, 0, len(siteRoles))
47+
for _, role := range siteRoles {
48+
siteRoleNames = append(siteRoleNames, role.Name)
49+
}
5550

5651
var selectedRoles []string
5752
if len(givenRoles) > 0 {
5853
// Make sure all of the given roles are valid site roles
5954
for _, givenRole := range givenRoles {
60-
if !slices.Contains(siteRoles, givenRole) {
61-
siteRolesPretty := strings.Join(siteRoles, ", ")
55+
if !slices.Contains(siteRoleNames, givenRole) {
56+
siteRolesPretty := strings.Join(siteRoleNames, ", ")
6257
return xerrors.Errorf("The role %s is not valid. Please use one or more of the following roles: %s\n", givenRole, siteRolesPretty)
6358
}
6459
}
@@ -67,7 +62,7 @@ func (r *RootCmd) userEditRoles() *serpent.Command {
6762
} else {
6863
selectedRoles, err = cliui.MultiSelect(inv, cliui.MultiSelectOptions{
6964
Message: "Select the roles you'd like to assign to the user",
70-
Options: siteRoles,
65+
Options: siteRoleNames,
7166
Defaults: userRoles.Roles,
7267
})
7368
if err != nil {

‎coderd/database/no_slim.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1+
//go:build slim
2+
13
package database
24

35
const (
4-
// This declaration protects against imports in slim builds, see
5-
// no_slim_slim.go.
6-
//nolint:revive,unused
7-
_DO_NOT_IMPORT_THIS_PACKAGE_IN_SLIM_BUILDS = "DO_NOT_IMPORT_THIS_PACKAGE_IN_SLIM_BUILDS"
6+
// This line fails to compile, preventing this package from being imported
7+
// in slim builds.
8+
_DO_NOT_IMPORT_THIS_PACKAGE_IN_SLIM_BUILDS = _DO_NOT_IMPORT_THIS_PACKAGE_IN_SLIM_BUILDS
89
)

‎coderd/database/no_slim_slim.go‎

Lines changed: 0 additions & 14 deletions
This file was deleted.

‎coderd/httpapi/authz.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
//go:build !slim
2+
3+
package httpapi
4+
5+
import (
6+
"context"
7+
"net/http"
8+
9+
"github.com/coder/coder/v2/coderd/rbac"
10+
)
11+
12+
// This is defined separately in slim builds to avoid importing the rbac
13+
// package, which is a large dependency.
14+
func SetAuthzCheckRecorderHeader(ctx context.Context, rw http.ResponseWriter) {
15+
if rec, ok := rbac.GetAuthzCheckRecorder(ctx); ok {
16+
// If you're here because you saw this header in a response, and you're
17+
// trying to investigate the code, here are a couple of notable things
18+
// for you to know:
19+
// - If any of the checks are `false`, they might not represent the whole
20+
// picture. There could be additional checks that weren't performed,
21+
// because processing stopped after the failure.
22+
// - The checks are recorded by the `authzRecorder` type, which is
23+
// configured on server startup for development and testing builds.
24+
// - If this header is missing from a response, make sure the response is
25+
// being written by calling `httpapi.Write`!
26+
rw.Header().Set("x-authz-checks", rec.String())
27+
}
28+
}

‎coderd/httpapi/authz_slim.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
//go:build slim
2+
3+
package httpapi
4+
5+
import (
6+
"context"
7+
"net/http"
8+
)
9+
10+
func SetAuthzCheckRecorderHeader(ctx context.Context, rw http.ResponseWriter) {
11+
// There's no RBAC on the agent API, so this is separately defined to
12+
// avoid importing the RBAC package, which is a large dependency.
13+
}

‎coderd/httpapi/httpapi.go‎

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"github.com/coder/websocket/wsjson"
2121

2222
"github.com/coder/coder/v2/coderd/httpapi/httpapiconstraints"
23-
"github.com/coder/coder/v2/coderd/rbac"
2423
"github.com/coder/coder/v2/coderd/tracing"
2524
"github.com/coder/coder/v2/codersdk"
2625
)
@@ -199,19 +198,7 @@ func Write(ctx context.Context, rw http.ResponseWriter, status int, response int
199198
_, span := tracing.StartSpan(ctx)
200199
defer span.End()
201200

202-
if rec, ok := rbac.GetAuthzCheckRecorder(ctx); ok {
203-
// If you're here because you saw this header in a response, and you're
204-
// trying to investigate the code, here are a couple of notable things
205-
// for you to know:
206-
// - If any of the checks are `false`, they might not represent the whole
207-
// picture. There could be additional checks that weren't performed,
208-
// because processing stopped after the failure.
209-
// - The checks are recorded by the `authzRecorder` type, which is
210-
// configured on server startup for development and testing builds.
211-
// - If this header is missing from a response, make sure the response is
212-
// being written by calling `httpapi.Write`!
213-
rw.Header().Set("x-authz-checks", rec.String())
214-
}
201+
SetAuthzCheckRecorderHeader(ctx, rw)
215202

216203
rw.Header().Set("Content-Type", "application/json; charset=utf-8")
217204
rw.WriteHeader(status)
@@ -228,9 +215,7 @@ func WriteIndent(ctx context.Context, rw http.ResponseWriter, status int, respon
228215
_, span := tracing.StartSpan(ctx)
229216
defer span.End()
230217

231-
if rec, ok := rbac.GetAuthzCheckRecorder(ctx); ok {
232-
rw.Header().Set("x-authz-checks", rec.String())
233-
}
218+
SetAuthzCheckRecorderHeader(ctx, rw)
234219

235220
rw.Header().Set("Content-Type", "application/json; charset=utf-8")
236221
rw.WriteHeader(status)

‎coderd/httpmw/authz.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
//go:build !slim
2+
13
package httpmw
24

35
import (

‎coderd/rbac/no_slim.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
//go:build slim
2+
3+
package rbac
4+
5+
const (
6+
// This line fails to compile, preventing this package from being imported
7+
// in slim builds.
8+
_DO_NOT_IMPORT_THIS_PACKAGE_IN_SLIM_BUILDS = _DO_NOT_IMPORT_THIS_PACKAGE_IN_SLIM_BUILDS
9+
)

‎coderd/rbac/roles.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -798,12 +798,12 @@ func OrganizationRoles(organizationID uuid.UUID) []Role {
798798
return roles
799799
}
800800

801-
// SiteRoles lists all roles that can be applied to a user.
801+
// SiteBuiltInRoles lists all roles that can be applied to a user.
802802
// This is the list of available roles, and not specific to a user
803803
//
804804
// This should be a list in a database, but until then we build
805805
// the list from the builtins.
806-
func SiteRoles() []Role {
806+
func SiteBuiltInRoles() []Role {
807807
var roles []Role
808808
for _, roleF := range builtInRoles {
809809
// Must provide some non-nil uuid to filter out org roles.

0 commit comments

Comments
 (0)