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 <ZenGround0@users.noreply.github.com>

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
 		}

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

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

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
 		}

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.

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,staticcheck // The file is RO
-	defer file.Close() //nolint:errcheck // The file is RO
+	cfgBs, err := ioutil.ReadAll(file)
-	return FromReader(file, def)
+	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
+			// filter lines from options
-			if _, found := defaults[strings.TrimSpace(nodeLines[i])]; (cfgDef == nil || found) && len(line) > 0 {
+			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())
 }

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)
+}

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 {