diff --git a/.golangci.yaml b/.golangci.yaml new file mode 100644 index 0000000..0d12a03 --- /dev/null +++ b/.golangci.yaml @@ -0,0 +1,24 @@ +version: "2" + +issues: + max-same-issues: 50 + +formatters: + enable: + - goimports # checks if the code and import statements are formatted according to the 'goimports' command + - gofumpt # Or "gofmt", # Enforce standard formatting + +linters: + enable: + - errcheck #Errcheck is a program for checking for unchecked errors in Go code. These unchecked errors can be critical bugs in some cases. + - govet # Vet examines Go source code and reports suspicious constructs. It is roughly the same as 'go vet' and uses its passes. [auto-fix] + - ineffassign # Detects when assignments to existing variables are not used. [fast] + - staticcheck # It's a set of rules from staticcheck. It's not the same thing as the staticcheck binary. The author of staticcheck doesn't support or approve the use of staticcheck as a library inside golangci-lint. [auto-fix] + - unused # Checks Go code for unused constants, variables, functions and types. + # Subective additional linters + - gocyclo # or "cyclop", # Detect cyclomatic complexity + - goconst # Detect repeated values that can be made constants + - misspell # Fix spelling errors + - unconvert # Detect unnecessary type conversions + - unparam # Detect unused function parameters + - dupword # Detect duplicate words in comments and string literals (e.g. “the the”, “is is”) diff --git a/Makefile b/Makefile index a366960..7bc41f2 100644 --- a/Makefile +++ b/Makefile @@ -70,7 +70,7 @@ endef all: info -.PHONY: build local_build logs major minor patch push run stop test version +.PHONY: build local_build logs major minor patch push run stop test version lint lint-fix format info: @echo -e "\n${GREEN} ${PROJECT_NAME} / Makefile ${RESET}\n" @@ -115,6 +115,25 @@ push: @git tag -fa 'v${APP_VERSION}' -m 'v${APP_VERSION}' @git push --follow-tags --set-upstream origin master +format: + @echo "\n>>> Formating code with golangci-lint…" + @golangci-lint fmt \ + --config .golangci.yaml \ + ./... + +lint: + @echo "\n>>> Running golangci-lint…" + @golangci-lint run \ + --config .golangci.yaml \ + --timeout 2m \ + ./... + +lint-fix: + @echo "\n>>> Auto-fixing with golangci-lint…" + @golangci-lint run --fix \ + --config .golangci.yaml \ + --timeout 2m \ + ./... MAJOR := $(shell echo ${APP_VERSION} | cut -d. -f1) MINOR := $(shell echo ${APP_VERSION} | cut -d. -f2) diff --git a/cmd/dish/cli_test.go b/cmd/dish/cli_test.go index a7a5157..67d3cc5 100644 --- a/cmd/dish/cli_test.go +++ b/cmd/dish/cli_test.go @@ -10,16 +10,23 @@ import ( func TestPrintHelp(t *testing.T) { oldStdout := os.Stdout - r, w, _ := os.Pipe() + r, w, err := os.Pipe() + if err != nil { + t.Errorf("failed to process pipe: %v", err) + } + os.Stdout = w printHelp() - w.Close() + if err := w.Close(); err != nil { + t.Errorf("pipe close: %v", err) + } + os.Stdout = oldStdout var buf bytes.Buffer - _, err := io.Copy(&buf, r) + _, err = io.Copy(&buf, r) if err != nil { t.Fatalf("failed to read from pipe: %v", err) } diff --git a/cmd/dish/main.go b/cmd/dish/main.go index e78acdb..1d0ddb5 100644 --- a/cmd/dish/main.go +++ b/cmd/dish/main.go @@ -23,7 +23,7 @@ func run(fs *flag.FlagSet, args []string, _, stderr io.Writer) int { return 1 } // Otherwise, print the error - fmt.Fprintln(stderr, "error loading config:", err) + fmt.Fprintln(stderr, "error loading config:", err) //nolint:errcheck return 2 } diff --git a/cmd/dish/runner_test.go b/cmd/dish/runner_test.go index e85e3fd..9f08c13 100644 --- a/cmd/dish/runner_test.go +++ b/cmd/dish/runner_test.go @@ -28,7 +28,6 @@ func compareResults(expected, actual []socket.Result) bool { } func TestFanInChannels(t *testing.T) { - testChannels := []chan socket.Result{} for range 3 { diff --git a/pkg/alert/alerter_test.go b/pkg/alert/alerter_test.go index 416ea2c..b4ed0f0 100644 --- a/pkg/alert/alerter_test.go +++ b/pkg/alert/alerter_test.go @@ -7,9 +7,7 @@ import ( ) func TestNewAlerter(t *testing.T) { - var ( - mockLogger = MockLogger{} - ) + mockLogger := MockLogger{} if alerterNil := NewAlerter(nil); alerterNil != nil { t.Error("expected nil, got alerter") diff --git a/pkg/alert/api_test.go b/pkg/alert/api_test.go index e8847cb..fa76540 100644 --- a/pkg/alert/api_test.go +++ b/pkg/alert/api_test.go @@ -10,7 +10,6 @@ import ( func TestNewAPISender(t *testing.T) { mockHTTPClient := &SuccessStatusHTTPClient{} - url := "https://abc123.xyz.com" headerName := "X-Api-Key" headerValue := "abc123" notifySuccess := false @@ -18,7 +17,7 @@ func TestNewAPISender(t *testing.T) { expected := &apiSender{ httpClient: mockHTTPClient, - url: url, + url: pushgatewayURL, headerName: headerName, headerValue: headerValue, notifySuccess: notifySuccess, @@ -27,7 +26,7 @@ func TestNewAPISender(t *testing.T) { } cfg := &config.Config{ - ApiURL: url, + ApiURL: pushgatewayURL, ApiHeaderName: headerName, ApiHeaderValue: headerValue, MachineNotifySuccess: notifySuccess, @@ -42,7 +41,6 @@ func TestNewAPISender(t *testing.T) { } func TestSend_API(t *testing.T) { - url := "https://abc123.xyz.com" headerName := "X-Api-Key" headerValue := "abc123" @@ -65,7 +63,7 @@ func TestSend_API(t *testing.T) { newConfig := func(headerName, headerValue string, notifySuccess, verbose bool) *config.Config { return &config.Config{ - ApiURL: url, + ApiURL: pushgatewayURL, MachineNotifySuccess: notifySuccess, Verbose: verbose, ApiHeaderName: headerName, diff --git a/pkg/alert/discord.go b/pkg/alert/discord.go index 5bb0629..0bde7ef 100644 --- a/pkg/alert/discord.go +++ b/pkg/alert/discord.go @@ -46,7 +46,6 @@ func NewDiscordSender(httpClient HTTPClient, config *config.Config, logger logge notifySuccess: config.TextNotifySuccess, url: parsedURL.String(), }, nil - } func (s *discordSender) send(message string, failedCount int) error { @@ -69,7 +68,6 @@ func (s *discordSender) send(message string, failedCount int) error { resp, err := handleSubmit(s.httpClient, http.MethodPost, s.url, bytes.NewBuffer(body), func(o *submitOptions) { o.headers["Authorization"] = "Bot " + strings.TrimSpace(s.botToken) }) - if err != nil { return fmt.Errorf("error submitting discord alert: %w", err) } diff --git a/pkg/alert/formatter.go b/pkg/alert/formatter.go index cf0fdea..8eeec51 100644 --- a/pkg/alert/formatter.go +++ b/pkg/alert/formatter.go @@ -7,7 +7,6 @@ import ( ) func FormatMessengerText(result socket.Result) string { - status := "failed" if result.Passed { status = "success" diff --git a/pkg/alert/pushgateway_test.go b/pkg/alert/pushgateway_test.go index 7b9835a..31a866b 100644 --- a/pkg/alert/pushgateway_test.go +++ b/pkg/alert/pushgateway_test.go @@ -7,18 +7,19 @@ import ( "go.vxn.dev/dish/pkg/config" ) +const pushgatewayURL = "https://abc123.xyz.com" + func TestNewPushgatewaySender(t *testing.T) { mockHTTPClient := &SuccessStatusHTTPClient{} mockLogger := &MockLogger{} - url := "https://abc123.xyz.com" instanceName := "test-instance" verbose := false notifySuccess := false expected := &pushgatewaySender{ httpClient: mockHTTPClient, - url: url, + url: pushgatewayURL, instanceName: "test-instance", notifySuccess: notifySuccess, verbose: verbose, @@ -27,7 +28,7 @@ func TestNewPushgatewaySender(t *testing.T) { } cfg := &config.Config{ - PushgatewayURL: url, + PushgatewayURL: pushgatewayURL, InstanceName: instanceName, Verbose: verbose, MachineNotifySuccess: notifySuccess, @@ -60,7 +61,6 @@ func TestNewPushgatewaySender(t *testing.T) { } func TestSend_Pushgateway(t *testing.T) { - url := "https://abc123.xyz.com" instanceName := "test-instance" successResults := Results{ @@ -213,7 +213,7 @@ func TestSend_Pushgateway(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - cfg := newConfig(url, tt.instanceName, tt.notifySuccess, tt.verbose) + cfg := newConfig(pushgatewayURL, tt.instanceName, tt.notifySuccess, tt.verbose) sender, err := NewPushgatewaySender(tt.client, cfg, &MockLogger{}) if err != nil { t.Fatalf("failed to create Pushgateway sender instance: %v", err) @@ -229,7 +229,7 @@ func TestSend_Pushgateway(t *testing.T) { func TestCreateMessage(t *testing.T) { cfg := &config.Config{ - PushgatewayURL: "https://abc123.xyz.com", + PushgatewayURL: pushgatewayURL, InstanceName: "test-instance", MachineNotifySuccess: false, Verbose: false, diff --git a/pkg/alert/telegram_test.go b/pkg/alert/telegram_test.go index 81a92ef..cd12b75 100644 --- a/pkg/alert/telegram_test.go +++ b/pkg/alert/telegram_test.go @@ -11,8 +11,8 @@ func TestNewTelegramSender(t *testing.T) { mockHTTPClient := &SuccessStatusHTTPClient{} mockLogger := &MockLogger{} - chatID := "-123" token := "abc123" + chatID := "-123" verbose := false notifySuccess := false diff --git a/pkg/alert/transport.go b/pkg/alert/transport.go index fb297ab..da4bb29 100644 --- a/pkg/alert/transport.go +++ b/pkg/alert/transport.go @@ -74,7 +74,11 @@ func handleSubmit(client HTTPClient, method string, url string, body io.Reader, // handleRead reads an HTTP response, ensures the status code is within the expected <200, 299> range and if not, logs the response body. func handleRead(res *http.Response, logger logger.Logger) error { - defer res.Body.Close() + defer func() { + if err := res.Body.Close(); err != nil { + logger.Errorf("failed to close response body: %v", err) + } + }() if res.StatusCode < http.StatusOK || res.StatusCode >= http.StatusMultipleChoices { body, err := io.ReadAll(res.Body) diff --git a/pkg/config/config.go b/pkg/config/config.go index 7859401..ab81b2d 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -96,7 +96,7 @@ func defineFlags(fs *flag.FlagSet, cfg *Config) { // If a flag is used for a supported config parameter, the config parameter's value is set according to the provided flag. Otherwise, a default value is used for the given parameter. func NewConfig(fs *flag.FlagSet, args []string) (*Config, error) { if fs == nil { - //fs = flag.CommandLine + // fs = flag.CommandLine return nil, fmt.Errorf("flagset argument cannot be nil") } diff --git a/pkg/logger/console_logger.go b/pkg/logger/console_logger.go index ed6a03c..d38e2c4 100644 --- a/pkg/logger/console_logger.go +++ b/pkg/logger/console_logger.go @@ -27,7 +27,7 @@ func NewConsoleLogger(verbose bool, out io.Writer) *consoleLogger { l := &consoleLogger{ stdLogger: log.New(out, "", log.LstdFlags), - withColors: !(os.Getenv("NO_COLOR") == "true") && verbose, + withColors: os.Getenv("NO_COLOR") != "true" && verbose, } l.logLevel = INFO diff --git a/pkg/logger/console_logger_test.go b/pkg/logger/console_logger_test.go index 068a31c..9ebfaf3 100644 --- a/pkg/logger/console_logger_test.go +++ b/pkg/logger/console_logger_test.go @@ -35,7 +35,10 @@ func TestNewConsoleLogger(t *testing.T) { } logger.Info("hello stderr") - w.Close() + if err := w.Close(); err != nil { + t.Errorf("failed to close capture pipe writer: %v", err) + } + var buf bytes.Buffer _, err := buf.ReadFrom(r) if err != nil { @@ -65,7 +68,6 @@ func TestNewConsoleLogger(t *testing.T) { if actual != expected { t.Fatalf("expected %s, got %s", expected, actual) } - }) t.Run("with colors when verbose and no env set", func(t *testing.T) { diff --git a/pkg/netrunner/runner.go b/pkg/netrunner/runner.go index 02704a8..d489c4a 100644 --- a/pkg/netrunner/runner.go +++ b/pkg/netrunner/runner.go @@ -92,7 +92,15 @@ func (runner *tcpRunner) RunTest(ctx context.Context, sock socket.Socket) socket if err != nil { return socket.Result{Socket: sock, Error: err, Passed: false} } - defer conn.Close() + + defer func() { + if err := conn.Close(); err != nil { + runner.logger.Errorf( + "failed to close TCP connection to %s: %v", + endpoint, err, + ) + } + }() return socket.Result{Socket: sock, Passed: true} } @@ -120,7 +128,12 @@ func (runner *httpRunner) RunTest(ctx context.Context, sock socket.Socket) socke if err != nil { return socket.Result{Socket: sock, Passed: false, Error: err} } - defer resp.Body.Close() + + defer func() { + if cerr := resp.Body.Close(); cerr != nil { + runner.logger.Errorf("failed to close body for %v", cerr) + } + }() if !slices.Contains(sock.ExpectedHTTPCodes, resp.StatusCode) { err = fmt.Errorf("expected codes: %v, got %d", sock.ExpectedHTTPCodes, resp.StatusCode) diff --git a/pkg/netrunner/runner_posix.go b/pkg/netrunner/runner_posix.go index 04163eb..f543daa 100644 --- a/pkg/netrunner/runner_posix.go +++ b/pkg/netrunner/runner_posix.go @@ -24,9 +24,11 @@ const ( echoRequest ICMPType = 8 ) -const ipStripHdr = 23 -const testID = 0x1234 -const testSeq = 0x0001 +const ( + ipStripHdr = 23 + testID = 0x1234 + testSeq = 0x0001 +) type icmpRunner struct { logger logger.Logger @@ -64,7 +66,15 @@ func (runner *icmpRunner) RunTest(ctx context.Context, sock socket.Socket) socke if err != nil { return socket.Result{Socket: sock, Error: fmt.Errorf("failed to create a non-privileged icmp socket: %w", err)} } - defer syscall.Close(sysSocket) + + defer func() { + if cerr := syscall.Close(sysSocket); cerr != nil { + runner.logger.Errorf( + "error closing ICMP socket (fd %d) for %s:%d: %v", + sysSocket, sock.Host, sock.Port, cerr, + ) + } + }() if runtime.GOOS == "darwin" { if err := syscall.SetsockoptInt(sysSocket, syscall.IPPROTO_IP, ipStripHdr, 1); err != nil { diff --git a/pkg/socket/cache_test.go b/pkg/socket/cache_test.go index e840dff..3fd5543 100644 --- a/pkg/socket/cache_test.go +++ b/pkg/socket/cache_test.go @@ -38,7 +38,7 @@ func TestHashUrlToFilePath(t *testing.T) { } func TestSaveSocketsToCache(t *testing.T) { - filePath := testFile(t, "randomhash.json", nil) + filePath := testFile(t, nil) cacheDir := filepath.Dir(filePath) if err := saveSocketsToCache(filePath, cacheDir, []byte(testSockets)); err != nil { @@ -61,14 +61,18 @@ func TestSaveSocketsToCache(t *testing.T) { func TestLoadSocketsFromCache(t *testing.T) { t.Run("Load Sockets From Cache", func(t *testing.T) { - filePath := testFile(t, "randomhash.json", []byte(testSockets)) + filePath := testFile(t, []byte(testSockets)) cacheTTL := uint(60) readerFromCache, _, err := loadCachedSockets(filePath, cacheTTL) if err != nil { t.Fatalf("expected no error, but got %v", err) } - defer readerFromCache.Close() + defer func() { + if cerr := readerFromCache.Close(); cerr != nil { + t.Fatalf("failed to close cache reader: %v", cerr) + } + }() readBytes, err := io.ReadAll(readerFromCache) if err != nil { @@ -81,7 +85,7 @@ func TestLoadSocketsFromCache(t *testing.T) { }) t.Run("Load Sockets From Expired Cache", func(t *testing.T) { - filePath := testFile(t, "randomhash.json", []byte(testSockets)) + filePath := testFile(t, []byte(testSockets)) cacheTTL := uint(0) // For some reason Windows tests in CI/CD think that 0 time has elapsed since the creation of the test file when it's being checked inside of loadCachedSockets, therefore the expired cache error is not returned. @@ -92,7 +96,12 @@ func TestLoadSocketsFromCache(t *testing.T) { if !errors.Is(err, ErrExpiredCache) { t.Errorf("expected error %v, but got %v", ErrExpiredCache, err) } - defer readerFromCache.Close() + + defer func() { + if cerr := readerFromCache.Close(); cerr != nil { + t.Fatalf("failed to close cache reader: %v", cerr) + } + }() readBytes, err := io.ReadAll(readerFromCache) if err != nil { diff --git a/pkg/socket/fetch.go b/pkg/socket/fetch.go index 6e16f3c..5b8a4f7 100644 --- a/pkg/socket/fetch.go +++ b/pkg/socket/fetch.go @@ -2,6 +2,7 @@ package socket import ( "bytes" + "errors" "fmt" "io" "net/http" @@ -37,10 +38,15 @@ func (f *fetchHandler) fetchSocketsFromFile(config *config.Config) (io.ReadClose } // copyBody copies the provided response body to the provided buffer. The body is closed. -func (f *fetchHandler) copyBody(body io.ReadCloser, buf *bytes.Buffer) error { - defer body.Close() +func (f *fetchHandler) copyBody(body io.ReadCloser, buf *bytes.Buffer) (err error) { + defer func() { + if cerr := body.Close(); cerr != nil { + cerr = fmt.Errorf("close error: %w", cerr) + err = errors.Join(cerr, err) + } + }() - _, err := buf.ReadFrom(body) + _, err = buf.ReadFrom(body) return err } diff --git a/pkg/socket/fetch_test.go b/pkg/socket/fetch_test.go index be3f59c..98eeaba 100644 --- a/pkg/socket/fetch_test.go +++ b/pkg/socket/fetch_test.go @@ -23,7 +23,7 @@ func TestNewFetchHandler(t *testing.T) { } func TestFetchSocketsFromFile(t *testing.T) { - filePath := testFile(t, "randomhash.json", []byte(testSockets)) + filePath := testFile(t, []byte(testSockets)) cfg := &config.Config{ Source: filePath, } @@ -34,7 +34,12 @@ func TestFetchSocketsFromFile(t *testing.T) { if err != nil { t.Fatalf("Failed to fetch sockets from file %v\n", err) } - defer reader.Close() + + defer func() { + if cerr := reader.Close(); cerr != nil { + t.Fatalf("failed to close file reader: %v", cerr) + } + }() fileData, err := io.ReadAll(reader) if err != nil { @@ -80,7 +85,7 @@ func TestFetchSocketsFromRemote(t *testing.T) { t.Run(tt.name, func(t *testing.T) { // Specify temp cache file & directory for each test separately // This fixes open file handles preventing the tests from succeeding on Windows - filePath := testFile(t, "randomhash.json", []byte(testSockets)) + filePath := testFile(t, []byte(testSockets)) tt.cfg.ApiCacheDirectory = filepath.Dir(filePath) fetchHandler := NewFetchHandler(&mockLogger{}) diff --git a/pkg/socket/helpers_test.go b/pkg/socket/helpers_test.go index b21d5ae..2d14fb5 100644 --- a/pkg/socket/helpers_test.go +++ b/pkg/socket/helpers_test.go @@ -13,9 +13,10 @@ const testSockets string = `{ "sockets": [ { "id": "vxn_dev_https", "socket_name // testFile creates a temporary file inside of a temporary directory with the provided filename and data. // The temporary directory including the file is removed when the test using it finishes. -func testFile(t *testing.T, filename string, data []byte) string { +func testFile(t *testing.T, data []byte) string { t.Helper() dir := t.TempDir() + filename := "randomhash.json" filepath := filepath.Join(dir, filename) diff --git a/pkg/socket/socket.go b/pkg/socket/socket.go index b444595..6ca8ba6 100644 --- a/pkg/socket/socket.go +++ b/pkg/socket/socket.go @@ -4,6 +4,7 @@ package socket import ( "encoding/json" + "errors" "fmt" "io" @@ -51,12 +52,19 @@ func PrintSockets(list *SocketList, logger logger.Logger) { } // LoadSocketList decodes a JSON encoded SocketList from the provided io.ReadCloser. -func LoadSocketList(reader io.ReadCloser) (*SocketList, error) { - defer reader.Close() - - list := new(SocketList) - if err := json.NewDecoder(reader).Decode(list); err != nil { - return nil, fmt.Errorf("error decoding sockets json: %w", err) +func LoadSocketList(reader io.ReadCloser) (list *SocketList, err error) { + // defer a closure that appends a Close() error to the returned err + defer func() { + if cerr := reader.Close(); cerr != nil { + cerr = fmt.Errorf("close error: %w", cerr) + err = errors.Join(cerr, err) + } + }() + + list = new(SocketList) + if err = json.NewDecoder(reader).Decode(list); err != nil { + err = fmt.Errorf("error decoding sockets JSON: %w", err) + return nil, err } return list, nil diff --git a/pkg/socket/socket_test.go b/pkg/socket/socket_test.go index bfd0a9e..3db6484 100644 --- a/pkg/socket/socket_test.go +++ b/pkg/socket/socket_test.go @@ -59,7 +59,7 @@ func TestLoadSocketList(t *testing.T) { func TestFetchSocketList(t *testing.T) { mockServer := newMockServer(t, "", "", testSockets, http.StatusOK) - validFile := testFile(t, "randomhash.json", []byte(testSockets)) + validFile := testFile(t, []byte(testSockets)) socketStringReader := io.NopCloser(bytes.NewBufferString(testSockets)) originalList, err := LoadSocketList(socketStringReader) if err != nil {