module context proto doesn’t need to care if global or not

backend/controller/modulecontext.go → backend/controller/module_context.go

@@ -9,7 +9,6 @@ import (
 	"connectrpc.com/connect"
 	ftlv1 "github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1"
 	"github.com/TBD54566975/ftl/backend/schema"
-	"github.com/TBD54566975/ftl/common/configuration"
 	cf "github.com/TBD54566975/ftl/common/configuration"
 	"github.com/TBD54566975/ftl/internal/slices"
 )
@@ -25,40 +24,18 @@ func moduleContextToProto(ctx context.Context, name string, schemas []*schema.Mo
 	}
 
 	// configs
-	configManager := configuration.ConfigFromContext(ctx)
-	configList, err := configManager.List(ctx)
+	configManager := cf.ConfigFromContext(ctx)
+	configMap, err := bytesMapFromConfigManager(ctx, configManager, name)
 	if err != nil {
 		return nil, err
 	}
-	configProtos := []*ftlv1.ModuleContextResponse_Config{}
-	for _, entry := range configList {
-		data, err := configManager.GetData(ctx, entry.Ref)
-		if err != nil {
-			return nil, err
-		}
-		configProtos = append(configProtos, &ftlv1.ModuleContextResponse_Config{
-			Ref:  configRefToProto(entry.Ref),
-			Data: data,
-		})
-	}
 
 	// secrets
-	secretsManager := configuration.SecretsFromContext(ctx)
-	secretsList, err := secretsManager.List(ctx)
+	secretsManager := cf.SecretsFromContext(ctx)
+	secretsMap, err := bytesMapFromConfigManager(ctx, secretsManager, name)
 	if err != nil {
 		return nil, err
 	}
-	secretProtos := []*ftlv1.ModuleContextResponse_Secret{}
-	for _, entry := range secretsList {
-		data, err := secretsManager.GetData(ctx, entry.Ref)
-		if err != nil {
-			return nil, err
-		}
-		secretProtos = append(secretProtos, &ftlv1.ModuleContextResponse_Secret{
-			Ref:  configRefToProto(entry.Ref),
-			Data: data,
-		})
-	}
 
 	// DSNs
 	dsnProtos := []*ftlv1.ModuleContextResponse_DSN{}
@@ -80,18 +57,41 @@ func moduleContextToProto(ctx context.Context, name string, schemas []*schema.Mo
 	}
 
 	return connect.NewResponse(&ftlv1.ModuleContextResponse{
-		Configs:   configProtos,
-		Secrets:   secretProtos,
+		Configs:   configMap,
+		Secrets:   secretsMap,
 		Databases: dsnProtos,
 	}), nil
 }
 
-func configRefToProto(r cf.Ref) *ftlv1.ModuleContextResponse_Ref {
-	protoRef := &ftlv1.ModuleContextResponse_Ref{
-		Name: r.Name,
+func bytesMapFromConfigManager[R cf.Role](ctx context.Context, manager *cf.Manager[R], moduleName string) (map[string][]byte, error) {
+	configList, err := manager.List(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	// module specific values must override global values
+	// put module specific values into moduleConfigMap, then merge with configMap
+	configMap := map[string][]byte{}
+	moduleConfigMap := map[string][]byte{}
+
+	for _, entry := range configList {
+		refModule, isModuleSpecific := entry.Module.Get()
+		if isModuleSpecific && refModule != moduleName {
+			continue
+		}
+		data, err := manager.GetData(ctx, entry.Ref)
+		if err != nil {
+			return nil, err
+		}
+		if !isModuleSpecific {
+			configMap[entry.Ref.Name] = data
+		} else {
+			moduleConfigMap[entry.Ref.Name] = data
+		}
 	}
-	if module, ok := r.Module.Get(); ok {
-		protoRef.Module = &module
+
+	for name, data := range moduleConfigMap {
+		configMap[name] = data
 	}
-	return protoRef
+	return configMap, nil
 }

backend/controller/module_context_test.go

@@ -0,0 +1,55 @@
+package controller
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/TBD54566975/ftl/backend/schema"
+	cf "github.com/TBD54566975/ftl/common/configuration"
+	"github.com/TBD54566975/ftl/internal/log"
+	"github.com/alecthomas/assert/v2"
+	"github.com/alecthomas/types/optional"
+)
+
+func TestModuleContextProto(t *testing.T) {
+	ctx := log.ContextWithNewDefaultLogger(context.Background())
+
+	moduleName := "test"
+
+	cp := cf.NewInMemoryProvider[cf.Configuration]()
+	cr := cf.NewInMemoryResolver[cf.Configuration]()
+	cm, err := cf.New(ctx, cr, []cf.Provider[cf.Configuration]{cp})
+	assert.NoError(t, err)
+	ctx = cf.ContextWithConfig(ctx, cm)
+
+	sp := cf.NewInMemoryProvider[cf.Secrets]()
+	sr := cf.NewInMemoryResolver[cf.Secrets]()
+	sm, err := cf.New(ctx, sr, []cf.Provider[cf.Secrets]{sp})
+	assert.NoError(t, err)
+	ctx = cf.ContextWithSecrets(ctx, sm)
+
+	// Set 50 configs and 50 global configs
+	// It's hard to tell if module config beats global configs because we are dealing with unordered maps, or because the logic is correct
+	// Repeating it 50 times hopefully gives us a good chance of catching inconsistencies
+	for i := range 50 {
+		key := fmt.Sprintf("key%d", i)
+
+		strValue := "HelloWorld"
+		globalStrValue := "GlobalHelloWorld"
+		cm.Set(ctx, cf.Ref{Module: optional.Some(moduleName), Name: key}, strValue)
+		cm.Set(ctx, cf.Ref{Module: optional.None[string](), Name: key}, globalStrValue)
+	}
+
+	response, err := moduleContextToProto(ctx, moduleName, []*schema.Module{
+		{
+			Name: moduleName,
+		},
+	})
+	assert.NoError(t, err)
+
+	for i := range 50 {
+		key := fmt.Sprintf("key%d", i)
+		assert.Equal(t, "\"HelloWorld\"", string(response.Msg.Configs[key]), "module configs should beat global configs")
+	}
+}

common/modulecontext/builder.go

@@ -2,71 +2,50 @@ package modulecontext
 
 import (
 	"context"
-	"fmt"
 
 	ftlv1 "github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1"
 	cf "github.com/TBD54566975/ftl/common/configuration"
-	"github.com/TBD54566975/ftl/internal/slices"
-	"github.com/alecthomas/types/optional"
 )
 
-type ref struct {
-	Module optional.Option[string]
-	Name   string
-}
-
-type refValuePair struct {
-	ref      ref
-	resolver resolver
-}
-
 type dsnEntry struct {
-	name     string
-	dbType   DBType
-	resolver stringResolver
-}
-
-type resolver interface {
-	Resolve() (isData bool, value any, data []byte, err error)
-}
-
-var _ resolver = valueResolver{}
-var _ resolver = dataResolver{}
-
-type stringResolver interface {
-	ResolveString() (string, error)
+	dbType DBType
+	dsn    string
 }
 
-var _ stringResolver = valueResolver{}
-
 type Builder struct {
 	moduleName string
-	configs    []refValuePair
-	secrets    []refValuePair
-	dsns       []dsnEntry
+	configs    map[string][]byte
+	secrets    map[string][]byte
+	dsns       map[string]dsnEntry
 }
 
 func NewBuilder(moduleName string) *Builder {
 	return &Builder{
 		moduleName: moduleName,
-		configs:    []refValuePair{},
-		secrets:    []refValuePair{},
-		dsns:       []dsnEntry{},
+		configs:    map[string][]byte{},
+		secrets:    map[string][]byte{},
+		dsns:       map[string]dsnEntry{},
 	}
 }
 
 func NewBuilderFromProto(moduleName string, response *ftlv1.ModuleContextResponse) *Builder {
+	configs := map[string][]byte{}
+	for name, bytes := range response.Configs {
+		configs[name] = bytes
+	}
+	secrets := map[string][]byte{}
+	for name, bytes := range response.Secrets {
+		secrets[name] = bytes
+	}
+	dsns := map[string]dsnEntry{}
+	for _, d := range response.Databases {
+		dsns[d.Name] = dsnEntry{dbType: DBType(d.Type), dsn: d.Dsn}
+	}
 	return &Builder{
 		moduleName: moduleName,
-		configs: slices.Map(response.Configs, func(c *ftlv1.ModuleContextResponse_Config) refValuePair {
-			return refValuePair{ref: refFromProto(c.Ref), resolver: dataResolver{data: c.Data}}
-		}),
-		secrets: slices.Map(response.Secrets, func(s *ftlv1.ModuleContextResponse_Secret) refValuePair {
-			return refValuePair{ref: refFromProto(s.Ref), resolver: dataResolver{data: s.Data}}
-		}),
-		dsns: slices.Map(response.Databases, func(d *ftlv1.ModuleContextResponse_DSN) dsnEntry {
-			return dsnEntry{name: d.Name, dbType: DBType(d.Type), resolver: valueResolver{value: d.Dsn}}
-		}),
+		configs:    configs,
+		secrets:    secrets,
+		dsns:       dsns,
 	}
 }
 
@@ -85,23 +64,17 @@ func (b *Builder) Build(ctx context.Context) (*ModuleContext, error) {
 		dbProvider:     NewDBProvider(),
 	}
 
-	if err := buildConfigOrSecrets[cf.Configuration](ctx, *moduleCtx.configManager, b.configs); err != nil {
+	if err := buildConfigOrSecrets[cf.Configuration](ctx, *moduleCtx.configManager, b.configs, b.moduleName); err != nil {
 		return nil, err
 	}
-	if err := buildConfigOrSecrets[cf.Secrets](ctx, *moduleCtx.secretsManager, b.secrets); err != nil {
+	if err := buildConfigOrSecrets[cf.Secrets](ctx, *moduleCtx.secretsManager, b.secrets, b.moduleName); err != nil {
 		return nil, err
 	}
-
-	for _, entry := range b.dsns {
-		dsn, err := entry.resolver.ResolveString()
-		if err != nil {
-			return nil, err
-		}
-		if err = moduleCtx.dbProvider.AddDSN(entry.name, entry.dbType, dsn); err != nil {
+	for name, entry := range b.dsns {
+		if err = moduleCtx.dbProvider.AddDSN(name, entry.dbType, entry.dsn); err != nil {
 			return nil, err
 		}
 	}
-
 	return moduleCtx, nil
 }
 
@@ -115,52 +88,11 @@ func newInMemoryConfigManager[R cf.Role](ctx context.Context) (*cf.Manager[R], e
 	return manager, nil
 }
 
-func buildConfigOrSecrets[R cf.Role](ctx context.Context, manager cf.Manager[R], items []refValuePair) error {
-	for _, item := range items {
-		isData, value, data, err := item.resolver.Resolve()
-		if err != nil {
+func buildConfigOrSecrets[R cf.Role](ctx context.Context, manager cf.Manager[R], valueMap map[string][]byte, moduleName string) error {
+	for name, data := range valueMap {
+		if err := manager.SetData(ctx, cf.Ref(cf.Ref{Name: name}), data); err != nil {
 			return err
 		}
-		if isData {
-			if err := manager.SetData(ctx, cf.Ref(item.ref), data); err != nil {
-				return err
-			}
-		} else {
-			if err := manager.Set(ctx, cf.Ref(item.ref), value); err != nil {
-				return err
-			}
-		}
 	}
 	return nil
 }
-
-func refFromProto(r *ftlv1.ModuleContextResponse_Ref) ref {
-	return ref{
-		Module: optional.Ptr(r.Module),
-		Name:   r.Name,
-	}
-}
-
-type valueResolver struct {
-	value any
-}
-
-func (r valueResolver) Resolve() (isData bool, value any, data []byte, err error) {
-	return false, r.value, nil, nil
-}
-
-func (r valueResolver) ResolveString() (string, error) {
-	str, ok := r.value.(string)
-	if !ok {
-		return "", fmt.Errorf("value is not a string: %v", r.value)
-	}
-	return str, nil
-}
-
-type dataResolver struct {
-	data []byte
-}
-
-func (r dataResolver) Resolve() (isData bool, value any, data []byte, err error) {
-	return true, nil, r.data, nil
-}