diff --git a/pkg/registry/apis/iam/resourcepermission/sql.go b/pkg/registry/apis/iam/resourcepermission/sql.go index e66542f2e76..5c5f7abe511 100644 --- a/pkg/registry/apis/iam/resourcepermission/sql.go +++ b/pkg/registry/apis/iam/resourcepermission/sql.go @@ -8,6 +8,8 @@ import ( "strings" "time" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "github.com/grafana/authlib/types" "github.com/grafana/grafana/apps/iam/pkg/apis/iam/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/iam/common" @@ -121,7 +123,6 @@ func (s *ResourcePermSqlBackend) latestUpdate(ctx context.Context, dbHelper *leg return maxUpdated.UnixMilli() } -// Get // getRbacAssignmentsWithTx queries resource permissions based on the provided ListResourcePermissionsQuery and groups them by resource (e.g. {folder.grafana.app, folders, fold1}) func (s *ResourcePermSqlBackend) getRbacAssignmentsWithTx(ctx context.Context, sql *legacysql.LegacyDatabaseHelper, tx *session.SessionTx, query *ListResourcePermissionsQuery) ([]rbacAssignment, error) { rawQuery, args, err := buildListResourcePermissionsQueryFromTemplate(sql, query) @@ -159,7 +160,7 @@ func (s *ResourcePermSqlBackend) getRbacAssignmentsWithTx(ctx context.Context, s func (s *ResourcePermSqlBackend) getResourcePermission(ctx context.Context, sql *legacysql.LegacyDatabaseHelper, tx *session.SessionTx, ns types.NamespaceInfo, name string) (*v0alpha1.ResourcePermission, error) { mapper, grn, err := s.splitResourceName(name) if err != nil { - return nil, err + return nil, apierrors.NewInternalError(err) } resourceQuery := &ListResourcePermissionsQuery{ @@ -170,19 +171,19 @@ func (s *ResourcePermSqlBackend) getResourcePermission(ctx context.Context, sql assignments, err := s.getRbacAssignmentsWithTx(ctx, sql, tx, resourceQuery) if err != nil { - return nil, err + return nil, apierrors.NewInternalError(err) } if len(assignments) == 0 { - return nil, fmt.Errorf("resource permission %q: %w", resourceQuery.Scopes, errNotFound) + return nil, apierrors.NewNotFound(v0alpha1.ResourcePermissionInfo.GroupResource(), name) } resourcePermission, err := s.toV0ResourcePermissions(assignments, ns.Value) if err != nil { - return nil, err + return nil, apierrors.NewInternalError(err) } if resourcePermission == nil { - return nil, fmt.Errorf("resource permission %q: %w", resourceQuery.Scopes, errNotFound) + return nil, apierrors.NewNotFound(v0alpha1.ResourcePermissionInfo.GroupResource(), name) } return &resourcePermission[0], nil @@ -402,8 +403,6 @@ func (s *ResourcePermSqlBackend) createResourcePermission( return timeNow().UnixMilli(), nil } -// Update - func (s *ResourcePermSqlBackend) updateResourcePermission(ctx context.Context, dbHelper *legacysql.LegacyDatabaseHelper, ns types.NamespaceInfo, mapper Mapper, grn *groupResourceName, v0ResourcePerm *v0alpha1.ResourcePermission) (int64, error) { if err := validateCreateAndUpdateInput(v0ResourcePerm, grn); err != nil { return 0, err @@ -412,8 +411,8 @@ func (s *ResourcePermSqlBackend) updateResourcePermission(ctx context.Context, d err := dbHelper.DB.GetSqlxSession().WithTransaction(ctx, func(tx *session.SessionTx) error { currentPerms, err := s.getResourcePermission(ctx, dbHelper, tx, ns, grn.string()) if err != nil { - if errors.Is(err, errNotFound) { - return fmt.Errorf("resource permissions not found: %w", errNotFound) + if apierrors.IsNotFound(err) { + return apierrors.NewNotFound(v0alpha1.ResourcePermissionInfo.GroupResource(), grn.string()) } s.logger.Error("could not get resource permissions", "orgID", ns.OrgID, "scope", grn.Name, "error", err.Error()) return fmt.Errorf("could not get the existing resource permissions for resource %s", grn.Name) @@ -514,8 +513,6 @@ func validateCreateAndUpdateInput(v0ResourcePerm *v0alpha1.ResourcePermission, g return nil } -// Delete - // deleteResourcePermission deletes resource permissions for a single ResourcePermission resource referenced by its name in the format -- (e.g. dashboard.grafana.app-dashboards-ad5rwqs) func (s *ResourcePermSqlBackend) deleteResourcePermission(ctx context.Context, sql *legacysql.LegacyDatabaseHelper, ns types.NamespaceInfo, name string) error { mapper, grn, err := s.splitResourceName(name) @@ -534,7 +531,6 @@ func (s *ResourcePermSqlBackend) deleteResourcePermission(ctx context.Context, s return err } - // run delete query _, err = sql.DB.GetSqlxSession().Exec(ctx, rawQuery, args...) if err != nil { s.logger.Error("could not delete resource permissions", "scope", scope, "orgID", ns.OrgID, err.Error()) diff --git a/pkg/registry/apis/iam/resourcepermission/sql_test.go b/pkg/registry/apis/iam/resourcepermission/sql_test.go index 7d6b492c590..0fd19133fa1 100644 --- a/pkg/registry/apis/iam/resourcepermission/sql_test.go +++ b/pkg/registry/apis/iam/resourcepermission/sql_test.go @@ -306,7 +306,7 @@ func TestIntegration_ResourcePermSqlBackend_getResourcePermission(t *testing.T) if tt.err != nil { require.Error(t, err) - require.ErrorIs(t, err, tt.err) + require.ErrorAs(t, err, &tt.err) return } require.NoError(t, err) @@ -533,7 +533,6 @@ func TestIntegration_ResourcePermSqlBackend_UpdateResourcePermission(t *testing. _, err = backend.updateResourcePermission(ctx, sql, types.NamespaceInfo{Value: "default", OrgID: 1}, mapper, grn, resourcePerm) require.Error(t, err) - require.ErrorIs(t, err, errNotFound) }) t.Run("should update resource permission", func(t *testing.T) { diff --git a/pkg/registry/apis/iam/resourcepermission/storage_backend.go b/pkg/registry/apis/iam/resourcepermission/storage_backend.go index 938745687e2..24f0f982823 100644 --- a/pkg/registry/apis/iam/resourcepermission/storage_backend.go +++ b/pkg/registry/apis/iam/resourcepermission/storage_backend.go @@ -104,7 +104,7 @@ func (s *ResourcePermSqlBackend) ListIterator(ctx context.Context, req *resource if err != nil { logger := s.logger.FromContext(ctx) logger.Error("Failed to get database helper", "error", err) - return 0, errDatabaseHelper + return 0, apierrors.NewInternalError(errDatabaseHelper) } iterator, err := s.newRoleIterator(ctx, dbHelper, ns, pagination) @@ -114,12 +114,12 @@ func (s *ResourcePermSqlBackend) ListIterator(ctx context.Context, req *resource }() } if err != nil { - return 0, err + return 0, apierrors.NewInternalError(err) } err = callback(iterator) if err != nil { - return 0, err + return 0, apierrors.NewInternalError(err) } return s.latestUpdate(ctx, dbHelper, ns), nil @@ -136,7 +136,7 @@ func (s *ResourcePermSqlBackend) ReadResource(ctx context.Context, req *resource ns, err := types.ParseNamespace(req.Key.Namespace) if err != nil { - rsp.Error = resource.AsErrorResult(err) + rsp.Error = resource.AsErrorResult(apierrors.NewBadRequest(err.Error())) return rsp } if ns.OrgID <= 0 { @@ -155,7 +155,6 @@ func (s *ResourcePermSqlBackend) ReadResource(ctx context.Context, req *resource // Hide the error from the user, but log it logger := s.logger.FromContext(ctx) logger.Error("Failed to get database helper", "error", err) - rsp.Error = resource.AsErrorResult(errDatabaseHelper) return rsp } @@ -166,15 +165,7 @@ func (s *ResourcePermSqlBackend) ReadResource(ctx context.Context, req *resource }) if err != nil { - if errors.Is(err, errNotFound) { - rsp.Error = resource.AsErrorResult( - apierrors.NewNotFound(v0alpha1.ResourcePermissionInfo.GroupResource(), req.Key.Name), - ) - } else if errors.Is(err, errUnknownGroupResource) || errors.Is(err, errInvalidName) { - rsp.Error = resource.AsErrorResult(apierrors.NewBadRequest(err.Error())) - } else { - rsp.Error = resource.AsErrorResult(err) - } + rsp.Error = resource.AsErrorResult(err) return rsp } @@ -260,7 +251,7 @@ func (s *ResourcePermSqlBackend) WriteEvent(ctx context.Context, event resource. var v0resourceperm *v0alpha1.ResourcePermission v0resourceperm, err = getResourcePermissionFromEvent(event) if err != nil { - return 0, err + return 0, apierrors.NewBadRequest(fmt.Sprintf("invalid resource permission in event: %v", err)) } if v0resourceperm.Name != event.Key.Name { @@ -294,7 +285,7 @@ func (s *ResourcePermSqlBackend) WriteEvent(ctx context.Context, event resource. } } default: - return 0, fmt.Errorf("unsupported event type: %v", event.Type) + return 0, apierrors.NewMethodNotSupported(v0alpha1.ResourcePermissionInfo.GroupResource(), event.Type.String()) } return rv, err diff --git a/pkg/registry/apis/iam/resourcepermission/storage_backend_test.go b/pkg/registry/apis/iam/resourcepermission/storage_backend_test.go index 0192251cf5c..e42dd8830e5 100644 --- a/pkg/registry/apis/iam/resourcepermission/storage_backend_test.go +++ b/pkg/registry/apis/iam/resourcepermission/storage_backend_test.go @@ -551,7 +551,7 @@ func TestIntegration_ResourcePermSqlBackend_ListIterator(t *testing.T) { }) require.Error(t, err) - require.Equal(t, expectedErr, err) + require.ErrorAs(t, err, &expectedErr) require.Zero(t, listRV) }) diff --git a/pkg/registry/apis/iam/resourcepermission/templates.go b/pkg/registry/apis/iam/resourcepermission/templates.go index 79a30fe70e8..039e653939c 100644 --- a/pkg/registry/apis/iam/resourcepermission/templates.go +++ b/pkg/registry/apis/iam/resourcepermission/templates.go @@ -35,8 +35,6 @@ func mustTemplate(filename string) *template.Template { panic(fmt.Sprintf("template file not found: %s", filename)) } -// List - type pageQueryTemplate struct { sqltemplate.SQLTemplate Query *PageQuery @@ -140,8 +138,6 @@ func buildListResourcePermissionsQueryFromTemplate(dbHelper *legacysql.LegacyDat return rawQuery, req.GetArgs(), nil } -// Create - type insertRoleTemplate struct { sqltemplate.SQLTemplate RoleTable string @@ -235,8 +231,6 @@ func buildInsertPermissionQuery(dbHelper *legacysql.LegacyDatabaseHelper, roleID return rawQuery, req.GetArgs(), nil } -// Update - type removePermissionTemplate struct { sqltemplate.SQLTemplate PermissionTable string @@ -268,8 +262,6 @@ func buildRemovePermissionQuery(dbHelper *legacysql.LegacyDatabaseHelper, scope, return rawQuery, req.GetArgs(), nil } -// Delete - type deleteResourcePermissionsQueryTemplate struct { sqltemplate.SQLTemplate Query *DeleteResourcePermissionsQuery