Skip to content

Commit

Permalink
Merge branch 'master' into petera/add-streaming-metrics
Browse files Browse the repository at this point in the history
  • Loading branch information
peterargue committed Jul 6, 2023
2 parents d6c6e27 + b94310b commit 7b304d2
Show file tree
Hide file tree
Showing 6 changed files with 321 additions and 89 deletions.
92 changes: 92 additions & 0 deletions config/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
## config
config is a package to hold all configuration values for each Flow component. This package centralizes configuration management providing access
to the entire FlowConfig and utilities to add a new config value, corresponding CLI flag, and validation.

### Package structure
The root config package contains the FlowConfig struct and the default config file [default-config.yml](https://github.com/onflow/flow-go/blob/master/config/default-config.yml). The `default-config.yml` file is the default configuration that is loaded when the config package is initialize.
The `default-config.yml` is a snapshot of all the configuration values defined for Flow.
Each subpackage contains configuration structs and utilities for components and their related subcomponents. These packages also contain the CLI flags for each configuration value. The [network](https://github.com/onflow/flow-go/tree/master/config/network) package
is a good example of this pattern. The network component is a large component made of many other large components and subcomponents. Each configuration
struct is defined for all of these network related components in the network subpackage and CLI flags.

### Overriding default values
The entire default config can be overridden using the `--config-file` CLI flag. When set the config package will attempt to parse the specified config file and override all the values
defined. A single default value can be overridden by setting the CLI flag for that specific config. For example `--network-connection-pruning=false` will override the default network connection pruning
config to false.
Override entire config file.
```shell
go build -tags relic -o flow-access-node ./cmd/access
./flow-access-node --config-file=config/config.yml
```
Override a single configuration value.
```shell
go build -tags relic -o flow-access-node ./cmd/access
./flow-access-node --network-connection-pruning=false
```
### Adding a new config value
Adding a new config to the FlowConfig can be done in a few easy steps.

1. Create a new subpackage in the config package for the new configuration structs to live. Although it is encouraged to put all configuration sub-packages in the config package
so that configuration can be updated in one place these sub-packages can live anywhere. This package will define the configuration structs and CLI flags for overriding.
```shell
mkdir example_config
```
2. Add a new CLI flag for the config value.
```go
const workersCLIFlag = "app-workers"
flags.String(workersCLIFlag, 1, "number of app workers")
```
The network package can be used as a good example of how to structure CLI flag initialization. All flags are initialized in a single function [InitializeNetworkFlags](https://github.com/onflow/flow-go/blob/master/config/network/flags.go#L80), this function is then used during flag initialization
of the [config package](https://github.com/onflow/flow-go/blob/master/config/base_flags.go#L22).
3. Add the config as a new field to an existing configuration struct or create a new one. Each configuration struct must be a field on the FlowConfig struct so that it is unmarshalled during configuration initialization.
Each field on a configuration struct must contain the following field tags.
1. `validate` - validate tag is used to perform validation on field structs using the [validator](https://github.com/go-playground/validator) package. In the example below you will notice
the `validate:"gt=0"` tag, this will ensure that the value of `AppWorkers` is greater than 0. The top level `FlowConfig` struct has a Validate method that performs struct validation. This
validation is done with the validator package, each validate tag on ever struct field and sub struct field will be validated and validation errors are returned.
2. `mapstructure` - mapstructure tag is used for unmarshalling and must match the CLI flag name defined in step or else the field will not be set when the config is unmarshalled.
```go
type MyComponentConfig struct {
AppWorkers int `validate:"gt=0" mapstructure:"app-workers"`
}
```
It's important to make sure that the CLI flag name matches the mapstructure field tag to avoid parsing errors.
4. Add the new config and a default value to the `default-config.yml` file. Ensure that the new property added matches the configuration struct structure for the subpackage the config belongs to.
```yaml
config-file: "./default-config.yml"
network-config:
...
my-component:
app-workers: 1
```
5. Finally, if a new struct was created add it as a new field to the FlowConfig. In the previous steps we added a new config struct and added a new property to the default-config.yml for this struct `my-component`. This property name
must match the mapstructure field tag for the struct when added to the FlowConfig.
```go
// FlowConfig Flow configuration.
type FlowConfig struct {
ConfigFile string `validate:"filepath" mapstructure:"config-file"`
NetworkConfig *network.Config `mapstructure:"network-config"`
MyComponentConfig *mypackage.MyComponentConfig `mapstructure:"my-component"`
}
```

### Nested structs
In an effort to keep the configuration yaml structure readable some configuration will be in nested properties. When this is the case the mapstructure `squash` tag can be used so that the corresponding nested struct will be
flattened before the configuration is unmarshalled. This is used in the network package which is a collection of configuration structs nested on the network.Config struct.
```go
type Config struct {
// UnicastRateLimitersConfig configuration for all unicast rate limiters.
UnicastRateLimitersConfig `mapstructure:",squash"`
...
}
```
`UnicastRateLimitersConfig` is a nested struct that defines configuration for unicast rate limiter component. In our configuration yaml structure you will see that all network configs are defined under the `network-config` property.

### Setting Aliases
Most configs will not be defined on the top layer FlowConfig but instead be defined on nested structs and in nested properties of the configuration yaml. When the default config is initially loaded the underlying config [viper](https://github.com/spf13/viper) store will store
each configuration with a key that is prefixed with each parent property. For example, because `network-connection-pruning` is found on the `network-config` property of the configuration yaml, the key used by the config store to
store this config value will be prefixed with `network` e.g.
```network.network-connection-pruning```

Later in the config process we bind the underlying config store with our pflag set, this allows us to override default values using CLI flags.
At this time the underlying config store would have 2 separate keys `network-connection-pruning` and `network.network-connection-pruning` for the same configuration value. This is because we don't use the network prefix for the CLI flags
used to override network configs. As a result, an alias must be set from `network.network-connection-pruning` -> `network-connection-pruning` so that they both point to the value loaded from the CLI flag. See [SetAliases](https://github.com/onflow/flow-go/blob/master/config/network/config.go#L84) in the network package for a reference.
75 changes: 14 additions & 61 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package config
import (
"bytes"
_ "embed"
"errors"
"fmt"
"path/filepath"
"regexp"
Expand All @@ -22,8 +23,14 @@ var (
validate *validator.Validate
//go:embed default-config.yml
configFile string

errPflagsNotParsed = errors.New("failed to bind flags to configuration values, pflags must be parsed before binding")
)

func init() {
initialize()
}

// FlowConfig Flow configuration.
type FlowConfig struct {
// ConfigFile used to set a path to a config.yml file used to override the default-config.yml file.
Expand Down Expand Up @@ -79,7 +86,7 @@ func DefaultConfig() (*FlowConfig, error) {
// Note: As configuration management is improved, this func should accept the entire Flow config as the arg to unmarshall new config values into.
func BindPFlags(c *FlowConfig, flags *pflag.FlagSet) (bool, error) {
if !flags.Parsed() {
return false, fmt.Errorf("failed to bind flags to configuration values, pflags must be parsed before binding")
return false, errPflagsNotParsed
}

// update the config store values from config file if --config-file flag is set
Expand Down Expand Up @@ -118,7 +125,7 @@ func Unmarshall(flowConfig *FlowConfig) error {
// enforce all fields are set on the FlowConfig struct
decoderConfig.ErrorUnset = true
// currently the entire flow configuration has not been moved to this package
// for now we all key's in the config which are unused.
// for now we allow key's in the config which are unused.
decoderConfig.ErrorUnused = false
})
if err != nil {
Expand All @@ -127,30 +134,6 @@ func Unmarshall(flowConfig *FlowConfig) error {
return nil
}

// Print prints current configuration keys and values.
// Returns:
// map[string]struct{}: map of keys to avoid printing if they were set by an config file.
// This is required because we still have other config values not migrated to the config package. When a
// config file is used currently only network-configs are set, we want to avoid printing the config file
// value and also the flag value.
func Print(info *zerolog.Event, flags *pflag.FlagSet) map[string]struct{} {
// only print config values if they were overridden with a config file
m := make(map[string]struct{})
if flags.Lookup(configFileFlagName).Changed {
for _, key := range conf.AllKeys() {
info.Str(key, fmt.Sprintf("%v", conf.Get(key)))
s := strings.Split(key, ".")
if len(s) == 2 {
m[s[1]] = struct{}{}
} else {
m[key] = struct{}{}
}
}
}

return m
}

// LogConfig logs configuration keys and values if they were overridden with a config file.
// It also returns a map of keys for which the values were set by a config file.
//
Expand Down Expand Up @@ -184,45 +167,12 @@ func LogConfig(logger *zerolog.Event, flags *pflag.FlagSet) map[string]struct{}
// keys do not match the CLI flags 1:1. ie: networking-connection-pruning -> network-config.networking-connection-pruning. After aliases
// are set the conf store will override values with any CLI flag values that are set as expected.
func setAliases() {
err := SetAliases(conf)
err := netconf.SetAliases(conf)
if err != nil {
panic(fmt.Errorf("failed to set network aliases: %w", err))
}
}

// SetAliases this func sets an aliases for each CLI flag defined for network config overrides to it's corresponding
// full key in the viper config store. This is required because in our config.yml file all configuration values for the
// Flow network are stored one level down on the network-config property. When the default config is bootstrapped viper will
// store these values with the "network-config." prefix on the config key, because we do not want to use CLI flags like --network-config.networking-connection-pruning
// to override default values we instead use cleans flags like --networking-connection-pruning and create an alias from networking-connection-pruning -> network-config.networking-connection-pruning
// to ensure overrides happen as expected.
// Args:
// *viper.Viper: instance of the viper store to register network config aliases on.
// Returns:
// error: if a flag does not have a corresponding key in the viper store.
func SetAliases(conf *viper.Viper) error {
m := make(map[string]string)
// create map of key -> full pathkey
// ie: "networking-connection-pruning" -> "network-config.networking-connection-pruning"
for _, key := range conf.AllKeys() {
s := strings.Split(key, ".")
// check len of s, we expect all network keys to have a single prefix "network-config"
// s should always contain only 2 elements
if len(s) == 2 {
m[s[1]] = key
}
}
// each flag name should correspond to exactly one key in our config store after it is loaded with the default config
for _, flagName := range netconf.AllFlagNames() {
fullKey, ok := m[flagName]
if !ok {
return fmt.Errorf("invalid network configuration missing configuration key flag name %s check config file and cli flags", flagName)
}
conf.RegisterAlias(fullKey, flagName)
}
return nil
}

// overrideConfigFile overrides the default config file by reading in the config file at the path set
// by the --config-file flag in our viper config store.
//
Expand All @@ -241,6 +191,9 @@ func overrideConfigFile(flags *pflag.FlagSet) (bool, error) {
if err != nil {
return false, fmt.Errorf("failed to read config file %s: %w", p, err)
}
if len(conf.AllKeys()) == 0 {
return false, fmt.Errorf("failed to read in config file no config values found")
}
return true, nil
}
return false, nil
Expand Down Expand Up @@ -286,7 +239,7 @@ func splitConfigPath(path string) (string, string) {
return dir, baseName
}

func init() {
func initialize() {
buf := bytes.NewBufferString(configFile)
conf.SetConfigType("yaml")
if err := conf.ReadConfig(buf); err != nil {
Expand Down
141 changes: 141 additions & 0 deletions config/config_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
package config

import (
"errors"
"fmt"
"os"
"strings"
"testing"

"github.com/go-playground/validator/v10"
"github.com/spf13/pflag"
"github.com/spf13/viper"
"github.com/stretchr/testify/require"

"github.com/onflow/flow-go/utils/unittest"
)

// TestBindPFlags ensures configuration is bound to the pflag set as expected and configuration values are overridden when set with CLI flags.
func TestBindPFlags(t *testing.T) {
t.Run("should override config values when any flag is set", func(t *testing.T) {
c := defaultConfig(t)
flags := testFlagSet(c)
err := flags.Set("networking-connection-pruning", "false")
require.NoError(t, err)
require.NoError(t, flags.Parse(nil))

configFileUsed, err := BindPFlags(c, flags)
require.NoError(t, err)
require.False(t, configFileUsed)
require.False(t, c.NetworkConfig.NetworkConnectionPruning)
})
t.Run("should return an error if flags are not parsed", func(t *testing.T) {
c := defaultConfig(t)
flags := testFlagSet(c)
configFileUsed, err := BindPFlags(&FlowConfig{}, flags)
require.False(t, configFileUsed)
require.Error(t, err)
require.True(t, errors.Is(err, errPflagsNotParsed))
})
}

// TestDefaultConfig ensures the default Flow config is created and returned without errors.
func TestDefaultConfig(t *testing.T) {
c := defaultConfig(t)
require.Equalf(t, "./default-config.yml", c.ConfigFile, "expected default config file to be used")
require.NoErrorf(t, c.Validate(), "unexpected error encountered validating default config")
unittest.IdentifierFixture()
}

// TestFlowConfig_Validate ensures the Flow validate returns the expected number of validator.ValidationErrors when incorrect
// fields are set.
func TestFlowConfig_Validate(t *testing.T) {
c := defaultConfig(t)
// set invalid config values
c.NetworkConfig.UnicastRateLimitersConfig.MessageRateLimit = -100
c.NetworkConfig.UnicastRateLimitersConfig.BandwidthRateLimit = -100
err := c.Validate()
require.Error(t, err)
errs, ok := errors.Unwrap(err).(validator.ValidationErrors)
require.True(t, ok)
require.Len(t, errs, 2)
}

// TestUnmarshall_UnsetFields ensures that if the config store has any missing config values an error is returned when the config is decoded into a Flow config.
func TestUnmarshall_UnsetFields(t *testing.T) {
conf = viper.New()
c := &FlowConfig{}
err := Unmarshall(c)
require.True(t, strings.Contains(err.Error(), "has unset fields"))
}

// Test_overrideConfigFile ensures configuration values can be overridden via the --config-file flag.
func Test_overrideConfigFile(t *testing.T) {
t.Run("should override the default config if --config-file is set", func(t *testing.T) {
file, err := os.CreateTemp("", "config-*.yml")
require.NoError(t, err)
defer os.Remove(file.Name())

var data = fmt.Sprintf(`config-file: "%s"
network-config:
networking-connection-pruning: false
`, file.Name())
_, err = file.Write([]byte(data))
require.NoError(t, err)
c := defaultConfig(t)
flags := testFlagSet(c)
err = flags.Set(configFileFlagName, file.Name())

require.NoError(t, err)
overridden, err := overrideConfigFile(flags)
require.NoError(t, err)
require.True(t, overridden)

// ensure config values overridden with values from our inline config
require.Equal(t, conf.GetString(configFileFlagName), file.Name())
require.False(t, conf.GetBool("networking-connection-pruning"))
})
t.Run("should return an error for missing --config file", func(t *testing.T) {
c := defaultConfig(t)
flags := testFlagSet(c)
err := flags.Set(configFileFlagName, "./missing-config.yml")
require.NoError(t, err)
overridden, err := overrideConfigFile(flags)
require.Error(t, err)
require.False(t, overridden)
})
t.Run("should not attempt to override config if --config-file is not set", func(t *testing.T) {
c := defaultConfig(t)
flags := testFlagSet(c)
overridden, err := overrideConfigFile(flags)
require.NoError(t, err)
require.False(t, overridden)
})
t.Run("should return an error for file types other than .yml", func(t *testing.T) {
file, err := os.CreateTemp("", "config-*.json")
require.NoError(t, err)
defer os.Remove(file.Name())
c := defaultConfig(t)
flags := testFlagSet(c)
err = flags.Set(configFileFlagName, file.Name())
require.NoError(t, err)
overridden, err := overrideConfigFile(flags)
require.Error(t, err)
require.False(t, overridden)
})
}

// defaultConfig resets the config store gets the default Flow config.
func defaultConfig(t *testing.T) *FlowConfig {
initialize()
c, err := DefaultConfig()
require.NoError(t, err)
return c
}

func testFlagSet(c *FlowConfig) *pflag.FlagSet {
flags := pflag.NewFlagSet("test", pflag.PanicOnError)
// initialize default flags
InitializePFlagSet(flags, c)
return flags
}
Loading

0 comments on commit 7b304d2

Please sign in to comment.