From 390548fbc2a648c46a68c92742d7551ae569c6cf Mon Sep 17 00:00:00 2001 From: Matthew Witkowski Date: Tue, 12 Sep 2023 16:46:20 -0400 Subject: [PATCH 1/6] Initial commits for change. Still need to verify everything works and fix tests. --- cmd/provenanced/config/manager.go | 8 ++++---- cmd/provenanced/config/manager_test.go | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cmd/provenanced/config/manager.go b/cmd/provenanced/config/manager.go index 0320da3fd7..a80d76b54a 100644 --- a/cmd/provenanced/config/manager.go +++ b/cmd/provenanced/config/manager.go @@ -386,13 +386,13 @@ func unquote(str string) string { // LoadConfigFromFiles loads configurations appropriately. func LoadConfigFromFiles(cmd *cobra.Command) error { - if err := loadUnmanagedConfig(cmd); err != nil { - return err - } if IsPacked(cmd) { return loadPackedConfig(cmd) } - return loadUnpackedConfig(cmd) + if err := loadUnpackedConfig(cmd); err != nil { + return err + } + return loadUnmanagedConfig(cmd) } // loadUnmanagedConfig reads the unmanaged config file into viper. It does not apply anything to any contexts though. diff --git a/cmd/provenanced/config/manager_test.go b/cmd/provenanced/config/manager_test.go index 46cf7d1a65..9188d141e3 100644 --- a/cmd/provenanced/config/manager_test.go +++ b/cmd/provenanced/config/manager_test.go @@ -170,7 +170,7 @@ func (s *ConfigManagerTestSuite) TestUnmanagedConfig() { assert.Equal(t, "bananas", actual, "unmanaged field value") }) - s.T().Run("unmanaged config entry does not override other config", func(t *testing.T) { + s.T().Run("unmanaged config entry does override other config", func(t *testing.T) { dCmd := s.makeDummyCmd() configDir := GetFullPathToConfigDir(dCmd) uFile := GetFullPathToUnmanagedConf(dCmd) @@ -180,8 +180,8 @@ func (s *ConfigManagerTestSuite) TestUnmanagedConfig() { ctx := client.GetClientContextFromCmd(dCmd) vpr := ctx.Viper actual := vpr.GetString("db_backend") - assert.NotEqual(t, "still bananas", actual, "unmanaged field value") - assert.Equal(t, DefaultTmConfig().DBBackend, actual, "unmanaged field default value") + assert.Equal(t, "still bananas", actual, "unmanaged field value") + assert.NotEqual(t, DefaultTmConfig().DBBackend, actual, "unmanaged field default value") }) s.T().Run("unmanaged config is read with unpacked files", func(t *testing.T) { From 313e435d88b41fa6a551e5d1e9fec78187fec508 Mon Sep 17 00:00:00 2001 From: Matthew Witkowski Date: Wed, 13 Sep 2023 13:34:35 -0400 Subject: [PATCH 2/6] Fix test name. --- cmd/provenanced/config/manager_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/provenanced/config/manager_test.go b/cmd/provenanced/config/manager_test.go index 9188d141e3..042b50289f 100644 --- a/cmd/provenanced/config/manager_test.go +++ b/cmd/provenanced/config/manager_test.go @@ -170,7 +170,7 @@ func (s *ConfigManagerTestSuite) TestUnmanagedConfig() { assert.Equal(t, "bananas", actual, "unmanaged field value") }) - s.T().Run("unmanaged config entry does override other config", func(t *testing.T) { + s.T().Run("unmanaged config entry overrides other config", func(t *testing.T) { dCmd := s.makeDummyCmd() configDir := GetFullPathToConfigDir(dCmd) uFile := GetFullPathToUnmanagedConf(dCmd) From 57960d6d40c6f08a8104ab973184e6ca1bf29e08 Mon Sep 17 00:00:00 2001 From: Matthew Witkowski Date: Wed, 13 Sep 2023 13:35:14 -0400 Subject: [PATCH 3/6] Fix LoadConfig to either load unpacked or packed, and then be overwritten by the unmanaged config. --- cmd/provenanced/config/manager.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/cmd/provenanced/config/manager.go b/cmd/provenanced/config/manager.go index a80d76b54a..0c4ca6bb4a 100644 --- a/cmd/provenanced/config/manager.go +++ b/cmd/provenanced/config/manager.go @@ -385,11 +385,13 @@ func unquote(str string) string { } // LoadConfigFromFiles loads configurations appropriately. -func LoadConfigFromFiles(cmd *cobra.Command) error { +func LoadConfigFromFiles(cmd *cobra.Command) (err error) { if IsPacked(cmd) { - return loadPackedConfig(cmd) + err = loadPackedConfig(cmd) + } else { + err = loadUnpackedConfig(cmd) } - if err := loadUnpackedConfig(cmd); err != nil { + if err != nil { return err } return loadUnmanagedConfig(cmd) From acbb807e9798234caca11c737c321ad7fc4a0b90 Mon Sep 17 00:00:00 2001 From: Matthew Witkowski Date: Wed, 13 Sep 2023 14:54:33 -0400 Subject: [PATCH 4/6] Add missing test cases that weren't covered before. --- cmd/provenanced/config/manager_test.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/cmd/provenanced/config/manager_test.go b/cmd/provenanced/config/manager_test.go index 042b50289f..1288f3b0ac 100644 --- a/cmd/provenanced/config/manager_test.go +++ b/cmd/provenanced/config/manager_test.go @@ -196,6 +196,27 @@ func (s *ConfigManagerTestSuite) TestUnmanagedConfig() { assert.Equal(t, "stuff", actual, "unmanaged field value") }) + s.T().Run("unmanaged config is read with invalid packed files", func(t *testing.T) { + dCmd := s.makeDummyCmd() + uFile := GetFullPathToUnmanagedConf(dCmd) + pFile := GetFullPathToPackedConf(dCmd) + SaveConfigs(dCmd, DefaultAppConfig(), DefaultTmConfig(), DefaultClientConfig(), false) + require.NoError(t, os.WriteFile(uFile, []byte("my-custom-entry = \"stuff\"\n"), 0o644), "writing unmanaged config") + require.NoError(t, os.WriteFile(pFile, []byte("kl234508923u5jl"), 0o644), "writing invalid data to packed config") + require.EqualError(t, LoadConfigFromFiles(dCmd), "packed config file parse error: invalid character 'k' looking for beginning of value", "should throw error with invalid packed config") + require.NoError(t, os.Remove(pFile), "removing packed config") + }) + + s.T().Run("unmanaged config is read with invalid unpacked files", func(t *testing.T) { + dCmd := s.makeDummyCmd() + uFile := GetFullPathToUnmanagedConf(dCmd) + pFile := GetFullPathToAppConf(dCmd) + SaveConfigs(dCmd, DefaultAppConfig(), DefaultTmConfig(), DefaultClientConfig(), false) + require.NoError(t, os.WriteFile(uFile, []byte("my-custom-entry = \"stuff\"\n"), 0o644), "writing unmanaged config") + require.NoError(t, os.WriteFile(pFile, []byte("kl234508923u5jl"), 0o644), "writing invalid data to app config") + require.EqualError(t, LoadConfigFromFiles(dCmd), "app config file merge error: While parsing config: toml: expected = after a key, but the document ends there", "should throw error with invalid packed config") + }) + s.T().Run("unmanaged config is read with packed config", func(t *testing.T) { dCmd := s.makeDummyCmd() uFile := GetFullPathToUnmanagedConf(dCmd) From 799f9f8321db75dceda9319b6fe302a33ad060dc Mon Sep 17 00:00:00 2001 From: Matthew Witkowski Date: Fri, 15 Sep 2023 11:16:13 -0400 Subject: [PATCH 5/6] Add changelog entry. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 664cc3efbe..d3b8eec1b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * Bump cometbft to v0.34.29 (from v0.34.28) [PR 1649](https://github.com/provenance-io/provenance/pull/1649). * Add genesis/init for Marker module send deny list addresses. [#1660](https://github.com/provenance-io/provenance/issues/1660) * Add automatic changelog entries for dependabot. [#1674](https://github.com/provenance-io/provenance/issues/1674) +* Change config load order so custom.toml is last [#1262](https://github.com/provenance-io/provenance/issues/1262) ### Bug Fixes From d256fca3c1e8e97ab8eced4652f653d8123ea82a Mon Sep 17 00:00:00 2001 From: Matthew Witkowski Date: Fri, 15 Sep 2023 11:17:29 -0400 Subject: [PATCH 6/6] Move changelog entry to bug fixes. --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d3b8eec1b9..9da6e32e0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,7 +63,6 @@ Ref: https://keepachangelog.com/en/1.0.0/ * Bump cometbft to v0.34.29 (from v0.34.28) [PR 1649](https://github.com/provenance-io/provenance/pull/1649). * Add genesis/init for Marker module send deny list addresses. [#1660](https://github.com/provenance-io/provenance/issues/1660) * Add automatic changelog entries for dependabot. [#1674](https://github.com/provenance-io/provenance/issues/1674) -* Change config load order so custom.toml is last [#1262](https://github.com/provenance-io/provenance/issues/1262) ### Bug Fixes @@ -71,6 +70,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * Fix for incorrect resource-id type casting on contract specification [#1647](https://github.com/provenance-io/provenance/issues/1647). * Allow restricted coins to be quarantined [#1626](https://github.com/provenance-io/provenance/issues/1626). * Prevent marker forced transfers from module accounts [#1626](https://github.com/provenance-io/provenance/issues/1626). +* Change config load order so custom.toml can override other config. [#1262](https://github.com/provenance-io/provenance/issues/1262) ### Client Breaking