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 (bp #8641) #8659

Merged
merged 3 commits into from
Feb 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,16 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Improvements
Copy link
Contributor

Choose a reason for hiding this comment

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

@alessio there is no pending section in this file for the next 0.41 release. Where do these entries go?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah I'm about to open a PR for that

Copy link
Contributor

Choose a reason for hiding this comment

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

@alexanderbez for now just don't add an entry. I think it's much easier to prep and finalize both changelog and release notes at release time. So we do the work only once. Wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree. But the changelog entries need to be tracked somewhere, right? How does the release manager know what to add?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

All merged PRs must be included in the release milestone


<<<<<<< HEAD
* Bump tendermint dependency to v0.34.7.
=======
### Bug Fixes

* (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
>>>>>>> 2f069c90b... server: fix interceptConfigs (#8641)

## [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
Original file line number Diff line number Diff line change
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 app.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 {
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
Original file line number Diff line number Diff line change
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