diff --git a/CHANGELOG.md b/CHANGELOG.md index 7317316..09e1854 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ TESTS: - Added etcd v2 provider tests. - Add test for degraded cluster (TestDegradedCluster). - Add more tests for etcd v3 viper provider. +- Add tests and correct panics for invalid topology config from viper. ## v2.0.1 diff --git a/providers/viper/moonlibs/convert.go b/providers/viper/moonlibs/convert.go index bd99807..3d353a2 100644 --- a/providers/viper/moonlibs/convert.go +++ b/providers/viper/moonlibs/convert.go @@ -1,19 +1,20 @@ package moonlibs import ( + "fmt" "log" "github.com/google/uuid" vshardrouter "github.com/tarantool/go-vshard-router/v2" ) -func (cfg *Config) Convert() map[vshardrouter.ReplicasetInfo][]vshardrouter.InstanceInfo { +func (cfg *Config) Convert() (map[vshardrouter.ReplicasetInfo][]vshardrouter.InstanceInfo, error) { if cfg.Topology.Instances == nil { - panic("instances is nil") + return nil, fmt.Errorf("no topology instances found") } if cfg.Topology.Clusters == nil { - panic("clusters is nil") + return nil, fmt.Errorf("no topology clusters found") } // prepare vshard router config @@ -22,7 +23,7 @@ func (cfg *Config) Convert() map[vshardrouter.ReplicasetInfo][]vshardrouter.Inst for rsName, rs := range cfg.Topology.Clusters { rsUUID, err := uuid.Parse(rs.ReplicasetUUID) if err != nil { - panic("Can't parse replicaset uuid: %s") + return nil, fmt.Errorf("invalid topology replicaset UUID: %s", rs.ReplicasetUUID) } rsInstances := make([]vshardrouter.InstanceInfo, 0) @@ -36,7 +37,7 @@ func (cfg *Config) Convert() map[vshardrouter.ReplicasetInfo][]vshardrouter.Inst if err != nil { log.Printf("Can't parse replicaset uuid: %s", err) - panic(err) + return nil, fmt.Errorf("invalid topology instance UUID: %s", instInfo.Box.InstanceUUID) } rsInstances = append(rsInstances, vshardrouter.InstanceInfo{ @@ -51,5 +52,5 @@ func (cfg *Config) Convert() map[vshardrouter.ReplicasetInfo][]vshardrouter.Inst }] = rsInstances } - return vshardRouterTopology + return vshardRouterTopology, nil } diff --git a/providers/viper/provider.go b/providers/viper/provider.go index 1c41ff3..00dea97 100644 --- a/providers/viper/provider.go +++ b/providers/viper/provider.go @@ -29,7 +29,7 @@ const ( ) type Convertable interface { - Convert() map[vshardrouter.ReplicasetInfo][]vshardrouter.InstanceInfo + Convert() (map[vshardrouter.ReplicasetInfo][]vshardrouter.InstanceInfo, error) } func NewProvider(ctx context.Context, v *srcviper.Viper, cfgType ConfigType) *Provider { @@ -53,7 +53,10 @@ func NewProvider(ctx context.Context, v *srcviper.Viper, cfgType ConfigType) *Pr panic(err) } - resultMap := cfg.Convert() + resultMap, err := cfg.Convert() + if err != nil { + panic(err) + } return &Provider{ctx: ctx, v: v, rs: resultMap} } diff --git a/providers/viper/provider_test.go b/providers/viper/provider_test.go index 3c926ce..b182ccb 100644 --- a/providers/viper/provider_test.go +++ b/providers/viper/provider_test.go @@ -132,17 +132,50 @@ func TestNewProvider_ETCD3(t *testing.T) { require.NoError(t, err) require.NotEmpty(t, getResponse.Kvs[0].Value) - etcdViper := viper.New() - err = etcdViper.AddRemoteProvider("etcd3", "http://127.0.0.1:2379", key) - require.NoError(t, err) + t.Run("ok reads config", func(t *testing.T) { + etcdViper := viper.New() + err = etcdViper.AddRemoteProvider("etcd3", "http://127.0.0.1:2379", key) + require.NoError(t, err) - etcdViper.SetConfigType("yaml") - err = etcdViper.ReadRemoteConfig() - require.NoError(t, err) + etcdViper.SetConfigType("yaml") + err = etcdViper.ReadRemoteConfig() + require.NoError(t, err) - provider := vprovider.NewProvider(ctx, etcdViper, vprovider.ConfigTypeTarantool3) + provider := vprovider.NewProvider(ctx, etcdViper, vprovider.ConfigTypeTarantool3) + anyProviderValidation(t, provider) + }) - anyProviderValidation(t, provider) + t.Run("invalid path", func(t *testing.T) { + etcdViper := viper.New() + + err = etcdViper.AddRemoteProvider("etcd3", "http://127.0.0.1:2379", "/some-invalid-path") + require.NoError(t, err) + + etcdViper.SetConfigType("yaml") + + err = etcdViper.ReadRemoteConfig() + require.Error(t, err, "path not found error") + }) + + t.Run("invalid config panics", func(t *testing.T) { + etcdViper := viper.New() + + emptyPath := "/some-empty-path" + + _, err := kv.Put(ctx, emptyPath, "") + require.NoError(t, err) + + err = etcdViper.AddRemoteProvider("etcd3", "http://127.0.0.1:2379", "/some-empty-path") + require.NoError(t, err) + + etcdViper.SetConfigType("yaml") + err = etcdViper.ReadRemoteConfig() + require.NoError(t, err) + + require.Panics(t, func() { + vprovider.NewProvider(ctx, etcdViper, vprovider.ConfigTypeTarantool3) + }) + }) } func anyProviderValidation(t testing.TB, provider *vprovider.Provider) { diff --git a/providers/viper/tarantool3/convert.go b/providers/viper/tarantool3/convert.go index 7bd0d8e..e4c39bb 100644 --- a/providers/viper/tarantool3/convert.go +++ b/providers/viper/tarantool3/convert.go @@ -1,10 +1,20 @@ package tarantool3 import ( + "fmt" + vshardrouter "github.com/tarantool/go-vshard-router/v2" ) -func (cfg *Config) Convert() map[vshardrouter.ReplicasetInfo][]vshardrouter.InstanceInfo { +func (cfg *Config) Convert() (map[vshardrouter.ReplicasetInfo][]vshardrouter.InstanceInfo, error) { + if cfg.Groups.Storages == nil { + return nil, fmt.Errorf("cant get groups storage from etcd") + } + + if cfg.Groups.Storages.Replicasets == nil { + return nil, fmt.Errorf("cant get storage replicasets from etcd") + } + m := make(map[vshardrouter.ReplicasetInfo][]vshardrouter.InstanceInfo) for rsName, rs := range cfg.Groups.Storages.Replicasets { @@ -21,5 +31,5 @@ func (cfg *Config) Convert() map[vshardrouter.ReplicasetInfo][]vshardrouter.Inst m[rsInfo] = instances } - return m + return m, nil }