From 7c3a8f9d07e203cad12680b7c326e11db40fb8de Mon Sep 17 00:00:00 2001 From: Petu Eusebiu Date: Fri, 6 May 2022 13:15:03 +0300 Subject: [PATCH] Report unknown keys when parsing configuration files Report missing mandatory ldap keys Signed-off-by: Petu Eusebiu --- pkg/api/config/config.go | 16 ----- pkg/api/controller.go | 7 -- pkg/cli/root.go | 149 +++++++++++++++++++++++++++------------ pkg/cli/root_test.go | 63 +++++++++++++++++ 4 files changed, 165 insertions(+), 70 deletions(-) diff --git a/pkg/api/config/config.go b/pkg/api/config/config.go index c1649db2..6dab2533 100644 --- a/pkg/api/config/config.go +++ b/pkg/api/config/config.go @@ -7,9 +7,7 @@ import ( "github.com/getlantern/deepcopy" distspec "github.com/opencontainers/distribution-spec/specs-go" "github.com/spf13/viper" - "zotregistry.io/zot/errors" extconf "zotregistry.io/zot/pkg/extensions/config" - "zotregistry.io/zot/pkg/log" "zotregistry.io/zot/pkg/storage" ) @@ -167,20 +165,6 @@ func (c *Config) Sanitize() *Config { return sanitizedConfig } -func (c *Config) Validate(log log.Logger) error { - // LDAP configuration - if c.HTTP.Auth != nil && c.HTTP.Auth.LDAP != nil { - l := c.HTTP.Auth.LDAP - if l.UserAttribute == "" { - log.Error().Str("userAttribute", l.UserAttribute).Msg("invalid LDAP configuration") - - return errors.ErrLDAPConfig - } - } - - return nil -} - // LoadAccessControlConfig populates config.AccessControl struct with values from config. func (c *Config) LoadAccessControlConfig(viperInstance *viper.Viper) error { if c.HTTP.RawAccessControl == nil { diff --git a/pkg/api/controller.go b/pkg/api/controller.go index 2bec765f..591b0703 100644 --- a/pkg/api/controller.go +++ b/pkg/api/controller.go @@ -102,13 +102,6 @@ func DumpRuntimeParams(log log.Logger) { } func (c *Controller) Run(reloadCtx context.Context) error { - // validate configuration - if err := c.Config.Validate(c.Log); err != nil { - c.Log.Error().Err(err).Msg("configuration validation failed") - - return err - } - // print the current configuration, but strip secrets c.Log.Info().Interface("params", c.Config.Sanitize()).Msg("configuration settings") diff --git a/pkg/cli/root.go b/pkg/cli/root.go index 15dbebb9..4513e957 100644 --- a/pkg/cli/root.go +++ b/pkg/cli/root.go @@ -201,31 +201,16 @@ func NewCliRootCmd() *cobra.Command { } func validateConfiguration(config *config.Config) error { - // enforce GC params - if config.Storage.GCDelay < 0 { - log.Error().Err(errors.ErrBadConfig). - Msgf("invalid garbage-collect delay %v specified", config.Storage.GCDelay) - - return errors.ErrBadConfig + if err := validateGC(config); err != nil { + return err } - if config.Storage.GCInterval < 0 { - log.Error().Err(errors.ErrBadConfig). - Msgf("invalid garbage-collect interval %v specified", config.Storage.GCInterval) - - return errors.ErrBadConfig + if err := validateLDAP(config); err != nil { + return err } - if !config.Storage.GC { - if config.Storage.GCDelay != 0 { - log.Warn().Err(errors.ErrBadConfig). - Msg("garbage-collect delay specified without enabling garbage-collect, will be ignored") - } - - if config.Storage.GCInterval != 0 { - log.Warn().Err(errors.ErrBadConfig). - Msg("periodic garbage-collect interval specified without enabling garbage-collect, will be ignored") - } + if err := validateSync(config); err != nil { + return err } // check authorization config, it should have basic auth enabled or ldap @@ -252,30 +237,6 @@ func validateConfiguration(config *config.Config) error { } } - // check glob patterns in sync config are compilable - if config.Extensions != nil && config.Extensions.Sync != nil { - for id, regCfg := range config.Extensions.Sync.Registries { - // check retry options are configured for sync - if regCfg.MaxRetries != nil && regCfg.RetryDelay == nil { - log.Error().Err(errors.ErrBadConfig).Msgf("extensions.sync.registries[%d].retryDelay"+ - " is required when using extensions.sync.registries[%d].maxRetries", id, id) - - return errors.ErrBadConfig - } - - if regCfg.Content != nil { - for _, content := range regCfg.Content { - ok := glob.ValidatePattern(content.Prefix) - if !ok { - log.Error().Err(glob.ErrBadPattern).Str("pattern", content.Prefix).Msg("sync pattern could not be compiled") - - return glob.ErrBadPattern - } - } - } - } - } - // enforce s3 driver on subpaths in case of using storage driver if config.Storage.SubPaths != nil { if len(config.Storage.SubPaths) > 0 { @@ -412,8 +373,14 @@ func LoadConfiguration(config *config.Config, configPath string) error { return err } - if len(metaData.Keys) == 0 || len(metaData.Unused) > 0 { - log.Error().Err(errors.ErrBadConfig).Msg("bad configuration, retry writing it") + if len(metaData.Keys) == 0 { + log.Error().Err(errors.ErrBadConfig).Msgf("config doesn't contain any key:value pair") + + return errors.ErrBadConfig + } + + if len(metaData.Unused) > 0 { + log.Error().Err(errors.ErrBadConfig).Msgf("unknown keys: %v", metaData.Unused) return errors.ErrBadConfig } @@ -465,3 +432,91 @@ func isDefaultPolicyConfig(cfg *config.Config) bool { return true } + +func validateLDAP(config *config.Config) error { + // LDAP mandatory configuration + if config.HTTP.Auth != nil && config.HTTP.Auth.LDAP != nil { + ldap := config.HTTP.Auth.LDAP + if ldap.UserAttribute == "" { + log.Error().Str("userAttribute", ldap.UserAttribute). + Msg("invalid LDAP configuration, missing mandatory key: userAttribute") + + return errors.ErrLDAPConfig + } + + if ldap.Address == "" { + log.Error().Str("address", ldap.Address). + Msg("invalid LDAP configuration, missing mandatory key: address") + + return errors.ErrLDAPConfig + } + + if ldap.BaseDN == "" { + log.Error().Str("basedn", ldap.BaseDN). + Msg("invalid LDAP configuration, missing mandatory key: basedn") + + return errors.ErrLDAPConfig + } + } + + return nil +} + +func validateGC(config *config.Config) error { + // enforce GC params + if config.Storage.GCDelay < 0 { + log.Error().Err(errors.ErrBadConfig). + Msgf("invalid garbage-collect delay %v specified", config.Storage.GCDelay) + + return errors.ErrBadConfig + } + + if config.Storage.GCInterval < 0 { + log.Error().Err(errors.ErrBadConfig). + Msgf("invalid garbage-collect interval %v specified", config.Storage.GCInterval) + + return errors.ErrBadConfig + } + + if !config.Storage.GC { + if config.Storage.GCDelay != 0 { + log.Warn().Err(errors.ErrBadConfig). + Msg("garbage-collect delay specified without enabling garbage-collect, will be ignored") + } + + if config.Storage.GCInterval != 0 { + log.Warn().Err(errors.ErrBadConfig). + Msg("periodic garbage-collect interval specified without enabling garbage-collect, will be ignored") + } + } + + return nil +} + +func validateSync(config *config.Config) error { + // check glob patterns in sync config are compilable + if config.Extensions != nil && config.Extensions.Sync != nil { + for id, regCfg := range config.Extensions.Sync.Registries { + // check retry options are configured for sync + if regCfg.MaxRetries != nil && regCfg.RetryDelay == nil { + log.Error().Err(errors.ErrBadConfig).Msgf("extensions.sync.registries[%d].retryDelay"+ + " is required when using extensions.sync.registries[%d].maxRetries", id, id) + + return errors.ErrBadConfig + } + + if regCfg.Content != nil { + for _, content := range regCfg.Content { + ok := glob.ValidatePattern(content.Prefix) + if !ok { + log.Error().Err(glob.ErrBadPattern).Str("pattern", content.Prefix).Msg("sync pattern could not be compiled") + + return glob.ErrBadPattern + } + } + } + } + } + + return nil +} diff --git a/pkg/cli/root_test.go b/pkg/cli/root_test.go index 3072aaa8..205ed35f 100644 --- a/pkg/cli/root_test.go +++ b/pkg/cli/root_test.go @@ -334,6 +334,69 @@ func TestVerify(t *testing.T) { So(func() { _ = cli.NewServerRootCmd().Execute() }, ShouldPanic) }) + Convey("Test verify config with unknown keys", t, func(c C) { + tmpfile, err := ioutil.TempFile("", "zot-test*.json") + So(err, ShouldBeNil) + defer os.Remove(tmpfile.Name()) // clean up + content := []byte(`{"distSpecVersion": "1.0.0", "storage": {"rootDirectory": "/tmp/zot"}, + "http": {"url": "127.0.0.1", "port": "8080", "ReadOnly": false}, + "log": {"level": "debug"}}`) + _, err = tmpfile.Write(content) + So(err, ShouldBeNil) + err = tmpfile.Close() + So(err, ShouldBeNil) + os.Args = []string{"cli_test", "verify", tmpfile.Name()} + So(func() { _ = cli.NewServerRootCmd().Execute() }, ShouldPanic) + }) + + Convey("Test verify config with missing basedn key", t, func(c C) { + tmpfile, err := ioutil.TempFile("", "zot-test*.json") + So(err, ShouldBeNil) + defer os.Remove(tmpfile.Name()) // clean up + content := []byte(`{"distSpecVersion": "1.0.0", "storage": {"rootDirectory": "/tmp/zot"}, + "http": {"auth": {"ldap": {"address": "ldap", "userattribute": "uid"}}, + "address": "127.0.0.1", "port": "8080", "ReadOnly": false}, + "log": {"level": "debug"}}`) + _, err = tmpfile.Write(content) + So(err, ShouldBeNil) + err = tmpfile.Close() + So(err, ShouldBeNil) + os.Args = []string{"cli_test", "verify", tmpfile.Name()} + So(func() { _ = cli.NewServerRootCmd().Execute() }, ShouldPanic) + }) + + Convey("Test verify config with missing address key", t, func(c C) { + tmpfile, err := ioutil.TempFile("", "zot-test*.json") + So(err, ShouldBeNil) + defer os.Remove(tmpfile.Name()) // clean up + content := []byte(`{"distSpecVersion": "1.0.0", "storage": {"rootDirectory": "/tmp/zot"}, + "http": {"auth": {"ldap": {"basedn": "ou=Users,dc=example,dc=org", "userattribute": "uid"}}, + "address": "127.0.0.1", "port": "8080", "ReadOnly": false}, + "log": {"level": "debug"}}`) + _, err = tmpfile.Write(content) + So(err, ShouldBeNil) + err = tmpfile.Close() + So(err, ShouldBeNil) + os.Args = []string{"cli_test", "verify", tmpfile.Name()} + So(func() { _ = cli.NewServerRootCmd().Execute() }, ShouldPanic) + }) + + Convey("Test verify config with missing userattribute key", t, func(c C) { + tmpfile, err := ioutil.TempFile("", "zot-test*.json") + So(err, ShouldBeNil) + defer os.Remove(tmpfile.Name()) // clean up + content := []byte(`{"distSpecVersion": "1.0.0", "storage": {"rootDirectory": "/tmp/zot"}, + "http": {"auth": {"ldap": {"basedn": "ou=Users,dc=example,dc=org", "address": "ldap"}}, + "address": "127.0.0.1", "port": "8080", "ReadOnly": false}, + "log": {"level": "debug"}}`) + _, err = tmpfile.Write(content) + So(err, ShouldBeNil) + err = tmpfile.Close() + So(err, ShouldBeNil) + os.Args = []string{"cli_test", "verify", tmpfile.Name()} + So(func() { _ = cli.NewServerRootCmd().Execute() }, ShouldPanic) + }) + Convey("Test verify good config", t, func(c C) { tmpfile, err := ioutil.TempFile("", "zot-test*.json") So(err, ShouldBeNil)