Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat:config:force existing users to opt into new defaults #10488

Merged
merged 13 commits into from
Mar 20, 2023
4 changes: 2 additions & 2 deletions cmd/lotus-miner/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ var configDefaultCmd = &cli.Command{
Action: func(cctx *cli.Context) error {
c := config.DefaultStorageMiner()

cb, err := config.ConfigUpdate(c, nil, !cctx.Bool("no-comment"))
cb, err := config.ConfigUpdate(c, nil, config.Commented(!cctx.Bool("no-comment")))
if err != nil {
return err
}
Expand Down Expand Up @@ -83,7 +83,7 @@ var configUpdateCmd = &cli.Command{

cfgDef := config.DefaultStorageMiner()

updated, err := config.ConfigUpdate(cfgNode, cfgDef, !cctx.Bool("no-comment"))
updated, err := config.ConfigUpdate(cfgNode, cfgDef, config.Commented(!cctx.Bool("no-comment")))
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/lotus-miner/init_restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ func restore(ctx context.Context, cctx *cli.Context, targetPath string, strConfi
return
}

ff, err := config.FromFile(cf, rcfg)
ff, err := config.FromFile(cf, config.SetDefault(func() (interface{}, error) { return rcfg, nil }))
if err != nil {
cerr = xerrors.Errorf("loading config: %w", err)
return
Expand Down
2 changes: 1 addition & 1 deletion cmd/lotus/backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func restore(cctx *cli.Context, r repo.Repo) error {
return
}

ff, err := config.FromFile(cf, rcfg)
ff, err := config.FromFile(cf, config.SetDefault(func() (interface{}, error) { return rcfg, nil }))
if err != nil {
cerr = xerrors.Errorf("loading config: %w", err)
return
Expand Down
4 changes: 2 additions & 2 deletions cmd/lotus/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ var configDefaultCmd = &cli.Command{
Action: func(cctx *cli.Context) error {
c := config.DefaultFullNode()

cb, err := config.ConfigUpdate(c, nil, !cctx.Bool("no-comment"))
cb, err := config.ConfigUpdate(c, nil, config.Commented(!cctx.Bool("no-comment")), config.DefaultKeepUncommented())
if err != nil {
return err
}
Expand Down Expand Up @@ -83,7 +83,7 @@ var configUpdateCmd = &cli.Command{

cfgDef := config.DefaultFullNode()

updated, err := config.ConfigUpdate(cfgNode, cfgDef, !cctx.Bool("no-comment"))
updated, err := config.ConfigUpdate(cfgNode, cfgDef, config.Commented(!cctx.Bool("no-comment")), config.DefaultKeepUncommented())
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion documentation/en/default-lotus-config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@
[Chainstore]
# type: bool
# env var: LOTUS_CHAINSTORE_ENABLESPLITSTORE
#EnableSplitstore = true
EnableSplitstore = true

[Chainstore.Splitstore]
# ColdStoreType specifies the type of the coldstore.
Expand Down
131 changes: 122 additions & 9 deletions node/config/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"fmt"
"io"
"io/ioutil"
"os"
"reflect"
"regexp"
Expand All @@ -17,17 +18,46 @@ import (

// FromFile loads config from a specified file overriding defaults specified in
// the def parameter. If file does not exist or is empty defaults are assumed.
func FromFile(path string, def interface{}) (interface{}, error) {
func FromFile(path string, opts ...LoadCfgOpt) (interface{}, error) {
var loadOpts cfgLoadOpts
var err error
for _, opt := range opts {
if err = opt(&loadOpts); err != nil {
return nil, xerrors.Errorf("failed to apply load cfg option: %w", err)
}
}
var def interface{}
if loadOpts.defaultCfg != nil {
def, err = loadOpts.defaultCfg()
if err != nil {
return nil, xerrors.Errorf("no config found")
}
}
// check for loadability
file, err := os.Open(path)
switch {
case os.IsNotExist(err):
if loadOpts.canFallbackOnDefault != nil {
if err := loadOpts.canFallbackOnDefault(); err != nil {
return nil, err
}
}
return def, nil
ZenGround0 marked this conversation as resolved.
Show resolved Hide resolved
case err != nil:
return nil, err
}

defer file.Close() //nolint:errcheck // The file is RO
return FromReader(file, def)
defer file.Close() //nolint:errcheck,staticcheck // The file is RO
cfgBs, err := ioutil.ReadAll(file)
if err != nil {
return nil, xerrors.Errorf("failed to read config for validation checks %w", err)
}
buf := bytes.NewBuffer(cfgBs)
if loadOpts.validate != nil {
if err := loadOpts.validate(buf.String()); err != nil {
return nil, xerrors.Errorf("config failed validation: %w", err)
}
}
return FromReader(buf, def)
}

// FromReader loads config from a reader instance.
Expand All @@ -46,7 +76,88 @@ func FromReader(reader io.Reader, def interface{}) (interface{}, error) {
return cfg, nil
}

func ConfigUpdate(cfgCur, cfgDef interface{}, comment bool) ([]byte, error) {
type cfgLoadOpts struct {
defaultCfg func() (interface{}, error)
canFallbackOnDefault func() error
validate func(string) error
}

type LoadCfgOpt func(opts *cfgLoadOpts) error

func SetDefault(f func() (interface{}, error)) LoadCfgOpt {
return func(opts *cfgLoadOpts) error {
opts.defaultCfg = f
return nil
}
}

func SetCanFallbackOnDefault(f func() error) LoadCfgOpt {
return func(opts *cfgLoadOpts) error {
opts.canFallbackOnDefault = f
return nil
}
}

func SetValidate(f func(string) error) LoadCfgOpt {
return func(opts *cfgLoadOpts) error {
opts.validate = f
return nil
}
}

func NoDefaultForSplitstoreTransition() error {
return xerrors.Errorf("FullNode config not found and fallback to default disallowed while we transition to splitstore discard default. Use `lotus config default` to set this repo up with a default config. Be sure to set `EnableSplitstore` to `false` if you are running a full archive node")
}

// Match the EnableSplitstore field
func MatchEnableSplitstoreField(s string) bool {
enableSplitstoreRx := regexp.MustCompile(`(?m)^\s*EnableSplitstore\s*=`)
return enableSplitstoreRx.MatchString(s)
}

func ValidateSplitstoreSet(cfgRaw string) error {
if !MatchEnableSplitstoreField(cfgRaw) {
return xerrors.Errorf("Config does not contain explicit set of EnableSplitstore field, refusing to load. Please explicitly set EnableSplitstore. Set it to false if you are running a full archival node")
}
return nil
}

type cfgUpdateOpts struct {
comment bool
keepUncommented func(string) bool
}

// UpdateCfgOpt is a functional option for updating the config
type UpdateCfgOpt func(opts *cfgUpdateOpts) error

// KeepUncommented sets a function for matching default valeus that should remain uncommented during
// a config update that comments out default values.
func KeepUncommented(f func(string) bool) UpdateCfgOpt {
return func(opts *cfgUpdateOpts) error {
opts.keepUncommented = f
return nil
}
}

func Commented(commented bool) UpdateCfgOpt {
return func(opts *cfgUpdateOpts) error {
opts.comment = commented
return nil
}
}

func DefaultKeepUncommented() UpdateCfgOpt {
return KeepUncommented(MatchEnableSplitstoreField)
}

// ConfigUpdate takes in a config and a default config and optionally comments out default values
func ConfigUpdate(cfgCur, cfgDef interface{}, opts ...UpdateCfgOpt) ([]byte, error) {
var updateOpts cfgUpdateOpts
for _, opt := range opts {
if err := opt(&updateOpts); err != nil {
return nil, xerrors.Errorf("failed to apply update cfg option to ConfigUpdate's config: %w", err)
}
}
var nodeStr, defStr string
if cfgDef != nil {
buf := new(bytes.Buffer)
Expand All @@ -68,7 +179,7 @@ func ConfigUpdate(cfgCur, cfgDef interface{}, comment bool) ([]byte, error) {
nodeStr = buf.String()
}

if comment {
if updateOpts.comment {
// create a map of default lines, so we can comment those out later
defLines := strings.Split(defStr, "\n")
defaults := map[string]struct{}{}
Expand Down Expand Up @@ -130,8 +241,10 @@ func ConfigUpdate(cfgCur, cfgDef interface{}, comment bool) ([]byte, error) {
}
}

// if there is the same line in the default config, comment it out it output
if _, found := defaults[strings.TrimSpace(nodeLines[i])]; (cfgDef == nil || found) && len(line) > 0 {
// filter lines from options
optsFilter := updateOpts.keepUncommented != nil && updateOpts.keepUncommented(line)
// if there is the same line in the default config, comment it out in output
if _, found := defaults[strings.TrimSpace(nodeLines[i])]; (cfgDef == nil || found) && len(line) > 0 && !optsFilter {
line = pad + "#" + line[len(pad):]
}
outLines = append(outLines, line)
Expand Down Expand Up @@ -159,5 +272,5 @@ func ConfigUpdate(cfgCur, cfgDef interface{}, comment bool) ([]byte, error) {
}

func ConfigComment(t interface{}) ([]byte, error) {
return ConfigUpdate(t, nil, true)
return ConfigUpdate(t, nil, Commented(true), DefaultKeepUncommented())
}
81 changes: 78 additions & 3 deletions node/config/load_test.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add some unit tests around EnableSplitstore handling?

Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,21 @@ import (
"github.com/stretchr/testify/assert"
)

func fullNodeDefault() (interface{}, error) { return DefaultFullNode(), nil }

func TestDecodeNothing(t *testing.T) {
//stm: @NODE_CONFIG_LOAD_FILE_002
assert := assert.New(t)

{
cfg, err := FromFile(os.DevNull, DefaultFullNode())
cfg, err := FromFile(os.DevNull, SetDefault(fullNodeDefault))
assert.Nil(err, "error should be nil")
assert.Equal(DefaultFullNode(), cfg,
"config from empty file should be the same as default")
}

{
cfg, err := FromFile("./does-not-exist.toml", DefaultFullNode())
cfg, err := FromFile("./does-not-exist.toml", SetDefault(fullNodeDefault))
assert.Nil(err, "error should be nil")
assert.Equal(DefaultFullNode(), cfg,
"config from not exisiting file should be the same as default")
Expand Down Expand Up @@ -58,9 +60,82 @@ func TestParitalConfig(t *testing.T) {
assert.NoError(err, "closing tmp file should not error")
defer os.Remove(fname) //nolint:errcheck

cfg, err := FromFile(fname, DefaultFullNode())
cfg, err := FromFile(fname, SetDefault(fullNodeDefault))
assert.Nil(err, "error should be nil")
assert.Equal(expected, cfg,
"config from reader should contain changes")
}
}

func TestValidateSplitstoreSet(t *testing.T) {
cfgSet := `
EnableSplitstore = false
`
assert.NoError(t, ValidateSplitstoreSet(cfgSet))
cfgSloppySet := `
EnableSplitstore = true
`
assert.NoError(t, ValidateSplitstoreSet(cfgSloppySet))

// Missing altogether
cfgMissing := `
[Chainstore]
# type: bool
# env var: LOTUS_CHAINSTORE_ENABLESPLITSTORE
# oops its mising

[Chainstore.Splitstore]
ColdStoreType = "discard"
`
err := ValidateSplitstoreSet(cfgMissing)
assert.Error(t, err)
cfgCommentedOut := `
# EnableSplitstore = false
`
err = ValidateSplitstoreSet(cfgCommentedOut)
assert.Error(t, err)
}

// Default config keeps EnableSplitstore field uncommented
func TestKeepEnableSplitstoreUncommented(t *testing.T) {
cfgStr, err := ConfigComment(DefaultFullNode())
assert.NoError(t, err)
assert.True(t, MatchEnableSplitstoreField(string(cfgStr)))

cfgStrFromDef, err := ConfigUpdate(DefaultFullNode(), DefaultFullNode(), Commented(true), DefaultKeepUncommented())
assert.NoError(t, err)
assert.True(t, MatchEnableSplitstoreField(string(cfgStrFromDef)))
}

// Loading a config with commented EnableSplitstore fails when setting validator
func TestValidateConfigSetsEnableSplitstore(t *testing.T) {
cfgCommentedOutEnableSS, err := ConfigUpdate(DefaultFullNode(), DefaultFullNode(), Commented(true))
assert.NoError(t, err)
// this comments out EnableSplitstore
assert.False(t, MatchEnableSplitstoreField(string(cfgCommentedOutEnableSS)))
ZenGround0 marked this conversation as resolved.
Show resolved Hide resolved

// write config with commented out EnableSplitstore to file
f, err := ioutil.TempFile("", "config.toml")
fname := f.Name()
assert.NoError(t, err)
defer func() {
err = f.Close()
assert.NoError(t, err)
os.Remove(fname) //nolint:errcheck
}()
_, err = f.WriteString(string(cfgCommentedOutEnableSS))
assert.NoError(t, err)

_, err = FromFile(fname, SetDefault(fullNodeDefault), SetValidate(ValidateSplitstoreSet))
assert.Error(t, err)
}

// Loading without a config file and a default fails if the default fallback is disabled
func TestFailToFallbackToDefault(t *testing.T) {
dir, err := ioutil.TempDir("", "dirWithNoFiles")
assert.NoError(t, err)
defer assert.NoError(t, os.RemoveAll(dir))
nonExistantFileName := dir + "/notarealfile"
_, err = FromFile(nonExistantFileName, SetDefault(fullNodeDefault), SetCanFallbackOnDefault(NoDefaultForSplitstoreTransition))
assert.Error(t, err)
}
10 changes: 9 additions & 1 deletion node/repo/fsrepo.go
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,15 @@ func (fsr *fsLockedRepo) Config() (interface{}, error) {
}

func (fsr *fsLockedRepo) loadConfigFromDisk() (interface{}, error) {
return config.FromFile(fsr.configPath, fsr.repoType.Config())
var opts []config.LoadCfgOpt
if fsr.repoType == FullNode {
opts = append(opts, config.SetCanFallbackOnDefault(config.NoDefaultForSplitstoreTransition))
opts = append(opts, config.SetValidate(config.ValidateSplitstoreSet))
}
opts = append(opts, config.SetDefault(func() (interface{}, error) {
return fsr.repoType.Config(), nil
}))
return config.FromFile(fsr.configPath, opts...)
}

func (fsr *fsLockedRepo) SetConfig(c func(interface{})) error {
Expand Down