Fix config add-dir regression (#406)

* WIP: Fix config add-dir regression

* split add-dir into add-chains and add-paths

* nicer error messages

* fix lint

* apply @fedekunze suggestions

* remove newline from error

Co-authored-by: Colin Axnér <25233464+colin-axner@users.noreply.github.com>

cmd/config.go

@@ -26,6 +26,7 @@ import (
 	"time"
 
 	"github.com/cosmos/cosmos-sdk/client/flags"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/cosmos/relayer/relayer"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
@@ -51,7 +52,8 @@ func configCmd() *cobra.Command {
 	cmd.AddCommand(
 		configShowCmd(),
 		configInitCmd(),
-		configAddDirCmd(),
+		configAddChainsCmd(),
+		configAddPathsCmd(),
 	)
 
 	return cmd
@@ -170,19 +172,17 @@ $ %s cfg i`, appName, defaultHome, appName)),
 	return cmd
 }
 
-func configAddDirCmd() *cobra.Command {
+func configAddChainsCmd() *cobra.Command {
 	cmd := &cobra.Command{
-		Use:     "add-dir [dir]",
-		Aliases: []string{"ad"},
-		Args:    cobra.ExactArgs(1),
-		Short: `Add new chains and paths to the configuration file from a
-		 directory full of chain and path configuration, useful for adding testnet configurations`,
+		Use:  "add-chains [/path/to/chains/]",
+		Args: cobra.ExactArgs(1),
+		Short: `Add new chains to the configuration file from a
+		 directory full of chain configurations, useful for adding testnet configurations`,
 		Example: strings.TrimSpace(fmt.Sprintf(`
-$ %s config add-dir configs/
-$ %s cfg ad configs/`, appName, appName)),
+$ %s config add-chains configs/chains`, appName)),
 		RunE: func(cmd *cobra.Command, args []string) (err error) {
 			var out *Config
-			if out, err = cfgFilesAdd(args[0]); err != nil {
+			if out, err = cfgFilesAddChains(args[0]); err != nil {
 				return err
 			}
 			return overWriteConfig(cmd, out)
@@ -192,7 +192,28 @@ $ %s cfg ad configs/`, appName, appName)),
 	return cmd
 }
 
-func cfgFilesAdd(dir string) (cfg *Config, err error) {
+func configAddPathsCmd() *cobra.Command {
+	cmd := &cobra.Command{
+		Use:  "add-paths [/path/to/paths/]",
+		Args: cobra.ExactArgs(1),
+		//nolint:lll
+		Short: `Add new paths to the configuration file from a directory full of path configurations, useful for adding testnet configurations. 
+		Chain configuration files must be added before calling this command.`,
+		Example: strings.TrimSpace(fmt.Sprintf(`
+$ %s config add-paths configs/paths`, appName)),
+		RunE: func(cmd *cobra.Command, args []string) (err error) {
+			var out *Config
+			if out, err = cfgFilesAddPaths(args[0]); err != nil {
+				return err
+			}
+			return overWriteConfig(cmd, out)
+		},
+	}
+
+	return cmd
+}
+
+func cfgFilesAddChains(dir string) (cfg *Config, err error) {
 	dir = path.Clean(dir)
 	files, err := ioutil.ReadDir(dir)
 	if err != nil {
@@ -209,60 +230,71 @@ func cfgFilesAdd(dir string) (cfg *Config, err error) {
 
 		byt, err := ioutil.ReadFile(pth)
 		if err != nil {
-			fmt.Printf("failed to read file %s, skipping...\n", pth)
-			continue
+			return nil, fmt.Errorf("failed to read file %s: %w", pth, err)
 		}
 
 		if err = json.Unmarshal(byt, c); err != nil {
-			fmt.Printf("failed to unmarshal file %s, skipping...\n", pth)
+			return nil, fmt.Errorf("failed to unmarshal file %s: %w", pth, err)
+		}
+
+		if err = cfg.AddChain(c); err != nil {
+			return nil, fmt.Errorf("failed to add chain%s: %w", pth, err)
+		}
+		fmt.Printf("added chain %s...\n", c.ChainID)
+	}
+	return cfg, nil
+}
+
+func cfgFilesAddPaths(dir string) (cfg *Config, err error) {
+	dir = path.Clean(dir)
+	files, err := ioutil.ReadDir(dir)
+	if err != nil {
+		return nil, err
+	}
+	cfg = config
+	for _, f := range files {
+		pth := fmt.Sprintf("%s/%s", dir, f.Name())
+		if f.IsDir() {
+			fmt.Printf("directory at %s, skipping...\n", pth)
 			continue
 		}
 
-		if c.ChainID == "" && c.Key == "" && c.RPCAddr == "" {
-			p := &relayer.Path{}
-			if err = json.Unmarshal(byt, p); err != nil {
-				fmt.Printf("failed to unmarshal file %s, skipping...\n", pth)
-				continue
-			}
+		byt, err := ioutil.ReadFile(pth)
+		if err != nil {
+			return nil, fmt.Errorf("failed to read file %s: %w", pth, err)
+		}
 
-			// In the case that order isn't added to the path, add it manually
-			if p.Src.Order == "" || p.Dst.Order == "" {
-				p.Src.Order = defaultOrder
-				p.Dst.Order = defaultOrder
-			}
+		p := &relayer.Path{}
+		if err = json.Unmarshal(byt, p); err != nil {
+			return nil, fmt.Errorf("failed to unmarshal file %s: %w", pth, err)
+		}
 
-			// If the version isn't added to the path, add it manually
-			if p.Src.Version == "" {
-				p.Src.Version = defaultVersion
-			}
-			if p.Dst.Version == "" {
-				p.Dst.Version = defaultVersion
-			}
+		// In the case that order isn't added to the path, add it manually
+		if p.Src.Order == "" || p.Dst.Order == "" {
+			p.Src.Order = defaultOrder
+			p.Dst.Order = defaultOrder
+		}
 
-			pthName := strings.Split(f.Name(), ".")[0]
-			if err = config.ValidatePath(p); err != nil {
-				fmt.Printf("%s: %s\n", pth, err.Error())
-				continue
-			}
-			if err = cfg.AddPath(pthName, p); err != nil {
-				fmt.Printf("%s: %s\n", pth, err.Error())
-				continue
-			}
+		// If the version isn't added to the path, add it manually
+		if p.Src.Version == "" {
+			p.Src.Version = defaultVersion
+		}
+		if p.Dst.Version == "" {
+			p.Dst.Version = defaultVersion
+		}
 
-			// For now, we assume that all chain files must have same filename as chain-id
-			// this is to ensure non-chain files (global config) does not get parsed into chain struct.
-			if c.ChainID != pthName {
-				fmt.Printf("Skipping non chain file: %s\n", f.Name())
-				continue
-			}
+		pthName := strings.Split(f.Name(), ".")[0]
+		if err = config.ValidatePath(p); err != nil {
+			return nil, fmt.Errorf("failed to validate path %s: %w", pth, err)
 		}
 
-		if err = cfg.AddChain(c); err != nil {
-			fmt.Printf("%s: %s\n", pth, err.Error())
-			continue
+		if err = cfg.AddPath(pthName, p); err != nil {
+			return nil, fmt.Errorf("failed to add path %s: %w", pth, err)
 		}
-		fmt.Printf("added chain %s...\n", c.ChainID)
+
+		fmt.Printf("added path %s...\n", pthName)
 	}
+
 	return cfg, nil
 }
 
@@ -448,10 +480,10 @@ func (c *Config) ValidatePath(p *relayer.Path) (err error) {
 		return fmt.Errorf("source must specify a version")
 	}
 	if err = c.ValidatePathEnd(p.Src); err != nil {
-		return err
+		return sdkerrors.Wrapf(err, "chain %s failed path validation", p.Src.ChainID)
 	}
 	if err = c.ValidatePathEnd(p.Dst); err != nil {
-		return err
+		return sdkerrors.Wrapf(err, "chain %s failed path validation", p.Dst.ChainID)
 	}
 	if _, err = p.GetStrategy(); err != nil {
 		return err
@@ -469,6 +501,11 @@ func (c *Config) ValidatePathEnd(pe *relayer.PathEnd) error {
 		return err
 	}
 
+	// if the identifiers are empty, don't do any validation
+	if pe.ClientID == "" && pe.ConnectionID == "" && pe.ChannelID == "" {
+		return nil
+	}
+
 	chain, err := c.Chains.Get(pe.ChainID)
 	if err != nil {
 		return err

docs/commands.md

@@ -37,7 +37,8 @@ NOTE: Most of the commands have aliases that make typing them much quicker (i.e.
     - [rly chains list](#rly-chains-list)
     - [rly chains show](#rly-chains-show)
   - [rly config](#rly-config)
-    - [rly config add-dir](#rly-config-add-dir)
+    - [rly config add-chains](#rly-config-add-chains)
+    - [rly config add-paths](#rly-config-add-paths)
     - [rly config init](#rly-config-init)
     - [rly config show](#rly-config-show)
   - [rly development](#rly-development)
@@ -253,17 +254,30 @@ Manage configuration file
 
 ### Subcommands
 
-* [rly config add-dir](#rly-config-add-dir)	 - Add new chains and paths to the configuration file from a directory full of chain and path configuration, useful for adding testnet configurations
+* [rly config add-chains](#rly-config-add-chains)	 - Add new chains to the configuration file from a directory full of chain configurations, useful for adding testnet configurations
+* [rly config add-paths](#rly-config-add-paths)	 - Add new paths to the configuration file from a directory full of path configurations, useful for adding testnet configurations. Must be called after adding chains and keys.
 * [rly config init](#rly-config-init)	 - Creates a default home directory at path defined by --home
 * [rly config show](#rly-config-show)	 - Prints current configuration
 
-## rly config add-dir
+## rly config add-chains
 
-Add new chains and paths to the configuration file from a directory full of chain and path configuration, useful for adding testnet configurations
+Add new chains to the configuration file from a directory full of chain configurations, useful for adding testnet configurations
 
 ### Synopsis
 
-Add new chains and paths to the configuration file from a directory full of chain and path configuration, useful for adding testnet configurations
+Add new chains to the configuration file from a directory full of chain configurations, useful for adding testnet configurations
+
+```
+rly config add-chains [dir] [flags]
+```
+
+## rly config add-paths
+
+Add new paths to the configuration file from a directory full of path configurations, useful for adding testnet configurations. Must be called after adding chains and keys.
+
+### Synopsis
+
+Add new paths to the configuration file from a directory full of path configurations, useful for adding testnet configurations. Must be called after adding chains and keys.
 
 ```
 rly config add-dir [dir] [flags]

scripts/nchainz

@@ -446,7 +446,7 @@ clients)
 
   echo "Generating rly configurations..."
   rly config init
-  rly config add-dir "$BASEDIR/config/"
+  rly config add-chains "$BASEDIR/config/chains"
 
   sleep 2
 
@@ -467,6 +467,7 @@ clients)
       done
       SEED=$(jq -r '.secret' "$f")
       echo "Key $(rly keys restore $chainid testkey "$SEED") imported from $chainid to relayer..."
+      rly config add-paths "$BASEDIR/config/paths"
 
       try=0
       f="$DATA/$chainid/n0/$DAEMON/config/genesis.json"

scripts/three-chainz

@@ -63,16 +63,19 @@ make install
 
 echo "Generating rly configurations..."
 rly config init
-rly config add-dir configs/three/
+rly config add-chains configs/three/chains
 
 SEED0=$(jq -r '.mnemonic' $GAIA_DATA/ibc-0/key_seed.json)
 SEED1=$(jq -r '.mnemonic' $GAIA_DATA/ibc-1/key_seed.json)
 SEED2=$(jq -r '.mnemonic' $GAIA_DATA/ibc-2/key_seed.json)
 echo "Key $(rly keys restore ibc-0 testkey "$SEED0") imported from ibc-0 to relayer..."
 echo "Key $(rly keys restore ibc-1 testkey "$SEED1") imported from ibc-1 to relayer..."
 echo "Key $(rly keys restore ibc-2 testkey "$SEED2") imported from ibc-2 to relayer..."
+
+rly config add-paths configs/three/paths
+
 echo "Creating light clients..."
 sleep 3
 rly light init ibc-0 -f
 rly light init ibc-1 -f
-rly light init ibc-2 -f
+rly light init ibc-2 -f

scripts/two-chainz

@@ -60,12 +60,15 @@ make install
 
 echo "Generating rly configurations..."
 rly config init
-rly config add-dir configs/demo/
+rly config add-chains configs/demo/chains
 
 SEED0=$(jq -r '.mnemonic' $GAIA_DATA/ibc-0/key_seed.json)
 SEED1=$(jq -r '.mnemonic' $GAIA_DATA/ibc-1/key_seed.json)
 echo "Key $(rly keys restore ibc-0 testkey "$SEED0") imported from ibc-0 to relayer..."
 echo "Key $(rly keys restore ibc-1 testkey "$SEED1") imported from ibc-1 to relayer..."
+
+rly config add-paths configs/demo/paths
+
 echo "Creating light clients..."
 sleep 3
 rly light init ibc-0 -f

scripts/two-chainz-akash

@@ -69,13 +69,16 @@ make install
 
 echo "Generating rly configurations..."
 rly config init
-rly config add-dir configs/akash/
+rly config add-chains configs/akash/chains
 
 SEED0=$(jq -r '.mnemonic' $GAIA_DATA/ibc-0/key_seed.json)
 SEED1=$(jq -r '.mnemonic' $GAIA_DATA/ibc-1/key_seed.json)
 echo "Key $(rly keys restore ibc-0 testkey "$SEED0") imported from ibc-0 to relayer..."
 echo "Key $(rly keys restore ibc-1 testkey "$SEED1") imported from ibc-1 to relayer..."
+
+rly config add-paths configs/akash/paths
+
 echo "Creating light clients..."
 sleep 3
 rly light init ibc-0 -f
-rly light init ibc-1 -f
+rly light init ibc-1 -f

scripts/two-chainz-wasmd

@@ -59,12 +59,15 @@ make install
 
 echo "Generating rly configurations..."
 rly config init
-rly config add-dir configs/wasmd/
+rly config add-chains configs/wasmd/chains
 
 SEED0=$(jq -r '.mnemonic' $WASMD_DATA/ibc-0/key_seed.json)
 SEED1=$(jq -r '.mnemonic' $WASMD_DATA/ibc-1/key_seed.json)
 echo "Key $(rly keys restore ibc-0 testkey "$SEED0") imported from ibc-0 to relayer..."
 echo "Key $(rly keys restore ibc-1 testkey "$SEED1") imported from ibc-1 to relayer..."
+
+rly config add-paths configs/wasmd/paths
+
 echo "Creating light clients..."
 sleep 3
 rly light init ibc-0 -f