Skip to content

Commit

Permalink
lint: Enable goconst
Browse files Browse the repository at this point in the history
goconst finds repeated strings that could be replaced by a constant:
https://github.com/jgautheron/goconst

Part of #217

Signed-off-by: Andrew Seigner <siggy@buoyant.io>
  • Loading branch information
siggy committed Feb 24, 2019
1 parent e300309 commit 9abd59f
Show file tree
Hide file tree
Showing 19 changed files with 93 additions and 92 deletions.
2 changes: 1 addition & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ linters:
enable:
- deadcode
- depguard
- goconst
- gofmt
- golint
- gosimple
Expand All @@ -20,7 +21,6 @@ linters:
# - dupl
# - gochecknoglobals
# - gochecknoinits
# - goconst
# - gocyclo
# - goimports
# - gosec
Expand Down
16 changes: 7 additions & 9 deletions cli/cmd/endpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"os"
"sort"
Expand All @@ -23,25 +22,24 @@ type endpointsOptions struct {
outputFormat string
}

var (
const (
podHeader = "POD"
)

// validate performs all validation on the command-line options.
// It returns the first error encountered, or `nil` if the options are valid.
func (o *endpointsOptions) validate() error {
switch o.outputFormat {
case "table", "json", "":
if o.outputFormat == tableOutput || o.outputFormat == jsonOutput {
return nil
}

return errors.New("--output currently only supports table and json")
return fmt.Errorf("--output currently only supports %s and %s", tableOutput, jsonOutput)
}

func newEndpointsOptions() *endpointsOptions {
return &endpointsOptions{
namespace: "",
outputFormat: "",
outputFormat: tableOutput,
}
}

Expand Down Expand Up @@ -89,7 +87,7 @@ requests.`,
}

cmd.PersistentFlags().StringVarP(&options.namespace, "namespace", "n", options.namespace, "Namespace of the specified endpoints (default: all namespaces)")
cmd.PersistentFlags().StringVarP(&options.outputFormat, "output", "o", options.outputFormat, "Output format; currently only \"table\" and \"json\" are supported (default \"table\")")
cmd.PersistentFlags().StringVarP(&options.outputFormat, "output", "o", options.outputFormat, fmt.Sprintf("Output format; one of: \"%s\" or \"%s\"", tableOutput, jsonOutput))

return cmd
}
Expand Down Expand Up @@ -166,13 +164,13 @@ func writeEndpointsToBuffer(endpoints *pb.EndpointsResponse, w *tabwriter.Writer
}

switch options.outputFormat {
case "table", "":
case tableOutput:
if len(endpointsTables) == 0 {
fmt.Fprintln(os.Stderr, "No endpoints found.")
os.Exit(0)
}
printEndpointsTables(endpointsTables, w, options, maxPodLength, maxNamespaceLength)
case "json":
case jsonOutput:
printEndpointsJSON(endpointsTables, w)
}
}
Expand Down
4 changes: 2 additions & 2 deletions cli/cmd/endpoints_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func TestEndpoints(t *testing.T) {
}, t)
})

options.outputFormat = "json"
options.outputFormat = jsonOutput
t.Run("Returns endpoints (json)", func(t *testing.T) {
testEndpointsCall(endpointsExp{
options: options,
Expand All @@ -42,7 +42,7 @@ func TestEndpoints(t *testing.T) {
}, t)
})

options.outputFormat = "json"
options.outputFormat = jsonOutput
t.Run("Returns all namespace endpoints (json)", func(t *testing.T) {
testEndpointsCall(endpointsExp{
options: options,
Expand Down
33 changes: 14 additions & 19 deletions cli/cmd/inject_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ type injectYAML struct {
testInjectOptions *injectOptions
}

func mkFilename(filename string, verbose bool) string {
if verbose {
return fmt.Sprintf("%s.verbose", filename)
}
return filename
}

func testUninjectAndInject(t *testing.T, tc injectYAML) {
file, err := os.Open("testdata/" + tc.inputFileName)
if err != nil {
Expand All @@ -34,10 +41,7 @@ func testUninjectAndInject(t *testing.T, tc injectYAML) {
}
diffTestdata(t, tc.goldenFileName, output.String())

reportFileName := tc.reportFileName
if verbose {
reportFileName += ".verbose"
}
reportFileName := mkFilename(tc.reportFileName, verbose)
diffTestdata(t, reportFileName, report.String())
}

Expand All @@ -46,18 +50,18 @@ func TestUninjectAndInject(t *testing.T) {
defaultOptions.linkerdVersion = "testinjectversion"

tlsOptions := newInjectOptions()
tlsOptions.linkerdVersion = "testinjectversion"
tlsOptions.linkerdVersion = defaultOptions.linkerdVersion
tlsOptions.tls = "optional"

proxyResourceOptions := newInjectOptions()
proxyResourceOptions.linkerdVersion = "testinjectversion"
proxyResourceOptions.linkerdVersion = defaultOptions.linkerdVersion
proxyResourceOptions.proxyCPURequest = "110m"
proxyResourceOptions.proxyMemoryRequest = "100Mi"
proxyResourceOptions.proxyCPULimit = "160m"
proxyResourceOptions.proxyMemoryLimit = "150Mi"

noInitContainerOptions := newInjectOptions()
noInitContainerOptions.linkerdVersion = "testinjectversion"
noInitContainerOptions.linkerdVersion = defaultOptions.linkerdVersion
noInitContainerOptions.noInitContainer = true

testCases := []injectYAML{
Expand Down Expand Up @@ -212,10 +216,7 @@ func testInjectCmd(t *testing.T, tc injectCmd) {
t.Fatalf("Expected no standard output, but got: %s", outBuffer)
}

stdErrGoldenFileName := tc.stdErrGoldenFileName
if verbose {
stdErrGoldenFileName += ".verbose"
}
stdErrGoldenFileName := mkFilename(tc.stdErrGoldenFileName, verbose)
diffTestdata(t, stdErrGoldenFileName, errBuffer.String())
}

Expand Down Expand Up @@ -266,10 +267,7 @@ func testInjectFilePath(t *testing.T, tc injectFilePath) {
}
diffTestdata(t, tc.expectedFile, actual.String())

stdErrFile := tc.stdErrFile
if verbose {
stdErrFile += ".verbose"
}
stdErrFile := mkFilename(tc.stdErrFile, verbose)
diffTestdata(t, stdErrFile, errBuf.String())
}

Expand All @@ -288,10 +286,7 @@ func testReadFromFolder(t *testing.T, resourceFolder string, expectedFolder stri
expectedFile := filepath.Join(expectedFolder, "injected_nginx_redis.yaml")
diffTestdata(t, expectedFile, actual.String())

stdErrFileName := filepath.Join(expectedFolder, "injected_nginx_redis.stderr")
if verbose {
stdErrFileName += ".verbose"
}
stdErrFileName := mkFilename(filepath.Join(expectedFolder, "injected_nginx_redis.stderr"), verbose)
diffTestdata(t, stdErrFileName, errBuf.String())
}

Expand Down
8 changes: 4 additions & 4 deletions cli/cmd/install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,27 +112,27 @@ func TestRender(t *testing.T) {
haOptions := newInstallOptions()
haOptions.highAvailability = true
haConfig, _ := validateAndBuildConfig(haOptions)
haConfig.UUID = "deaab91a-f4ab-448a-b7d1-c832a2fa0a60"
haConfig.UUID = defaultConfig.UUID

haWithOverridesOptions := newInstallOptions()
haWithOverridesOptions.highAvailability = true
haWithOverridesOptions.controllerReplicas = 2
haWithOverridesOptions.proxyCPURequest = "400m"
haWithOverridesOptions.proxyMemoryRequest = "300Mi"
haWithOverridesConfig, _ := validateAndBuildConfig(haWithOverridesOptions)
haWithOverridesConfig.UUID = "deaab91a-f4ab-448a-b7d1-c832a2fa0a60"
haWithOverridesConfig.UUID = defaultConfig.UUID

noInitContainerOptions := newInstallOptions()
noInitContainerOptions.noInitContainer = true
noInitContainerConfig, _ := validateAndBuildConfig(noInitContainerOptions)
noInitContainerConfig.UUID = "deaab91a-f4ab-448a-b7d1-c832a2fa0a60"
noInitContainerConfig.UUID = defaultConfig.UUID

noInitContainerWithProxyAutoInjectOptions := newInstallOptions()
noInitContainerWithProxyAutoInjectOptions.noInitContainer = true
noInitContainerWithProxyAutoInjectOptions.proxyAutoInject = true
noInitContainerWithProxyAutoInjectOptions.tls = "optional"
noInitContainerWithProxyAutoInjectConfig, _ := validateAndBuildConfig(noInitContainerWithProxyAutoInjectOptions)
noInitContainerWithProxyAutoInjectConfig.UUID = "deaab91a-f4ab-448a-b7d1-c832a2fa0a60"
noInitContainerWithProxyAutoInjectConfig.UUID = defaultConfig.UUID

testCases := []struct {
config installConfig
Expand Down
8 changes: 5 additions & 3 deletions cli/cmd/profile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,11 @@ func TestValidateOptions(t *testing.T) {
t.Fatalf("validateOptions returned unexpected error: %s (expected: %s) for options: %+v", err, exp, options)
}

serviceName := "service-name"

options = newProfileOptions()
options.template = true
options.name = "service-name"
options.name = serviceName
options.namespace = ""
exp = fmt.Errorf("invalid namespace \"%s\": [a DNS-1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')]", options.namespace)
err = options.validate()
Expand All @@ -113,7 +115,7 @@ func TestValidateOptions(t *testing.T) {

options = newProfileOptions()
options.template = true
options.name = "service-name"
options.name = serviceName
options.namespace = "invalid/namespace"
exp = fmt.Errorf("invalid namespace \"%s\": [a DNS-1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')]", options.namespace)
err = options.validate()
Expand All @@ -123,7 +125,7 @@ func TestValidateOptions(t *testing.T) {

options = newProfileOptions()
options.template = true
options.name = "service-name"
options.name = serviceName
options.namespace = "7eet-ns"
err = options.validate()
if err != nil {
Expand Down
13 changes: 8 additions & 5 deletions cli/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package cmd

import (
"bytes"
"errors"
"fmt"
"os"
"regexp"
Expand All @@ -21,6 +20,10 @@ import (

const (
defaultNamespace = "linkerd"

jsonOutput = "json"
tableOutput = "table"
wideOutput = "wide"
)

var (
Expand Down Expand Up @@ -182,23 +185,23 @@ func newStatOptionsBase() *statOptionsBase {
return &statOptionsBase{
namespace: "default",
timeWindow: "1m",
outputFormat: "",
outputFormat: tableOutput,
}
}

func (o *statOptionsBase) validateOutputFormat() error {
switch o.outputFormat {
case "table", "json", "":
case tableOutput, jsonOutput:
return nil
default:
return errors.New("--output currently only supports table and json")
return fmt.Errorf("--output currently only supports %s and %s", tableOutput, jsonOutput)
}
}

func renderStats(buffer bytes.Buffer, options *statOptionsBase) string {
var out string
switch options.outputFormat {
case "json":
case jsonOutput:
out = buffer.String()
default:
// strip left padding on the first column
Expand Down
16 changes: 8 additions & 8 deletions cli/cmd/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ This command will only display traffic which is sent to a service that has a Ser
cmd.PersistentFlags().StringVarP(&options.timeWindow, "time-window", "t", options.timeWindow, "Stat window (for example: \"10s\", \"1m\", \"10m\", \"1h\")")
cmd.PersistentFlags().StringVar(&options.toResource, "to", options.toResource, "If present, shows outbound stats to the specified resource")
cmd.PersistentFlags().StringVar(&options.toNamespace, "to-namespace", options.toNamespace, "Sets the namespace used to lookup the \"--to\" resource; by default the current \"--namespace\" is used")
cmd.PersistentFlags().StringVarP(&options.outputFormat, "output", "o", options.outputFormat, "Output format; currently only \"table\" (default), \"wide\", and \"json\" are supported")
cmd.PersistentFlags().StringVarP(&options.outputFormat, "output", "o", options.outputFormat, fmt.Sprintf("Output format; one of: \"%s\", \"%s\", or \"%s\"", tableOutput, wideOutput, jsonOutput))

return cmd
}
Expand Down Expand Up @@ -142,15 +142,15 @@ func writeRouteStatsToBuffer(resp *pb.TopRoutesResponse, w *tabwriter.Writer, op
sort.Strings(resources)

switch options.outputFormat {
case "table", "wide", "":
case tableOutput, wideOutput:
for _, resource := range resources {
if len(tables) > 1 {
fmt.Fprintf(w, "==> %s <==\t\f", resource)
}
printRouteTable(tables[resource], w, options)
fmt.Fprintln(w)
}
case "json":
case jsonOutput:
printRouteJSON(tables, w, options)
}
}
Expand All @@ -168,7 +168,7 @@ func printRouteTable(stats []*routeRowStats, w *tabwriter.Writer, options *route
fmt.Sprintf(routeTemplate, "ROUTE"),
authorityColumn,
}
outputActual := options.toResource != "" && options.outputFormat == "wide"
outputActual := options.toResource != "" && options.outputFormat == wideOutput
if outputActual {
headers = append(headers, []string{
"EFFECTIVE_SUCCESS",
Expand Down Expand Up @@ -276,15 +276,15 @@ func printRouteJSON(tables map[string][]*routeRowStats, w *tabwriter.Writer, opt

func (o *routesOptions) validateOutputFormat() error {
switch o.outputFormat {
case "table", "json", "":
case tableOutput, jsonOutput:
return nil
case "wide":
case wideOutput:
if o.toResource == "" {
return errors.New("wide output is only available when --to is specified")
return fmt.Errorf("%s output is only available when --to is specified", wideOutput)
}
return nil
default:
return fmt.Errorf("--output currently only supports table, wide, and json")
return fmt.Errorf("--output currently only supports %s, %s, and %s", tableOutput, wideOutput, jsonOutput)
}
}

Expand Down
2 changes: 1 addition & 1 deletion cli/cmd/routes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func TestRoutes(t *testing.T) {
}, t)
})

options.outputFormat = "json"
options.outputFormat = jsonOutput
t.Run("Returns route stats (json)", func(t *testing.T) {
testRoutesCall(routesParamsExp{
routes: []string{"/a", "/b", "/c"},
Expand Down
6 changes: 3 additions & 3 deletions cli/cmd/stat.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ If no resource name is specified, displays stats about all resources of the spec
cmd.PersistentFlags().StringVar(&options.fromResource, "from", options.fromResource, "If present, restricts outbound stats from the specified resource name")
cmd.PersistentFlags().StringVar(&options.fromNamespace, "from-namespace", options.fromNamespace, "Sets the namespace used from lookup the \"--from\" resource; by default the current \"--namespace\" is used")
cmd.PersistentFlags().BoolVar(&options.allNamespaces, "all-namespaces", options.allNamespaces, "If present, returns stats across all namespaces, ignoring the \"--namespace\" flag")
cmd.PersistentFlags().StringVarP(&options.outputFormat, "output", "o", options.outputFormat, "Output format; currently only \"table\" (default) and \"json\" are supported")
cmd.PersistentFlags().StringVarP(&options.outputFormat, "output", "o", options.outputFormat, "Output format; one of: \"table\" or \"json\"")

return cmd
}
Expand Down Expand Up @@ -279,13 +279,13 @@ func writeStatsToBuffer(rows []*pb.StatTable_PodGroup_Row, w *tabwriter.Writer,
}

switch options.outputFormat {
case "table", "wide", "":
case tableOutput, wideOutput:
if len(statTables) == 0 {
fmt.Fprintln(os.Stderr, "No traffic found.")
os.Exit(0)
}
printStatTables(statTables, w, maxNameLength, maxNamespaceLength, options)
case "json":
case jsonOutput:
printStatJSON(statTables, w)
}
}
Expand Down
4 changes: 2 additions & 2 deletions cli/cmd/stat_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func TestStat(t *testing.T) {
}, t)
})

options.outputFormat = "json"
options.outputFormat = jsonOutput
t.Run("Returns namespace stats (json)", func(t *testing.T) {
testStatCall(paramsExp{
counts: &public.PodCounts{
Expand Down Expand Up @@ -58,7 +58,7 @@ func TestStat(t *testing.T) {
}, t)
})

options.outputFormat = "json"
options.outputFormat = jsonOutput
t.Run("Returns all namespace stats (json)", func(t *testing.T) {
testStatCall(paramsExp{
counts: &public.PodCounts{
Expand Down
Loading

0 comments on commit 9abd59f

Please sign in to comment.