fix: reduce the number of api calls from Workspace Create Modal (#9735)

(cherry picked from commit 20ed126)
determined-ai · Jul 29, 2024 · ed47fb0 · ed47fb0
1 parent e345871
commit ed47fb0
Show file tree

Hide file tree

Showing 18 changed files with 969 additions and 948 deletions.
diff --git a/harness/determined/common/api/bindings.py b/harness/determined/common/api/bindings.py
diff --git a/master/internal/api_master.go b/master/internal/api_master.go
@@ -370,16 +370,5 @@ func (a *apiServer) GetKubernetesResourceManagers(
 	ctx context.Context,
 	req *apiv1.GetKubernetesResourceManagersRequest,
 ) (*apiv1.GetKubernetesResourceManagersResponse, error) {
-	u, _, err := grpcutil.GetUser(ctx)
-	if err != nil {
-		return nil, err
-	}
-	permErr, err := cluster.AuthZProvider.Get().CanGetMasterConfig(ctx, u)
-	if err != nil {
-		return nil, err
-	} else if permErr != nil {
-		return nil, permErr
-	}
-
 	return &apiv1.GetKubernetesResourceManagersResponse{ResourceManagers: a.m.config.GetKubernetesClusterNames()}, nil
 }
diff --git a/master/internal/api_workspace.go b/master/internal/api_workspace.go
@@ -625,17 +625,12 @@ func (a *apiServer) DeleteWorkspaceNamespaceBindings(ctx context.Context,
 func (a *apiServer) deleteWorkspaceNamespaceBinding(ctx context.Context,
 	clusterNames []string, workspaceID int32, curUser *model.User,
 ) error {
-	w, err := a.GetWorkspaceByID(ctx, workspaceID, *curUser, true)
-	if err != nil {
-		return err
-	}
-
 	if err := workspace.AuthZProvider.Get().
-		CanSetWorkspaceNamespaceBindings(ctx, *curUser, w); err != nil {
+		CanSetWorkspaceNamespaceBindings(ctx, *curUser); err != nil {
 		return status.Error(codes.PermissionDenied, err.Error())
 	}
 	if len(clusterNames) > 0 {
-		err = db.Bun().RunInTx(ctx, nil, func(ctx context.Context, tx bun.Tx) error {
+		err := db.Bun().RunInTx(ctx, nil, func(ctx context.Context, tx bun.Tx) error {
 			deletedBindings, err := workspace.DeleteWorkspaceNamespaceBindings(ctx, int(workspaceID), clusterNames, &tx)
 			if err != nil {
 				return err
@@ -851,7 +846,7 @@ func (a *apiServer) setWorkspaceNamespaceBindings(ctx context.Context,
 	}()
 	wkspID := int(w.Id)
 	if err := workspace.AuthZProvider.Get().
-		CanSetWorkspaceNamespaceBindings(ctx, *curUser, w); err != nil {
+		CanSetWorkspaceNamespaceBindings(ctx, *curUser); err != nil {
 		return nil, status.Error(codes.PermissionDenied, err.Error())
 	}
 
@@ -1030,7 +1025,7 @@ func (a *apiServer) setResourceQuotas(ctx context.Context,
 ) (*apiv1.SetResourceQuotasResponse, error) {
 	license.RequireLicense("set resource quota")
 	if err := workspace.AuthZProvider.Get().
-		CanSetResourceQuotas(ctx, *curUser, w); err != nil {
+		CanSetResourceQuotas(ctx, *curUser); err != nil {
 		return nil, status.Error(codes.PermissionDenied, err.Error())
 	}
 

diff --git a/master/internal/workspace/authz_basic_impl.go b/master/internal/workspace/authz_basic_impl.go
@@ -160,7 +160,7 @@ func (a *WorkspaceAuthZBasic) CanCreateWorkspaceWithCheckpointStorageConfig(
 
 // CanSetWorkspaceNamespaceBindings retruns an error if the user is not a cluster admin.
 func (a *WorkspaceAuthZBasic) CanSetWorkspaceNamespaceBindings(
-	ctx context.Context, curUser model.User, workspace *workspacev1.Workspace,
+	ctx context.Context, curUser model.User,
 ) error {
 	if !curUser.Admin {
 		return fmt.Errorf("only admins may set workspace bindings")
@@ -170,7 +170,6 @@ func (a *WorkspaceAuthZBasic) CanSetWorkspaceNamespaceBindings(
 
 // CanSetResourceQuotas returns an error if the user is not a cluster admin.
 func (a *WorkspaceAuthZBasic) CanSetResourceQuotas(ctx context.Context, curUser model.User,
-	workspace *workspacev1.Workspace,
 ) error {
 	if !curUser.Admin {
 		return fmt.Errorf("only admins may set resource quotas")

diff --git a/master/internal/workspace/authz_iface.go b/master/internal/workspace/authz_iface.go
@@ -44,10 +44,8 @@ type WorkspaceAuthZ interface {
 	CanCreateWorkspace(ctx context.Context, curUser model.User) error
 	CanCreateWorkspaceWithAgentUserGroup(ctx context.Context, curUser model.User) error
 	CanCreateWorkspaceWithCheckpointStorageConfig(ctx context.Context, curUser model.User) error
-	CanSetWorkspaceNamespaceBindings(ctx context.Context, curUser model.User,
-		workspace *workspacev1.Workspace) error
-	CanSetResourceQuotas(ctx context.Context, curUser model.User,
-		workspace *workspacev1.Workspace) error
+	CanSetWorkspaceNamespaceBindings(ctx context.Context, curUser model.User) error
+	CanSetResourceQuotas(ctx context.Context, curUser model.User) error
 
 	// PATCH /api/v1/workspaces/:workspace_id
 	CanSetWorkspacesName(

diff --git a/master/internal/workspace/authz_permissive.go b/master/internal/workspace/authz_permissive.go
@@ -157,18 +157,18 @@ func (p *WorkspaceAuthZPermissive) CanSetWorkspacesDefaultPools(
 
 // CanSetWorkspaceNamespaceBindings calls RBAC authz but enforces basic authz.
 func (p *WorkspaceAuthZPermissive) CanSetWorkspaceNamespaceBindings(
-	ctx context.Context, curUser model.User, workspace *workspacev1.Workspace,
+	ctx context.Context, curUser model.User,
 ) error {
-	_ = (&WorkspaceAuthZRBAC{}).CanSetWorkspaceNamespaceBindings(ctx, curUser, workspace)
-	return (&WorkspaceAuthZBasic{}).CanSetWorkspaceNamespaceBindings(ctx, curUser, workspace)
+	_ = (&WorkspaceAuthZRBAC{}).CanSetWorkspaceNamespaceBindings(ctx, curUser)
+	return (&WorkspaceAuthZBasic{}).CanSetWorkspaceNamespaceBindings(ctx, curUser)
 }
 
 // CanSetResourceQuotas calls RBAC authz but enforces basic authz.
 func (p *WorkspaceAuthZPermissive) CanSetResourceQuotas(
-	ctx context.Context, curUser model.User, workspace *workspacev1.Workspace,
+	ctx context.Context, curUser model.User,
 ) error {
-	_ = (&WorkspaceAuthZRBAC{}).CanSetResourceQuotas(ctx, curUser, workspace)
-	return (&WorkspaceAuthZBasic{}).CanSetResourceQuotas(ctx, curUser, workspace)
+	_ = (&WorkspaceAuthZRBAC{}).CanSetResourceQuotas(ctx, curUser)
+	return (&WorkspaceAuthZBasic{}).CanSetResourceQuotas(ctx, curUser)
 }
 
 func init() {

diff --git a/master/internal/workspace/authz_rbac.go b/master/internal/workspace/authz_rbac.go
@@ -369,7 +369,7 @@ func (r *WorkspaceAuthZRBAC) CanSetWorkspacesDefaultPools(
 
 // CanSetWorkspaceNamespaceBindings determines whether a user can set a workspace namespace bindng.
 func (r *WorkspaceAuthZRBAC) CanSetWorkspaceNamespaceBindings(ctx context.Context,
-	curUser model.User, workspace *workspacev1.Workspace,
+	curUser model.User,
 ) (err error) {
 	fields := audit.ExtractLogFields(ctx)
 	addInfoWithoutWorkspace(curUser, fields,
@@ -378,13 +378,13 @@ func (r *WorkspaceAuthZRBAC) CanSetWorkspaceNamespaceBindings(ctx context.Contex
 		audit.LogFromErr(fields, err)
 	}()
 
-	return db.DoesPermissionMatch(ctx, curUser.ID, &workspace.Id,
+	return db.DoesPermissionMatch(ctx, curUser.ID, nil,
 		rbacv1.PermissionType_PERMISSION_TYPE_SET_WORKSPACE_NAMESPACE_BINDINGS)
 }
 
 // CanSetResourceQuotas determines whether a user can set a resource quota on a workspace.
 func (r *WorkspaceAuthZRBAC) CanSetResourceQuotas(ctx context.Context,
-	curUser model.User, workspace *workspacev1.Workspace,
+	curUser model.User,
 ) (err error) {
 	fields := audit.ExtractLogFields(ctx)
 	addInfoWithoutWorkspace(curUser, fields,
@@ -393,7 +393,7 @@ func (r *WorkspaceAuthZRBAC) CanSetResourceQuotas(ctx context.Context,
 		audit.LogFromErr(fields, err)
 	}()
 
-	return db.DoesPermissionMatch(ctx, curUser.ID, &workspace.Id,
+	return db.DoesPermissionMatch(ctx, curUser.ID, nil,
 		rbacv1.PermissionType_PERMISSION_TYPE_SET_RESOURCE_QUOTAS)
 }