From c51a0df1ef469372d8bb4051ad3bb93ed49a4d5e Mon Sep 17 00:00:00 2001
From: William Baker <wbaker@gmail.com>
Date: Wed, 15 Dec 2021 20:57:01 -0600
Subject: [PATCH] feat: error out when config file contains 1.x config values
 (#22996)

* feat: error out when config file contains invalid options

* feat: debug logging when loading a config file

* fix: only detect flags from 1.x

* test: update tests to use toml configs
---
 cmd/influxd/launcher/cmd.go      | 69 ++++++++++++++++++++++++++++++++
 cmd/influxd/launcher/cmd_test.go | 69 ++++++++++++++++++++++++++++++++
 cmd/influxd/launcher/launcher.go |  4 ++
 3 files changed, 142 insertions(+)
 create mode 100644 cmd/influxd/launcher/cmd_test.go

diff --git a/cmd/influxd/launcher/cmd.go b/cmd/influxd/launcher/cmd.go
index be73b481ed..5f6cdcd649 100644
--- a/cmd/influxd/launcher/cmd.go
+++ b/cmd/influxd/launcher/cmd.go
@@ -6,6 +6,7 @@ import (
 	"os"
 	"path/filepath"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/influxdata/influxdb/v2/bolt"
@@ -31,6 +32,14 @@ const (
 	MaxInt = 1<<uint(strconv.IntSize-1) - 1
 )
 
+func errInvalidFlags(flags []string, configFile string) error {
+	return fmt.Errorf(
+		"error: found flags from an InfluxDB 1.x configuration in config file at %s - see https://docs.influxdata.com/influxdb/latest/reference/config-options/ for flags supported on this version of InfluxDB: %s",
+		configFile,
+		strings.Join(flags, ","),
+	)
+}
+
 // NewInfluxdCommand constructs the root of the influxd CLI, along with a `run` subcommand.
 // The `run` subcommand is set as the default to execute.
 func NewInfluxdCommand(ctx context.Context, v *viper.Viper) (*cobra.Command, error) {
@@ -46,6 +55,11 @@ func NewInfluxdCommand(ctx context.Context, v *viper.Viper) (*cobra.Command, err
 		return nil, err
 	}
 
+	// Error out if invalid flags are found in the config file. This may indicate trying to launch 2.x using a 1.x config.
+	if invalidFlags := invalidFlags(v); len(invalidFlags) > 0 {
+		return nil, errInvalidFlags(invalidFlags, v.ConfigFileUsed())
+	}
+
 	runCmd := &cobra.Command{
 		Use:  "run",
 		RunE: cmd.RunE,
@@ -67,6 +81,17 @@ func NewInfluxdCommand(ctx context.Context, v *viper.Viper) (*cobra.Command, err
 	return cmd, nil
 }
 
+func invalidFlags(v *viper.Viper) []string {
+	var invalid []string
+	for _, k := range v.AllKeys() {
+		if inOneDotExFlagsList(k) {
+			invalid = append(invalid, k)
+		}
+	}
+
+	return invalid
+}
+
 func setCmdDescriptions(cmd *cobra.Command) {
 	cmd.Short = "Start the influxd server"
 	cmd.Long = `
@@ -610,3 +635,47 @@ func (o *InfluxdOpts) BindCliOpts() []cli.Opt {
 		},
 	}
 }
+
+var (
+	oneDotExFlagsList = []string{
+		// "reporting-disabled" is valid in both 1x and 2x configs
+		"bind-address", // global setting is called "http-bind-address" on 2x
+
+		// Remaining flags, when parsed from a 1.x config file, will be in sub-sections prefixed by these headers:
+		"collectd.",
+		"continuous_queries.",
+		"coordinator.",
+		"data.",
+		"graphite.",
+		"http.",
+		"logging.",
+		"meta.",
+		"monitor.",
+		"opentsdb.",
+		"retention.",
+		"shard-precreation.",
+		"subscriber.",
+		"tls.",
+		"udp.",
+	}
+)
+
+// compareFlags checks if a given flag from the read configuration matches one from the list. If the value from the list
+// ends in a ".", the given flag is check for that prefix. Otherwise, the flag is checked for equality.
+func compareFlags(key, fromList string) bool {
+	if strings.HasSuffix(fromList, ".") {
+		return strings.HasPrefix(key, fromList)
+	}
+
+	return strings.EqualFold(key, fromList)
+}
+
+func inOneDotExFlagsList(key string) bool {
+	for _, f := range oneDotExFlagsList {
+		if compareFlags(key, f) {
+			return true
+		}
+	}
+
+	return false
+}
diff --git a/cmd/influxd/launcher/cmd_test.go b/cmd/influxd/launcher/cmd_test.go
new file mode 100644
index 0000000000..167fa85f60
--- /dev/null
+++ b/cmd/influxd/launcher/cmd_test.go
@@ -0,0 +1,69 @@
+package launcher
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/spf13/viper"
+	"github.com/stretchr/testify/require"
+)
+
+func TestInvalidFlags(t *testing.T) {
+	t.Parallel()
+
+	v2config := `
+bolt-path = "/db/.influxdbv2/influxd.bolt"
+engine-path = "/db/.influxdbv2/engine"
+http-bind-address = ":8086"
+`
+
+	v1config := `
+reporting-disabled = false
+
+# Bind address to use for the RPC service for backup and restore.
+bind-address = "127.0.0.1:8088"
+
+[http]
+  flux-enabled = false
+
+[data]
+  index-version = "inmem"`
+
+	tests := []struct {
+		name   string
+		config string
+		want   []string
+	}{
+		{
+			name:   "empty config",
+			config: "",
+			want:   []string(nil),
+		},
+		{
+			name:   "v2 config",
+			config: v2config,
+			want:   []string(nil),
+		},
+		{
+			name:   "v1 config",
+			config: v1config,
+			want:   []string{"http.flux-enabled", "data.index-version", "bind-address"},
+		},
+		{
+			name:   "mixed config",
+			config: v2config + v1config,
+			want:   []string{"http.flux-enabled", "data.index-version", "bind-address"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			r := strings.NewReader(tt.config)
+			v := viper.GetViper()
+			v.SetConfigType("toml")
+			require.NoError(t, v.ReadConfig(r))
+			got := invalidFlags(v)
+			require.ElementsMatch(t, tt.want, got)
+		})
+	}
+}
diff --git a/cmd/influxd/launcher/launcher.go b/cmd/influxd/launcher/launcher.go
index 69583bc12e..10444d755d 100644
--- a/cmd/influxd/launcher/launcher.go
+++ b/cmd/influxd/launcher/launcher.go
@@ -216,6 +216,10 @@ func (m *Launcher) run(ctx context.Context, opts *InfluxdOpts) (err error) {
 	)
 	m.initTracing(opts)
 
+	if p := opts.Viper.ConfigFileUsed(); p != "" {
+		m.log.Debug("loaded config file", zap.String("path", p))
+	}
+
 	// Parse feature flags.
 	// These flags can be used to modify the remaining setup logic in this method.
 	// They will also be injected into the contexts of incoming HTTP requests at runtime,