NotificationsYou must be signed in to change notification settings
Fork913
Star10k

chore: avoid depending on rbac in slim builds#17959

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

ethanndickson merged 2 commits intomainfromethan/avoid-rbac-dependency-slim

May 22, 2025

Merged

chore: avoid depending on rbac in slim builds#17959

ethanndickson merged 2 commits intomainfromethan/avoid-rbac-dependency-slim

May 22, 2025

Conversation

Copy link

Member

ethanndickson commentedMay 21, 2025•
edited
Loading

I noticed thecoder-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/rbaccoderd/rbac/no_slim.go:8:2: initialization cycle: _DO_NOT_IMPORT_THIS_PACKAGE_IN_SLIM_BUILDS refers to itselfmake: *** [Makefile:224: build/coder-slim_2.22.1-devel+7e46d24b4_linux_amd64] Error 1

Before and after forcoder-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                                   │          │          │         │└─────────┴─────────────────────────────────────────┴──────────┴──────────┴─────────┘

Copy link

MemberAuthor

ethanndickson commentedMay 21, 2025

chore: avoid depending on rbac in slim builds #17959 👈(View in Graphite)
main

This stack of pull requests is managed byGraphite. Learn more aboutstacking.

github-actionsbot assignedethanndickson

May 21, 2025

ethanndickson commented

May 21, 2025

View reviewed changes

cli/usereditroles.go

		@@ -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)

Copy link

MemberAuthor

ethanndicksonMay 21, 2025•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I think making coderd the source of truth for site roles (and not whatever is embedded in the binary) is sensible here. There's a comment from a few years ago about moving the list of site roles to the database, so if that ever happens we'd need to switch to fetching them anyway.

EDIT: See below.