fix(cmd/influxd): don't validate unused paths in `upgrade` (#20052)

cmd/influxd/upgrade/config.go

@@ -7,6 +7,7 @@ import (
 	"bytes"
 	"fmt"
 	"io/ioutil"
+	"os"
 	"path/filepath"
 	"strings"
 
@@ -61,12 +62,54 @@ var configValueTransforms = map[string]func(interface{}) interface{}{
 	},
 }
 
-func translateV1ConfigPath(configFile string) string {
+func loadV1Config(configFile string) (*configV1, *map[string]interface{}, error) {
-	return filepath.Join(filepath.Dir(configFile), "config.toml")
+	_, err := os.Stat(configFile)
+	if err != nil {
+		return nil, nil, fmt.Errorf("1.x config file '%s' does not exist", configFile)
+	}
+
+	// load 1.x config content into byte array
+	bs, err := load(configFile)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	// parse it into simplified v1 config used as return value
+	var configV1 configV1
+	_, err = toml.Decode(string(bs), &configV1)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	// parse into a generic config map
+	var cAny map[string]interface{}
+	_, err = toml.Decode(string(bs), &cAny)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	return &configV1, &cAny, nil
 }
 
-// upgradeConfig upgrades existing 1.x (ie. typically influxdb.conf) configuration file to 2.x influxdb.toml file.
+func load(path string) ([]byte, error) {
-func upgradeConfig(configFile string, targetOptions optionsV2, log *zap.Logger) (*configV1, error) {
+	bs, err := ioutil.ReadFile(path)
+	if err != nil {
+		return nil, err
+	}
+
+	// From master-1.x/cmd/influxd/run/config.go:
+	// Handle any potential Byte-Order-Marks that may be in the config file.
+	// This is for Windows compatibility only.
+	// See https://github.com/influxdata/telegraf/issues/1378 and
+	// https://github.com/influxdata/influxdb/issues/8965.
+	bom := unicode.BOMOverride(transform.Nop)
+	bs, _, err = transform.Bytes(bom, bs)
+
+	return bs, err
+}
+
+// upgradeConfig upgrades existing 1.x configuration file to 2.x influxdb.toml file.
+func upgradeConfig(v1Config map[string]interface{}, targetOptions optionsV2, log *zap.Logger) (string, error) {
 	// create and initialize helper
 	cu := &configUpgrader{
 		rules:           configMapRules,
@@ -74,47 +117,17 @@ func upgradeConfig(configFile string, targetOptions optionsV2, log *zap.Logger)
 		log:             log,
 	}
 
-	// load 1.x config content into byte array
+	cTransformed := cu.transform(v1Config)
-	bs, err := cu.load(configFile)
-	if err != nil {
-		return nil, err
-	}
-
-	// parse it into simplified v1 config used as return value
-	var configV1 configV1
-	_, err = toml.Decode(string(bs), &configV1)
-	if err != nil {
-		return nil, err
-	}
-
-	// parse into a generic config map
-	var cAny map[string]interface{}
-	_, err = toml.Decode(string(bs), &cAny)
-	if err != nil {
-		return nil, err
-	}
-
-	// transform the config according to rules
-	cTransformed := cu.transform(cAny)
-	if err != nil {
-		return nil, err
-	}
 
 	// update new config with upgrade command options
 	cu.updateV2Config(cTransformed, targetOptions)
 
-	// save new config
+	configFileV2, err := cu.save(cTransformed, targetOptions.configPath)
-	configFileV2 := translateV1ConfigPath(configFile)
-	configFileV2, err = cu.save(cTransformed, configFileV2)
 	if err != nil {
-		return nil, err
+		return "", err
 	}
 
-	log.Info("Config file upgraded.",
+	return configFileV2, nil
-		zap.String("1.x config", configFile),
-		zap.String("2.x config", configFileV2))
-
-	return &configV1, nil
 }
 
 // configUpgrader is a helper used by `upgrade-config` command.
@@ -133,23 +146,6 @@ func (cu *configUpgrader) updateV2Config(config map[string]interface{}, targetOp
 	}
 }
 
-func (cu *configUpgrader) load(path string) ([]byte, error) {
-	bs, err := ioutil.ReadFile(path)
-	if err != nil {
-		return nil, err
-	}
-
-	// From master-1.x/cmd/influxd/run/config.go:
-	// Handle any potential Byte-Order-Marks that may be in the config file.
-	// This is for Windows compatibility only.
-	// See https://github.com/influxdata/telegraf/issues/1378 and
-	// https://github.com/influxdata/influxdb/issues/8965.
-	bom := unicode.BOMOverride(transform.Nop)
-	bs, _, err = transform.Bytes(bom, bs)
-
-	return bs, err
-}
-
 func (cu *configUpgrader) save(config map[string]interface{}, path string) (string, error) {
 	buf := new(bytes.Buffer)
 	if err := toml.NewEncoder(buf).Encode(&config); err != nil {

cmd/influxd/upgrade/config_test.go

@@ -7,27 +7,16 @@ import (
 
 	"github.com/BurntSushi/toml"
 	"github.com/google/go-cmp/cmp"
+	"github.com/influxdata/influxdb/v2/pkg/testing/assert"
 	"github.com/stretchr/testify/require"
-	"go.uber.org/zap"
 	"go.uber.org/zap/zaptest"
 )
 
 func TestConfigUpgrade(t *testing.T) {
-	targetOtions := optionsV2{
-		boltPath:   "/db/.influxdbv2/influxd.bolt",
-		enginePath: "/db/.influxdbv2/engine",
-	}
-
-	var typicalRetval, emptyRetval configV1
-	_, err := toml.Decode("[meta]\ndir=\"/var/lib/influxdb/meta\"\n[data]\ndir=\"/var/lib/influxdb/data\"\nwal-dir=\"/var/lib/influxdb/wal\"\n[http]\nbind-address=\":8086\"\nhttps-enabled=false",
-		&typicalRetval)
-	require.NoError(t, err)
-
 	type testCase struct {
 		name     string
 		config1x string
 		config2x string
-		retval   *configV1
 	}
 
 	var testCases = []testCase{
@@ -35,25 +24,21 @@ func TestConfigUpgrade(t *testing.T) {
 			name:     "minimal",
 			config1x: testConfigV1minimal,
 			config2x: testConfigV2minimal,
-			retval:   &typicalRetval,
 		},
 		{
 			name:     "default",
 			config1x: testConfigV1default,
 			config2x: testConfigV2default,
-			retval:   &typicalRetval,
 		},
 		{
 			name:     "empty",
 			config1x: testConfigV1empty,
 			config2x: testConfigV2empty,
-			retval:   &emptyRetval,
 		},
 		{
 			name:     "obsolete / arrays",
 			config1x: testConfigV1obsoleteArrays,
 			config2x: testConfigV2obsoleteArrays,
-			retval:   &typicalRetval,
 		},
 	}
 
@@ -66,13 +51,23 @@ func TestConfigUpgrade(t *testing.T) {
 			configFileV2 := filepath.Join(filepath.Dir(configFile), "config.toml")
 			err := ioutil.WriteFile(configFile, []byte(tc.config1x), 0444)
 			require.NoError(t, err)
-			retval, err := upgradeConfig(configFile, targetOtions, zaptest.NewLogger(t))
+
+			targetOtions := optionsV2{
+				boltPath:   "/db/.influxdbv2/influxd.bolt",
+				enginePath: "/db/.influxdbv2/engine",
+				configPath: configFileV2,
+			}
+
+			var rawV1Config map[string]interface{}
+			if _, err = toml.Decode(tc.config1x, &rawV1Config); err != nil {
+				t.Fatal(err)
+			}
+			retval, err := upgradeConfig(rawV1Config, targetOtions, zaptest.NewLogger(t))
 			if err != nil {
 				t.Fatal(err)
 			}
-			if diff := cmp.Diff(tc.retval, retval); diff != "" {
+			assert.Equal(t, retval, configFileV2)
-				t.Fatal(diff)
+
-			}
 			var actual, expected map[string]interface{}
 			if _, err = toml.Decode(tc.config2x, &expected); err != nil {
 				t.Fatal(err)
@@ -86,12 +81,68 @@ func TestConfigUpgrade(t *testing.T) {
 		})
 	}
 }
-func TestConfigUpgradeFileNotExists(t *testing.T) {
+
-	targetOtions := optionsV2{}
+func TestConfigLoadFile(t *testing.T) {
+	var typicalRetval, emptyRetval configV1
+	_, err := toml.Decode(
+		"[meta]\ndir=\"/var/lib/influxdb/meta\"\n[data]\ndir=\"/var/lib/influxdb/data\"\nwal-dir=\"/var/lib/influxdb/wal\"\n[http]\nbind-address=\":8086\"\nhttps-enabled=false",
+		&typicalRetval,
+	)
+	require.NoError(t, err)
+
+	type testCase struct {
+		name     string
+		config1x string
+		retval   *configV1
+	}
+
+	var testCases = []testCase{
+		{
+			name:     "minimal",
+			config1x: testConfigV1minimal,
+			retval:   &typicalRetval,
+		},
+		{
+			name:     "default",
+			config1x: testConfigV1default,
+			retval:   &typicalRetval,
+		},
+		{
+			name:     "empty",
+			config1x: testConfigV1empty,
+			retval:   &emptyRetval,
+		},
+		{
+			name:     "obsolete / arrays",
+			config1x: testConfigV1obsoleteArrays,
+			retval:   &typicalRetval,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			tmpdir := t.TempDir()
+			configFile := filepath.Join(tmpdir, "influxdb.conf")
+			err := ioutil.WriteFile(configFile, []byte(tc.config1x), 0444)
+			require.NoError(t, err)
+			retval, _, err := loadV1Config(configFile)
+			require.NoError(t, err)
+
+			if diff := cmp.Diff(tc.retval, retval); diff != "" {
+				t.Fatal(diff)
+			}
+		})
+	}
+}
+
+func TestConfigLoadFileNotExists(t *testing.T) {
 	configFile := "/there/is/no/such/path/influxdb.conf"
 
 	// try upgrade
-	_, err := upgradeConfig(configFile, targetOtions, zap.NewNop())
+
+	_, _, err := loadV1Config(configFile)
 	if err == nil {
 		t.Fatal("error expected")
 	}

cmd/influxd/upgrade/upgrade.go

@@ -91,6 +91,7 @@ type optionsV2 struct {
 	cliConfigsPath string
 	enginePath     string
 	cqPath         string
+	configPath     string
 	userName       string
 	password       string
 	orgName        string
@@ -120,12 +121,6 @@ var options = struct {
 
 func NewCommand(v *viper.Viper) *cobra.Command {
 
-	// source flags
-	v1dir, err := influxDirV1()
-	if err != nil {
-		panic("error fetching default InfluxDB 1.x dir: " + err.Error())
-	}
-
 	// target flags
 	v2dir, err := fs.InfluxDir()
 	if err != nil {
@@ -142,7 +137,10 @@ func NewCommand(v *viper.Viper) *cobra.Command {
       3. Creates influx CLI configurations.
       4. Exports any 1.x continuous queries to disk.
 
-    If the config file is not available, 1.x db folder (--v1-dir options) is taken as an input.
+    If --config-file is not passed, 1.x db folder (--v1-dir options) is taken as an input. If neither option is given,
+    the CLI will search for config under ${HOME}/.influxdb/ and /etc/influxdb/. If config can't be found, the CLI assumes
+    a standard V1 directory structure under ${HOME}/.influxdb/.
+
     Target 2.x database dir is specified by the --engine-path option. If changed, the bolt path should be changed as well.
 `,
 		RunE: runUpgradeE,
@@ -151,10 +149,9 @@ func NewCommand(v *viper.Viper) *cobra.Command {
 
 	opts := []cli.Opt{
 		{
-			DestP:   &options.source.dbDir,
+			DestP: &options.source.dbDir,
-			Flag:    "v1-dir",
+			Flag:  "v1-dir",
-			Default: v1dir,
+			Desc:  "path to source 1.x db directory containing meta, data and wal sub-folders",
-			Desc:    "path to source 1.x db directory containing meta, data and wal sub-folders",
 		},
 		{
 			DestP:   &options.verbose,
@@ -237,10 +234,9 @@ func NewCommand(v *viper.Viper) *cobra.Command {
 			Short:   't',
 		},
 		{
-			DestP:   &options.source.configFile,
+			DestP: &options.source.configFile,
-			Flag:    "config-file",
+			Flag:  "config-file",
-			Default: influxConfigPathV1(),
+			Desc:  "optional: Custom InfluxDB 1.x config file path, else the default config file",
-			Desc:    "optional: Custom InfluxDB 1.x config file path, else the default config file",
 		},
 		{
 			DestP:   &options.logLevel,
@@ -319,6 +315,23 @@ func runUpgradeE(*cobra.Command, []string) error {
 		return errors.New("unknown log level; supported levels are debug, info, warn and error")
 	}
 
+	if options.source.configFile != "" && options.source.dbDir != "" {
+		return errors.New("only one of --v1-dir or --config-file may be specified")
+	}
+
+	if options.source.configFile == "" && options.source.dbDir == "" {
+		// Try finding config at usual paths
+		options.source.configFile = influxConfigPathV1()
+		// If not found, try loading a V1 dir under HOME.
+		if options.source.configFile == "" {
+			v1dir, err := influxDirV1()
+			if err != nil {
+				return fmt.Errorf("error fetching default InfluxDB 1.x dir: %w", err)
+			}
+			options.source.dbDir = v1dir
+		}
+	}
+
 	ctx := context.Background()
 	config := zap.NewProductionConfig()
 	config.Level = zap.NewAtomicLevelAt(lvl)
@@ -329,17 +342,12 @@ func runUpgradeE(*cobra.Command, []string) error {
 		return err
 	}
 
-	err = validatePaths(&options.source, &options.target)
+	var v1Config *configV1
-	if err != nil {
+	var genericV1ops *map[string]interface{}
-		return err
-	}
 
-	log.Info("Starting InfluxDB 1.x upgrade")
-
-	var authEnabled bool
 	if options.source.configFile != "" {
-		log.Info("Upgrading config file", zap.String("file", options.source.configFile))
+		// If config is present, use it to set data paths.
-		v1Config, err := upgradeConfig(options.source.configFile, options.target, log)
+		v1Config, genericV1ops, err = loadV1Config(options.source.configFile)
 		if err != nil {
 			return err
 		}
@@ -347,7 +355,31 @@ func runUpgradeE(*cobra.Command, []string) error {
 		options.source.dataDir = v1Config.Data.Dir
 		options.source.walDir = v1Config.Data.WALDir
 		options.source.dbURL = v1Config.dbURL()
-		authEnabled = v1Config.Http.AuthEnabled
+
+		options.target.configPath = filepath.Join(filepath.Dir(options.source.configFile), "config.toml")
+	} else {
+		// Otherwise, assume a standard directory layout.
+		options.source.populateDirs()
+	}
+
+	err = validatePaths(&options.source, &options.target)
+	if err != nil {
+		return err
+	}
+
+	log.Info("Starting InfluxDB 1.x upgrade")
+
+	if genericV1ops != nil {
+		log.Info("Upgrading config file", zap.String("file", options.source.configFile))
+		v2ConfigPath, err := upgradeConfig(*genericV1ops, options.target, log)
+		if err != nil {
+			return err
+		}
+		log.Info(
+			"Config file upgraded.",
+			zap.String("1.x config", options.source.configFile),
+			zap.String("2.x config", v2ConfigPath),
+		)
 	} else {
 		log.Info("No InfluxDB 1.x config file specified, skipping its upgrade")
 	}
@@ -416,7 +448,7 @@ func runUpgradeE(*cobra.Command, []string) error {
 	if err != nil {
 		return err
 	}
-	if usersUpgraded > 0 && !authEnabled {
+	if usersUpgraded > 0 && !v1Config.Http.AuthEnabled {
 		log.Warn(
 			"V1 users were upgraded, but V1 auth was not enabled. Existing clients will fail authentication against V2 if using invalid credentials.",
 		)
@@ -430,29 +462,25 @@ func runUpgradeE(*cobra.Command, []string) error {
 // validatePaths ensures that all filesystem paths provided as input
 // are usable by the upgrade command
 func validatePaths(sourceOpts *optionsV1, targetOpts *optionsV2) error {
-	fi, err := os.Stat(sourceOpts.dbDir)
+	if sourceOpts.dbDir != "" {
-	if err != nil {
+		fi, err := os.Stat(sourceOpts.dbDir)
-		return fmt.Errorf("1.x DB dir '%s' does not exist", sourceOpts.dbDir)
+		if err != nil {
+			return fmt.Errorf("1.x DB dir '%s' does not exist", sourceOpts.dbDir)
+		}
+		if !fi.IsDir() {
+			return fmt.Errorf("1.x DB dir '%s' is not a directory", sourceOpts.dbDir)
+		}
 	}
-	if !fi.IsDir() {
-		return fmt.Errorf("1.x DB dir '%s' is not a directory", sourceOpts.dbDir)
-	}
-	sourceOpts.populateDirs()
 
 	metaDb := filepath.Join(sourceOpts.metaDir, "meta.db")
-	_, err = os.Stat(metaDb)
+	_, err := os.Stat(metaDb)
 	if err != nil {
 		return fmt.Errorf("1.x meta.db '%s' does not exist", metaDb)
 	}
 
-	if sourceOpts.configFile != "" {
+	if targetOpts.configPath != "" {
-		_, err = os.Stat(sourceOpts.configFile)
+		if _, err := os.Stat(targetOpts.configPath); err == nil {
-		if err != nil {
+			return fmt.Errorf("file present at target path for upgraded 2.x config file '%s'", targetOpts.configPath)
-			return fmt.Errorf("1.x config file '%s' does not exist", sourceOpts.configFile)
-		}
-		v2Config := translateV1ConfigPath(sourceOpts.configFile)
-		if _, err := os.Stat(v2Config); err == nil {
-			return fmt.Errorf("file present at target path for upgraded 2.x config file '%s'", v2Config)
 		}
 	}
 
@@ -460,7 +488,7 @@ func validatePaths(sourceOpts *optionsV1, targetOpts *optionsV2) error {
 		return fmt.Errorf("file present at target path for upgraded 2.x bolt DB: '%s'", targetOpts.boltPath)
 	}
 
-	if fi, err = os.Stat(targetOpts.enginePath); err == nil {
+	if fi, err := os.Stat(targetOpts.enginePath); err == nil {
 		if !fi.IsDir() {
 			return fmt.Errorf("upgraded 2.x engine path '%s' is not a directory", targetOpts.enginePath)
 		}

cmd/influxd/upgrade/upgrade_test.go

@@ -33,6 +33,8 @@ func TestPathValidations(t *testing.T) {
 		dbDir:      v1Dir,
 		configFile: "",
 	}
+	sourceOpts.populateDirs()
+
 	targetOpts := &optionsV2{
 		boltPath:       boltPath,
 		cliConfigsPath: configsPath,