Skip to content

Commit

Permalink
fix: prevent duplicate configs, secrets, databases within module (#1336)
Browse files Browse the repository at this point in the history 
Fixes #1121

This adds backend schema validation preventing duplicate configs,
secrets, and databases within a module. This also adds go-runtime
validation preventing duplicate databases. Go-runtime validation for
duplicate configs/secrets was added in PR
#1328
  • Loading branch information
@deniseli
deniseli authored Apr 26, 2024
1 parent 866e079 commit 64b2f21
Show file tree
Hide file tree
Showing 5 changed files with 188 additions and 32 deletions.
64 changes: 64 additions & 0 deletions backend/schema/validate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/TBD54566975/ftl/internal/cron"
"github.com/TBD54566975/ftl/internal/errors"
dc "github.com/TBD54566975/ftl/internal/reflect"
ftlslices "github.com/TBD54566975/ftl/internal/slices"
)

var (
Expand Down Expand Up @@ -312,6 +313,10 @@ func ValidateModule(module *Module) error {
}
return next()
})

// Scan declarations for duplicates
merr = append(merr, findDuplicateDecls(module.Decls)...)

merr = cleanErrors(merr)
sort.SliceStable(module.Decls, func(i, j int) bool {
iDecl := module.Decls[i]
Expand All @@ -326,6 +331,65 @@ func ValidateModule(module *Module) error {
return errors.Join(merr...)
}

func findDuplicateDecls(decls []Decl) []error {
merr := []error{}

configs := make(map[string][]int)
secrets := make(map[string][]int)
dbs := make(map[string]int)

for i, d := range decls {
c, ok := d.(*Config)
if ok { // found a config
if _, ok := configs[c.Name]; !ok { // no matches possible
configs[c.Name] = []int{i}
continue
}
matchIdx, foundMatch := ftlslices.Find(configs[c.Name], func(j int) bool {
maybeMatch, _ := decls[j].(*Config)
return maybeMatch.Type.String() == c.Type.String()
})
if foundMatch {
match, _ := decls[matchIdx].(*Config)
merr = append(merr, errorf(d, "duplicate config declaration at %d:%d", match.Pos.Line, match.Pos.Column))
} else {
configs[c.Name] = append(configs[c.Name], i)
}
continue
}

s, ok := d.(*Secret)
if ok { // found a secret
if _, ok := secrets[s.Name]; !ok { // no matches possible
secrets[s.Name] = []int{i}
continue
}
matchIdx, foundMatch := ftlslices.Find(secrets[s.Name], func(j int) bool {
maybeMatch, _ := decls[j].(*Secret)
return maybeMatch.Type.String() == s.Type.String()
})
if foundMatch {
match, _ := decls[matchIdx].(*Secret)
merr = append(merr, errorf(d, "duplicate secret declaration at %d:%d", match.Pos.Line, match.Pos.Column))
} else {
secrets[s.Name] = append(secrets[s.Name], i)
}
continue
}

db, ok := d.(*Database)
if ok { // found a database
if _, ok := dbs[db.Name]; !ok { // no matches
dbs[db.Name] = i
continue
}
match, _ := decls[dbs[db.Name]].(*Database)
merr = append(merr, errorf(d, "duplicate database declaration at %d:%d", match.Pos.Line, match.Pos.Column))
}
}
return merr
}

// getDeclSortingPriority (used for schema sorting) is pulled out into it's own switch so the Go sumtype check will fail
// if a new Decl is added but not explicitly prioritized
func getDeclSortingPriority(decl Decl) int {
Expand Down
80 changes: 79 additions & 1 deletion backend/schema/validate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,85 @@ func TestValidate(t *testing.T) {
verb a(HttpRequest<two.Data>) HttpResponse<two.Data, Empty>
+ingress http GET /a
}
`},
`,
},
{name: "DuplicateConfigsSimple",
schema: `
module one {
config FTL_ENDPOINT String
config FTL_ENDPOINT String
}
`,
errs: []string{
"4:41-41: duplicate config declaration at 3:41",
},
},
{name: "DuplicateConfigsDiffTypes",
schema: `
module one {
config FTL_ENDPOINT String
config FTL_ENDPOINT Any
}
`,
},
{name: "DuplicateConfigsMultiple",
schema: `
module one {
config FTL_ENDPOINT String
config FTL_ENDPOINT Any
config FTL_ENDPOINT String
config FTL_ENDPOINT String
}
`,
errs: []string{
"5:41-41: duplicate config declaration at 3:41",
"6:41-41: duplicate config declaration at 3:41",
},
},
{name: "DuplicateSecretsSimple",
schema: `
module one {
secret MY_SECRET String
secret MY_SECRET String
}
`,
errs: []string{
"4:41-41: duplicate secret declaration at 3:41",
},
},
{name: "DuplicateSecretsDiffTypes",
schema: `
module one {
secret MY_SECRET String
secret MY_SECRET Any
}
`,
},
{name: "DuplicateSecretsMultiple",
schema: `
module one {
secret MY_SECRET String
secret MY_SECRET Any
secret MY_SECRET String
secret MY_SECRET String
}
`,
errs: []string{
"5:41-41: duplicate secret declaration at 3:41",
"6:41-41: duplicate secret declaration at 3:41",
},
},
{name: "DuplicateDatabasesSimple",
schema: `
module one {
database MY_DB
database MY_DB
}
`,
errs: []string{
"4:41-41: duplicate database declaration at 3:41",
},
},
}

for _, test := range tests {
Expand Down
14 changes: 12 additions & 2 deletions go-runtime/compile/schema.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,16 +259,15 @@ func parseConfigDecl(pctx *parseContext, node *ast.CallExpr, fn *types.Func) {
}

// Check for duplicates
_, endCol := goNodePosToSchemaPos(node)
for _, d := range pctx.module.Decls {
c, ok := d.(*schema.Config)
if ok && c.Name == name && c.Type.String() == st.String() {
_, endCol := goNodePosToSchemaPos(node)
pctx.errors.add(errorf(node, "duplicate config declaration at %d:%d-%d", c.Pos.Line, c.Pos.Column, endCol))
return
}
s, ok := d.(*schema.Secret)
if ok && s.Name == name && s.Type.String() == st.String() {
_, endCol := goNodePosToSchemaPos(node)
pctx.errors.add(errorf(node, "duplicate secret declaration at %d:%d-%d", s.Pos.Line, s.Pos.Column, endCol))
return
}
Expand All @@ -293,6 +292,17 @@ func parseDatabaseDecl(pctx *parseContext, node *ast.CallExpr) {
pctx.errors.add(errorf(node, "config and secret declarations must have a single string literal argument"))
return
}

// Check for duplicates
_, endCol := goNodePosToSchemaPos(node)
for _, d := range pctx.module.Decls {
db, ok := d.(*schema.Database)
if ok && db.Name == name {
pctx.errors.add(errorf(node, "duplicate database declaration at %d:%d-%d", db.Pos.Line, db.Pos.Column, endCol))
return
}
}

decl := &schema.Database{
Pos: goPosToSchemaPos(node.Pos()),
Name: name,
Expand Down
59 changes: 30 additions & 29 deletions go-runtime/compile/schema_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,35 +249,36 @@ func TestErrorReporting(t *testing.T) {
filename+":10:13-35: config and secret declarations must have a single string literal argument\n"+
filename+":13:18-52: duplicate config declaration at 12:18-52\n"+
filename+":16:18-49: duplicate secret declaration at 15:18-49\n"+
filename+":19:2-10: unsupported type \"error\" for field \"BadParam\"\n"+
filename+":22:2-17: unsupported type \"uint64\" for field \"AnotherBadParam\"\n"+
filename+":25:3-3: unexpected token \"verb\" (expected Directive)\n"+
filename+":31:36-39: unsupported request type \"ftl/failing.Request\"\n"+
filename+":31:50-50: unsupported response type \"ftl/failing.Response\"\n"+
filename+":32:16-29: call first argument must be a function in an ftl module\n"+
filename+":33:2-46: call must have exactly three arguments\n"+
filename+":34:16-25: call first argument must be a function\n"+
filename+":39:1-2: must have at most two parameters (context.Context, struct)\n"+
filename+":39:69-69: unsupported response type \"ftl/failing.Response\"\n"+
filename+":44:22-27: first parameter must be of type context.Context but is ftl/failing.Request\n"+
filename+":44:37-43: second parameter must be a struct but is string\n"+
filename+":44:53-53: unsupported response type \"ftl/failing.Response\"\n"+
filename+":49:43-47: second parameter must not be ftl.Unit\n"+
filename+":49:59-59: unsupported response type \"ftl/failing.Response\"\n"+
filename+":54:1-2: first parameter must be context.Context\n"+
filename+":54:18-18: unsupported response type \"ftl/failing.Response\"\n"+
filename+":59:1-2: must have at most two results (struct, error)\n"+
filename+":59:41-44: unsupported request type \"ftl/failing.Request\"\n"+
filename+":64:1-2: must at least return an error\n"+
filename+":64:36-39: unsupported request type \"ftl/failing.Request\"\n"+
filename+":68:35-38: unsupported request type \"ftl/failing.Request\"\n"+
filename+":68:48-48: must return an error but is ftl/failing.Response\n"+
filename+":73:41-44: unsupported request type \"ftl/failing.Request\"\n"+
filename+":73:55-55: first result must be a struct but is string\n"+
filename+":73:63-63: must return an error but is string\n"+
filename+":73:63-63: second result must not be ftl.Unit\n"+
filename+":80:1-1: verb \"WrongResponse\" already exported\n"+
filename+":86:2-12: struct field unexported must be exported by starting with an uppercase letter",
filename+":19:14-44: duplicate database declaration at 18:14-44\n"+
filename+":22:2-10: unsupported type \"error\" for field \"BadParam\"\n"+
filename+":25:2-17: unsupported type \"uint64\" for field \"AnotherBadParam\"\n"+
filename+":28:3-3: unexpected token \"verb\" (expected Directive)\n"+
filename+":34:36-39: unsupported request type \"ftl/failing.Request\"\n"+
filename+":34:50-50: unsupported response type \"ftl/failing.Response\"\n"+
filename+":35:16-29: call first argument must be a function in an ftl module\n"+
filename+":36:2-46: call must have exactly three arguments\n"+
filename+":37:16-25: call first argument must be a function\n"+
filename+":42:1-2: must have at most two parameters (context.Context, struct)\n"+
filename+":42:69-69: unsupported response type \"ftl/failing.Response\"\n"+
filename+":47:22-27: first parameter must be of type context.Context but is ftl/failing.Request\n"+
filename+":47:37-43: second parameter must be a struct but is string\n"+
filename+":47:53-53: unsupported response type \"ftl/failing.Response\"\n"+
filename+":52:43-47: second parameter must not be ftl.Unit\n"+
filename+":52:59-59: unsupported response type \"ftl/failing.Response\"\n"+
filename+":57:1-2: first parameter must be context.Context\n"+
filename+":57:18-18: unsupported response type \"ftl/failing.Response\"\n"+
filename+":62:1-2: must have at most two results (struct, error)\n"+
filename+":62:41-44: unsupported request type \"ftl/failing.Request\"\n"+
filename+":67:1-2: must at least return an error\n"+
filename+":67:36-39: unsupported request type \"ftl/failing.Request\"\n"+
filename+":71:35-38: unsupported request type \"ftl/failing.Request\"\n"+
filename+":71:48-48: must return an error but is ftl/failing.Response\n"+
filename+":76:41-44: unsupported request type \"ftl/failing.Request\"\n"+
filename+":76:55-55: first result must be a struct but is string\n"+
filename+":76:63-63: must return an error but is string\n"+
filename+":76:63-63: second result must not be ftl.Unit\n"+
filename+":83:1-1: verb \"WrongResponse\" already exported\n"+
filename+":89:2-12: struct field unexported must be exported by starting with an uppercase letter",
)
}

Expand Down
3 changes: 3 additions & 0 deletions go-runtime/compile/testdata/failing/failing.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ var duplConfig = ftl.Config[string]("FTL_ENDPOINT")
var goodSecret = ftl.Secret[string]("MY_SECRET")
var duplSecret = ftl.Secret[string]("MY_SECRET")

var goodDB = ftl.PostgresDatabase("testDb")
var duplDB = ftl.PostgresDatabase("testDb")

type Request struct {
BadParam error
}
Expand Down

0 comments on commit 64b2f21

Please sign in to comment.