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

server: fix interceptConfigs #8641

Merged
merged 11 commits into from Feb 22, 2021
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -64,6 +64,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/evidence) [#8461](https://github.com/cosmos/cosmos-sdk/pull/8461) Fix bech32 prefix in evidence validator address conversion
* (x/slashing) [\#8427](https://github.com/cosmos/cosmos-sdk/pull/8427) Fix query signing infos command
* (server) [\#8399](https://github.com/cosmos/cosmos-sdk/pull/8399) fix gRPC-web flag default value
* (server) [\#8641](https://github.com/cosmos/cosmos-sdk/pull/8641) Fix Tendermint and application configuration reading from file

## [v0.41.1](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.41.1) - 2021-02-17

Expand Down
29 changes: 16 additions & 13 deletions server/util.go
Expand Up @@ -184,11 +184,11 @@ func SetCmdServerContext(cmd *cobra.Command, serverCtx *Context) error {
func interceptConfigs(rootViper *viper.Viper) (*tmcfg.Config, error) {
rootDir := rootViper.GetString(flags.FlagHome)
configPath := filepath.Join(rootDir, "config")
configFile := filepath.Join(configPath, "config.toml")
tmCfgFile := filepath.Join(configPath, "config.toml")

conf := tmcfg.DefaultConfig()

switch _, err := os.Stat(configFile); {
switch _, err := os.Stat(tmCfgFile); {
case os.IsNotExist(err):
tmcfg.EnsureRoot(rootDir)

Expand All @@ -200,7 +200,7 @@ func interceptConfigs(rootViper *viper.Viper) (*tmcfg.Config, error) {
conf.P2P.RecvRate = 5120000
conf.P2P.SendRate = 5120000
conf.Consensus.TimeoutCommit = 5 * time.Second
tmcfg.WriteConfigFile(configFile, conf)
tmcfg.WriteConfigFile(tmCfgFile, conf)

case err != nil:
return nil, err
Expand All @@ -209,34 +209,37 @@ func interceptConfigs(rootViper *viper.Viper) (*tmcfg.Config, error) {
rootViper.SetConfigType("toml")
rootViper.SetConfigName("config")
rootViper.AddConfigPath(configPath)

if err := rootViper.ReadInConfig(); err != nil {
return nil, fmt.Errorf("failed to read in config.toml: %w", err)
return nil, fmt.Errorf("failed to read in %s: %w", tmCfgFile, err)
}
}

// Read into the configuration whatever data the viper instance has for it
// This may come from the configuration file above but also any of the other sources
// viper uses
// Read into the configuration whatever data the viper instance has for it.
// This may come from the configuration file above but also any of the other
// sources viper uses.
if err := rootViper.Unmarshal(conf); err != nil {
return nil, err
}

conf.SetRoot(rootDir)

appConfigFilePath := filepath.Join(configPath, "app.toml")
if _, err := os.Stat(appConfigFilePath); os.IsNotExist(err) {
appCfgFilePath := filepath.Join(configPath, "app.toml")
if _, err := os.Stat(appCfgFilePath); os.IsNotExist(err) {
appConf, err := config.ParseConfig(rootViper)
if err != nil {
return nil, fmt.Errorf("failed to parse app.toml: %w", err)
return nil, fmt.Errorf("failed to parse %s: %w", appCfgFilePath, err)
}

config.WriteConfigFile(appConfigFilePath, appConf)
config.WriteConfigFile(appCfgFilePath, appConf)
}

rootViper.SetConfigType("toml")
rootViper.SetConfigName("app")
rootViper.AddConfigPath(configPath)
if err := rootViper.ReadInConfig(); err != nil {
return nil, fmt.Errorf("failed to read in app.toml: %w", err)

if err := rootViper.MergeInConfig(); err != nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was causing any user value set in Tendermint's config to be wiped out. We need to merge configurations, not overwrite them.

return nil, fmt.Errorf("failed to merge configuration: %w", err)
}

return conf, nil
Expand Down
70 changes: 0 additions & 70 deletions server/util_test.go
Expand Up @@ -162,76 +162,6 @@ func TestInterceptConfigsPreRunHandlerReadsAppToml(t *testing.T) {
}
}

func TestInterceptConfigsPreRunHandlerDoesNotMixConfigFiles(t *testing.T) {
// The goal of this test is to make sure that app.toml and config.toml
// are separate files and that mixing values does not work
const testDbBackend = "awesome_test_db"
const testHaltTime = 1337
const testHaltHeight = 2001

tempDir := t.TempDir()
err := os.Mkdir(path.Join(tempDir, "config"), os.ModePerm)
if err != nil {
t.Fatalf("creating config dir failed: %v", err)
}
configTomlPath := path.Join(tempDir, "config", "config.toml")
writer, err := os.Create(configTomlPath)
if err != nil {
t.Fatalf("creating config.toml file failed: %v", err)
}

// Put a value in config.toml that should be in app.toml
_, err = writer.WriteString(fmt.Sprintf("halt-time = %d\ndb_backend = \"%s\"\n", testHaltTime, testDbBackend))
if err != nil {
t.Fatalf("Failed writing string to config.toml: %v", err)
}

if err := writer.Close(); err != nil {
t.Fatalf("Failed closing config.toml: %v", err)
}

appTomlPath := path.Join(tempDir, "config", "app.toml")
writer, err = os.Create(appTomlPath)
if err != nil {
t.Fatalf("creating app.toml file failed %v", err)
}

// Put a different value in app.toml
_, err = writer.WriteString(fmt.Sprintf("halt-height = %d\n", testHaltHeight))
if err != nil {
t.Fatalf("Failed writing string to app.toml: %v", err)
}

if err := writer.Close(); err != nil {
t.Fatalf("Failed closing app.toml: %v", err)
}

cmd := StartCmd(nil, tempDir)
cmd.PreRunE = preRunETestImpl

serverCtx := &Context{}
ctx := context.WithValue(context.Background(), ServerContextKey, serverCtx)

if err := cmd.ExecuteContext(ctx); err != CancelledInPreRun {
t.Fatalf("function failed with [%T] %v", err, err)
}

// check that the intended value from config.toml is used
if testDbBackend != serverCtx.Config.DBBackend {
t.Error("DBPath was not set from config.toml")
}

// The value from app.toml should be used for this
if testHaltHeight != serverCtx.Viper.GetInt("halt-height") {
t.Error("Halt height is not using provided value")
}

// The value from config.toml should not be used, default is used instead
if 0 != serverCtx.Viper.GetInt("halt-time") {
t.Error("Halt time is not using default")
}
}

func TestInterceptConfigsPreRunHandlerReadsFlags(t *testing.T) {
const testAddr = "tcp://127.1.2.3:12345"
tempDir := t.TempDir()
Expand Down