Skip to content

Commit

Permalink
fix: validate module schema on deployment (#747)
Browse files Browse the repository at this point in the history
The previous change resulted in relative refs within a module not being
written back to the schema before committing to the DB.
  • Loading branch information
alecthomas authored Jan 8, 2024
1 parent 4a1c55b commit f299d5f
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 7 deletions.
18 changes: 11 additions & 7 deletions backend/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,8 @@ func (s *Service) CreateDeployment(ctx context.Context, req *connect.Request[ftl
logger.Errorf(err, "Invalid module schema")
return nil, fmt.Errorf("%s: %w", "invalid module schema", err)
}
if err := s.validateWholeSchema(ctx, module); err != nil {
module, err = s.validateWholeSchema(ctx, module)
if err != nil {
logger.Errorf(err, "Invalid module schema")
return nil, fmt.Errorf("%s: %w", "invalid module schema", err)
}
Expand All @@ -710,16 +711,19 @@ func (s *Service) CreateDeployment(ctx context.Context, req *connect.Request[ftl
}

// Load schemas for existing modules, combine with our new one, and validate as a whole.
func (s *Service) validateWholeSchema(ctx context.Context, module *schema.Module) error {
func (s *Service) validateWholeSchema(ctx context.Context, module *schema.Module) (*schema.Module, error) {
existingModules, err := s.dal.GetActiveDeploymentSchemas(ctx)
if err != nil {
return fmt.Errorf("%s: %w", "could not get existing schemas", err)
return nil, fmt.Errorf("%s: %w", "could not get existing schemas", err)
}
schemaMap := ftlmaps.FromSlice(existingModules, func(el *schema.Module) (string, *schema.Module) { return el.Name, el })
schemaMap[module.Name] = module
fullSchema := &schema.Schema{Modules: maps.Values(schemaMap)}
_, err = schema.Validate(fullSchema)
return err
schema, err := schema.Validate(fullSchema)
if err != nil {
return nil, fmt.Errorf("%s: %w", "invalid schema", err)
}
return schema.Module(module.Name).MustGet(), nil
}

func (s *Service) getDeployment(ctx context.Context, name string) (*model.Deployment, error) {
Expand Down Expand Up @@ -1053,11 +1057,11 @@ func (s *Service) getActiveSchema(ctx context.Context) (*schema.Schema, error) {
if err != nil {
return nil, err
}
return &schema.Schema{
return schema.Validate(&schema.Schema{
Modules: slices.Map(deployments, func(d dal.Deployment) *schema.Module {
return d.Schema
}),
}, nil
})
}

// Copied from the Apache-licensed connect-go source.
Expand Down
11 changes: 11 additions & 0 deletions backend/schema/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/alecthomas/participle/v2"
"github.com/alecthomas/participle/v2/lexer"
"github.com/alecthomas/types"
"golang.org/x/exp/maps"
"google.golang.org/protobuf/proto"
)
Expand Down Expand Up @@ -312,6 +313,16 @@ type Schema struct {
Modules []*Module `parser:"@@*" protobuf:"2"`
}

// Module returns the named module if it exists.
func (s *Schema) Module(name string) types.Option[*Module] {
for _, module := range s.Modules {
if module.Name == name {
return types.Some(module)
}
}
return types.None[*Module]()
}

func (s *Schema) DataMap() map[DataRef]*Data {
dataTypes := map[DataRef]*Data{}
for _, module := range s.Modules {
Expand Down
2 changes: 2 additions & 0 deletions backend/schema/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,12 @@ func Validate(schema *Schema) (*Schema, error) {
dataRefs := []*DataRef{}
merr := []error{}
ingress := map[string]*Verb{}

// Inject builtins.
builtins := Builtins()
schema.Modules = slices.DeleteFunc(schema.Modules, func(m *Module) bool { return m.Name == builtins.Name })
schema.Modules = append([]*Module{builtins}, schema.Modules...)

// Map from reference to the module it is defined in.
localModule := map[*Ref]*Module{}

Expand Down

0 comments on commit f299d5f

Please sign in to comment.