From 43da1084669fa1d91bd0b1bb22b4d459b477ba4b Mon Sep 17 00:00:00 2001 From: ZenGround0 <5515260+ZenGround0@users.noreply.github.com> Date: Mon, 20 Mar 2023 12:19:14 -0400 Subject: [PATCH] feat:config:force existing users to opt into new defaults (#10488) * Config default does not comment out EnableSplitstore * Loadability check * Remove test used for debugging * regexp for properly safe check that config is set * regexp for safely matching the EnableSpitstore field in the config * Add instructions for undeleting config and remind users to set splitstore false for full archive * UpdateConfig small docs and functional opts * make gen * Lint * Fix * nil pointer check on validate * Unit testing of EnableSplitstore cases * Address Review --------- Co-authored-by: zenground0 --- cmd/lotus-miner/config.go | 4 +- cmd/lotus-miner/init_restore.go | 2 +- cmd/lotus/backup.go | 2 +- cmd/lotus/config.go | 4 +- documentation/en/default-lotus-config.toml | 2 +- node/config/load.go | 131 +++++++++++++++++++-- node/config/load_test.go | 81 ++++++++++++- node/repo/fsrepo.go | 10 +- 8 files changed, 216 insertions(+), 20 deletions(-) diff --git a/cmd/lotus-miner/config.go b/cmd/lotus-miner/config.go index 652426583..b7af1b2e5 100644 --- a/cmd/lotus-miner/config.go +++ b/cmd/lotus-miner/config.go @@ -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 } @@ -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 } diff --git a/cmd/lotus-miner/init_restore.go b/cmd/lotus-miner/init_restore.go index 618825a27..f3ab9d04a 100644 --- a/cmd/lotus-miner/init_restore.go +++ b/cmd/lotus-miner/init_restore.go @@ -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 diff --git a/cmd/lotus/backup.go b/cmd/lotus/backup.go index 4bdd21322..efbac3e2b 100644 --- a/cmd/lotus/backup.go +++ b/cmd/lotus/backup.go @@ -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 diff --git a/cmd/lotus/config.go b/cmd/lotus/config.go index fcb7e2b08..4b323bfb2 100644 --- a/cmd/lotus/config.go +++ b/cmd/lotus/config.go @@ -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 } @@ -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 } diff --git a/documentation/en/default-lotus-config.toml b/documentation/en/default-lotus-config.toml index c51321714..4e6cba4a3 100644 --- a/documentation/en/default-lotus-config.toml +++ b/documentation/en/default-lotus-config.toml @@ -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. diff --git a/node/config/load.go b/node/config/load.go index 6f5731ea1..29cb15d27 100644 --- a/node/config/load.go +++ b/node/config/load.go @@ -4,6 +4,7 @@ import ( "bytes" "fmt" "io" + "io/ioutil" "os" "reflect" "regexp" @@ -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 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. @@ -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) @@ -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{}{} @@ -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) @@ -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()) } diff --git a/node/config/load_test.go b/node/config/load_test.go index 17e185be2..cab5268df 100644 --- a/node/config/load_test.go +++ b/node/config/load_test.go @@ -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") @@ -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) + // assert that this config comments out EnableSplitstore + assert.False(t, MatchEnableSplitstoreField(string(cfgCommentedOutEnableSS))) + + // 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) +} diff --git a/node/repo/fsrepo.go b/node/repo/fsrepo.go index 3012daaca..2dbedd5e7 100644 --- a/node/repo/fsrepo.go +++ b/node/repo/fsrepo.go @@ -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 {