Skip to content

Commit

Permalink
Runtime: Return feature flags from the GetInstance RPC (#4766)
Browse files Browse the repository at this point in the history
* Track feature flags in the runtime's registry

* Self review
  • Loading branch information
begelundmuller authored Apr 30, 2024
1 parent 2b40143 commit d0cba69
Show file tree
Hide file tree
Showing 12 changed files with 1,147 additions and 1,068 deletions.
2,003 changes: 1,011 additions & 992 deletions proto/gen/rill/runtime/v1/api.pb.go

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions proto/gen/rill/runtime/v1/api.pb.validate.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions proto/gen/rill/runtime/v1/runtime.swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3774,6 +3774,10 @@ definitions:
type: object
additionalProperties:
type: string
featureFlags:
type: object
additionalProperties:
type: boolean
annotations:
type: object
additionalProperties:
Expand Down
1 change: 1 addition & 0 deletions proto/rill/runtime/v1/api.proto
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ message Instance {
repeated Connector project_connectors = 13;
map<string, string> variables = 7;
map<string, string> project_variables = 8;
map<string, bool> feature_flags = 22;
map<string, string> annotations = 14;
bool embed_catalog = 6;
bool watch_repo = 15;
Expand Down
9 changes: 7 additions & 2 deletions runtime/compilers/rillv1/parse_rillyaml.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ type RillYAML struct {
Connectors []*ConnectorDef
Variables []*VariableDef
Defaults map[ResourceKind]yaml.Node
Features []string
FeatureFlags map[string]bool
}

// ConnectorDef is a subtype of RillYAML, defining connectors required by the project
Expand Down Expand Up @@ -135,6 +135,11 @@ func (p *Parser) parseRillYAML(ctx context.Context, path string) error {
}
}

featureFlags := map[string]bool{}
for _, f := range tmp.Features {
featureFlags[f] = true
}

res := &RillYAML{
Title: tmp.Title,
Description: tmp.Description,
Expand All @@ -147,7 +152,7 @@ func (p *Parser) parseRillYAML(ctx context.Context, path string) error {
ResourceKindMetricsView: tmp.Dashboards,
ResourceKindMigration: tmp.Migrations,
},
Features: tmp.Features,
FeatureFlags: featureFlags,
}

for i, c := range tmp.Connectors {
Expand Down
2 changes: 2 additions & 0 deletions runtime/drivers/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ type Instance struct {
// ProjectVariables contains default variables from rill.yaml
// (NOTE: This can always be reproduced from rill.yaml, so it's really just a handy cache of the values.)
ProjectVariables map[string]string `db:"project_variables"`
// FeatureFlags contains feature flags configured in rill.yaml
FeatureFlags map[string]bool `db:"feature_flags"`
// Annotations to enrich activity events (like usage tracking)
Annotations map[string]string
// EmbedCatalog tells the runtime to store the instance's catalog in its OLAP store instead
Expand Down
1 change: 1 addition & 0 deletions runtime/drivers/sqlite/migrations/0021.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE instances ADD COLUMN feature_flags TEXT NOT NULL DEFAULT '{}';
55 changes: 38 additions & 17 deletions runtime/drivers/sqlite/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ func (c *connection) findInstances(_ context.Context, whereClause string, args .
project_connectors,
variables,
project_variables,
feature_flags,
annotations,
embed_catalog,
watch_repo
Expand All @@ -63,7 +64,7 @@ func (c *connection) findInstances(_ context.Context, whereClause string, args .
var res []*drivers.Instance
for rows.Next() {
// sqlite doesn't support maps need to read as bytes and convert to map
var variables, projectVariables, annotations, connectors, projectConnectors []byte
var variables, projectVariables, featureFlags, annotations, connectors, projectConnectors []byte
i := &drivers.Instance{}
err := rows.Scan(
&i.ID,
Expand All @@ -80,6 +81,7 @@ func (c *connection) findInstances(_ context.Context, whereClause string, args .
&projectConnectors,
&variables,
&projectVariables,
&featureFlags,
&annotations,
&i.EmbedCatalog,
&i.WatchRepo,
Expand All @@ -98,17 +100,22 @@ func (c *connection) findInstances(_ context.Context, whereClause string, args .
return nil, err
}

i.Variables, err = mapFromJSON(variables)
i.Variables, err = mapFromJSON[string](variables)
if err != nil {
return nil, err
}

i.ProjectVariables, err = mapFromJSON(projectVariables)
i.ProjectVariables, err = mapFromJSON[string](projectVariables)
if err != nil {
return nil, err
}

i.Annotations, err = mapFromJSON(annotations)
i.FeatureFlags, err = mapFromJSON[bool](featureFlags)
if err != nil {
return nil, err
}

i.Annotations, err = mapFromJSON[string](annotations)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -149,6 +156,11 @@ func (c *connection) CreateInstance(_ context.Context, inst *drivers.Instance) e
return err
}

featureFlags, err := mapToJSON(inst.FeatureFlags)
if err != nil {
return err
}

annotations, err := mapToJSON(inst.Annotations)
if err != nil {
return err
Expand All @@ -173,11 +185,12 @@ func (c *connection) CreateInstance(_ context.Context, inst *drivers.Instance) e
project_connectors,
variables,
project_variables,
feature_flags,
annotations,
embed_catalog,
watch_repo
)
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17)
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18)
`,
inst.ID,
inst.Environment,
Expand All @@ -193,6 +206,7 @@ func (c *connection) CreateInstance(_ context.Context, inst *drivers.Instance) e
projectConnectors,
variables,
projectVariables,
featureFlags,
annotations,
inst.EmbedCatalog,
inst.WatchRepo,
Expand All @@ -213,27 +227,32 @@ func (c *connection) EditInstance(_ context.Context, inst *drivers.Instance) err
ctx := context.Background()

// sqlite doesn't support maps need to convert to json and write as bytes array
variables, err := mapToJSON(inst.Variables)
connectors, err := json.Marshal(inst.Connectors)
if err != nil {
return err
}

projectVariables, err := mapToJSON(inst.ProjectVariables)
projectConnectors, err := json.Marshal(inst.ProjectConnectors)
if err != nil {
return err
}

annotations, err := mapToJSON(inst.Annotations)
variables, err := mapToJSON(inst.Variables)
if err != nil {
return err
}

connectors, err := json.Marshal(inst.Connectors)
projectVariables, err := mapToJSON(inst.ProjectVariables)
if err != nil {
return err
}

projectConnectors, err := json.Marshal(inst.ProjectConnectors)
featureFlags, err := mapToJSON(inst.FeatureFlags)
if err != nil {
return err
}

annotations, err := mapToJSON(inst.Annotations)
if err != nil {
return err
}
Expand All @@ -255,9 +274,10 @@ func (c *connection) EditInstance(_ context.Context, inst *drivers.Instance) err
project_connectors = $11,
variables = $12,
project_variables = $13,
annotations = $14,
embed_catalog = $15,
watch_repo = $16
feature_flags = $14,
annotations = $15,
embed_catalog = $16,
watch_repo = $17
WHERE id = $1
`,
inst.ID,
Expand All @@ -273,6 +293,7 @@ func (c *connection) EditInstance(_ context.Context, inst *drivers.Instance) err
projectConnectors,
variables,
projectVariables,
featureFlags,
annotations,
inst.EmbedCatalog,
inst.WatchRepo,
Expand All @@ -295,15 +316,15 @@ func (c *connection) DeleteInstance(_ context.Context, id string) error {
return err
}

func mapToJSON(data map[string]string) ([]byte, error) {
func mapToJSON[T any](data map[string]T) ([]byte, error) {
return json.Marshal(data)
}

func mapFromJSON(data []byte) (map[string]string, error) {
func mapFromJSON[T any](data []byte) (map[string]T, error) {
if len(data) == 0 {
return map[string]string{}, nil
return map[string]T{}, nil
}
var m map[string]string
var m map[string]T
err := json.Unmarshal(data, &m)
return m, err
}
Expand Down
2 changes: 2 additions & 0 deletions runtime/reconcilers/project_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,8 @@ func (r *ProjectParserReconciler) reconcileProjectConfig(ctx context.Context, pa
}
inst.ProjectVariables = vars

inst.FeatureFlags = parser.RillYAML.FeatureFlags

// TODO: Passing "false" guards against infinite cancellations and restarts of the controller,
// but it also ignores potential consistency issues where we update connector config without evicting cached connctions,
// or where we update variables and don't re-evaluate all resources.
Expand Down
67 changes: 40 additions & 27 deletions runtime/server/instances.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func (s *Server) ListInstances(ctx context.Context, req *runtimev1.ListInstances

pbs := make([]*runtimev1.Instance, len(instances))
for i, inst := range instances {
pbs[i] = instanceToPB(inst)
pbs[i] = instanceToPB(inst, true)
}

return &runtimev1.ListInstancesResponse{Instances: pbs}, nil
Expand All @@ -43,8 +43,14 @@ func (s *Server) GetInstance(ctx context.Context, req *runtimev1.GetInstanceRequ

s.addInstanceRequestAttributes(ctx, req.InstanceId)

if !auth.GetClaims(ctx).CanInstance(req.InstanceId, auth.ReadInstance) {
return nil, ErrForbidden
sensitiveAccess := auth.GetClaims(ctx).CanInstance(req.InstanceId, auth.ReadInstance)
if !sensitiveAccess {
// Regular project viewers can access non-sensitive instance information.
// NOTE: ReadObjects is not the right permission to use, but it's the closest permission that regular project viewers have.
// TODO: We should split ReadInstance into an admin-level and viewer-level permission instead.
if !auth.GetClaims(ctx).CanInstance(req.InstanceId, auth.ReadObjects) {
return nil, ErrForbidden
}
}

inst, err := s.runtime.Instance(ctx, req.InstanceId)
Expand All @@ -56,7 +62,7 @@ func (s *Server) GetInstance(ctx context.Context, req *runtimev1.GetInstanceRequ
}

return &runtimev1.GetInstanceResponse{
Instance: instanceToPB(inst),
Instance: instanceToPB(inst, sensitiveAccess),
}, nil
}

Expand Down Expand Up @@ -96,7 +102,7 @@ func (s *Server) CreateInstance(ctx context.Context, req *runtimev1.CreateInstan
}

return &runtimev1.CreateInstanceResponse{
Instance: instanceToPB(inst),
Instance: instanceToPB(inst, true),
}, nil
}

Expand Down Expand Up @@ -156,6 +162,7 @@ func (s *Server) EditInstance(ctx context.Context, req *runtimev1.EditInstanceRe
ProjectConnectors: oldInst.ProjectConnectors,
Variables: variables,
ProjectVariables: oldInst.ProjectVariables,
FeatureFlags: oldInst.FeatureFlags,
Annotations: annotations,
EmbedCatalog: valOrDefault(req.EmbedCatalog, oldInst.EmbedCatalog),
WatchRepo: valOrDefault(req.WatchRepo, oldInst.WatchRepo),
Expand All @@ -167,7 +174,7 @@ func (s *Server) EditInstance(ctx context.Context, req *runtimev1.EditInstanceRe
}

return &runtimev1.EditInstanceResponse{
Instance: instanceToPB(inst),
Instance: instanceToPB(inst, true),
}, nil
}

Expand Down Expand Up @@ -258,28 +265,34 @@ func (s *Server) WatchLogs(req *runtimev1.WatchLogsRequest, srv runtimev1.Runtim
}, lvl)
}

func instanceToPB(inst *drivers.Instance) *runtimev1.Instance {
olapConnector := inst.OLAPConnector
if inst.ProjectOLAPConnector != "" {
olapConnector = inst.ProjectOLAPConnector
}

return &runtimev1.Instance{
InstanceId: inst.ID,
OlapConnector: olapConnector,
RepoConnector: inst.RepoConnector,
AdminConnector: inst.AdminConnector,
AiConnector: inst.AIConnector,
CreatedOn: timestamppb.New(inst.CreatedOn),
UpdatedOn: timestamppb.New(inst.UpdatedOn),
Connectors: inst.Connectors,
ProjectConnectors: inst.ProjectConnectors,
Variables: inst.Variables,
ProjectVariables: inst.ProjectVariables,
Annotations: inst.Annotations,
EmbedCatalog: inst.EmbedCatalog,
WatchRepo: inst.WatchRepo,
func instanceToPB(inst *drivers.Instance, sensitive bool) *runtimev1.Instance {
pb := &runtimev1.Instance{
InstanceId: inst.ID,
CreatedOn: timestamppb.New(inst.CreatedOn),
UpdatedOn: timestamppb.New(inst.UpdatedOn),
FeatureFlags: inst.FeatureFlags,
}

if sensitive {
olapConnector := inst.OLAPConnector
if inst.ProjectOLAPConnector != "" {
olapConnector = inst.ProjectOLAPConnector
}

pb.OlapConnector = olapConnector
pb.RepoConnector = inst.RepoConnector
pb.AdminConnector = inst.AdminConnector
pb.AiConnector = inst.AIConnector
pb.Connectors = inst.Connectors
pb.ProjectConnectors = inst.ProjectConnectors
pb.Variables = inst.Variables
pb.ProjectVariables = inst.ProjectVariables
pb.Annotations = inst.Annotations
pb.EmbedCatalog = inst.EmbedCatalog
pb.WatchRepo = inst.WatchRepo
}

return pb
}

func valOrDefault[T any](ptr *T, def T) T {
Expand Down
6 changes: 6 additions & 0 deletions web-common/src/proto/gen/rill/runtime/v1/api_pb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,11 @@ export class Instance extends Message<Instance> {
*/
projectVariables: { [key: string]: string } = {};

/**
* @generated from field: map<string, bool> feature_flags = 22;
*/
featureFlags: { [key: string]: boolean } = {};

/**
* @generated from field: map<string, string> annotations = 14;
*/
Expand Down Expand Up @@ -293,6 +298,7 @@ export class Instance extends Message<Instance> {
{ no: 13, name: "project_connectors", kind: "message", T: Connector, repeated: true },
{ no: 7, name: "variables", kind: "map", K: 9 /* ScalarType.STRING */, V: {kind: "scalar", T: 9 /* ScalarType.STRING */} },
{ no: 8, name: "project_variables", kind: "map", K: 9 /* ScalarType.STRING */, V: {kind: "scalar", T: 9 /* ScalarType.STRING */} },
{ no: 22, name: "feature_flags", kind: "map", K: 9 /* ScalarType.STRING */, V: {kind: "scalar", T: 8 /* ScalarType.BOOL */} },
{ no: 14, name: "annotations", kind: "map", K: 9 /* ScalarType.STRING */, V: {kind: "scalar", T: 9 /* ScalarType.STRING */} },
{ no: 6, name: "embed_catalog", kind: "scalar", T: 8 /* ScalarType.BOOL */ },
{ no: 15, name: "watch_repo", kind: "scalar", T: 8 /* ScalarType.BOOL */ },
Expand Down
Loading

1 comment on commit d0cba69

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.