Skip to content

Commit 8b125d6

Browse files
EmyrkEmyrk
authored
chore: Implement joins with golang templates (#6429)
* feat: Implement view for workspace builds to include rbac info * Removes the need to fetch the workspace to run an rbac check. * chore: Use workspace build as RBAC object * chore: Use golang templates instead of sqlc files
1 parent a666539 commit 8b125d6

36 files changed

+894
-660
lines changed

‎coderd/audit/diff.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ type Auditable interface {
1414
database.User |
1515
database.Workspace |
1616
database.GitSSHKey |
17-
database.WorkspaceBuild |
17+
database.WorkspaceBuildRBAC |
1818
database.AuditableGroup |
1919
database.License
2020
}

‎coderd/audit/request.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ func ResourceTarget[T Auditable](tgt T) string {
6262
return typed.Username
6363
case database.Workspace:
6464
return typed.Name
65-
case database.WorkspaceBuild:
65+
case database.WorkspaceBuildRBAC:
6666
// this isn't used
6767
return ""
6868
case database.GitSSHKey:
@@ -89,7 +89,7 @@ func ResourceID[T Auditable](tgt T) uuid.UUID {
8989
return typed.ID
9090
case database.Workspace:
9191
return typed.ID
92-
case database.WorkspaceBuild:
92+
case database.WorkspaceBuildRBAC:
9393
return typed.ID
9494
case database.GitSSHKey:
9595
return typed.UserID
@@ -114,7 +114,7 @@ func ResourceType[T Auditable](tgt T) database.ResourceType {
114114
return database.ResourceTypeUser
115115
case database.Workspace:
116116
return database.ResourceTypeWorkspace
117-
case database.WorkspaceBuild:
117+
case database.WorkspaceBuildRBAC:
118118
return database.ResourceTypeWorkspaceBuild
119119
case database.GitSSHKey:
120120
return database.ResourceTypeGitSshKey

‎coderd/autobuild/executor/lifecycle_executor.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ func isEligibleForAutoStartStop(ws database.Workspace) bool {
204204

205205
func getNextTransition(
206206
ws database.Workspace,
207-
priorHistory database.WorkspaceBuild,
207+
priorHistory database.WorkspaceBuildRBAC,
208208
priorJob database.ProvisionerJob,
209209
) (
210210
validTransition database.WorkspaceTransition,
@@ -239,7 +239,7 @@ func getNextTransition(
239239

240240
// TODO(cian): this function duplicates most of api.postWorkspaceBuilds. Refactor.
241241
// See: https://github.com/coder/coder/issues/1401
242-
func build(ctx context.Context, store database.Store, workspace database.Workspace, trans database.WorkspaceTransition, priorHistory database.WorkspaceBuild, priorJob database.ProvisionerJob) error {
242+
func build(ctx context.Context, store database.Store, workspace database.Workspace, trans database.WorkspaceTransition, priorHistory database.WorkspaceBuildRBAC, priorJob database.ProvisionerJob) error {
243243
template, err := store.GetTemplateByID(ctx, workspace.TemplateID)
244244
if err != nil {
245245
return xerrors.Errorf("get workspace template: %w", err)

‎coderd/autobuild/executor/lifecycle_executor_test.go

+3-4
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,16 @@ package executor_test
22

33
import (
44
"context"
5-
"os"
65
"testing"
76
"time"
87

9-
"go.uber.org/goleak"
10-
118
"github.com/google/uuid"
9+
"go.uber.org/goleak"
1210

1311
"github.com/coder/coder/coderd/autobuild/executor"
1412
"github.com/coder/coder/coderd/coderdtest"
1513
"github.com/coder/coder/coderd/database"
14+
"github.com/coder/coder/coderd/database/dbtestutil"
1615
"github.com/coder/coder/coderd/schedule"
1716
"github.com/coder/coder/coderd/util/ptr"
1817
"github.com/coder/coder/codersdk"
@@ -493,7 +492,7 @@ func TestExecutorWorkspaceAutostopNoWaitChangedMyMind(t *testing.T) {
493492
}
494493

495494
func TestExecutorAutostartMultipleOK(t *testing.T) {
496-
if os.Getenv("DB") == "" {
495+
if !dbtestutil.UsingRealDatabase() {
497496
t.Skip(`This test only really works when using a "real" database, similar to a HA setup`)
498497
}
499498

‎coderd/database/db.go

+12
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import (
1616

1717
"github.com/jmoiron/sqlx"
1818
"golang.org/x/xerrors"
19+
20+
"github.com/coder/coder/coderd/database/sqlxqueries"
1921
)
2022

2123
// Store contains all queryable database functions.
@@ -37,11 +39,21 @@ type DBTX interface {
3739
QueryRowContext(context.Context, string, ...interface{}) *sql.Row
3840
SelectContext(ctx context.Context, dest interface{}, query string, args ...interface{}) error
3941
GetContext(ctx context.Context, dest interface{}, query string, args ...interface{}) error
42+
43+
// Extends the sqlx interface
44+
sqlx.QueryerContext
4045
}
4146

4247
// New creates a new database store using a SQL database connection.
4348
func New(sdb *sql.DB) Store {
4449
dbx := sqlx.NewDb(sdb, "postgres")
50+
// Load the embedded queries. If this fails, some of our queries
51+
// will never work. This is a fatal developer error that should never
52+
// happen.
53+
_, err := sqlxqueries.LoadQueries()
54+
if err != nil {
55+
panic(xerrors.Errorf("load queries: %w", err))
56+
}
4557
return &sqlQuerier{
4658
db: dbx,
4759
sdb: dbx,

‎coderd/database/dbauthz/querier.go

+25-61
Original file line numberDiff line numberDiff line change
@@ -1167,25 +1167,12 @@ func (q *querier) GetWorkspaces(ctx context.Context, arg database.GetWorkspacesP
11671167
return q.db.GetAuthorizedWorkspaces(ctx, arg, prep)
11681168
}
11691169

1170-
func (q *querier) GetLatestWorkspaceBuildByWorkspaceID(ctx context.Context, workspaceID uuid.UUID) (database.WorkspaceBuild, error) {
1171-
if _, err := q.GetWorkspaceByID(ctx, workspaceID); err != nil {
1172-
return database.WorkspaceBuild{}, err
1173-
}
1174-
return q.db.GetLatestWorkspaceBuildByWorkspaceID(ctx, workspaceID)
1170+
func (q *querier) GetLatestWorkspaceBuildByWorkspaceID(ctx context.Context, workspaceID uuid.UUID) (database.WorkspaceBuildRBAC, error) {
1171+
return fetch(q.log, q.auth, q.db.GetLatestWorkspaceBuildByWorkspaceID)(ctx, workspaceID)
11751172
}
11761173

1177-
func (q *querier) GetLatestWorkspaceBuildsByWorkspaceIDs(ctx context.Context, ids []uuid.UUID) ([]database.WorkspaceBuild, error) {
1178-
// This is not ideal as not all builds will be returned if the workspace cannot be read.
1179-
// This should probably be handled differently? Maybe join workspace builds with workspace
1180-
// ownership properties and filter on that.
1181-
for _, id := range ids {
1182-
_, err := q.GetWorkspaceByID(ctx, id)
1183-
if err != nil {
1184-
return nil, err
1185-
}
1186-
}
1187-
1188-
return q.db.GetLatestWorkspaceBuildsByWorkspaceIDs(ctx, ids)
1174+
func (q *querier) GetLatestWorkspaceBuildsByWorkspaceIDs(ctx context.Context, ids []uuid.UUID) ([]database.WorkspaceBuildRBAC, error) {
1175+
return fetchWithPostFilter(q.auth, q.db.GetLatestWorkspaceBuildsByWorkspaceIDs)(ctx, ids)
11891176
}
11901177

11911178
func (q *querier) GetWorkspaceAgentByID(ctx context.Context, id uuid.UUID) (database.WorkspaceAgent, error) {
@@ -1263,35 +1250,16 @@ func (q *querier) GetWorkspaceAppsByAgentID(ctx context.Context, agentID uuid.UU
12631250
return q.db.GetWorkspaceAppsByAgentID(ctx, agentID)
12641251
}
12651252

1266-
func (q *querier) GetWorkspaceBuildByID(ctx context.Context, buildID uuid.UUID) (database.WorkspaceBuild, error) {
1267-
build, err := q.db.GetWorkspaceBuildByID(ctx, buildID)
1268-
if err != nil {
1269-
return database.WorkspaceBuild{}, err
1270-
}
1271-
if _, err := q.GetWorkspaceByID(ctx, build.WorkspaceID); err != nil {
1272-
return database.WorkspaceBuild{}, err
1273-
}
1274-
return build, nil
1253+
func (q *querier) GetWorkspaceBuildByID(ctx context.Context, buildID uuid.UUID) (database.WorkspaceBuildRBAC, error) {
1254+
return fetch(q.log, q.auth, q.db.GetWorkspaceBuildByID)(ctx, buildID)
12751255
}
12761256

1277-
func (q *querier) GetWorkspaceBuildByJobID(ctx context.Context, jobID uuid.UUID) (database.WorkspaceBuild, error) {
1278-
build, err := q.db.GetWorkspaceBuildByJobID(ctx, jobID)
1279-
if err != nil {
1280-
return database.WorkspaceBuild{}, err
1281-
}
1282-
// Authorized fetch
1283-
_, err = q.GetWorkspaceByID(ctx, build.WorkspaceID)
1284-
if err != nil {
1285-
return database.WorkspaceBuild{}, err
1286-
}
1287-
return build, nil
1257+
func (q *querier) GetWorkspaceBuildByJobID(ctx context.Context, jobID uuid.UUID) (database.WorkspaceBuildRBAC, error) {
1258+
return fetch(q.log, q.auth, q.db.GetWorkspaceBuildByJobID)(ctx, jobID)
12881259
}
12891260

1290-
func (q *querier) GetWorkspaceBuildByWorkspaceIDAndBuildNumber(ctx context.Context, arg database.GetWorkspaceBuildByWorkspaceIDAndBuildNumberParams) (database.WorkspaceBuild, error) {
1291-
if _, err := q.GetWorkspaceByID(ctx, arg.WorkspaceID); err != nil {
1292-
return database.WorkspaceBuild{}, err
1293-
}
1294-
return q.db.GetWorkspaceBuildByWorkspaceIDAndBuildNumber(ctx, arg)
1261+
func (q *querier) GetWorkspaceBuildByWorkspaceIDAndBuildNumber(ctx context.Context, arg database.GetWorkspaceBuildByWorkspaceIDAndBuildNumberParams) (database.WorkspaceBuildRBAC, error) {
1262+
return fetch(q.log, q.auth, q.db.GetWorkspaceBuildByWorkspaceIDAndBuildNumber)(ctx, arg)
12951263
}
12961264

12971265
func (q *querier) GetWorkspaceBuildParameters(ctx context.Context, workspaceBuildID uuid.UUID) ([]database.WorkspaceBuildParameter, error) {
@@ -1305,11 +1273,20 @@ func (q *querier) GetWorkspaceBuildParameters(ctx context.Context, workspaceBuil
13051273
return q.db.GetWorkspaceBuildParameters(ctx, workspaceBuildID)
13061274
}
13071275

1308-
func (q *querier) GetWorkspaceBuildsByWorkspaceID(ctx context.Context, arg database.GetWorkspaceBuildsByWorkspaceIDParams) ([]database.WorkspaceBuild, error) {
1309-
if _, err := q.GetWorkspaceByID(ctx, arg.WorkspaceID); err != nil {
1276+
func (q *querier) GetWorkspaceBuildsByWorkspaceID(ctx context.Context, arg database.GetWorkspaceBuildsByWorkspaceIDParams) ([]database.WorkspaceBuildRBAC, error) {
1277+
builds, err := q.db.GetWorkspaceBuildsByWorkspaceID(ctx, arg)
1278+
if err != nil {
1279+
return nil, err
1280+
}
1281+
if len(builds) == 0 {
1282+
return []database.WorkspaceBuildRBAC{}, nil
1283+
}
1284+
// All builds come from the same workspace, so we only need to check the first one.
1285+
err = q.authorizeContext(ctx, rbac.ActionRead, builds[0])
1286+
if err != nil {
13101287
return nil, err
13111288
}
1312-
return q.db.GetWorkspaceBuildsByWorkspaceID(ctx, arg)
1289+
return builds, nil
13131290
}
13141291

13151292
func (q *querier) GetWorkspaceByAgentID(ctx context.Context, agentID uuid.UUID) (database.Workspace, error) {
@@ -1369,11 +1346,7 @@ func (q *querier) GetWorkspaceResourcesByJobID(ctx context.Context, jobID uuid.U
13691346
if err != nil {
13701347
return nil, err
13711348
}
1372-
workspace, err := q.db.GetWorkspaceByID(ctx, build.WorkspaceID)
1373-
if err != nil {
1374-
return nil, err
1375-
}
1376-
obj = workspace
1349+
obj = build
13771350
default:
13781351
return nil, xerrors.Errorf("unknown job type: %s", job.Type)
13791352
}
@@ -1414,12 +1387,7 @@ func (q *querier) InsertWorkspaceBuildParameters(ctx context.Context, arg databa
14141387
return err
14151388
}
14161389

1417-
workspace, err := q.db.GetWorkspaceByID(ctx, build.WorkspaceID)
1418-
if err != nil {
1419-
return err
1420-
}
1421-
1422-
err = q.authorizeContext(ctx, rbac.ActionUpdate, workspace)
1390+
err = q.authorizeContext(ctx, rbac.ActionUpdate, build)
14231391
if err != nil {
14241392
return err
14251393
}
@@ -1483,11 +1451,7 @@ func (q *querier) UpdateWorkspaceBuildByID(ctx context.Context, arg database.Upd
14831451
return database.WorkspaceBuild{}, err
14841452
}
14851453

1486-
workspace, err := q.db.GetWorkspaceByID(ctx, build.WorkspaceID)
1487-
if err != nil {
1488-
return database.WorkspaceBuild{}, err
1489-
}
1490-
err = q.authorizeContext(ctx, rbac.ActionUpdate, workspace.RBACObject())
1454+
err = q.authorizeContext(ctx, rbac.ActionUpdate, build)
14911455
if err != nil {
14921456
return database.WorkspaceBuild{}, err
14931457
}

0 commit comments

Comments
 (0)