Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
ethanndickson merged 2 commits intomainfromethan/avoid-rbac-dependency-slim
May 22, 2025
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletionscli/testdata/coder_users_edit-roles_--help.golden
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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.
Expand Down
29 changes: 12 additions & 17 deletionscli/usereditroles.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -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),
},
Expand All@@ -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

@ethanndicksonethanndicksonMay 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.

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)
}
}
Expand All@@ -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 {
Expand Down
9 changes: 5 additions & 4 deletionscoderd/database/no_slim.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
//go:build slim

package database

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"
// This line fails to compile, preventing this package from being imported
// in slim builds.
_DO_NOT_IMPORT_THIS_PACKAGE_IN_SLIM_BUILDS = _DO_NOT_IMPORT_THIS_PACKAGE_IN_SLIM_BUILDS
)
14 changes: 0 additions & 14 deletionscoderd/database/no_slim_slim.go
View file
Open in desktop

This file was deleted.

28 changes: 28 additions & 0 deletionscoderd/httpapi/authz.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -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())
}
}
13 changes: 13 additions & 0 deletionscoderd/httpapi/authz_slim.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -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.
}
19 changes: 2 additions & 17 deletionscoderd/httpapi/httpapi.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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"
)
Expand DownExpand Up@@ -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)
Expand All@@ -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)
Expand Down
2 changes: 2 additions & 0 deletionscoderd/httpmw/authz.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
//go:build !slim

package httpmw

import (
Expand Down
9 changes: 9 additions & 0 deletionscoderd/rbac/no_slim.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
//go:build slim

package rbac

const (
// This line fails to compile, preventing this package from being imported
// in slim builds.
_DO_NOT_IMPORT_THIS_PACKAGE_IN_SLIM_BUILDS = _DO_NOT_IMPORT_THIS_PACKAGE_IN_SLIM_BUILDS
)
4 changes: 2 additions & 2 deletionscoderd/rbac/roles.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -798,12 +798,12 @@ func OrganizationRoles(organizationID uuid.UUID) []Role {
return roles
}

//SiteRoles lists all roles that can be applied to a user.
//SiteBuiltInRoles lists all roles that can be applied to a user.
// This is the list of available roles, and not specific to a user
//
// This should be a list in a database, but until then we build
// the list from the builtins.
funcSiteRoles() []Role {
funcSiteBuiltInRoles() []Role {
var roles []Role
for _, roleF := range builtInRoles {
// Must provide some non-nil uuid to filter out org roles.
Expand Down
4 changes: 2 additions & 2 deletionscoderd/rbac/roles_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -34,7 +34,7 @@ func (a authSubject) Subjects() []authSubject { return []authSubject{a} }
// rules. If this is incorrect, that is a mistake.
func TestBuiltInRoles(t *testing.T) {
t.Parallel()
for _, r := range rbac.SiteRoles() {
for _, r := range rbac.SiteBuiltInRoles() {
r := r
t.Run(r.Identifier.String(), func(t *testing.T) {
t.Parallel()
Expand DownExpand Up@@ -997,7 +997,7 @@ func TestIsOrgRole(t *testing.T) {
func TestListRoles(t *testing.T) {
t.Parallel()

siteRoles := rbac.SiteRoles()
siteRoles := rbac.SiteBuiltInRoles()
siteRoleNames := make([]string, 0, len(siteRoles))
for _, role := range siteRoles {
siteRoleNames = append(siteRoleNames, role.Identifier.Name)
Expand Down
2 changes: 1 addition & 1 deletioncoderd/roles.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -43,7 +43,7 @@ func (api *API) AssignableSiteRoles(rw http.ResponseWriter, r *http.Request) {
return
}

httpapi.Write(ctx, rw, http.StatusOK, assignableRoles(actorRoles.Roles, rbac.SiteRoles(), dbCustomRoles))
httpapi.Write(ctx, rw, http.StatusOK, assignableRoles(actorRoles.Roles, rbac.SiteBuiltInRoles(), dbCustomRoles))
}

// assignableOrgRoles returns all org wide roles that can be assigned.
Expand Down
2 changes: 1 addition & 1 deletiondocs/reference/cli/users_edit-roles.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

Loading

[8]ページ先頭

©2009-2025 Movatter.jp