From ee9fd178aff52f0379b48246a2a371673983c504 Mon Sep 17 00:00:00 2001 From: krehermann Date: Mon, 13 Feb 2023 09:23:38 -0800 Subject: [PATCH] Support dynamic TOML (#8421) --- core/cmd/app.go | 40 ++++++++------ core/cmd/app_test.go | 128 +++++++++++++++++++++++++++++++++++++++++++ core/main_test.go | 2 +- docs/CHANGELOG.md | 2 + 4 files changed, 154 insertions(+), 18 deletions(-) create mode 100644 core/cmd/app_test.go diff --git a/core/cmd/app.go b/core/cmd/app.go index e3a86b019b5..153af82df63 100644 --- a/core/cmd/app.go +++ b/core/cmd/app.go @@ -71,7 +71,7 @@ func NewApp(client *Client) *cli.App { }, cli.StringSliceFlag{ Name: "config, c", - Usage: "TOML configuration file(s) via flag, or raw TOML via env var. If used, legacy env vars must not be set. Multiple files can be used (-c configA.toml -c configB.toml), and they are applied in order with duplicated fields overriding any earlier values.", + Usage: "TOML configuration file(s) via flag, or raw TOML via env var. If used, legacy env vars must not be set. Multiple files can be used (-c configA.toml -c configB.toml), and they are applied in order with duplicated fields overriding any earlier values. If the env var is specified, it is always processed last with the effect of being the final override.", EnvVar: "CL_CONFIG", }, cli.StringFlag{ @@ -83,22 +83,9 @@ func NewApp(client *Client) *cli.App { if c.IsSet("config") { // TOML var opts chainlink.GeneralConfigOpts - if configTOML := v2.EnvConfig.Get(); configTOML != "" { - if err := opts.ParseConfig(configTOML); err != nil { - return errors.Wrapf(err, "failed to parse env var %q", v2.EnvConfig) - } - } else { - fileNames := c.StringSlice("config") - for _, fileName := range fileNames { - b, err := os.ReadFile(fileName) - if err != nil { - return errors.Wrapf(err, "failed to read config file: %s", fileName) - } - if err := opts.ParseConfig(string(b)); err != nil { - return errors.Wrapf(err, "failed to parse file: %s", fileName) - } - } - } + + fileNames := c.StringSlice("config") + loadOpts(&opts, fileNames...) secretsTOML := "" if c.IsSet("secrets") { @@ -1231,3 +1218,22 @@ func logDeprecatedClientEnvWarnings(lggr logger.Logger) { lggr.Errorf("ADMIN_CREDENTIALS_FILE env var has been deprecated and will be removed in a future release. Use flag instead: --admin-credentials-file=%s", s) } } + +// loadOpts applies file configs and then overlays env config +func loadOpts(opts *chainlink.GeneralConfigOpts, fileNames ...string) error { + for _, fileName := range fileNames { + b, err := os.ReadFile(fileName) + if err != nil { + return errors.Wrapf(err, "failed to read config file: %s", fileName) + } + if err := opts.ParseConfig(string(b)); err != nil { + return errors.Wrapf(err, "failed to parse file: %s", fileName) + } + } + if configTOML := v2.EnvConfig.Get(); configTOML != "" { + if err := opts.ParseConfig(configTOML); err != nil { + return errors.Wrapf(err, "failed to parse env var %q", v2.EnvConfig) + } + } + return nil +} diff --git a/core/cmd/app_test.go b/core/cmd/app_test.go new file mode 100644 index 00000000000..07203bf232c --- /dev/null +++ b/core/cmd/app_test.go @@ -0,0 +1,128 @@ +package cmd + +import ( + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/pelletier/go-toml/v2" + v2 "github.com/smartcontractkit/chainlink/core/config/v2" + "github.com/smartcontractkit/chainlink/core/services/chainlink" + "github.com/stretchr/testify/require" +) + +var ( + setInFile = "set in config file" + setInEnv = "set in env" + + testEnvContents = fmt.Sprintf("P2P.V2.AnnounceAddresses = ['%s']", setInEnv) + + testConfigFileContents = chainlink.Config{ + Core: v2.Core{ + RootDir: &setInFile, + P2P: v2.P2P{ + V2: v2.P2PV2{ + AnnounceAddresses: &[]string{setInFile}, + ListenAddresses: &[]string{setInFile}, + }, + }, + }, + } +) + +func makeTestConfigFile(t *testing.T) string { + d := t.TempDir() + p := filepath.Join(d, "test.toml") + + b, err := toml.Marshal(testConfigFileContents) + require.NoError(t, err) + + require.NoError(t, os.WriteFile(p, b, 0777)) + return p +} + +func Test_loadOpts(t *testing.T) { + type args struct { + opts *chainlink.GeneralConfigOpts + fileNames []string + envVar string + } + tests := []struct { + name string + args args + wantErr bool + wantOpts *chainlink.GeneralConfigOpts + }{ + { + name: "env only", + args: args{ + opts: new(chainlink.GeneralConfigOpts), + envVar: testEnvContents, + }, + wantOpts: &chainlink.GeneralConfigOpts{ + Config: chainlink.Config{ + Core: v2.Core{ + P2P: v2.P2P{ + V2: v2.P2PV2{ + AnnounceAddresses: &[]string{setInEnv}, + }, + }, + }, + }, + }, + }, + + { + name: "files only", + args: args{ + opts: new(chainlink.GeneralConfigOpts), + fileNames: []string{makeTestConfigFile(t)}, + }, + wantOpts: &chainlink.GeneralConfigOpts{ + Config: testConfigFileContents, + }, + }, + { + name: "file error", + args: args{ + opts: new(chainlink.GeneralConfigOpts), + fileNames: []string{"notexist"}, + }, + wantErr: true, + }, + + { + name: "env overlay of file", + args: args{ + opts: new(chainlink.GeneralConfigOpts), + fileNames: []string{makeTestConfigFile(t)}, + envVar: testEnvContents, + }, + wantOpts: &chainlink.GeneralConfigOpts{ + Config: chainlink.Config{ + Core: v2.Core{ + RootDir: &setInFile, + P2P: v2.P2P{ + V2: v2.P2PV2{ + // env should override this specific field + AnnounceAddresses: &[]string{setInEnv}, + ListenAddresses: &[]string{setInFile}, + }, + }, + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.args.envVar != "" { + t.Setenv(string(v2.EnvConfig), tt.args.envVar) + } + if err := loadOpts(tt.args.opts, tt.args.fileNames...); (err != nil) != tt.wantErr { + t.Errorf("loadOpts() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/core/main_test.go b/core/main_test.go index d0d8b173084..843549ff68f 100644 --- a/core/main_test.go +++ b/core/main_test.go @@ -76,7 +76,7 @@ func ExampleRun() { // --admin-credentials-file FILE optional, applies only in client mode when making remote API calls. If provided, FILE containing admin credentials will be used for logging in, allowing to avoid an additional login step. If `FILE` is missing, it will be ignored. Defaults to /apicredentials // --remote-node-url URL optional, applies only in client mode when making remote API calls. If provided, URL will be used as the remote Chainlink API endpoint (default: "http://localhost:6688") // --insecure-skip-verify optional, applies only in client mode when making remote API calls. If turned on, SSL certificate verification will be disabled. This is mostly useful for people who want to use Chainlink with a self-signed TLS certificate - // --config value, -c value TOML configuration file(s) via flag, or raw TOML via env var. If used, legacy env vars must not be set. Multiple files can be used (-c configA.toml -c configB.toml), and they are applied in order with duplicated fields overriding any earlier values. [$CL_CONFIG] + // --config value, -c value TOML configuration file(s) via flag, or raw TOML via env var. If used, legacy env vars must not be set. Multiple files can be used (-c configA.toml -c configB.toml), and they are applied in order with duplicated fields overriding any earlier values. If the env var is specified, it is always processed last with the effect of being the final override. [$CL_CONFIG] // --secrets value, -s value TOML configuration file for secrets. Must be set if and only if config is set. // --help, -h show help // --version, -v print the version diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index c5e8fdf51c7..0f6573f2a65 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -29,6 +29,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Removed `KEEPER_TURN_FLAG_ENABLED` as all networks/nodes have switched this to `true` now. The variable should be completely removed my NOPs. - Removed `Keeper.UpkeepCheckGasPriceEnabled` config (`KEEPER_CHECK_UPKEEP_GAS_PRICE_FEATURE_ENABLED` in old env var configuration) as this feature is deprecated now. The variable should be completely removed by NOPs. +- TOML env var `CL_CONFIG` always processed as the last configuration, with the effect of being the final override +of any values provided via configuration files. ### Fixed