Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce/validate configuration of services #4035

Merged
merged 17 commits into from
Jul 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions changelog/unreleased/validate-config.md
Original file line number Diff line number Diff line change
@@ -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
7 changes: 4 additions & 3 deletions cmd/revad/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,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
Expand Down Expand Up @@ -144,7 +144,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 {
Expand All @@ -153,7 +153,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)
}
Expand Down
15 changes: 6 additions & 9 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
24 changes: 6 additions & 18 deletions internal/grpc/services/applicationauth/applicationauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -46,7 +45,7 @@ type service struct {
am appauth.Manager
}

func (c *config) init() {
func (c *config) ApplyDefaults() {
if c.Driver == "" {
c.Driver = "json"
}
Expand All @@ -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,
}

Expand Down
24 changes: 7 additions & 17 deletions internal/grpc/services/appprovider/appprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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,
}

Expand Down
8 changes: 6 additions & 2 deletions internal/grpc/services/appprovider/appprovider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package appprovider
import (
"testing"

"github.com/cs3org/reva/pkg/utils/cfg"
"github.com/mitchellh/mapstructure"
"github.com/stretchr/testify/assert"
)
Expand Down Expand Up @@ -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)
}
})
}
}
19 changes: 5 additions & 14 deletions internal/grpc/services/appregistry/appregistry.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
}
Expand All @@ -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])
Expand Down
20 changes: 5 additions & 15 deletions internal/grpc/services/authprovider/authprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -48,7 +48,7 @@ type config struct {
blockedUsers []string
}

func (c *config) init() {
func (c *config) ApplyDefaults() {
if c.AuthManager == "" {
c.AuthManager = "json"
}
Expand All @@ -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")
Expand Down Expand Up @@ -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
}

Expand All @@ -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),
Expand Down
Loading