Config update command

CHANGELOG.md

@@ -37,7 +37,10 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## Unreleased
 
-* nothing
+### Improvements
+
+* Alias the `config unpack` command to `config update`. It can be used to update config files to include new fields [PR 1233](https://github.com/provenance-io/provenance/pull/1233).
+* When loading the unpacked configs, always load the defaults before reading the files (instead of only loading the defaults if the file doesn't exist) [PR 1233](https://github.com/provenance-io/provenance/pull/1233).
 
 ---
 

cmd/provenanced/cmd/config.go

@@ -217,14 +217,17 @@ Settings that are their default value will not be included.
 // ConfigUnpackCmd returns a CLI command for creating the several config toml files.
 func ConfigUnpackCmd() *cobra.Command {
 	cmd := &cobra.Command{
-		Use:   "unpack",
-		Short: "Unpack configuration into separate config files",
+		Use:     "unpack",
+		Aliases: []string{"update"},
+		Short:   "Unpack configuration into separate config files",
 		Long: fmt.Sprintf(`Unpack configuration into separate config files.
 
 Splits the %[1]s file into %[2]s, %[3]s, and %[4]s.
 Settings defined through environment variables will be included in the unpacked files.
 Default values are filled in appropriately.
 
+This can also be used to update the config files using the current template so they include all current fields.
+
 `, provconfig.PackedConfFilename, provconfig.AppConfFilename, provconfig.TmConfFilename, provconfig.ClientConfFilename),
 		Example: fmt.Sprintf(`$ %[1]s unpack`, configCmdStart),
 		Args:    cobra.ExactArgs(0),
@@ -312,10 +315,17 @@ func runConfigGetCmd(cmd *cobra.Command, args []string) error {
 }
 
 // runConfigSetCmd sets values as provided.
-// The first return value is whether or not to include help with the output of an error.
+// The first return value is whether to include help with the output of an error.
 // This will only ever be true if an error is also returned.
 // The second return value is any error encountered.
 func runConfigSetCmd(cmd *cobra.Command, args []string) (bool, error) {
+	if len(args) == 0 {
+		return true, errors.New("no key/value pairs provided")
+	}
+	if len(args)%2 != 0 {
+		return true, errors.New("an even number of arguments are required when setting values")
+	}
+
 	// Warning: This wipes out all the viper setup stuff up to this point.
 	// It needs to be done so that just the file values or defaults are loaded
 	// without considering environment variables.
@@ -344,12 +354,6 @@ func runConfigSetCmd(cmd *cobra.Command, args []string) (bool, error) {
 		return false, fmt.Errorf("couldn't get client config: %w", ccerr)
 	}
 
-	if len(args) == 0 {
-		return true, errors.New("no key/value pairs provided")
-	}
-	if len(args)%2 != 0 {
-		return true, errors.New("an even number of arguments are required when setting values")
-	}
 	keyCount := len(args) / 2
 	keys := make([]string, keyCount)
 	vals := make([]string, keyCount)

cmd/provenanced/cmd/config_test.go

@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
+	"os"
 	"regexp"
 	"strings"
 	"testing"
@@ -49,6 +50,8 @@ func (s *ConfigTestSuite) SetupTest() {
 	s.Home = s.T().TempDir()
 	s.T().Logf("%s Home: %s", s.T().Name(), s.Home)
 
+	pioconfig.SetProvenanceConfig("confcoin", 5)
+
 	encodingConfig := sdksim.MakeTestEncodingConfig()
 	clientCtx := client.Context{}.
 		WithCodec(encodingConfig.Codec).
@@ -450,7 +453,7 @@ func (s *ConfigTestSuite) TestConfigChanged() {
 	}
 	expectedAppOutLines := []string{
 		s.makeAppDiffHeaderLines(),
-		fmt.Sprintf(`minimum-gas-prices="%s" (default="")`, pioconfig.GetProvenanceConfig().ProvenanceMinGasPrices),
+		allEqual("app"),
 		"",
 	}
 	expectedTMOutLines := []string{
@@ -753,9 +756,7 @@ func (s *ConfigTestSuite) TestConfigSetMulti() {
 
 func (s *ConfigTestSuite) TestPackUnpack() {
 	s.T().Run("pack", func(t *testing.T) {
-		expectedPacked := map[string]string{
-			"minimum-gas-prices": "1905nhash",
-		}
+		expectedPacked := map[string]string{}
 		expectedPackedJSON, jerr := json.MarshalIndent(expectedPacked, "", "  ")
 		require.NoError(t, jerr, "making expected json")
 		expectedPackedJSONStr := string(expectedPackedJSON)
@@ -810,3 +811,123 @@ func (s *ConfigTestSuite) TestPackUnpack() {
 		assert.True(t, provconfig.FileExists(clientFile), "file exists: client")
 	})
 }
+
+func (s *ConfigTestSuite) TestEmptyPackedConfigHasDefaultMinGas() {
+	expected := provconfig.DefaultAppConfig().MinGasPrices
+	s.Require().NotEqual("", expected, "default MinGasPrices")
+
+	// Pack the config, then rewrite the file to be an empty object.
+	pcmd := s.getConfigCmd()
+	pcmd.SetArgs([]string{"pack"})
+	err := pcmd.Execute()
+	s.Require().NoError(err, "pack the config")
+	s.Require().NoError(os.WriteFile(provconfig.GetFullPathToPackedConf(pcmd), []byte("{}"), 0o644), "writing empty packed config")
+
+	// Now read the config and check that the min gas prices are the default that we want.
+	ncmd := s.getConfigCmd()
+	err = provconfig.LoadConfigFromFiles(ncmd)
+	s.Require().NoError(err, "LoadConfigFromFiles")
+
+	appConfig, err := provconfig.ExtractAppConfig(ncmd)
+	s.Require().NoError(err, "ExtractAppConfig")
+	actual := appConfig.MinGasPrices
+	s.Assert().Equal(expected, actual, "MinGasPrices")
+}
+
+func (s *ConfigTestSuite) TestUpdate() {
+	// Write a dumb version of each file that is missing almost everything, but has an easily identifiable comment.
+	customComment := "# There's no way this is comment is in the real config."
+	uFileField := "bananas"
+	uFileValue := "no it's not"
+	uFileContents := fmt.Sprintf("%s\n%s = %q\n", customComment, uFileField, uFileValue)
+	dbBackend := "bananas"
+	tFileContents := fmt.Sprintf("%s\n%s = %q\n", customComment, "db_backend", dbBackend)
+	minGasPrices := "not a lot"
+	aFileContents := fmt.Sprintf("%s\n%s = %q\n", customComment, "minimum-gas-prices", minGasPrices)
+	chainId := "this-will-never-work"
+	cFileContents := fmt.Sprintf("%s\n%s = %q\n", customComment, "chain-id", chainId)
+	configCmd := s.getConfigCmd()
+	configDir := provconfig.GetFullPathToConfigDir(configCmd)
+	uFile := provconfig.GetFullPathToUnmanagedConf(configCmd)
+	tFile := provconfig.GetFullPathToTmConf(configCmd)
+	aFile := provconfig.GetFullPathToAppConf(configCmd)
+	cFile := provconfig.GetFullPathToClientConf(configCmd)
+	s.Require().NoError(os.MkdirAll(configDir, 0o755), "making config dir")
+	s.Require().NoError(os.WriteFile(uFile, []byte(uFileContents), 0o644), "writing unmanaged config")
+	s.Require().NoError(os.WriteFile(tFile, []byte(tFileContents), 0o644), "writing tm config")
+	s.Require().NoError(os.WriteFile(aFile, []byte(aFileContents), 0o644), "writing app config")
+	s.Require().NoError(os.WriteFile(cFile, []byte(cFileContents), 0o644), "writing client config")
+
+	// Load the config from files.
+	// Usually this is done in the pre-run handler, but that's defined in the root command,
+	// so this one doesn't know about it.
+	err := provconfig.LoadConfigFromFiles(configCmd)
+	s.Require().NoError(err, "loading config from files")
+
+	// Run the command!
+	args := []string{"update"}
+	configCmd.SetArgs(args)
+	err = configCmd.Execute()
+	s.Require().NoError(err, "%s %s - unexpected error in execution", configCmd.Name(), args)
+
+	s.Run("unmanaged file is unchanged", func() {
+		actualUFileContents, err := os.ReadFile(uFile)
+		s.Require().NoError(err, "reading unmanaged file: %s", uFile)
+		s.Assert().Equal(uFileContents, string(actualUFileContents), "unmanaged file contents")
+	})
+
+	s.Run("tm file has been updated", func() {
+		actualTFileContents, err := os.ReadFile(tFile)
+		s.Require().NoError(err, "reading tm file: %s", tFile)
+		s.Assert().NotEqual(tFileContents, string(actualTFileContents), "tm file contents")
+		lines := strings.Split(string(actualTFileContents), "\n")
+		s.Assert().Greater(len(lines), 2, "number of lines in tm file.")
+	})
+
+	s.Run("app file has been updated", func() {
+		actualAFileContents, err := os.ReadFile(aFile)
+		s.Require().NoError(err, "reading app file: %s", aFile)
+		s.Assert().NotEqual(aFileContents, string(actualAFileContents), "app file contents")
+		lines := strings.Split(string(actualAFileContents), "\n")
+		s.Assert().Greater(len(lines), 2, "number of lines in app file.")
+	})
+
+	s.Run("client file has been updated", func() {
+		actualCFileContents, err := os.ReadFile(cFile)
+		s.Require().NoError(err, "reading client file: %s", cFile)
+		s.Assert().NotEqual(aFileContents, string(actualCFileContents), "client file contents")
+		lines := strings.Split(string(actualCFileContents), "\n")
+		s.Assert().Greater(len(lines), 2, "number of lines in client file.")
+	})
+
+	err = provconfig.LoadConfigFromFiles(configCmd)
+	s.Require().NoError(err, "loading config from files")
+
+	s.Run("tm db_backend value unchanged", func() {
+		tmConfig, err := provconfig.ExtractTmConfig(configCmd)
+		s.Require().NoError(err, "ExtractTmConfig")
+		actual := tmConfig.DBBackend
+		s.Assert().Equal(dbBackend, actual, "DBBackend")
+	})
+
+	s.Run("app minimum-gas-prices value unchanged", func() {
+		appConfig, err := provconfig.ExtractAppConfig(configCmd)
+		s.Require().NoError(err, "ExtractAppConfig")
+		actual := appConfig.MinGasPrices
+		s.Assert().Equal(minGasPrices, actual, "MinGasPrices")
+	})
+
+	s.Run("client chain-id value unchanged", func() {
+		clientConfig, err := provconfig.ExtractClientConfig(configCmd)
+		s.Require().NoError(err, "ExtractClientConfig")
+		actual := clientConfig.ChainID
+		s.Assert().Equal(chainId, actual, "ChainID")
+	})
+
+	s.Run("unmanaged config entry still applies", func() {
+		ctx := client.GetClientContextFromCmd(configCmd)
+		vpr := ctx.Viper
+		actual := vpr.GetString(uFileField)
+		s.Assert().Equal(uFileValue, actual, "unmanaged config entry")
+	})
+}

cmd/provenanced/config/manager.go

@@ -17,6 +17,8 @@ import (
 	"github.com/cosmos/cosmos-sdk/client/flags"
 	"github.com/cosmos/cosmos-sdk/server"
 	serverconfig "github.com/cosmos/cosmos-sdk/server/config"
+
+	"github.com/provenance-io/provenance/internal/pioconfig"
 )
 
 // PackConfig generates and saves the packed config file then removes the individual config files.
@@ -63,7 +65,7 @@ func FileExists(fullFilePath string) bool {
 // ExtractAppConfig creates an app/cosmos config from the command context.
 func ExtractAppConfig(cmd *cobra.Command) (*serverconfig.Config, error) {
 	v := server.GetServerContextFromCmd(cmd).Viper
-	conf := serverconfig.DefaultConfig()
+	conf := DefaultAppConfig()
 	if err := v.Unmarshal(conf); err != nil {
 		return nil, fmt.Errorf("error extracting app config: %w", err)
 	}
@@ -141,11 +143,18 @@ func ExtractClientConfigAndMap(cmd *cobra.Command) (*ClientConfig, FieldValueMap
 	return conf, fields, nil
 }
 
+// DefaultAppConfig gets our default app config.
+func DefaultAppConfig() *serverconfig.Config {
+	rv := serverconfig.DefaultConfig()
+	rv.MinGasPrices = pioconfig.GetProvenanceConfig().ProvenanceMinGasPrices
+	return rv
+}
+
 // GetAllConfigDefaults gets a field map from the defaults of all the configs.
 func GetAllConfigDefaults() FieldValueMap {
 	rv := FieldValueMap{}
 	rv.AddEntriesFrom(
-		MakeFieldValueMap(serverconfig.DefaultConfig(), false),
+		MakeFieldValueMap(DefaultAppConfig(), false),
 		removeUndesirableTmConfigEntries(MakeFieldValueMap(tmconfig.DefaultConfig(), false)),
 		MakeFieldValueMap(DefaultClientConfig(), false),
 	)
@@ -400,13 +409,14 @@ func loadUnpackedConfig(cmd *cobra.Command) error {
 	// Both the server context and client context should be using the same Viper, so this is good for both.
 	vpr := server.GetServerContextFromCmd(cmd).Viper
 
-	// Load the tendermint config if it exists, or else defaults.
+	// Load the tendermint config defaults, then file if it exists.
+	tdErr := addFieldMapToViper(vpr, MakeFieldValueMap(tmconfig.DefaultConfig(), false))
+	if tdErr != nil {
+		return fmt.Errorf("tendermint config defaults load error: %w", tdErr)
+	}
 	switch _, err := os.Stat(tmConfFile); {
 	case os.IsNotExist(err):
-		lerr := addFieldMapToViper(vpr, MakeFieldValueMap(tmconfig.DefaultConfig(), false))
-		if lerr != nil {
-			return fmt.Errorf("tendermint config file load error: %w", lerr)
-		}
+		// Do nothing.
 	case err != nil:
 		return fmt.Errorf("tendermint config file stat error: %w", err)
 	default:
@@ -417,13 +427,14 @@ func loadUnpackedConfig(cmd *cobra.Command) error {
 		}
 	}
 
-	// Load the app/cosmos config if it exists, or else defaults.
+	// Load the app/cosmos config defaults, then file if it exists.
+	adErr := addFieldMapToViper(vpr, MakeFieldValueMap(DefaultAppConfig(), false))
+	if adErr != nil {
+		return fmt.Errorf("app config defaults load error: %w", adErr)
+	}
 	switch _, err := os.Stat(appConfFile); {
 	case os.IsNotExist(err):
-		lerr := addFieldMapToViper(vpr, MakeFieldValueMap(serverconfig.DefaultConfig(), false))
-		if lerr != nil {
-			return fmt.Errorf("app config load error: %w", lerr)
-		}
+		// Do nothing.
 	case err != nil:
 		return fmt.Errorf("app config file stat error: %w", err)
 	default:
@@ -434,13 +445,14 @@ func loadUnpackedConfig(cmd *cobra.Command) error {
 		}
 	}
 
-	// Load the client config if it exists, or else defaults.
+	// Load the client config defaults, then file if it exists.
+	cdErr := addFieldMapToViper(vpr, MakeFieldValueMap(DefaultClientConfig(), false))
+	if cdErr != nil {
+		return fmt.Errorf("client config defaults load error: %w", cdErr)
+	}
 	switch _, err := os.Stat(clientConfFile); {
 	case os.IsNotExist(err):
-		lerr := addFieldMapToViper(vpr, MakeFieldValueMap(DefaultClientConfig(), false))
-		if lerr != nil {
-			return fmt.Errorf("client config file load error: %w", lerr)
-		}
+		// Do nothing.
 	case err != nil:
 		return fmt.Errorf("client config file stat error: %w", err)
 	default:
@@ -474,7 +486,7 @@ func loadPackedConfig(cmd *cobra.Command) error {
 	}
 
 	// Start with the defaults
-	appConfigMap := MakeFieldValueMap(serverconfig.DefaultConfig(), false)
+	appConfigMap := MakeFieldValueMap(DefaultAppConfig(), false)
 	tmConfigMap := MakeFieldValueMap(tmconfig.DefaultConfig(), false)
 	clientConfigMap := MakeFieldValueMap(DefaultClientConfig(), false)
 
@@ -530,28 +542,9 @@ func loadPackedConfig(cmd *cobra.Command) error {
 }
 
 func addFieldMapToViper(vpr *viper.Viper, fvmap FieldValueMap) error {
-	configMap := make(map[string]interface{})
-	for k, v := range fvmap {
-		configMap[k] = v.Interface()
-	}
-	// The telemetry.global-labels field in the app config struct is a `[][]string`.
-	// But in serverconfig.GetConfig, it expects viper to return it as a `[]interface{}`.
-	// Then each element of that is expected to also be a `[]interface{}`.
-	// So we need to convert that field before adding it to viper.
-	if gli, hasGL := configMap["telemetry.global-labels"]; hasGL {
-		newv := make([]interface{}, 0)
-		if gli != nil {
-			if gl, ok := gli.([][]string); ok {
-				for _, p := range gl {
-					var newp []interface{}
-					for _, k := range p {
-						newp = append(newp, k)
-					}
-					newv = append(newv, newp)
-				}
-			}
-		}
-		configMap["telemetry.global-labels"] = newv
+	configMap, err := fvmap.AsConfigMap()
+	if err != nil {
+		return err
 	}
 	return vpr.MergeConfigMap(configMap)
 }