8000 chore: remove parallel queries in the same transaction · coder/coder@1e1cb20 · GitHub
[go: up one dir, main page]
Skip to content

Commit 1e1cb20

Browse files
EmyrkEmyrk
committed
chore: remove parallel queries in the same transaction
Parallel concurrent queries cannot be run in the same tx
1 parent c1b35bf commit 1e1cb20

File tree

1 file changed

+43
-64
lines changed

1 file changed

+43
-64
lines changed

‎coderd/dynamicparameters/render.go

Lines changed: 43 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"time"
1010

1111
"github.com/google/uuid"
12-
"golang.org/x/sync/errgroup"
1312
"golang.org/x/xerrors"
1413

1514
"github.com/coder/coder/v2/apiversion"
@@ -244,8 +243,6 @@ func (r *dynamicRenderer) getWorkspaceOwnerData(ctx context.Context, ownerID uui
244243
return nil // already fetched
245244
}
246245

247-
var g errgroup.Group
248-
249246
// You only need to be able to read the organization member to get the owner
250247
// data. Only the terraform files can therefore leak more information than the
251248
// caller should have access to. All this info should be public assuming you can
@@ -266,72 +263,54 @@ func (r *dynamicRenderer) getWorkspaceOwnerData(ctx context.Context, ownerID uui
266263
return xerrors.Errorf("fetch user: %w", err)
267264
}
268265

269-
var ownerRoles []previewtypes.WorkspaceOwnerRBACRole
270-
g.Go(func() error {
271-
// nolint:gocritic // This is kind of the wrong query to use here, but it
272-
// matches how the provisioner currently works. We should figure out
273-
// something that needs less escalation but has the correct behavior.
274-
row, err := r.db.GetAuthorizationUserRoles(dbauthz.AsProvisionerd(ctx), ownerID)
275-
if err != nil {
276-
return err
277-
}
278-
roles, err := row.RoleNames()
279-
if err != nil {
280-
return err
266+
// nolint:gocritic // This is kind of the wrong query to use here, but it
267+
// matches how the provisioner currently works. We should figure out
268+
// something that needs less escalation but has the correct behavior.
269+
row, err := r.db.GetAuthorizationUserRoles(dbauthz.AsProvisionerd(ctx), ownerID)
270+
if err != nil {
271+
return xerrors.Errorf("user roles: %w", err)
272+
}
273+
roles, err := row.RoleNames()
274+
if err != nil {
275+
return xerrors.Errorf("expand roles: %w", err)
276+
}
277+
ownerRoles := make([]previewtypes.WorkspaceOwnerRBACRole, 0, len(roles))
278+
for _, it := range roles {
279+
if it.OrganizationID != uuid.Nil && it.OrganizationID != r.data.templateVersion.OrganizationID {
280+
continue
281281
}
282-
ownerRoles = make([]previewtypes.WorkspaceOwnerRBACRole, 0, len(roles))
283-
for _, it := range roles {
284-
if it.OrganizationID != uuid.Nil && it.OrganizationID != r.data.templateVersion.OrganizationID {
285-
continue
286-
}
287-
var orgID string
288-
if it.OrganizationID != uuid.Nil {
289-
orgID = it.OrganizationID.String()
290-
}
291-
ownerRoles = append(ownerRoles, previewtypes.WorkspaceOwnerRBACRole{
292-
Name: it.Name,
293-
OrgID: orgID,
294-
})
282+
var orgID string
283+
if it.OrganizationID != uuid.Nil {
284+
orgID = it.OrganizationID.String()
295285
}
296-
return nil
297-
})
286+
ownerRoles = append(ownerRoles, previewtypes.WorkspaceOwnerRBACRole{
287+
Name: it.Name,
288+
OrgID: orgID,
289+
})
290+
}
298291

299-
var publicKey string
300-
g.Go(func() error {
301-
// The correct public key has to be sent. This will not be leaked
302-
// unless the template leaks it.
303-
// nolint:gocritic
304-
key, err := r.db.GetGitSSHKey(dbauthz.AsProvisionerd(ctx), ownerID)
305-
if err != nil {
306-
return err
307-
}
308-
publicKey = key.PublicKey
309-
return nil
310-
})
292+
// The correct public key has to be sent. This will not be leaked
293+
// unless the template leaks it.
294+
// nolint:gocritic
295+
key, err := r.db.GetGitSSHKey(dbauthz.AsProvisionerd(ctx), ownerID)
296+
if err != nil {
297+
return xerrors.Errorf("ssh key: %w", err)
298+
}
311299

312-
var groupNames []string
313-
g.Go(func() error {
314-
// The groups need to be sent to preview. These groups are not exposed to the
315-
// user, unless the template does it through the parameters. Regardless, we need
316-
// the correct groups, and a user might not have read access.
317-
// nolint:gocritic
318-
groups, err := r.db.GetGroups(dbauthz.AsProvisionerd(ctx), database.GetGroupsParams{
319-
OrganizationID: r.data.templateVersion.OrganizationID,
320-
HasMemberID: ownerID,
321-
})
322-
if err != nil {
323-
return err
324-
}
325-
groupNames = make([]string, 0, len(groups))
326-
for _, it := range groups {
327-
groupNames = append(groupNames, it.Group.Name)
328-
}
329-
return nil
300+
// The groups need to be sent to preview. These groups are not exposed to the
301+
// user, unless the template does it through the parameters. Regardless, we need
302+
// the correct groups, and a user might not have read access.
303+
// nolint:gocritic
304+
groups, err := r.db.GetGroups(dbauthz.AsProvisionerd(ctx), database.GetGroupsParams{
305+
OrganizationID: r.data.templateVersion.OrganizationID,
306+
HasMemberID: ownerID,
330307
})
331-
332-
err = g.Wait()
333308
if err != nil {
334-
return err
309+
return xerrors.Errorf("groups: %w", err)
310+
}
311+
groupNames := make([]string, 0, len(groups))
312+
for _, it := range groups {
313+
groupNames = append(groupNames, it.Group.Name)
335314
}
336315

337316
r.currentOwner = &previewtypes.WorkspaceOwner{
@@ -341,7 +320,7 @@ func (r *dynamicRenderer) getWorkspaceOwnerData(ctx context.Context, ownerID uui
341320
Email: mem.Email,
342321
LoginType: string(user.LoginType),
343322
RBACRoles: ownerRoles,
344-
SSHPublicKey: publicKey,
323+
SSHPublicKey: key.PublicKey,
345324
Groups: groupNames,
346325
}
347326
return nil

0 commit comments

Comments
 (0)
0