Skip to content

Commit

Permalink
QA on APIs
Browse files Browse the repository at this point in the history
  • Loading branch information
begelundmuller committed Oct 26, 2023
1 parent b2856e3 commit d7a99a5
Show file tree
Hide file tree
Showing 16 changed files with 374 additions and 577 deletions.
4 changes: 2 additions & 2 deletions admin/database/postgres/postgres.go
Original file line number Diff line number Diff line change
Expand Up @@ -1268,7 +1268,7 @@ func (c *connection) FindVirtualFiles(ctx context.Context, projectID, branch str
err := c.getDB(ctx).SelectContext(ctx, &res, `
SELECT path, data, deleted, updated_on
FROM virtual_files
WHERE project_id=$1 AND branch=$2 AND (updated_on>$3 OR updated_on=$3 AND afterPath>$4)
WHERE project_id=$1 AND branch=$2 AND (updated_on>$3 OR updated_on=$3 AND path>$4)
ORDER BY updated_on, path LIMIT $5
`, projectID, branch, afterUpdatedOn, afterPath, limit)
if err != nil {
Expand Down Expand Up @@ -1302,7 +1302,7 @@ func (c *connection) UpdateVirtualFileDeleted(ctx context.Context, projectID, br
data = ''::BYTEA,
deleted = TRUE,
updated_on = now()
WHERE project_id=$1, branch=$2, path=$3`, projectID, branch, path)
WHERE project_id=$1 AND branch=$2 AND path=$3`, projectID, branch, path)
return checkUpdateRow("virtual file", res, err)
}

Expand Down
2 changes: 1 addition & 1 deletion admin/deployments.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func (s *Service) createDeployment(ctx context.Context, opts *createDeploymentOp
Name: "admin",
Type: "admin",
Config: map[string]string{
"host": s.opts.ExternalURL,
"admin_url": s.opts.ExternalURL,
"access_token": adminAuthToken,
"project_id": opts.ProjectID,
"branch": opts.ProdBranch,
Expand Down
6 changes: 6 additions & 0 deletions admin/reports.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@ package admin

import (
"context"
"fmt"
"time"

"github.com/rilldata/rill/admin/database"
runtimev1 "github.com/rilldata/rill/proto/gen/rill/runtime/v1"
"github.com/rilldata/rill/runtime"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)

// TriggerReport triggers an ad-hoc run of a report
Expand Down Expand Up @@ -81,6 +84,9 @@ func (s *Service) TriggerReconcileAndAwaitReport(ctx context.Context, depl *data

r, err := rt.GetResource(ctx, reportReq)
if err != nil {
if s, ok := status.FromError(err); !ok || s.Code() != codes.NotFound {
return fmt.Errorf("failed to poll for report: %w", err)
}
if oldSpecVersion != nil {
// Success - previously the report was found, now we cannot find it anymore
return nil
Expand Down
88 changes: 69 additions & 19 deletions admin/server/reports.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@ import (
"context"
"errors"
"path"
"strconv"
"strings"
"time"

"github.com/google/uuid"
"github.com/rilldata/rill/admin/database"
"github.com/rilldata/rill/admin/server/auth"
adminv1 "github.com/rilldata/rill/proto/gen/rill/admin/v1"
runtimev1 "github.com/rilldata/rill/proto/gen/rill/runtime/v1"
"github.com/rilldata/rill/runtime/compilers/rillv1"
"github.com/rilldata/rill/runtime/pkg/observability"
"go.opentelemetry.io/otel/attribute"
"golang.org/x/exp/slices"
Expand Down Expand Up @@ -117,17 +118,18 @@ func (s *Server) EditReport(ctx context.Context, req *adminv1.EditReportRequest)
if err != nil {
return nil, status.Errorf(codes.InvalidArgument, "could not get report: %s", err.Error())
}
annotations := parseReportAnnotations(spec)

if spec.Security == nil || !spec.Security.ManagedByUi {
if !annotations.AdminManaged {
return nil, status.Error(codes.FailedPrecondition, "can't edit report because it was not created from the UI")
}

isOwner := claims.OwnerType() == auth.OwnerTypeUser && spec.Security.OwnerUserId == claims.OwnerID()
isOwner := claims.OwnerType() == auth.OwnerTypeUser && annotations.AdminOwnerUserID == claims.OwnerID()
if !permissions.ManageReports && !isOwner {
return nil, status.Error(codes.PermissionDenied, "does not have permission to edit report")
}

data, err := yamlForManagedReport(req.Options, spec.Security.OwnerUserId)
data, err := yamlForManagedReport(req.Options, annotations.AdminOwnerUserID)
if err != nil {
return nil, status.Errorf(codes.InvalidArgument, "failed to generate report YAML: %s", err.Error())
}
Expand Down Expand Up @@ -187,8 +189,9 @@ func (s *Server) UnsubscribeReport(ctx context.Context, req *adminv1.Unsubscribe
if err != nil {
return nil, status.Errorf(codes.InvalidArgument, "could not get report: %s", err.Error())
}
annotations := parseReportAnnotations(spec)

if spec.Security == nil || !spec.Security.ManagedByUi {
if !annotations.AdminManaged {
return nil, status.Error(codes.FailedPrecondition, "can't edit report because it was not created from the UI")
}

Expand Down Expand Up @@ -224,7 +227,7 @@ func (s *Server) UnsubscribeReport(ctx context.Context, req *adminv1.Unsubscribe
return nil, status.Errorf(codes.Internal, "failed to update virtual file: %s", err.Error())
}
} else {
data, err := yamlForManagedReport(opts, spec.Security.OwnerUserId)
data, err := yamlForManagedReport(opts, annotations.AdminOwnerUserID)
if err != nil {
return nil, status.Errorf(codes.InvalidArgument, "failed to generate report YAML: %s", err.Error())
}
Expand Down Expand Up @@ -285,12 +288,13 @@ func (s *Server) DeleteReport(ctx context.Context, req *adminv1.DeleteReportRequ
if err != nil {
return nil, status.Errorf(codes.InvalidArgument, "could not get report: %s", err.Error())
}
annotations := parseReportAnnotations(spec)

if spec.Security == nil || !spec.Security.ManagedByUi {
if !annotations.AdminManaged {
return nil, status.Error(codes.FailedPrecondition, "can't edit report because it was not created from the UI")
}

isOwner := claims.OwnerType() == auth.OwnerTypeUser && spec.Security.OwnerUserId == claims.OwnerID()
isOwner := claims.OwnerType() == auth.OwnerTypeUser && annotations.AdminOwnerUserID == claims.OwnerID()
if !permissions.ManageReports && !isOwner {
return nil, status.Error(codes.PermissionDenied, "does not have permission to edit report")
}
Expand Down Expand Up @@ -345,8 +349,9 @@ func (s *Server) TriggerReport(ctx context.Context, req *adminv1.TriggerReportRe
if err != nil {
return nil, status.Errorf(codes.InvalidArgument, "could not get report: %s", err.Error())
}
annotations := parseReportAnnotations(spec)

isOwner := spec.Security != nil && claims.OwnerType() == auth.OwnerTypeUser && spec.Security.OwnerUserId == claims.OwnerID()
isOwner := claims.OwnerType() == auth.OwnerTypeUser && annotations.AdminOwnerUserID == claims.OwnerID()
if !permissions.ManageReports && !isOwner {
return nil, status.Error(codes.PermissionDenied, "does not have permission to edit report")
}
Expand Down Expand Up @@ -380,11 +385,10 @@ func virtualFilePathForManagedReport(name string) string {
}

func yamlForManagedReport(opts *adminv1.ReportOptions, ownerUserID string) ([]byte, error) {
res := rillv1.ReportYAML{}
res := reportYAML{}
res.Kind = "report"
res.Title = opts.Title
res.Refresh = &rillv1.ScheduleYAML{
Cron: opts.RefreshCron,
}
res.Refresh.Cron = opts.RefreshCron
res.Query.Name = opts.QueryName
res.Query.ArgsJSON = opts.QueryArgsJson
res.Export.Format = opts.ExportFormat.String()
Expand All @@ -393,17 +397,17 @@ func yamlForManagedReport(opts *adminv1.ReportOptions, ownerUserID string) ([]by
res.Email.Template.EditURL = "" // TODO: Add
res.Email.Template.ExportURL = "" // TODO: Add
res.Email.Recipients = opts.Recipients
res.Security.ManagedByUI = true
res.Security.OwnerUserID = ownerUserID
res.Annotations.AdminOwnerUserID = ownerUserID
res.Annotations.AdminManaged = true
res.Annotations.AdminNonce = time.Now().Format(time.RFC3339Nano)
return yaml.Marshal(res)
}

func yamlForCommittedReport(opts *adminv1.ReportOptions) ([]byte, error) {
res := rillv1.ReportYAML{}
res := reportYAML{}
res.Kind = "report"
res.Title = opts.Title
res.Refresh = &rillv1.ScheduleYAML{
Cron: opts.RefreshCron,
}
res.Refresh.Cron = opts.RefreshCron
res.Query.Name = opts.QueryName
res.Query.ArgsJSON = opts.QueryArgsJson // TODO: Format as YAML
res.Export.Format = opts.ExportFormat.String() // TODO: Format as pretty string
Expand All @@ -429,3 +433,49 @@ func recreateReportOptionsFromSpec(spec *runtimev1.ReportSpec) (*adminv1.ReportO
opts.Recipients = spec.EmailRecipients
return opts, nil
}

// reportYAML is derived from rillv1.ReportYAML, but adapted for generating (as opposed to parsing) the report YAML.
type reportYAML struct {
Kind string `yaml:"kind"`
Title string `yaml:"title"`
Refresh struct {
Cron string `yaml:"cron"`
} `yaml:"refresh"`
Query struct {
Name string `yaml:"name"`
Args map[string]any `yaml:"args,omitempty"`
ArgsJSON string `yaml:"args_json,omitempty"`
} `yaml:"query"`
Export struct {
Format string `yaml:"format"`
Limit uint `yaml:"limit"`
} `yaml:"export"`
Email struct {
Recipients []string `yaml:"recipients"`
Template struct {
OpenURL string `yaml:"open_url"`
EditURL string `yaml:"edit_url"`
ExportURL string `yaml:"export_url"`
} `yaml:"template"`
} `yaml:"email"`
Annotations reportAnnotations `yaml:"annotations,omitempty"`
}

type reportAnnotations struct {
AdminOwnerUserID string `yaml:"admin_owner_user_id"`
AdminManaged bool `yaml:"admin_managed"`
AdminNonce string `yaml:"admin_nonce"` // To ensure spec version gets updated on writes, to enable polling in TriggerReconcileAndAwaitReport
}

func parseReportAnnotations(r *runtimev1.ReportSpec) reportAnnotations {
if r.Annotations == nil {
return reportAnnotations{}
}

res := reportAnnotations{}
res.AdminOwnerUserID = r.Annotations["admin_owner_user_id"]
res.AdminManaged, _ = strconv.ParseBool(r.Annotations["admin_managed"])
res.AdminNonce = r.Annotations["admin_nonce"]

return res
}
1 change: 1 addition & 0 deletions cli/cmd/runtime/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"golang.org/x/sync/errgroup"

// Load connectors and reconcilers for runtime
_ "github.com/rilldata/rill/runtime/drivers/admin"
_ "github.com/rilldata/rill/runtime/drivers/athena"
_ "github.com/rilldata/rill/runtime/drivers/azure"
_ "github.com/rilldata/rill/runtime/drivers/bigquery"
Expand Down
Loading

0 comments on commit d7a99a5

Please sign in to comment.