Repository: NetApp/harvest
Pull request #70 (2021-05-26 16:36:06)

Add harvest doctor command
* Use `gopkg.in/yaml.v3` to parse yml - better parser with comment support
* Create a `HarvestConfig` struct - biggest advantage of this struct is it allows us to move away from [StringlyTyped](http://wiki.c2.com/?StringlyTyped) code
* Add `harvest doctor` cmd which:
  * Validates config file passed via `--config`
  * Prints a sanitized version of the config file: removing comments, usernames, passwords, and auth tokens - the goal is to include the output of this for every issue
* Testcase for sanitizing and struct creation

Remaining
* Evaluate the effort to replace the current config parsing code with `HarvestConfig` struct
* Add more checks to `checkAll`
* Evaluate the effort to create structs for everything in `conf/` folder. Haven't seen anything that looks problematic from a schema point of view

The goal is to merge this into main after review and continue improving

cgrinds: Addressed review comments and updated testcase to include addr and host 

Patch:
diff --git a/cmd/harvest/harvest.go b/cmd/harvest/harvest.go
index d3381df0..30813f66 100644
--- a/cmd/harvest/harvest.go
+++ b/cmd/harvest/harvest.go
@@ -21,6 +21,7 @@ import (
 	"goharvest2/cmd/harvest/config"
 	"goharvest2/cmd/harvest/stub"
 	"goharvest2/cmd/harvest/version"
+	"goharvest2/cmd/tools/doctor"
 	"goharvest2/cmd/tools/grafana"
 	"goharvest2/cmd/tools/zapi"
 	"goharvest2/pkg/conf"
@@ -628,6 +629,7 @@ func init() {
 	rootCmd.AddCommand(manageCmd("restart", true))
 	rootCmd.AddCommand(manageCmd("kill", true))
 	rootCmd.AddCommand(config.ConfigCmd, zapi.ZapiCmd, grafana.GrafanaCmd, stub.NewCmd)
+	rootCmd.AddCommand(doctor.Cmd)
 
 	rootCmd.PersistentFlags().StringVar(&opts.config, "config", "./harvest.yml", "harvest config file path")
 	rootCmd.Version = version.String()
diff --git a/cmd/tools/doctor/doctor.go b/cmd/tools/doctor/doctor.go
new file mode 100644
index 00000000..4cec850e
--- /dev/null
+++ b/cmd/tools/doctor/doctor.go
@@ -0,0 +1,123 @@
+package doctor
+
+import (
+	"fmt"
+	"github.com/spf13/cobra"
+	"goharvest2/pkg/conf"
+	"gopkg.in/yaml.v3"
+	"io/ioutil"
+	"os"
+)
+
+type options struct {
+	ShouldPrintConfig bool
+}
+
+var opts = &options{
+	ShouldPrintConfig: false,
+}
+
+var Cmd = &cobra.Command{
+	Use:   "doctor",
+	Short: "Check for potential problems",
+	Long:  "Check for potential problems",
+	Run:   doDoctorCmd,
+}
+
+func doDoctorCmd(cmd *cobra.Command, _ []string) {
+	var config = cmd.Root().PersistentFlags().Lookup("config")
+	doDoctor(config.Value.String())
+}
+
+func doDoctor(path string) {
+	contents, err := ioutil.ReadFile(path)
+	if err != nil {
+		fmt.Printf("error reading config file. err=%+v\n", err)
+		return
+	}
+	if opts.ShouldPrintConfig {
+		printRedactedConfig(path, contents)
+	}
+	checkAll(path, contents)
+}
+
+func checkAll(path string, contents []byte) {
+	// TODO add checks here, see https://github.com/NetApp/harvest/issues/16
+	// print nothing and exit with 0 when all checks pass
+
+	// Validate that the config file can be parsed
+	harvestConfig := &conf.HarvestConfig{}
+	err := yaml.Unmarshal(contents, harvestConfig)
+	if err != nil {
+		fmt.Printf("error reading config file=[%s] %+v\n", path, err)
+		os.Exit(1)
+		return
+	}
+	os.Exit(0)
+}
+
+func printRedactedConfig(path string, contents []byte) string {
+	root := &yaml.Node{}
+	err := yaml.Unmarshal(contents, root)
+	if err != nil {
+		fmt.Printf("error reading config file=[%s] %+v\n", path, err)
+		return ""
+	}
+	var nodes []*yaml.Node
+	collectNodes(root, &nodes)
+	sanitize(nodes)
+	removeComments(root)
+
+	marshaled, err := yaml.Marshal(root)
+	if err != nil {
+		fmt.Printf("error marshalling yaml sanitized from config file=[%s] %+v\n", path, err)
+		return ""
+	}
+	result := string(marshaled)
+	fmt.Println(result)
+	return result
+}
+
+func sanitize(nodes []*yaml.Node) {
+	sanitizeWords := []string{"username", "password", "grafana_api_token", "token",
+		"host", "addr"}
+	for i, node := range nodes {
+		if node == nil {
+			continue
+		}
+		if node.Kind == yaml.ScalarNode && node.ShortTag() == "!!str" {
+			// Update this list of conditionals if there are other tokens you want to sanitize
+			value := node.Value
+			for _, word := range sanitizeWords {
+				if value == word {
+					nodes[i+1].SetString("-REDACTED-")
+				}
+			}
+		}
+		removeComments(node)
+	}
+}
+
+func removeComments(node *yaml.Node) {
+	// Strip all comments since they may contain sensitive information
+	node.HeadComment = ""
+	node.LineComment = ""
+	node.FootComment = ""
+}
+
+func collectNodes(root *yaml.Node, nodes *[]*yaml.Node) {
+	for _, node := range root.Content {
+		*nodes = append(*nodes, node)
+		collectNodes(node, nodes)
+	}
+}
+
+func init() {
+	Cmd.Flags().BoolVarP(
+		&opts.ShouldPrintConfig,
+		"print",
+		"p",
+		false,
+		"print config to console with sensitive info redacted",
+	)
+}
diff --git a/go.mod b/go.mod
index ad6cb057..01ab86cb 100644
--- a/go.mod
+++ b/go.mod
@@ -5,4 +5,5 @@ go 1.15
 require (
 	github.com/hashicorp/go-version v1.3.0
 	github.com/spf13/cobra v1.1.3
+	gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b
 )
diff --git a/go.sum b/go.sum
index 33187a3d..7c41e10b 100644
--- a/go.sum
+++ b/go.sum
@@ -101,8 +101,10 @@ github.com/kisielk/errcheck v1.1.0/go.mod h1:EZBBE59ingxPouuu3KfxchcWSUPOHkagtvW
 github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
 github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=
 github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc=
+github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI=
 github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
 github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
+github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/magiconair/properties v1.8.1/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
 github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU=
@@ -273,6 +275,7 @@ google.golang.org/grpc v1.20.1/go.mod h1:10oTOabMzJvdu6/UiuZezV6QK5dSlG84ov/aaiq
 google.golang.org/grpc v1.21.1/go.mod h1:oYelfM1adQP15Ek0mdvEgi9Df8B9CZIaU1084ijfRaM=
 gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
+gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY=
 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI=
 gopkg.in/ini.v1 v1.51.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
@@ -281,6 +284,8 @@ gopkg.in/yaml.v2 v2.0.0-20170812160011-eb3733d160e7/go.mod h1:JAlM8MvJe8wmxCU4Bl
 gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ=
+gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b h1:h8qDotaEPuJATrMmW04NCwg7v22aHH28wwpauUhK9Oo=
+gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
diff --git a/pkg/conf/conf.go b/pkg/conf/conf.go
index f39823c9..74df45e8 100644
--- a/pkg/conf/conf.go
+++ b/pkg/conf/conf.go
@@ -168,3 +168,61 @@ func GetUniqueExporters(p *node.Node, configFp string) ([]string, error) {
 	}
 	return resultExporters, nil
 }
+
+// Pointers need to be used for struct members where you need
+// to distinguish missing values from zero values. See
+// https://github.com/go-yaml/yaml/issues/113 for details
+// The downside of making all members pointers is accessing
+// the values requires more dereferencing - see doctor_test.go
+
+type Consul struct {
+	Host        *string   `yaml:"host,omitempty"`
+	ServiceName *string   `yaml:"service_name,omitempty"`
+	Tags        *[]string `yaml:"tags,omitempty"`
+}
+
+type Tools struct {
+	GrafanaApiToken *string `yaml:"grafana_api_token,omitempty"`
+}
+
+type Poller struct {
+	Datacenter     *string   `yaml:"datacenter,omitempty"`
+	Addr           *string   `yaml:"addr,omitempty"`
+	AuthStyle      *string   `yaml:"auth_style,omitempty"`
+	Username       *string   `yaml:"username,omitempty"`
+	Password       *string   `yaml:"password,omitempty"`
+	UseInsecureTls *bool     `yaml:"use_insecure_tls,omitempty"`
+	SslCert        *string   `yaml:"ssl_cert,omitempty"`
+	SslKey         *string   `yaml:"ssl_key,omitempty"`
+	LogMaxBytes    *int64    `yaml:"log_max_bytes,omitempty"`
+	LogMaxFiles    *int      `yaml:"log_max_files,omitempty"`
+	Exporters      *[]string `yaml:"exporters,omitempty"`
+	Collectors     *[]string `yaml:"collectors,omitempty"`
+}
+
+type Exporter struct {
+	Port              *int      `yaml:"port,omitempty"`
+	Type              *string   `yaml:"exporter,omitempty"`
+	Addr              *string   `yaml:"addr,omitempty"`
+	LocalHttpAddr     *string   `yaml:"local_http_addr,omitempty"`
+	GlobalPrefix      *string   `yaml:"global_prefix,omitempty"`
+	AllowedAddrs      *[]string `yaml:"allow_addrs,omitempty"`
+	AllowedAddrsRegex *[]string `yaml:"allow_addrs_regex,omitempty"`
+	CacheMaxKeep      *string   `yaml:"cache_max_keep,omitempty"`
+	ShouldAddMetaTags *bool     `yaml:"add_meta_tags,omitempty"`
+	Consul            *Consul   `yaml:"consul,omitempty"`
+
+	// InfluxDB specific
+	Bucket        *string `yaml:"bucket,omitempty"`
+	Org           *string `yaml:"org,omitempty"`
+	Token         *string `yaml:"token,omitempty"`
+	Precision     *string `yaml:"precision,omitempty"`
+	ClientTimeout *string `yaml:"client_timeout,omitempty"`
+}
+
+type HarvestConfig struct {
+	Tools     *Tools               `yaml:"Tools,omitempty"`
+	Exporters *map[string]Exporter `yaml:"Exporters,omitempty"`
+	Pollers   *map[string]Poller   `yaml:"Pollers,omitempty"`
+	Defaults  *Poller              `yaml:"Defaults,omitempty"`
+}


Test patch:
diff --git a/cmd/tools/doctor/doctor_test.go b/cmd/tools/doctor/doctor_test.go
new file mode 100644
index 00000000..c03b7cea
--- /dev/null
+++ b/cmd/tools/doctor/doctor_test.go
@@ -0,0 +1,66 @@
+package doctor
+
+import (
+	"fmt"
+	"goharvest2/pkg/conf"
+	"gopkg.in/yaml.v3"
+	"io/ioutil"
+	"strings"
+	"testing"
+)
+
+func TestRedaction(t *testing.T) {
+	assertRedacted(t, `foo: bar`, `foo: bar`)
+	assertRedacted(t, `username: pass`, `username: -REDACTED-`)
+	assertRedacted(t, `password: f`, `password: -REDACTED-`)
+	assertRedacted(t, `grafana_api_token: secret`, `grafana_api_token: -REDACTED-`)
+	assertRedacted(t, `token: secret`, `token: -REDACTED-`)
+	assertRedacted(t, "# foo\nusername: pass\n#foot", `username: -REDACTED-`)
+	assertRedacted(t, `host: 1.2.3.4`, `host: -REDACTED-`)
+	assertRedacted(t, `addr: 1.2.3.4`, `addr: -REDACTED-`)
+}
+
+func assertRedacted(t *testing.T, input, redacted string) {
+	redacted = strings.TrimSpace(redacted)
+	input = printRedactedConfig("test", []byte(input))
+	input = strings.TrimSpace(input)
+	if input != redacted {
+		t.Fatalf(`input=[%s] != redacted=[%s]`, input, redacted)
+	}
+}
+
+func TestConfigToStruct(t *testing.T) {
+	path := "testConfig.yml"
+	contents, err := ioutil.ReadFile(path)
+	if err != nil {
+		fmt.Printf("error reading config file %+v\n", err)
+		return
+	}
+	harvestConfig := &conf.HarvestConfig{}
+	err = yaml.Unmarshal(contents, harvestConfig)
+	if err != nil {
+		fmt.Printf("error reading config file=[%s] %+v\n", path, err)
+		return
+	}
+
+	if harvestConfig.Defaults.Addr != nil {
+		t.Fatalf(`expected harvestConfig.Defaults.Addr to be nil, actual=[%+v]`,
+			harvestConfig.Defaults.Addr)
+	}
+	if len(*harvestConfig.Defaults.Collectors) != 2 {
+		t.Fatalf(`expected two default collectors, actual=%+v`,
+			*harvestConfig.Defaults.Collectors)
+	}
+
+	allowedRegexes := (*harvestConfig.Exporters)["influxy"].AllowedAddrsRegex
+	if (*allowedRegexes)[0] != "^192.168.0.\\d+$" {
+		t.Fatalf(`expected allow_addrs_regex to be ^192.168.0.\d+$ actual=%+v`,
+			(*allowedRegexes)[0])
+	}
+
+	collectors := (*harvestConfig.Pollers)["infinity2"].Collectors
+	if (*collectors)[0] != "Zapi" {
+		t.Fatalf(`expected infinity2 collectors to contain Zapi actual=%+v`,
+			(*collectors)[0])
+	}
+}
diff --git a/cmd/tools/doctor/testConfig.yml b/cmd/tools/doctor/testConfig.yml
new file mode 100644
index 00000000..9e548c60
--- /dev/null
+++ b/cmd/tools/doctor/testConfig.yml
@@ -0,0 +1,81 @@
+Tools:
+  grafana_api_token: 'aaa-bbb-ccc-ddd'
+  token: [ 'aaa-bbb-ccc-ddd' ]
+
+Exporters:
+  prometheus:
+    exporter: Prometheus
+    addr: 0.0.0.0
+    port: 12990
+  pluto: # name of your exporter, can be any valid yaml string
+    exporter: Prometheus
+    port: 12990              # ignored since this exporter contains a consul block
+    consul: # consul block added because we want service discovery
+      host: 1.2.3.4          # ip or hostname of Consul cluster
+      service_name: monitor  # all pollers exporting to pluto will be registered with the service name of 'monitor'
+      tags:
+        - netapp             # each of the pollers exporting to pluto will be tagged with two tags: 'netapp' and 'harvest'
+        - harvest
+  influxy:
+    exporter: InfluxDB
+    addr: localhost
+    bucket: harvest
+    org: harvest
+    token: abcdefghijklmnopqrstuvwxyz
+    allow_addrs_regex:
+      - ^192.168.0.\d+$
+
+Defaults:
+  collectors:
+    - Zapi
+    - ZapiPerf
+  use_insecure_tls: false
+  username: myuser
+  password: mypasw
+
+Pollers:
+  # this is a special/optional poller that provides stats about harvest itself
+  # to remove this poller, simply delete the section "unix"
+  unix:
+    datacenter: local
+    addr: localhost
+    log_max_files: 1
+    collectors:
+      - Unix
+    exporters:
+      - prometheus
+
+  zeros:
+    log_max_files: 0
+    use_insecure_tls: false
+    collectors: [ ]
+
+  # add your Cdot / 7Mode pollers below, example:
+  cluster-01:
+    datacenter: DC-01
+    addr: 10.0.0.1
+    auth_style: basic_auth
+    username: myuser
+    password: mypasw
+    use_insecure_tls: true   # since we don't use certificate authentication
+    exporters:
+      - prometheus1
+  #no need to add collectors, already defined in Defaults!
+
+  # example where we use certificate authentication
+  infinity:
+    datacenter: DC-01
+    addr: 10.0.0.2
+    auth_style: certificate_auth
+    ssl_cert: /path/to/cert.pem
+    ssl_key: /path/to/key.key
+    use_insecure_tls: false  # if harvest should be able to authenticate your server, set this true
+    # otherwise set it to false (if cluster has no valid server certificates)
+    # all the other parameters should be the same
+
+  infinity2:
+    datacenter: DC-01
+    addr: 10.0.0.2
+    collectors: [ Zapi ]
+
+ll: grafana_api_token grafana_api_token