Skip to content

Commit 74f2295

Browse files
authored
fix: Prevent overriding config with default flag values (#2642)
Closes: evstack/ev-abci#245 The ignite app should be bumped and ev-abci jobs should be bumped after this.
1 parent f415cd3 commit 74f2295

File tree

2 files changed

+43
-0
lines changed

2 files changed

+43
-0
lines changed

pkg/config/config.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,11 @@ func LoadFromViper(inputViper *viper.Viper) (Config, error) {
402402

403403
// then override with settings from input viper (higher precedence)
404404
for _, key := range inputViper.AllKeys() {
405+
// we must not override config with default flags value
406+
if !inputViper.IsSet(key) {
407+
continue
408+
}
409+
405410
// Handle special case for prefixed keys
406411
if after, ok := strings.CutPrefix(key, FlagPrefixEvnode); ok {
407412
// Strip the prefix for the merged viper

pkg/config/config_test.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,44 @@ signer:
279279
require.Equal(t, "http://yaml-da:26657", cfgFromViper.DA.Address, "DA.Address should match YAML")
280280
require.Equal(t, "file", cfgFromViper.Signer.SignerType, "Signer.SignerType should match YAML")
281281
require.Equal(t, "something/config", cfgFromViper.Signer.SignerPath, "Signer.SignerPath should match YAML")
282+
283+
// Test that default flag values don't override config values when not explicitly set
284+
t.Run("default flag values don't override config when not explicitly set", func(t *testing.T) {
285+
// Create a viper instance that simulates having default values but IsSet() returns false
286+
flagViper := viper.New()
287+
flagViper.Set(FlagRootDir, tempDir)
288+
289+
// Create a command and parse empty args to simulate flags with defaults but not explicitly set
290+
cmd = &cobra.Command{Use: "test"}
291+
AddFlags(cmd)
292+
AddGlobalFlags(cmd, "test")
293+
294+
// Bind the flags to viper - this populates viper with default values
295+
// but since no args were provided, IsSet() will return false for these flags
296+
err = flagViper.BindPFlags(cmd.Flags())
297+
require.NoError(t, err)
298+
err = flagViper.BindPFlags(cmd.PersistentFlags())
299+
require.NoError(t, err)
300+
301+
// Parse empty command line - this means all flags have default values but IsSet() = false
302+
err = cmd.ParseFlags([]string{})
303+
require.NoError(t, err)
304+
305+
// Sanity check.
306+
// Verify that IsSet returns false for flag values (this is the key behavior we're testing)
307+
require.False(t, flagViper.IsSet("rollkit.node.aggregator"), "Flag should not be considered 'set' when using default")
308+
require.False(t, flagViper.IsSet("rollkit.node.block_time"), "Flag should not be considered 'set' when using default")
309+
require.False(t, flagViper.IsSet("rollkit.da.address"), "Flag should not be considered 'set' when using default")
310+
311+
cfgFromViper, err = LoadFromViper(flagViper)
312+
require.NoError(t, err)
313+
314+
// These values should come from YAML, not be overridden by flag defaults
315+
require.True(t, cfgFromViper.Node.Aggregator, "Node.Aggregator should remain true from YAML, not overridden by flag default")
316+
require.Equal(t, "5s", cfgFromViper.Node.BlockTime.String(), "Node.BlockTime should remain 5s from YAML, not overridden by flag default")
317+
require.Equal(t, "http://yaml-da:26657", cfgFromViper.DA.Address, "DA.Address should remain from YAML, not overridden by flag default")
318+
require.Equal(t, "something/config", cfgFromViper.Signer.SignerPath, "Signer.SignerPath should remain from YAML, not overridden by flag default")
319+
})
282320
}
283321

284322
func TestDAConfig_GetDataNamespace(t *testing.T) {

0 commit comments

Comments
 (0)