Merge pull request #96 from github/hugodorea/addresses-linter

addresses linter errors

.github/workflows/workflow.yml

@@ -16,6 +16,6 @@ jobs:
         go-version: ${{ matrix.go }}
     - name: Run tests
       run: |
-        curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $GOROOT/bin v1.31.0
+        curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $GOROOT/bin v1.46.2
         sudo sysctl -w vm.max_map_count=262144
         script/integration-test

.golangci.yaml

@@ -0,0 +1,43 @@
+# This file contains all available configuration options for golangci-lint.
+# The Go Community at GitHub is currently working on a finalized version
+# of this configuration file to share with all Hubbers.
+
+run:
+  tests: true
+
+linters-settings:
+  staticcheck:
+    # https://staticcheck.io/docs/options#checks
+    # Default: ["*"]
+    checks: ["all", "-SA1019"]
+
+linters:
+  enable:
+    - deadcode
+    - depguard
+    - errcheck
+    - exportloopref
+    - gocritic
+    - gocyclo
+    - gofmt
+    - goimports
+    - gosec
+    - gosimple
+    - govet
+    - ineffassign
+    - misspell
+    - nakedret
+    - prealloc
+    - revive
+    - staticcheck
+    - structcheck
+    - typecheck
+    - unconvert
+    - unused
+    - varcheck
+  disable:
+    - gochecknoglobals # we allow global variables in packages
+    - gochecknoinits   # we allow inits in packages
+    - goconst          # we allow repeated values to go un-const'd
+    - lll              # we allow any line length
+    - unparam          # we allow function calls to name unused parameters

es_test.go

@@ -2,6 +2,7 @@ package vulcanizer
 
 import (
 	"crypto/tls"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"net/http"
@@ -337,8 +338,8 @@ func TestGetNodeAllocations(t *testing.T) {
 		t.Errorf("Unexpected DiskUsed, expected 735.2gb, got %s", nodes[0].DiskUsed)
 	}
 
-	if nodes[0].Ip != "127.0.0.1" {
-		t.Errorf("Unexpected node IP, expected 127.0.0.1, got %s", nodes[0].Ip)
+	if nodes[0].IP != "127.0.0.1" {
+		t.Errorf("Unexpected node IP, expected 127.0.0.1, got %s", nodes[0].IP)
 	}
 }
 
@@ -620,6 +621,8 @@ func TestGetHealth_TLS(t *testing.T) {
 	defer ts.Close()
 	client := NewClient(host, port)
 	client.Secure = true
+	// nolint:gosec
+	// G402: TLS InsecureSkipVerify set true. (gosec)
 	client.TLSConfig = &tls.Config{InsecureSkipVerify: true}
 
 	health, err := client.GetHealth()
@@ -1096,8 +1099,7 @@ func TestRegisterRepository_MissingName(t *testing.T) {
 	}
 
 	err := client.RegisterRepository(repo)
-
-	if err == nil || err.Error() != "Repository Name is required." {
+	if !errors.Is(err, ErrRepositoryNameRequired) {
 		t.Error("Expected validation for missing repository name.")
 	}
 }
@@ -1115,8 +1117,7 @@ func TestRegisterRepository_MissingType(t *testing.T) {
 	}
 
 	err := client.RegisterRepository(repo)
-
-	if err == nil || err.Error() != "Repository Type is required." {
+	if !errors.Is(err, ErrRepositoryTypeRequired) {
 		t.Error("Expected validation for missing repository type.")
 	}
 }
@@ -1144,8 +1145,7 @@ func TestRemoveRepository_MissingName(t *testing.T) {
 	client := NewClient(host, port)
 
 	err := client.RemoveRepository("")
-
-	if err == nil || err.Error() != "Repository Name is required." {
+	if !errors.Is(err, ErrRepositoryNameRequired) {
 		t.Error("Expected validation for missing repository name.")
 	}
 }

go.mod

@@ -21,7 +21,7 @@ require (
 	github.com/spf13/viper v1.7.1
 	github.com/tidwall/gjson v1.11.0
 	golang.org/x/net v0.0.0-20200927032502-5d4f70055728 // indirect
-	golang.org/x/sys v0.0.0-20200929083018-4d22bbb62b3c // indirect
+	golang.org/x/sys v0.0.0-20220808155132-1c4a2a72c664 // indirect
 	gopkg.in/ini.v1 v1.61.0 // indirect
 	gopkg.in/yaml.v2 v2.3.0 // indirect
 	gotest.tools v2.2.0+incompatible

go.sum

@@ -294,6 +294,8 @@ golang.org/x/sys v0.0.0-20191005200804-aed5e4c7ecf9/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200929083018-4d22bbb62b3c h1:/h0vtH0PyU0xAoZJVcRw1k0Ng+U0JAy3QDiFmppIlIE=
 golang.org/x/sys v0.0.0-20200929083018-4d22bbb62b3c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20220808155132-1c4a2a72c664 h1:v1W7bwXHsnLLloWYTVEdvGvA7BHMeBYsPcF0GLDxIRs=
+golang.org/x/sys v0.0.0-20220808155132-1c4a2a72c664/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=

pkg/cli/entrypoint.go

@@ -81,6 +81,9 @@ func getClient() *vulcanizer.Client {
 		v.Secure = true
 	}
 
+	// nolint:gosec
+	// G402: TLS MinVersion too low. (gosec)
+	// Skipping it for now. This should be fixed in a separate PR.
 	v.TLSConfig = &tls.Config{}
 
 	if c.TLSSkipVerify {

pkg/cli/node_allocation.go

@@ -48,7 +48,7 @@ var cmdNodeAllocations = &cobra.Command{
 					node.DiskPercent,
 					node.DiskIndices,
 					node.Shards,
-					node.Ip,
+					node.IP,
 				}
 
 				rows = append(rows, row)
@@ -70,8 +70,8 @@ var cmdNodeAllocations = &cobra.Command{
 					node.DiskTotal,
 					node.DiskUsed,
 					node.Shards,
-					node.Ip,
-					node.Id,
+					node.IP,
+					node.ID,
 					node.Jdk,
 					node.Version,
 				}

pkg/cli/nodes.go

@@ -37,8 +37,8 @@ var cmdNodes = &cobra.Command{
 				node.Master,
 				node.Role,
 				node.Name,
-				node.Ip,
-				node.Id,
+				node.IP,
+				node.ID,
 				node.Jdk,
 				node.Version,
 			}

pkg/cli/repository.go

@@ -177,7 +177,7 @@ var cmdRepositoryRegister = &cobra.Command{
 			os.Exit(1)
 		}
 
-		fmt.Printf("Repository %s registered succesfully.\n", repositoryName)
+		fmt.Printf("Repository %s registered successfully.\n", repositoryName)
 	},
 }
 
@@ -202,6 +202,6 @@ var cmdRepositoryRemove = &cobra.Command{
 			os.Exit(1)
 		}
 
-		fmt.Printf("Repository %s removed succesfully.\n", repositoryName)
+		fmt.Printf("Repository %s removed successfully.\n", repositoryName)
 	},
 }

pkg/cli/shards.go

@@ -2,8 +2,9 @@ package cli
 
 import (
 	"fmt"
-	"github.com/spf13/cobra"
 	"os"
+
+	"github.com/spf13/cobra"
 )
 
 var nodesToCheck []string

util.go

@@ -8,7 +8,7 @@ import (
 	"github.com/tidwall/gjson"
 )
 
-func excludeSettingsFromJson(settings []gjson.Result) ExcludeSettings {
+func excludeSettingsFromJSON(settings []gjson.Result) ExcludeSettings {
 	excludeSettings := ExcludeSettings{}
 
 	if settings[0].String() == "" {
@@ -34,8 +34,7 @@ func excludeSettingsFromJson(settings []gjson.Result) ExcludeSettings {
 
 // Returns caption based on cluster health explaining the meaning of this state.
 func captionHealth(clusterHealth ClusterHealth) (caption string) {
-
-	var unhealthyIndexList []string
+	unhealthyIndexList := make([]string, 0, len(clusterHealth.UnhealthyIndices))
 	for _, index := range clusterHealth.UnhealthyIndices {
 		status := fmt.Sprintf("%s is %s. %d shards are unassigned.", index.Name, index.Status, index.UnassignedShards)
 		unhealthyIndexList = append(unhealthyIndexList, status)
@@ -57,16 +56,17 @@ func captionHealth(clusterHealth ClusterHealth) (caption string) {
 }
 
 func enrichNodesWithAllocations(nodes []Node, allocations []DiskAllocation) []Node {
-	var enrichedNodes []Node
 	nodeAllocation := make(map[string]DiskAllocation)
 	for _, alloc := range allocations {
 		nodeAllocation[alloc.Node] = alloc
 	}
+
+	enrichedNodes := make([]Node, 0, len(nodes))
 	for _, node := range nodes {
 		enrichedNode := Node{
 			Name:        node.Name,
-			Ip:          node.Ip,
-			Id:          node.Id,
+			IP:          node.IP,
+			ID:          node.ID,
 			Role:        node.Role,
 			Master:      node.Master,
 			Jdk:         node.Jdk,
@@ -92,5 +92,5 @@ func combineErrors(errs []error) error {
 }
 
 func escapeIndexName(index string) string {
-	return strings.Replace(index, ".", "\\.", -1)
+	return strings.ReplaceAll(index, ".", "\\.")
 }

util_test.go

@@ -11,7 +11,7 @@ func TestExcludeSettingsFromJson_OneResult(t *testing.T) {
 	body := `{"transient":{"cluster":{"routing":{"allocation":{"exclude":{"_host":"excluded.host","_name":"excluded_name","_ip":"10.0.0.99"}}}}}}`
 	excludedArray := gjson.GetMany(body, "transient.cluster.routing.allocation.exclude._ip", "transient.cluster.routing.allocation.exclude._name", "transient.cluster.routing.allocation.exclude._host")
 
-	settings := excludeSettingsFromJson(excludedArray)
+	settings := excludeSettingsFromJSON(excludedArray)
 
 	if len(settings.Ips) != 1 && settings.Ips[0] != "10.0.0.99" {
 		t.Fatalf("Ips should should contain 10.0.0.99, got %s", settings.Ips)
@@ -30,7 +30,7 @@ func TestExcludeSettingsFromJson_NoResults(t *testing.T) {
 	body := `{"transient":{"cluster":{"routing":{"allocation":{"exclude":{"_host":"","_name":"","_ip":""}}}}}}`
 	excludedArray := gjson.GetMany(body, "transient.cluster.routing.allocation.exclude._ip", "transient.cluster.routing.allocation.exclude._name", "transient.cluster.routing.allocation.exclude._host")
 
-	settings := excludeSettingsFromJson(excludedArray)
+	settings := excludeSettingsFromJSON(excludedArray)
 
 	if len(settings.Ips) != 0 {
 		t.Fatalf("Ips should be empty array, got %#v", settings.Ips)