Enforce/validate configuration of services (#4035)

changelog/unreleased/validate-config.md

@@ -0,0 +1,7 @@
+Enhancement: Enforce/validate configuration of services
+
+Every driver can now specify some validation rules on the
+configuration. If the validation rules are not respected,
+reva will bail out on startup with a clear error.
+
+https://github.com/cs3org/reva/pull/4035

cmd/revad/runtime/runtime.go

@@ -119,7 +119,7 @@ func New(config *config.Config, opt ...Option) (*Reva, error) {
 		return nil, err
 	}
 
-	serverless, err := newServerless(config, log)
+	serverless, err := newServerless(ctx, config, log)
 	if err != nil {
 		watcher.Clean()
 		return nil, err
@@ -181,7 +181,7 @@ func servicesAddresses(cfg *config.Config) map[string]grace.Addressable {
 	return a
 }
 
-func newServerless(config *config.Config, log *zerolog.Logger) (*rserverless.Serverless, error) {
+func newServerless(ctx context.Context, config *config.Config, log *zerolog.Logger) (*rserverless.Serverless, error) {
 	sl := make(map[string]rserverless.Service)
 	logger := log.With().Str("pkg", "serverless").Logger()
 	if err := config.Serverless.ForEach(func(name string, config map[string]any) error {
@@ -190,7 +190,8 @@ func newServerless(config *config.Config, log *zerolog.Logger) (*rserverless.Ser
 			return fmt.Errorf("serverless service %s does not exist", name)
 		}
 		log := logger.With().Str("service", name).Logger()
-		svc, err := new(config, &log)
+		ctx := appctx.WithLogger(ctx, &log)
+		svc, err := new(ctx, config)
 		if err != nil {
 			return errors.Wrapf(err, "serverless service %s could not be initialized", name)
 		}

go.mod

@@ -14,6 +14,8 @@ require (
 	github.com/cenkalti/backoff v2.2.1+incompatible
 	github.com/ceph/go-ceph v0.15.0
 	github.com/cheggaaa/pb v1.0.29
+	github.com/coreos/go-oidc/v3 v3.5.0
+	github.com/creasty/defaults v1.7.0
 	github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e
 	github.com/cs3org/go-cs3apis v0.0.0-20230606135123-b799d47a6648
 	github.com/dgraph-io/ristretto v0.1.1
@@ -23,6 +25,8 @@ require (
 	github.com/glpatcern/go-mime v0.0.0-20221026162842-2a8d71ad17a9
 	github.com/go-chi/chi/v5 v5.0.8
 	github.com/go-ldap/ldap/v3 v3.4.4
+	github.com/go-playground/locales v0.14.1
+	github.com/go-playground/universal-translator v0.18.1
 	github.com/go-playground/validator/v10 v10.11.2
 	github.com/go-sql-driver/mysql v1.7.0
 	github.com/golang-jwt/jwt v3.2.2+incompatible
@@ -76,12 +80,6 @@ require (
 	gotest.tools v2.2.0+incompatible
 )
 
-require (
-	github.com/creasty/defaults v1.7.0 // indirect
-	github.com/go-jose/go-jose/v3 v3.0.0 // indirect
-	github.com/hashicorp/go-msgpack/v2 v2.1.0 // indirect
-)
-
 require (
 	github.com/Azure/go-ntlmssp v0.0.0-20220621081337-cb9428e4ac1e // indirect
 	github.com/Masterminds/goutils v1.1.1 // indirect
@@ -92,22 +90,20 @@ require (
 	github.com/bmizerany/pat v0.0.0-20170815010413-6226ea591a40 // indirect
 	github.com/cespare/xxhash v1.1.0 // indirect
 	github.com/cespare/xxhash/v2 v2.1.2 // indirect
-	github.com/coreos/go-oidc/v3 v3.5.0
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/dolthub/vitess v0.0.0-20221031111135-9aad77e7b39f // indirect
 	github.com/dustin/go-humanize v1.0.0 // indirect
 	github.com/fatih/color v1.13.0 // indirect
 	github.com/fsnotify/fsnotify v1.4.9 // indirect
 	github.com/go-asn1-ber/asn1-ber v1.5.4 // indirect
+	github.com/go-jose/go-jose/v3 v3.0.0 // indirect
 	github.com/go-kit/kit v0.10.0 // indirect
 	github.com/go-kit/log v0.2.1 // indirect
 	github.com/go-logfmt/logfmt v0.5.1 // indirect
 	github.com/go-logr/logr v1.2.3 // indirect
 	github.com/go-logr/stdr v1.2.2 // indirect
 	github.com/go-openapi/errors v0.20.2 // indirect
 	github.com/go-openapi/strfmt v0.21.2 // indirect
-	github.com/go-playground/locales v0.14.1 // indirect
-	github.com/go-playground/universal-translator v0.18.1 // indirect
 	github.com/go-stack/stack v1.8.1 // indirect
 	github.com/gocraft/dbr/v2 v2.7.2 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
@@ -116,6 +112,7 @@ require (
 	github.com/google/flatbuffers v2.0.6+incompatible // indirect
 	github.com/hashicorp/go-immutable-radix v1.0.0 // indirect
 	github.com/hashicorp/go-msgpack v1.1.5 // indirect
+	github.com/hashicorp/go-msgpack/v2 v2.1.0 // indirect
 	github.com/hashicorp/golang-lru v0.5.4 // indirect
 	github.com/hashicorp/raft v1.4.0 // indirect
 	github.com/hashicorp/yamux v0.0.0-20180604194846-3520598351bb // indirect

go.sum

@@ -308,8 +308,6 @@ github.com/creasty/defaults v1.7.0 h1:eNdqZvc5B509z18lD8yc212CAqJNvfT1Jq6L8WowdB
 github.com/creasty/defaults v1.7.0/go.mod h1:iGzKe6pbEHnpMPtfDXZEr0NVxWnPTjb1bbDy08fPzYM=
 github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e h1:tqSPWQeueWTKnJVMJffz4pz0o1WuQxJ28+5x5JgaHD8=
 github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e/go.mod h1:XJEZ3/EQuI3BXTp/6DUzFr850vlxq11I6satRtz0YQ4=
-github.com/cs3org/go-cs3apis v0.0.0-20230508132523-e0d062e63b3b h1:UCO7Rnf5bvIvRtETguV8IaTx73cImLlFWxrApCB0QsQ=
-github.com/cs3org/go-cs3apis v0.0.0-20230508132523-e0d062e63b3b/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY=
 github.com/cs3org/go-cs3apis v0.0.0-20230606135123-b799d47a6648 h1:gBz1JSC2u6o/TkUhWSdJZvacyTsVUzDouegRzvrJye4=
 github.com/cs3org/go-cs3apis v0.0.0-20230606135123-b799d47a6648/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY=
 github.com/cyberdelia/templates v0.0.0-20141128023046-ca7fffd4298c/go.mod h1:GyV+0YP4qX0UQ7r2MoYZ+AvYDp12OF5yg4q8rGnyNh4=

internal/grpc/services/applicationauth/applicationauth.go

@@ -27,8 +27,7 @@ import (
 	"github.com/cs3org/reva/pkg/errtypes"
 	"github.com/cs3org/reva/pkg/rgrpc"
 	"github.com/cs3org/reva/pkg/rgrpc/status"
-	"github.com/mitchellh/mapstructure"
-	"github.com/pkg/errors"
+	"github.com/cs3org/reva/pkg/utils/cfg"
 	"google.golang.org/grpc"
 )
 
@@ -46,7 +45,7 @@ type service struct {
 	am   appauth.Manager
 }
 
-func (c *config) init() {
+func (c *config) ApplyDefaults() {
 	if c.Driver == "" {
 		c.Driver = "json"
 	}
@@ -63,30 +62,19 @@ func getAppAuthManager(ctx context.Context, c *config) (appauth.Manager, error)
 	return nil, errtypes.NotFound("driver not found: " + c.Driver)
 }
 
-func parseConfig(m map[string]interface{}) (*config, error) {
-	c := &config{}
-	if err := mapstructure.Decode(m, c); err != nil {
-		err = errors.Wrap(err, "error decoding conf")
-		return nil, err
-	}
-	return c, nil
-}
-
 // New creates a app auth provider svc.
 func New(ctx context.Context, m map[string]interface{}) (rgrpc.Service, error) {
-	c, err := parseConfig(m)
-	if err != nil {
+	var c config
+	if err := cfg.Decode(m, &c); err != nil {
 		return nil, err
 	}
-	c.init()
-
-	am, err := getAppAuthManager(ctx, c)
+	am, err := getAppAuthManager(ctx, &c)
 	if err != nil {
 		return nil, err
 	}
 
 	service := &service{
-		conf: c,
+		conf: &c,
 		am:   am,
 	}
 

internal/grpc/services/appprovider/appprovider.go

@@ -40,8 +40,8 @@ import (
 	"github.com/cs3org/reva/pkg/rgrpc/status"
 	"github.com/cs3org/reva/pkg/rgrpc/todo/pool"
 	"github.com/cs3org/reva/pkg/sharedconf"
+	"github.com/cs3org/reva/pkg/utils/cfg"
 	"github.com/juliangruber/go-intersect"
-	"github.com/mitchellh/mapstructure"
 	"google.golang.org/grpc"
 )
 
@@ -65,43 +65,33 @@ type config struct {
 	Language            string                            `mapstructure:"language"`
 }
 
-func (c *config) init() {
+func (c *config) ApplyDefaults() {
 	if c.Driver == "" {
 		c.Driver = "demo"
 	}
 	c.AppProviderURL = sharedconf.GetGatewaySVC(c.AppProviderURL)
 	c.GatewaySvc = sharedconf.GetGatewaySVC(c.GatewaySvc)
 }
 
-func parseConfig(m map[string]interface{}) (*config, error) {
-	c := &config{}
-	if err := mapstructure.Decode(m, c); err != nil {
-		return nil, err
-	}
-	c.init()
-	return c, nil
-}
-
 // New creates a new AppProviderService.
 func New(ctx context.Context, m map[string]interface{}) (rgrpc.Service, error) {
-	c, err := parseConfig(m)
-	if err != nil {
+	var c config
+	if err := cfg.Decode(m, &c); err != nil {
 		return nil, err
 	}
 
 	// read and register custom mime types if configured
-	err = registerMimeTypes(c.CustomMimeTypesJSON)
-	if err != nil {
+	if err := registerMimeTypes(c.CustomMimeTypesJSON); err != nil {
 		return nil, err
 	}
 
-	provider, err := getProvider(ctx, c)
+	provider, err := getProvider(ctx, &c)
 	if err != nil {
 		return nil, err
 	}
 
 	service := &service{
-		conf:     c,
+		conf:     &c,
 		provider: provider,
 	}
 

internal/grpc/services/appprovider/appprovider_test.go

@@ -21,6 +21,7 @@ package appprovider
 import (
 	"testing"
 
+	"github.com/cs3org/reva/pkg/utils/cfg"
 	"github.com/mitchellh/mapstructure"
 	"github.com/stretchr/testify/assert"
 )
@@ -80,9 +81,12 @@ func Test_parseConfig(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := parseConfig(tt.m)
+			var got config
+			err := cfg.Decode(tt.m, &got)
 			assert.Equal(t, tt.wantErr, err)
-			assert.Equal(t, tt.want, got)
+			if tt.wantErr == nil {
+				assert.Equal(t, tt.want, &got)
+			}
 		})
 	}
 }

internal/grpc/services/appregistry/appregistry.go

@@ -27,7 +27,7 @@ import (
 	"github.com/cs3org/reva/pkg/errtypes"
 	"github.com/cs3org/reva/pkg/rgrpc"
 	"github.com/cs3org/reva/pkg/rgrpc/status"
-	"github.com/mitchellh/mapstructure"
+	"github.com/cs3org/reva/pkg/utils/cfg"
 	"google.golang.org/grpc"
 )
 
@@ -56,20 +56,20 @@ type config struct {
 	Drivers map[string]map[string]interface{} `mapstructure:"drivers"`
 }
 
-func (c *config) init() {
+func (c *config) ApplyDefaults() {
 	if c.Driver == "" {
 		c.Driver = "static"
 	}
 }
 
 // New creates a new StorageRegistryService.
 func New(ctx context.Context, m map[string]interface{}) (rgrpc.Service, error) {
-	c, err := parseConfig(m)
-	if err != nil {
+	var c config
+	if err := cfg.Decode(m, &c); err != nil {
 		return nil, err
 	}
 
-	reg, err := getRegistry(ctx, c)
+	reg, err := getRegistry(ctx, &c)
 	if err != nil {
 		return nil, err
 	}
@@ -81,15 +81,6 @@ func New(ctx context.Context, m map[string]interface{}) (rgrpc.Service, error) {
 	return svc, nil
 }
 
-func parseConfig(m map[string]interface{}) (*config, error) {
-	c := &config{}
-	if err := mapstructure.Decode(m, c); err != nil {
-		return nil, err
-	}
-	c.init()
-	return c, nil
-}
-
 func getRegistry(ctx context.Context, c *config) (app.Registry, error) {
 	if f, ok := registry.NewFuncs[c.Driver]; ok {
 		return f(ctx, c.Drivers[c.Driver])

internal/grpc/services/authprovider/authprovider.go

@@ -33,7 +33,7 @@ import (
 	"github.com/cs3org/reva/pkg/rgrpc/status"
 	"github.com/cs3org/reva/pkg/sharedconf"
 	"github.com/cs3org/reva/pkg/user"
-	"github.com/mitchellh/mapstructure"
+	"github.com/cs3org/reva/pkg/utils/cfg"
 	"github.com/pkg/errors"
 	"google.golang.org/grpc"
 )
@@ -48,7 +48,7 @@ type config struct {
 	blockedUsers []string
 }
 
-func (c *config) init() {
+func (c *config) ApplyDefaults() {
 	if c.AuthManager == "" {
 		c.AuthManager = "json"
 	}
@@ -62,16 +62,6 @@ type service struct {
 	blockedUsers user.BlockedUsers
 }
 
-func parseConfig(m map[string]interface{}) (*config, error) {
-	c := &config{}
-	if err := mapstructure.Decode(m, c); err != nil {
-		err = errors.Wrap(err, "error decoding conf")
-		return nil, err
-	}
-	c.init()
-	return c, nil
-}
-
 func getAuthManager(ctx context.Context, manager string, m map[string]map[string]interface{}) (auth.Manager, *plugin.RevaPlugin, error) {
 	if manager == "" {
 		return nil, nil, errtypes.InternalError("authsvc: driver not configured for auth manager")
@@ -101,8 +91,8 @@ func getAuthManager(ctx context.Context, manager string, m map[string]map[string
 
 // New returns a new AuthProviderServiceServer.
 func New(ctx context.Context, m map[string]interface{}) (rgrpc.Service, error) {
-	c, err := parseConfig(m)
-	if err != nil {
+	var c config
+	if err := cfg.Decode(m, &c); err != nil {
 		return nil, err
 	}
 
@@ -112,7 +102,7 @@ func New(ctx context.Context, m map[string]interface{}) (rgrpc.Service, error) {
 	}
 
 	svc := &service{
-		conf:         c,
+		conf:         &c,
 		authmgr:      authManager,
 		plugin:       plug,
 		blockedUsers: user.NewBlockedUsersSet(c.blockedUsers),