refactor(cmd): fix error handling and linter warnings in CLI and loaders (5/5)

.github/linters/.golangci.yml

@@ -1,4 +1,48 @@
+---
 version: "2"
-
-issues:
-  new-from-rev: origin/main
+linters:
+  enable:
+    - gocritic
+    - gosec
+    - revive
+    - unconvert
+    - unparam
+    - wastedassign
+    - whitespace
+  settings:
+    errcheck:
+      check-blank: true
+    gocritic:
+      disabled-checks:
+        - singleCaseSwitch
+        - appendAssign
+    revive:
+      severity: warning
+  exclusions:
+    generated: lax
+    presets:
+      - comments
+      - common-false-positives
+      - legacy
+      - std-error-handling
+    rules:
+      - linters:
+          - dupl
+          - goconst
+          - gosec
+        path: _test\.go
+    paths:
+      - third_party$
+      - builtin$
+      - examples$
+formatters:
+  enable:
+    - gofmt
+    - goimports
+  exclusions:
+    generated: lax
+    paths:
+      - third_party$
+      - builtin$
+      - examples$
+      - \.pb\.go

cmd/deploy/deploy.go

@@ -84,10 +84,10 @@ func newDeployment(cfgPath string, testing bool) (*deploy.Deployment, error) {
 		return nil, err
 	}
 	if cfg.Cluster == nil {
-		return nil, fmt.Errorf("Cluster not specified")
+		return nil, fmt.Errorf("cluster not specified")
 	}
 	if cfg.Ingress == nil {
-		return nil, fmt.Errorf("Ingress not specified")
+		return nil, fmt.Errorf("ingress not specified")
 	}
 	if cfg.CNI == nil {
 		return nil, fmt.Errorf("CNI not specified")

cmd/root.go

@@ -54,7 +54,9 @@ environment.`,
 				return fmt.Errorf("error reading config: %w", err)
 			}
 		}
-		viper.BindPFlags(cmd.Flags())
+		if err := viper.BindPFlags(cmd.Flags()); err != nil {
+			return err
+		}
 		viper.SetDefault("report_usage", false)
 		return nil
 	}

cmd/topology/topology_test.go

@@ -150,7 +150,7 @@ func TestReset(t *testing.T) {
 	}
 	fConfigRelative, closer := writeTopology(t, tWithConfigRelative)
 	defer closer()
-	tWithConfigDNE := &tpb.Topology{
+	tWithConfigMissing := &tpb.Topology{
 		Nodes: []*tpb.Node{{
 			Name:   "resettable1",
 			Vendor: tpb.Vendor(1001),
@@ -164,15 +164,15 @@ func TestReset(t *testing.T) {
 			Vendor: tpb.Vendor(1001),
 			Config: &tpb.Config{
 				ConfigData: &tpb.Config_File{
-					File: "dne",
+					File: "missing",
 				},
 			},
 		}, {
 			Name:   "notresettable1",
 			Vendor: tpb.Vendor(1002),
 		}},
 	}
-	fConfigDNE, closer := writeTopology(t, tWithConfigDNE)
+	fConfigMissing, closer := writeTopology(t, tWithConfigMissing)
 	defer closer()
 	node.Vendor(tpb.Vendor(1001), NewR)
 	node.Vendor(tpb.Vendor(1002), NewNR)
@@ -206,15 +206,15 @@ func TestReset(t *testing.T) {
 		desc: "valid topology push with relative file location",
 		args: []string{"reset", fConfigRelative.Name(), "--skip", "--push"},
 	}, {
-		desc:    "valid topology push with config DNE",
-		args:    []string{"reset", fConfigDNE.Name(), "--skip", "--push"},
+		desc:    "valid topology push with config missing",
+		args:    []string{"reset", fConfigMissing.Name(), "--skip", "--push"},
 		wantErr: "no such file or directory",
 	}, {
-		desc: "valid topology push with config DNE single device",
-		args: []string{"reset", fConfigDNE.Name(), "--skip", "--push", "resettable1"},
+		desc: "valid topology push with config missing single device",
+		args: []string{"reset", fConfigMissing.Name(), "--skip", "--push", "resettable1"},
 	}, {
-		desc:    "valid topology push with config DNE single device invalid",
-		args:    []string{"reset", fConfigDNE.Name(), "--skip", "--push", "dne"},
+		desc:    "valid topology push with config missing single device invalid",
+		args:    []string{"reset", fConfigMissing.Name(), "--skip", "--push", "missing"},
 		wantErr: "not found",
 	}}
 
@@ -234,8 +234,7 @@ func TestReset(t *testing.T) {
 	}()
 	rCmd.PersistentFlags().String("kubecfg", "", "")
 	rCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error {
-		viper.BindPFlags(cmd.Flags())
-		return nil
+		return viper.BindPFlags(cmd.Flags())
 	}
 	buf := bytes.NewBuffer([]byte{})
 	rCmd.SetOut(buf)
@@ -370,7 +369,7 @@ func TestGenerateRing(t *testing.T) {
 		wantErr: "links must be positive",
 	}, {
 		desc:    "file not found",
-		args:    []string{"generate", "ring", "dne.textproto", "2", "8"},
+		args:    []string{"generate", "ring", "missing.textproto", "2", "8"},
 		wantErr: "no such file",
 	}, {
 		desc:    "empty topology",
@@ -571,10 +570,10 @@ func TestPush(t *testing.T) {
 	defer os.Remove(confFile.Name())
 	tWithConfig := &tpb.Topology{
 		Nodes: []*tpb.Node{{
-			Name:   "configable",
+			Name:   "configurable",
 			Vendor: tpb.Vendor(1003),
 		}, {
-			Name:   "notconfigable",
+			Name:   "notconfigurable",
 			Vendor: tpb.Vendor(1004),
 		}},
 	}
@@ -594,22 +593,22 @@ func TestPush(t *testing.T) {
 	}, {
 		desc:    "missing args",
 		wantErr: "invalid args",
-		args:    []string{"push", fConfig.Name(), "configable"},
+		args:    []string{"push", fConfig.Name(), "configurable"},
 	}, {
 		desc:    "no file",
-		args:    []string{"push", fConfig.Name(), "configable", "filedne"},
+		args:    []string{"push", fConfig.Name(), "configurable", "filemissing"},
 		wantErr: "no such file",
 	}, {
 		desc:    "valid file invalid device",
 		args:    []string{"push", fConfig.Name(), "foo", confFile.Name()},
 		wantErr: `node "foo" not found`,
 	}, {
-		desc:    "valid file notconfigable device",
-		args:    []string{"push", fConfig.Name(), "notconfigable", confFile.Name()},
+		desc:    "valid file notconfigurable device",
+		args:    []string{"push", fConfig.Name(), "notconfigurable", confFile.Name()},
 		wantErr: "does not implement ConfigPusher",
 	}, {
 		desc: "valid file",
-		args: []string{"push", fConfig.Name(), "configable", confFile.Name()},
+		args: []string{"push", fConfig.Name(), "configurable", confFile.Name()},
 	}}
 
 	rCmd := New()

deploy/deploy.go

@@ -110,7 +110,10 @@ type Deployment struct {
 }
 
 func (d *Deployment) String() string {
-	b, _ := json.MarshalIndent(d, "", "\t")
+	b, err := json.MarshalIndent(d, "", "\t")
+	if err != nil {
+		return fmt.Sprintf("Deployment: %+v (marshal error: %v)", *d, err)
+	}
 	return string(b)
 }
 
@@ -219,7 +222,7 @@ func (d *Deployment) Deploy(ctx context.Context, kubecfg string) (rerr error) {
 
 	ctx, cancel := context.WithCancel(ctx)
 
-	// Watch the containter status of the pods so we can fail if a container fails to start running.
+	// Watch the container status of the pods so we can fail if a container fails to start running.
 	if w, err := pods.NewWatcher(ctx, kClient, cancel); err != nil {
 		log.Warningf("Failed to start pod watcher: %v", err)
 	} else {

load/deploy.go

@@ -13,7 +13,7 @@ import (
 
 var yamlNodeType = reflect.TypeOf(yaml.Node{})
 
-// open is overriden in tests.
+// open is overridden in tests.
 var open = os.Open
 
 // A Spec represents a structure that yaml can be decoded into.  The type is the
@@ -43,7 +43,7 @@ func Register(kind string, spec *Spec) {
 // A Config represents a KNE deployment configuration.
 type Config struct {
 	Path       string      // Path of the configuration file
-	Dir        string      // Absolute path of the diretory Path is in
+	Dir        string      // Absolute path of the directory Path is in
 	Config     interface{} // The configuration structure
 	Deployment interface{} // Filled by Config.Decode
 
@@ -179,7 +179,7 @@ func (c *Config) decode(v reflect.Value, path []string, tag reflect.StructTag) (
 				}
 			case "spec":
 				if sf.Type != yamlNodeType {
-					return fmt.Errorf("%s is not of type %v\n", strings.Join(append(path, sf.Name), "."), yamlNodeType)
+					return fmt.Errorf("%s is not of type %v", strings.Join(append(path, sf.Name), "."), yamlNodeType)
 				}
 				node := sv.Interface().(yaml.Node)
 				spec = &node
@@ -193,9 +193,9 @@ func (c *Config) decode(v reflect.Value, path []string, tag reflect.StructTag) (
 		switch {
 		case kind == "" && spec == nil:
 		case kind == "":
-			return fmt.Errorf("spec field without kind: %s\n", strings.Join(path, "."))
+			return fmt.Errorf("spec field without kind: %s", strings.Join(path, "."))
 		case spec == nil:
-			return fmt.Errorf("kind field without spec: %s\n", strings.Join(path, "."))
+			return fmt.Errorf("kind field without spec: %s", strings.Join(path, "."))
 		default:
 			// kind and spec have been supplied.