Add tests and correct panics for invalid topology config from viper

tarantool · Jan 23, 2025 · f01c756 · f01c756
1 parent 91e5d7a
commit f01c756
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 18 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
 }