From 99b1c027cf8df282296ce20293adfdebd1fb99ca Mon Sep 17 00:00:00 2001 From: Steffen Vogel Date: Sat, 15 Jun 2024 19:15:53 +0200 Subject: [PATCH] fix: Linter warnings Signed-off-by: Steffen Vogel --- .golangci.yaml | 129 +++++++++++++++++++++++++++---- cmd/gontc/gontc.go | 22 ++++-- internal/execvpe/execvpe.go | 23 +++--- internal/prque/prque.go | 6 +- internal/utils/ip.go | 10 ++- pkg/capture.go | 12 ++- pkg/capture_interface.go | 6 +- pkg/capture_pcapgo.go | 7 +- pkg/capture_test.go | 33 ++++---- pkg/cmd.go | 18 +++-- pkg/debug_instance.go | 6 +- pkg/debug_instance_breakpoint.go | 2 +- pkg/debug_msg.go | 13 +++- pkg/debug_vscode.go | 14 +++- pkg/exec.go | 1 + pkg/filter_test.go | 2 + pkg/gont.go | 4 +- pkg/host.go | 6 +- pkg/host_ping.go | 11 ++- pkg/link.go | 18 +++-- pkg/main_test.go | 3 +- pkg/nat.go | 7 +- pkg/network.go | 6 +- pkg/network_files.go | 2 +- pkg/ns_node.go | 4 +- pkg/options/bridge.go | 2 +- pkg/options/debug/tracepoint.go | 2 +- pkg/options/filters/network.go | 4 +- pkg/options/options.go | 8 +- pkg/switch.go | 4 +- pkg/trace.go | 16 ++-- pkg/trace/event.go | 2 + pkg/trace/log.go | 2 +- pkg/trace/trace.go | 13 +++- pkg/trace_test.go | 2 +- test/tracee1/main.go | 5 +- 36 files changed, 299 insertions(+), 126 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 580587f8..ef596ac0 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,23 +1,118 @@ -# SPDX-FileCopyrightText: 2023 Steffen Vogel +# SPDX-FileCopyrightText: 2023-2024 Steffen Vogel # SPDX-License-Identifier: Apache-2.0 +linters-settings: + misspell: + locale: US + + exhaustive: + default-signifies-exhaustive: true + + gomodguard: + blocked: + modules: + - github.com/pkg/errors: + recommendations: + - errors + + tagliatelle: + case: + use-field-name: true + rules: + json: snake + yaml: snake + xml: snake + + gci: + sections: + - standard + - default + - prefix(cunicu.li/skeleton) + - blank + - dot + + custom-order: true + linters: enable: - - errname + - asciicheck # Simple linter to check that your code does not contain non-ASCII identifiers + - bidichk # Checks for dangerous unicode character sequences + - bodyclose # checks whether HTTP response body is closed successfully + - contextcheck # check the function whether use a non-inherited context + - decorder # check declaration order and count of types, constants, variables and functions + - dogsled # Checks assignments with too many blank identifiers (e.g. x, _, _, _, := f()) + - dupl # Tool for code clone detection + - durationcheck # check for two durations multiplied together + - errcheck # Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases + - errchkjson # Checks types passed to the json encoding functions. Reports unsupported types and optionally reports occasions, where the check for the returned error can be omitted. + - errname # Checks that sentinel errors are prefixed with the `Err` and error types are suffixed with the `Error`. + - errorlint # errorlint is a linter for that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13. + - exhaustive # check exhaustiveness of enum switch statements - copyloopvar - - gci - - gochecknoglobals - - gocognit - - gofmt - - misspell - - tagliatelle - - whitespace - - revive - - gosec - - nestif - - nolintlint - - prealloc + - forcetypeassert # finds forced type assertions + - gci # Gci control golang package import order and make it always deterministic. + - gochecknoglobals # Checks that no globals are present in Go code + - gochecknoinits # Checks that no init functions are present in Go code + - gocognit # Computes and checks the cognitive complexity of functions + - goconst # Finds repeated strings that could be replaced by a constant + - gocritic # The most opinionated Go source code linter + - err113 # Golang linter to check the errors handling expressions + - gofmt # Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification + - gofumpt # Gofumpt checks whether code was gofumpt-ed. + - goheader # Checks is file header matches to pattern + - goimports # Goimports does everything that gofmt does. Additionally it checks unused imports + - gomoddirectives # Manage the use of 'replace', 'retract', and 'excludes' directives in go.mod. + - gomodguard # Allow and block list linter for direct Go module dependencies. This is different from depguard where there are different block types for example version constraints and module recommendations. + - goprintffuncname # Checks that printf-like functions are named with `f` at the end + - gosec # Inspects source code for security problems + - gosimple # Linter for Go source code that specializes in simplifying a code + - govet # Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string + - grouper # An analyzer to analyze expression groups. + - importas # Enforces consistent import aliases + - ineffassign # Detects when assignments to existing variables are not used + - misspell # Finds commonly misspelled English words in comments + - nakedret # Finds naked returns in functions greater than a specified function length + - nilerr # Finds the code that returns nil even if it checks that the error is not nil. + - nilnil # Checks that there is no simultaneous return of `nil` error and an invalid value. + - noctx # noctx finds sending http request without context.Context + - predeclared # find code that shadows one of Go's predeclared identifiers + - revive # golint replacement, finds style mistakes + - staticcheck # Staticcheck is a go vet on steroids, applying a ton of static analysis checks + - stylecheck # Stylecheck is a replacement for golint + - tagliatelle # Checks the struct tags. + - tenv # tenv is analyzer that detects using os.Setenv instead of t.Setenv since Go1.17 + - tparallel # tparallel detects inappropriate usage of t.Parallel() method in your Go test codes + - typecheck # Like the front-end of a Go compiler, parses and type-checks Go code + - unconvert # Remove unnecessary type conversions + - unparam # Reports unused function parameters + - unused # Checks Go code for unused constants, variables, functions and types + - wastedassign # wastedassign finds wasted assignment statements + - whitespace # Tool for detection of leading and trailing whitespace -linters-settings: - revive: - severity: warning + disable: + - containedctx # containedctx is a linter that detects struct contained context.Context field + - cyclop # checks function and package cyclomatic complexity + - depguard # Go linter that checks if package imports are in a list of acceptable packages + - forbidigo # Forbids identifiers + - funlen # Tool for detection of long functions + - gocyclo # Computes and checks the cyclomatic complexity of functions + - godot # Check if comments end in a period + - godox # Tool for detection of FIXME, TODO and other comment keywords + - gomnd # An analyzer to detect magic numbers. + - ireturn # Accept Interfaces, Return Concrete Types + - lll # Reports long lines + - maintidx # maintidx measures the maintainability index of each function. + - makezero # Finds slice declarations with non-zero initial length + - nestif # Reports deeply nested if statements + - nlreturn # nlreturn checks for a new line before return and branch statements to increase code clarity + - nolintlint # Reports ill-formed or insufficient nolint directives + - paralleltest # paralleltest detects missing usage of t.Parallel() method in your Go test + - prealloc # Finds slice declarations that could potentially be preallocated + - promlinter # Check Prometheus metrics naming via promlint + - rowserrcheck # checks whether Err of rows is checked successfully + - sqlclosecheck # Checks that sql.Rows and sql.Stmt are closed. + - testpackage # linter that makes you use a separate _test package + - thelper # thelper detects golang test helpers without t.Helper() call and checks the consistency of test helpers + - varnamelen # checks that the length of a variable's name matches its scope + - wrapcheck # Checks that errors returned from external packages are wrapped + - wsl # Whitespace Linter - Forces you to use empty lines! diff --git a/cmd/gontc/gontc.go b/cmd/gontc/gontc.go index ed997888..bd2620f0 100644 --- a/cmd/gontc/gontc.go +++ b/cmd/gontc/gontc.go @@ -18,6 +18,13 @@ import ( "golang.org/x/exp/slices" ) +var ( + ErrNonExistingNetwork = errors.New("non-existing Gont network") + ErrNonExistingNode = errors.New("non-existing Gont node") + ErrNotEnoughArguments = errors.New("not enough arguments") + ErrUnknownSubCommand = errors.New("unknown sub-command") +) + // Set via ldflags (see Makefile) var tag string //nolint:gochecknoglobals @@ -53,8 +60,7 @@ func main() { var err error var network, node string - logger := internal.SetupLogging() - defer logger.Sync() //nolint:errcheck + internal.SetupLogging() if err := g.CheckCaps(); err != nil { fmt.Printf("error: %s\n", err) @@ -102,7 +108,7 @@ func main() { err = nil default: - err = fmt.Errorf("unknown sub-command: %s", subcmd) + err = fmt.Errorf("%w: %s", ErrUnknownSubCommand, subcmd) } if err != nil { @@ -119,7 +125,7 @@ func networkNode(args []string) (string, string, error) { c := strings.SplitN(args[1], "/", 2) if len(c) == 1 { // no network in name if len(networks) == 0 { - return "", "", errors.New("no-existing Gont network") + return "", "", ErrNonExistingNetwork } network = networks[0] @@ -129,14 +135,14 @@ func networkNode(args []string) (string, string, error) { node = c[1] if !slices.Contains(networks, network) { - return "", "", fmt.Errorf("non-existing network '%s'", network) + return "", "", fmt.Errorf("%w: %s", ErrNonExistingNetwork, network) } } nodes := g.NodeNames(network) if !slices.Contains(nodes, node) { - return "", "", fmt.Errorf("non-existing node '%s' in network '%s'", node, network) + return "", "", fmt.Errorf("%w '%s' in network '%s'", ErrNonExistingNode, node, network) } return network, node, nil @@ -188,11 +194,11 @@ func clean(args []string) error { func exec(network, node string, args []string) error { if len(flag.Args()) <= 1 { - return fmt.Errorf("not enough arguments") + return ErrNotEnoughArguments } if network == "" { - return fmt.Errorf("there is no active Gont network") + return ErrNonExistingNetwork } if err := os.Setenv("GONT_NETWORK", network); err != nil { diff --git a/internal/execvpe/execvpe.go b/internal/execvpe/execvpe.go index b371121e..35779d09 100644 --- a/internal/execvpe/execvpe.go +++ b/internal/execvpe/execvpe.go @@ -7,6 +7,7 @@ package execvpe // See: https://sourceware.org/git/?p=glibc.git;a=blob;f=posix/execvpe.c import ( + "errors" "os" "path/filepath" "strings" @@ -40,7 +41,7 @@ func Execvpe(argv0 string, argv []string, envp []string) error { if strings.Contains(argv0, "/") { err := syscall.Exec(argv0, argv, envp) - if err == syscall.ENOEXEC { + if errors.Is(err, syscall.ENOEXEC) { if err := maybeScriptExecute(argv0, argv, envp); err != nil { return err } @@ -57,34 +58,34 @@ func Execvpe(argv0 string, argv []string, envp []string) error { gotEacces := false for _, p := range strings.Split(path, ":") { argv0 := filepath.Join(p, argv0) - if err = syscall.Exec(argv0, argv, envp); err == syscall.ENOEXEC { + if err = syscall.Exec(argv0, argv, envp); errors.Is(err, syscall.ENOEXEC) { err = maybeScriptExecute(argv0, argv, envp) } - switch err { - case syscall.EACCES: + switch { + case errors.Is(err, syscall.EACCES): // Record that we got a 'Permission denied' error. If we end // up finding no executable we can use, we want to diagnose // that we did find one but were denied access. gotEacces = true - case syscall.ENOENT: + case errors.Is(err, syscall.ENOENT): fallthrough - case syscall.ESTALE: + case errors.Is(err, syscall.ESTALE): fallthrough - case syscall.ENOTDIR: + case errors.Is(err, syscall.ENOTDIR): fallthrough + // Those errors indicate the file is missing or not executable // by us, in which case we want to just try the next path // directory. - - case syscall.ENODEV: + case errors.Is(err, syscall.ENODEV): fallthrough - case syscall.ETIMEDOUT: + case errors.Is(err, syscall.ETIMEDOUT): + // Some strange file systems like AFS return even // stranger error numbers. They cannot reasonably mean // anything else so ignore those, too. - default: // Some other error means we found an executable file, but // something went wrong executing it; return the error to our diff --git a/internal/prque/prque.go b/internal/prque/prque.go index 49ecd6f0..74f4a758 100644 --- a/internal/prque/prque.go +++ b/internal/prque/prque.go @@ -31,7 +31,7 @@ func (q heapl) Swap(i, j int) { } func (q *heapl) Push(x any) { - *q = append(*q, x.(Item)) + *q = append(*q, x.(Item)) //nolint:forcetypeassert } func (q *heapl) Pop() any { @@ -64,9 +64,7 @@ func (q *PriorityQueue) Pop() Item { q.lock.Lock() defer q.lock.Unlock() - item := heap.Pop(&q.heap).(Item) - - return item + return heap.Pop(&q.heap).(Item) //nolint:forcetypeassert } func (q *PriorityQueue) Oldest() time.Time { diff --git a/internal/utils/ip.go b/internal/utils/ip.go index ef2faec4..3c81ee59 100644 --- a/internal/utils/ip.go +++ b/internal/utils/ip.go @@ -12,12 +12,14 @@ import ( func ipToInt(ip net.IP) (*big.Int, int) { val := &big.Int{} val.SetBytes([]byte(ip)) - if len(ip) == net.IPv4len { + + switch { + case len(ip) == net.IPv4len: return val, 32 - } else if len(ip) == net.IPv6len { + case len(ip) == net.IPv6len: return val, 128 - } else { - panic(fmt.Errorf("Unsupported address length %d", len(ip))) + default: + panic(fmt.Sprintf("unsupported address length %d", len(ip))) } } diff --git a/pkg/capture.go b/pkg/capture.go index 6937820b..9dcd298e 100644 --- a/pkg/capture.go +++ b/pkg/capture.go @@ -126,12 +126,15 @@ func NewCapture(opts ...CaptureOption) *Capture { // Count returns the total number of captured packets func (c *Capture) Count() uint64 { - return uint64(c.count.Load()) + return c.count.Load() } func (c *Capture) Flush() error { for c.queue.Len() > 0 { - p := c.queue.Pop().(CapturePacket) + p, ok := c.queue.Pop().(CapturePacket) + if !ok { + continue + } if err := c.writePacket(p); err != nil { return err @@ -239,7 +242,10 @@ out: break } - p := c.queue.Pop().(CapturePacket) + p, ok := c.queue.Pop().(CapturePacket) + if !ok { + continue + } if err := c.writePacket(p); err != nil { c.logger.Error("Failed to handle packet. Stop capturing...", zap.Error(err)) diff --git a/pkg/capture_interface.go b/pkg/capture_interface.go index 59123904..ba0d093d 100644 --- a/pkg/capture_interface.go +++ b/pkg/capture_interface.go @@ -37,10 +37,10 @@ func (ci *captureInterface) readPackets(c *Capture) { if err := ci.readPacket(c); err != nil { if errors.Is(err, io.EOF) { break - } else { - c.logger.Error("Failed to read packet data", zap.Error(err)) - continue } + + c.logger.Error("Failed to read packet data", zap.Error(err)) + continue } } } diff --git a/pkg/capture_pcapgo.go b/pkg/capture_pcapgo.go index 3757b18c..ef2be043 100644 --- a/pkg/capture_pcapgo.go +++ b/pkg/capture_pcapgo.go @@ -6,6 +6,7 @@ package gont import ( + "errors" "fmt" "github.com/gopacket/gopacket/layers" @@ -13,6 +14,8 @@ import ( "golang.org/x/net/bpf" ) +var ErrFilterExpressionsRequireCGO = errors.New("libpcap filter expressions require CGo") + const CGoPCAP = false type pcapgoPacketSource struct { @@ -25,7 +28,7 @@ func (c *Capture) createPCAPHandle(name string) (packetSource, error) { return nil, fmt.Errorf("failed to open PCAP handle: %w", err) } - if err := hdl.SetCaptureLength(int(c.SnapshotLength)); err != nil { + if err := hdl.SetCaptureLength(c.SnapshotLength); err != nil { return nil, fmt.Errorf("failed to set capture length: %w", err) } @@ -34,7 +37,7 @@ func (c *Capture) createPCAPHandle(name string) (packetSource, error) { } if c.FilterExpression != "" { - return nil, fmt.Errorf("libpcap filter expressions require CGo") + return nil, ErrFilterExpressionsRequireCGO } if c.FilterInstructions != nil { diff --git a/pkg/capture_test.go b/pkg/capture_test.go index 9e73f307..94ac66d0 100644 --- a/pkg/capture_test.go +++ b/pkg/capture_test.go @@ -49,7 +49,7 @@ func TestCaptureNetwork(t *testing.T) { } }() - cb := func(p g.CapturePacket) { + cb := func(_ g.CapturePacket) { // fmt.Println("Callback", p.String()) } @@ -80,7 +80,12 @@ func TestCaptureNetwork(t *testing.T) { co.FilterPackets(func(p *g.CapturePacket) bool { pp := p.Decode(gopacket.DecodeOptions{}) if layer := pp.Layer(layers.LayerTypeICMPv6); layer != nil { - typec := layer.(*layers.ICMPv6).TypeCode.Type() + layer, ok := layer.(*layers.ICMPv6) + if !ok { + return false + } + + typec := layer.TypeCode.Type() return typec == layers.ICMPv6TypeEchoRequest || typec == layers.ICMPv6TypeEchoReply } @@ -131,7 +136,7 @@ func TestCaptureNetwork(t *testing.T) { h1veth0 := h1.Interface("veth0") h2veth0 := h2.Interface("veth0") - pkt, _, intf, eof := nextPacket(t, rd) + pkt, intf, eof := nextPacket(t, rd) require.Equal(t, eof, false, "Expected more packets") require.Equal(t, intf.Name, "h1/veth0", "Invalid 1st packet") @@ -151,35 +156,35 @@ func TestCaptureNetwork(t *testing.T) { h1veth0.Addresses[0].IP.String(), ) - _, _, intf, eof = nextPacket(t, rd) + _, intf, eof = nextPacket(t, rd) require.False(t, eof, "Expected more packets") require.Equal(t, intf.Name, "sw1/veth-h1", "Invalid 2nd packet") - _, _, intf, eof = nextPacket(t, rd) + _, intf, eof = nextPacket(t, rd) require.False(t, eof, "Expected more packets") require.Equal(t, intf.Name, "sw1/veth-h2", "Invalid 3rd packet") - _, _, intf, eof = nextPacket(t, rd) + _, intf, eof = nextPacket(t, rd) require.False(t, eof, "Expected more packets") require.Equal(t, intf.Name, "h2/veth0", "Invalid 4th packet") - _, _, intf, eof = nextPacket(t, rd) + _, intf, eof = nextPacket(t, rd) require.False(t, eof, "Expected more packets") require.Equal(t, intf.Name, "h2/veth0", "Invalid 5th packet") - _, _, intf, eof = nextPacket(t, rd) + _, intf, eof = nextPacket(t, rd) require.False(t, eof, "Expected more packets") require.Equal(t, intf.Name, "sw1/veth-h2", "Invalid 6th packet: %s", intf.Name) - _, _, intf, eof = nextPacket(t, rd) + _, intf, eof = nextPacket(t, rd) require.False(t, eof, "Expected more packets") require.Equal(t, intf.Name, "sw1/veth-h1", "Invalid 7th packet") - _, _, intf, eof = nextPacket(t, rd) + _, intf, eof = nextPacket(t, rd) require.False(t, eof, "Expected more packets") require.Equal(t, intf.Name, "h1/veth0", "Invalid 7th packet") - _, _, _, eof = nextPacket(t, rd) + _, _, eof = nextPacket(t, rd) require.True(t, eof, "Expected EOF") require.Equal(t, rd.NInterfaces(), 4, "Invalid number of interfaces") @@ -192,11 +197,11 @@ func TestCaptureNetwork(t *testing.T) { require.NoError(t, err, "Failed to close file") } -func nextPacket(t *testing.T, rd *pcapgo.NgReader) (gopacket.Packet, *gopacket.CaptureInfo, *pcapgo.NgInterface, bool) { +func nextPacket(t *testing.T, rd *pcapgo.NgReader) (gopacket.Packet, *pcapgo.NgInterface, bool) { data, ci, err := rd.ZeroCopyReadPacketData() if err != nil { if errors.Is(err, io.EOF) { - return nil, nil, nil, true + return nil, nil, true } require.NoError(t, err, "Failed to read packet data") @@ -205,5 +210,5 @@ func nextPacket(t *testing.T, rd *pcapgo.NgReader) (gopacket.Packet, *gopacket.C intf, err := rd.Interface(ci.InterfaceIndex) require.NoError(t, err, "Received packet from unknown interface") - return gopacket.NewPacket(data, layers.LinkTypeEthernet, gopacket.Default), &ci, &intf, false + return gopacket.NewPacket(data, layers.LinkTypeEthernet, gopacket.Default), &intf, false } diff --git a/pkg/cmd.go b/pkg/cmd.go index 900bd717..da6979c1 100644 --- a/pkg/cmd.go +++ b/pkg/cmd.go @@ -6,6 +6,7 @@ package gont import ( "bytes" "context" + "errors" "fmt" "io" "log" @@ -19,6 +20,8 @@ import ( "go.uber.org/zap/zapio" ) +var ErrExitPrematurely = errors.New("process exited prematurely") + //nolint:gochecknoglobals var DefaultPreserveEnvVars = []string{ "PATH", @@ -80,8 +83,7 @@ func (n *NamespaceNode) Command(name string, args ...any) *Cmd { } for _, arg := range args { - switch arg := arg.(type) { - case ExecCmdOption: + if arg, ok := arg.(ExecCmdOption); ok { arg.ApplyExecCmd(c.Cmd) } } @@ -243,9 +245,9 @@ func (c *Cmd) tracer() *Tracer { return t } else if t := c.node.network.Tracer; t != nil { return t - } else { - return nil } + + return nil } func (c *Cmd) debugger() *Debugger { @@ -255,9 +257,9 @@ func (c *Cmd) debugger() *Debugger { return d } else if d := c.node.network.Debugger; d != nil { return d - } else { - return nil } + + return nil } func (c *Cmd) extraEnvFile(envName string, f *os.File) { @@ -348,7 +350,7 @@ func (c *Cmd) stoppedStart() error { zap.Int("trap_cause", ws.TrapCause())) if ws.Exited() { - return fmt.Errorf("process exited prematurely") + return ErrExitPrematurely } if !ws.Stopped() { @@ -375,6 +377,8 @@ func (c *Cmd) stoppedStart() error { c.logger.Debug("Detached from tracee") return nil + + default: } if err = syscall.PtraceCont(wpid, 0); err != nil { diff --git a/pkg/debug_instance.go b/pkg/debug_instance.go index d6d31e8e..90626c92 100644 --- a/pkg/debug_instance.go +++ b/pkg/debug_instance.go @@ -159,6 +159,8 @@ func (d *debuggerInstance) run() { case proc.StopManual: d.logger.Debug("Process stopped manually") return + + default: } } } @@ -352,8 +354,8 @@ func (d *debuggerInstance) createBreakpointsForLocation(tp *Tracepoint) error { } func ptrace(request int, pid int, addr uintptr, data uintptr) error { - if _, _, e1 := syscall.Syscall6(syscall.SYS_PTRACE, uintptr(request), uintptr(pid), uintptr(addr), uintptr(data), 0, 0); e1 != 0 { - return syscall.Errno(e1) + if _, _, e1 := syscall.Syscall6(syscall.SYS_PTRACE, uintptr(request), uintptr(pid), addr, data, 0, 0); e1 != 0 { + return e1 } return nil diff --git a/pkg/debug_instance_breakpoint.go b/pkg/debug_instance_breakpoint.go index 5c931112..ee2bcac1 100644 --- a/pkg/debug_instance_breakpoint.go +++ b/pkg/debug_instance_breakpoint.go @@ -105,7 +105,7 @@ func (bpi *breakpointInstance) createWatchpoint(d *debuggerInstance, thr *api.Th return nil } -func (bpi *breakpointInstance) traceEvent(t *api.Thread, d *debuggerInstance) trace.Event { +func (bpi *breakpointInstance) traceEvent(t *api.Thread, _ *debuggerInstance) trace.Event { var msg string if bpi.message == nil { msg = fmt.Sprintf("Hit breakpoint %d: %s", bpi.ID, bpi.Name) diff --git a/pkg/debug_msg.go b/pkg/debug_msg.go index bcb37792..8b5aa15a 100644 --- a/pkg/debug_msg.go +++ b/pkg/debug_msg.go @@ -8,12 +8,19 @@ package gont import ( + "errors" "fmt" "strings" "github.com/go-delve/delve/service/api" ) +var ( + ErrEmptyEvaluationString = errors.New("empty evaluation string") + ErrLogPointInvalidFormat = errors.New("invalid log point format") + ErrInvalidDebugMessage = errors.New("invalid debug message format") +) + type debugMessage struct { format string args []string @@ -39,7 +46,7 @@ func parseDebugMessage(msg string) (*debugMessage, error) { if braceCount--; braceCount == 0 { argStr := strings.TrimSpace(string(argSlice)) if len(argStr) == 0 { - return nil, fmt.Errorf("empty evaluation string") + return nil, ErrEmptyEvaluationString } args = append(args, argStr) formatSlice = append(formatSlice, '%', 's') @@ -53,7 +60,7 @@ func parseDebugMessage(msg string) (*debugMessage, error) { } else { switch r { case '}': - return nil, fmt.Errorf("invalid log point format, unexpected '}'") + return nil, fmt.Errorf("%w, unexpected '}'", ErrLogPointInvalidFormat) case '{': if braceCount++; braceCount == 1 { isArg, argSlice = true, []rune{} @@ -64,7 +71,7 @@ func parseDebugMessage(msg string) (*debugMessage, error) { } } if isArg || len(formatSlice) == 0 { - return nil, fmt.Errorf("invalid debug message format") + return nil, ErrInvalidDebugMessage } return &debugMessage{ diff --git a/pkg/debug_vscode.go b/pkg/debug_vscode.go index 1235bd4d..f93ac734 100644 --- a/pkg/debug_vscode.go +++ b/pkg/debug_vscode.go @@ -20,11 +20,15 @@ import ( "go.uber.org/zap" ) +var ErrFailedToFindWorkspaceDir = errors.New("failed to find workspace directory") + +//nolint:tagliatelle type vscTasksConfig struct { Version string `json:"version"` Tasks []vscTask `json:"tasks"` } +//nolint:tagliatelle type vscTask struct { Label string `json:"label"` Type string `json:"type"` @@ -35,22 +39,26 @@ type vscTask struct { ProblemMatcher *vscProblemMatcher `json:"problemMatcher,omitempty"` } +//nolint:tagliatelle type vscProblemMatcher struct { Owner string `json:"owner,omitempty"` Pattern *vscPattern `json:"pattern,omitempty"` Background *vscBackground `json:"background,omitempty"` } +//nolint:tagliatelle type vscBackground struct { ActiveOnStart bool `json:"activeOnStart,omitempty"` BeginsPattern string `json:"beginsPattern,omitempty"` EndsPattern string `json:"endsPattern,omitempty"` } +//nolint:tagliatelle type vscPattern struct { Regexp string `json:"regexp,omitempty"` } +//nolint:tagliatelle type vscLaunchConfig struct { Version string `json:"version,omitempty"` Configurations []vscConfiguration `json:"configurations,omitempty"` @@ -63,6 +71,7 @@ type vscPresentation struct { Panel string `json:"panel,omitempty"` } +//nolint:tagliatelle type vscConfiguration struct { Name string `json:"name,omitempty"` Type string `json:"type,omitempty"` @@ -75,6 +84,7 @@ type vscConfiguration struct { Mode string `json:"mode,omitempty"` } +//nolint:tagliatelle type vscCompound struct { Name string `json:"name,omitempty"` StopAll bool `json:"stopAll,omitempty"` @@ -88,7 +98,7 @@ type vscCompound struct { // instances // If an empty dir is passed, we attempt to find the workspace directory by searching for a // parent directory which contains either a .vscode, go.mod or .git -func (d *Debugger) WriteVSCodeConfigs(dir string, stopOnEntry bool) error { +func (d *Debugger) WriteVSCodeConfigs(dir string, _ bool) error { if dir == "" { wd, err := os.Getwd() if err != nil { @@ -97,7 +107,7 @@ func (d *Debugger) WriteVSCodeConfigs(dir string, stopOnEntry bool) error { var ok bool if dir, ok = findWorkspaceDir(wd); !ok { - return errors.New("failed to find workspace directory") + return ErrFailedToFindWorkspaceDir } } diff --git a/pkg/exec.go b/pkg/exec.go index 304320ee..8916faf7 100644 --- a/pkg/exec.go +++ b/pkg/exec.go @@ -22,6 +22,7 @@ const ( persNoRandomize = 0x0040000 // ADDR_NO_RANDOMIZE ) +//nolint:gochecknoinits func init() { unshare := os.Getenv("GONT_UNSHARE") node := os.Getenv("GONT_NODE") diff --git a/pkg/filter_test.go b/pkg/filter_test.go index 2a0a7bbc..2ab31d23 100644 --- a/pkg/filter_test.go +++ b/pkg/filter_test.go @@ -14,6 +14,7 @@ import ( "golang.org/x/sys/unix" ) +//nolint:dupl func TestFilterIPv4(t *testing.T) { n, err := g.NewNetwork(*nname, globalNetworkOptions...) require.NoError(t, err, "Failed to create network") @@ -52,6 +53,7 @@ func TestFilterIPv4(t *testing.T) { require.Error(t, err, "Succeeded to ping h1") } +//nolint:dupl func TestFilterIPv6(t *testing.T) { n, err := g.NewNetwork(*nname, globalNetworkOptions...) require.NoError(t, err, "Failed to create network") diff --git a/pkg/gont.go b/pkg/gont.go index 7ceb1fc5..2b8008d4 100644 --- a/pkg/gont.go +++ b/pkg/gont.go @@ -22,6 +22,8 @@ const ( bridgeInterfaceName = "br" ) +var ErrMissingPrivileges = errors.New("missing NET_ADMIN capabilities") + // Option is the base type for all functional options. type Option any @@ -29,7 +31,7 @@ type Option any func CheckCaps() error { c := cap.GetProc() if v, err := c.GetFlag(cap.Effective, cap.SYS_ADMIN); err != nil || !v { - return errors.New("missing NET_ADMIN capabilities") + return ErrMissingPrivileges } return nil } diff --git a/pkg/host.go b/pkg/host.go index 639d5a9c..a90cb560 100644 --- a/pkg/host.go +++ b/pkg/host.go @@ -116,13 +116,13 @@ func (h *Host) ConfigureInterface(i *Interface) error { if !i.EnableDAD { fn := filepath.Join("/proc/sys/net/ipv6/conf", i.Name, "accept_dad") if err := h.WriteProcFS(fn, "0"); err != nil { - return fmt.Errorf("failed to enabled IPv6 forwarding: %s", err) + return fmt.Errorf("failed to disable IPv6 DAD: %w", err) } } for _, addr := range i.Addresses { if err := i.AddAddress(&addr); err != nil { - return fmt.Errorf("failed to add link address: %s", err) + return fmt.Errorf("failed to add link address: %w", err) } } @@ -131,7 +131,7 @@ func (h *Host) ConfigureInterface(i *Interface) error { func (h *Host) Traceroute(o *Host, opts ...any) error { if h.network != o.network { - return fmt.Errorf("hosts must be on same network") + return ErrDifferentNetworks } opts = append(opts, o) diff --git a/pkg/host_ping.go b/pkg/host_ping.go index 1419ce65..6bbb84ce 100644 --- a/pkg/host_ping.go +++ b/pkg/host_ping.go @@ -12,6 +12,11 @@ import ( "go.uber.org/zap/zapio" ) +var ( + ErrFailedToFindAddress = errors.New("failed to find address") + ErrPacketsLost = errors.New("packets lost") +) + func (h *Host) Ping(o *Host) (*ping.Statistics, error) { return h.PingWithOptions(o, "ip", 1, 5*time.Second, time.Second, true) } @@ -31,13 +36,13 @@ func (h *Host) PingWithOptions(o *Host, net string, count int, timeout time.Dura p.Interval = intv if h.network != o.network { - return nil, fmt.Errorf("hosts must be on same network") + return nil, ErrDifferentNetworks } // Find first IP address of first interface ip := o.LookupAddress(net) if ip == nil { - return nil, errors.New("failed to find address") + return nil, ErrFailedToFindAddress } logger := h.logger.Named("pinger") @@ -82,7 +87,7 @@ func (h *Host) PingWithOptions(o *Host, net string, count int, timeout time.Dura } if lost := p.PacketsSent - p.PacketsRecv; lost > 0 { - err = fmt.Errorf("lost %d packets", lost) + err = fmt.Errorf("%w: %d", ErrPacketsLost, lost) } return p.Statistics(), err diff --git a/pkg/link.go b/pkg/link.go index 685a6d60..ebdfd6ff 100644 --- a/pkg/link.go +++ b/pkg/link.go @@ -14,6 +14,13 @@ import ( "golang.org/x/sys/unix" ) +var ( + ErrDifferentNetworks = errors.New("nodes are belonging to different networks") + ErrMissingNode = errors.New("cant establish link between interfaces without node") + ErrInterfaceNameTooLong = errors.New("interface names are too long") + ErrAddLinkToSelf = errors.New("failed to link the node with itself") +) + type VethOption interface { ApplyVeth(ve *nl.Veth) } @@ -26,19 +33,19 @@ func (n *Network) AddLink(l, r *Interface, opts ...Option) error { var err error if len(l.Name) > syscall.IFNAMSIZ-1 || len(r.Name) > syscall.IFNAMSIZ-1 { - return fmt.Errorf("interface names are too long. max_len=%d", syscall.IFNAMSIZ-1) + return fmt.Errorf("%w. max_len=%d", ErrInterfaceNameTooLong, syscall.IFNAMSIZ-1) } if l.Node == nil || r.Node == nil { - return errors.New("cant establish link between interfaces without node") + return ErrMissingNode } if l.Node == r.Node { - return errors.New("failed to link the node with itself") + return ErrAddLinkToSelf } if l.Node.Network() != r.Node.Network() { - return errors.New("nodes are belonging to different networks") + return ErrDifferentNetworks } // Create Veth pair @@ -63,8 +70,7 @@ func (n *Network) AddLink(l, r *Interface, opts ...Option) error { // Apply options for _, opt := range opts { - switch opt := opt.(type) { - case VethOption: + if opt, ok := opt.(VethOption); ok { opt.ApplyVeth(veth) } } diff --git a/pkg/main_test.go b/pkg/main_test.go index 7a9e9638..59158260 100644 --- a/pkg/main_test.go +++ b/pkg/main_test.go @@ -42,8 +42,7 @@ func setupLogging() *zap.Logger { } func TestMain(m *testing.M) { - logger := setupLogging() - defer logger.Sync() //nolint:errcheck + setupLogging() flag.Parse() diff --git a/pkg/nat.go b/pkg/nat.go index eec4a81b..60f942b2 100644 --- a/pkg/nat.go +++ b/pkg/nat.go @@ -47,9 +47,8 @@ func (n *Network) AddNAT(name string, opts ...Option) (*NAT, error) { } // Apply NAT options - for _, o := range opts { - switch opt := o.(type) { - case NATOption: + for _, opt := range opts { + if opt, ok := opt.(NATOption); ok { opt.ApplyNAT(nat) } } @@ -63,7 +62,7 @@ func (n *Network) AddNAT(name string, opts ...Option) (*NAT, error) { return nat, nil } -func (n *Network) AddHostNAT(name string, opts ...Option) (*NAT, error) { +func (n *Network) AddHostNAT(_ string, opts ...Option) (*NAT, error) { host := n.HostNode if err := host.EnableForwarding(); err != nil { diff --git a/pkg/network.go b/pkg/network.go index 45b6a387..6ad8ce66 100644 --- a/pkg/network.go +++ b/pkg/network.go @@ -20,6 +20,8 @@ import ( "go.uber.org/zap" ) +var ErrCreateNode = errors.New("failed to create node") + type NetworkOption interface { ApplyNetwork(n *Network) } @@ -119,7 +121,7 @@ func NewNetwork(name string, opts ...NetworkOption) (*Network, error) { n.HostNode = HostNode(n) if n.HostNode == nil { - return nil, errors.New("failed to create host node") + return nil, ErrCreateNode } if err := n.GenerateHostsFile(); err != nil { @@ -286,7 +288,7 @@ func (n *Network) KeyLogPipe(secretsType uint32) (*os.File, error) { } if len(capturesWithKeys) == 0 { - return nil, nil + return nil, nil //nolint:nilnil } rd, wr, err := os.Pipe() diff --git a/pkg/network_files.go b/pkg/network_files.go index 6c496c13..0831f277 100644 --- a/pkg/network_files.go +++ b/pkg/network_files.go @@ -154,7 +154,7 @@ func readNSSwitchConfig(fn string) (map[string][]string, error) { return nil, err } - return nil, nil + return m, nil } func writeNSSwitchConfig(fn string, config map[string][]string) error { diff --git a/pkg/ns_node.go b/pkg/ns_node.go index 7dca4dfc..e45b7297 100644 --- a/pkg/ns_node.go +++ b/pkg/ns_node.go @@ -126,7 +126,7 @@ func (n *Network) AddNamespaceNode(name string, opts ...Option) (*NamespaceNode, return nil, err } if err := unix.Mount(src, dst, "", syscall.MS_BIND, ""); err != nil { - return nil, fmt.Errorf("failed to bind mount netns fd: %s", err) + return nil, fmt.Errorf("failed to bind mount netns fd: %w", err) } n.Register(node) @@ -256,7 +256,7 @@ func (n *NamespaceNode) ConfigureInterface(i *Interface) error { n.Interfaces = append(n.Interfaces, i) if err := n.network.GenerateHostsFile(); err != nil { - return fmt.Errorf("failed to update hosts file") + return fmt.Errorf("failed to update hosts file: %w", err) } return nil diff --git a/pkg/options/bridge.go b/pkg/options/bridge.go index b452d8a4..2a03ab75 100644 --- a/pkg/options/bridge.go +++ b/pkg/options/bridge.go @@ -26,7 +26,7 @@ func (vf VLANFiltering) ApplyBridge(b *nl.Bridge) { b.VlanFiltering = &v } -// AgingTime configures the bridge's FDB entries ageing time, ie the number of seconds a MAC address will be kept in the FDB after a packet has been received from that address. +// AgingTime configures the bridge's FDB entries aging time, ie the number of seconds a MAC address will be kept in the FDB after a packet has been received from that address. // After this time has passed, entries are cleaned up. type AgingTime time.Duration diff --git a/pkg/options/debug/tracepoint.go b/pkg/options/debug/tracepoint.go index c3194821..3eba6d34 100644 --- a/pkg/options/debug/tracepoint.go +++ b/pkg/options/debug/tracepoint.go @@ -163,7 +163,7 @@ type UserData struct { } func (u UserData) ApplyTracepoint(b *g.Tracepoint) { - b.UserData = any(u.Data) + b.UserData = u.Data } func Data(d any) UserData { diff --git a/pkg/options/filters/network.go b/pkg/options/filters/network.go index 8c3e781a..3a621425 100644 --- a/pkg/options/filters/network.go +++ b/pkg/options/filters/network.go @@ -36,7 +36,7 @@ func ipOffsetLen(ip net.IP, dir direction) (uint32, uint32) { } func network(dir direction, netw *net.IPNet) Statement { - offset, len := ipOffsetLen(netw.IP, dir) + offset, length := ipOffsetLen(netw.IP, dir) fromAddr, toAddr := utils.AddressRange(netw) return Statement{ @@ -45,7 +45,7 @@ func network(dir direction, netw *net.IPNet) Statement { DestRegister: 1, Base: expr.PayloadBaseNetworkHeader, Offset: offset, - Len: len, + Len: length, }, &expr.Range{ Op: expr.CmpOpEq, diff --git a/pkg/options/options.go b/pkg/options/options.go index 0bfba300..3aeb36ab 100644 --- a/pkg/options/options.go +++ b/pkg/options/options.go @@ -6,9 +6,9 @@ package options // Customize clones and extends a list of options without altering the list of base options. func Customize[T any](opts []T, extraOptions ...T) []T { - new := make([]T, 0, len(opts)+len(extraOptions)) - new = append(new, opts...) - new = append(new, extraOptions...) + n := make([]T, 0, len(opts)+len(extraOptions)) + n = append(n, opts...) + n = append(n, extraOptions...) - return new + return n } diff --git a/pkg/switch.go b/pkg/switch.go index 1d31e35a..670c92f6 100644 --- a/pkg/switch.go +++ b/pkg/switch.go @@ -108,12 +108,12 @@ func (sw *Switch) ConfigureInterface(i *Interface) error { sw.logger.Info("Connecting interface to bridge master", zap.Any("intf", i)) br, err := sw.nlHandle.LinkByName(bridgeInterfaceName) if err != nil { - return fmt.Errorf("failed to find bridge intf: %s", err) + return fmt.Errorf("failed to find bridge interface: %w", err) } l, err := sw.nlHandle.LinkByName(i.Name) if err != nil { - return fmt.Errorf("failed to find new bridge interface: %s", err) + return fmt.Errorf("failed to find new bridge interface: %w", err) } // Attach interface to bridge diff --git a/pkg/trace.go b/pkg/trace.go index 92c31c39..6ec6f2db 100644 --- a/pkg/trace.go +++ b/pkg/trace.go @@ -113,7 +113,10 @@ func (t *Tracer) Start() error { func (t *Tracer) Flush() error { for t.queue.Len() > 0 { - p := t.queue.Pop().(trace.Event) + p, ok := t.queue.Pop().(trace.Event) + if !ok { + continue + } if err := t.writeEvent(p); err != nil { return err @@ -161,10 +164,10 @@ func (t *Tracer) Pipe() (*os.File, error) { if _, err := e.ReadFrom(rd); err != nil { if errors.Is(err, io.EOF) { break - } else { - t.logger.Warn("Failed to read tracepoint from log", zap.Error(err)) - continue } + + t.logger.Warn("Failed to read tracepoint from log", zap.Error(err)) + continue } t.newEvent(e) @@ -222,7 +225,10 @@ out: break } - e := t.queue.Pop().(trace.Event) + e, ok := t.queue.Pop().(trace.Event) + if !ok { + continue + } if err := t.writeEvent(e); err != nil { t.logger.Error("Failed to handle event. Stop tracing...", zap.Error(err)) diff --git a/pkg/trace/event.go b/pkg/trace/event.go index 3a4fdf28..7b5a85cf 100644 --- a/pkg/trace/event.go +++ b/pkg/trace/event.go @@ -22,6 +22,7 @@ var ( em cbor.EncMode ) +//nolint:gochecknoinits func init() { dm, _ = cbor.DecOptions{ DefaultMapType: reflect.TypeOf(map[string]any{}), @@ -53,6 +54,7 @@ const ( type EventCallback func(e Event) +//nolint:tagliatelle type Event struct { Timestamp time.Time `cbor:"time" json:"time"` Type string `cbor:"type" json:"type"` diff --git a/pkg/trace/log.go b/pkg/trace/log.go index abeb16fa..39444bd2 100644 --- a/pkg/trace/log.go +++ b/pkg/trace/log.go @@ -22,7 +22,7 @@ type traceCore struct { fields []zapcore.Field } -func (c *traceCore) Enabled(lvl zapcore.Level) bool { +func (c *traceCore) Enabled(_ zapcore.Level) bool { return eventWriter != nil || eventCallback != nil } diff --git a/pkg/trace/trace.go b/pkg/trace/trace.go index a8b3e899..f27b2cd7 100644 --- a/pkg/trace/trace.go +++ b/pkg/trace/trace.go @@ -5,6 +5,7 @@ package trace import ( "bufio" + "errors" "fmt" "io" "os" @@ -16,6 +17,12 @@ import ( // The following functions are intended to by used used for instrumentation of Go code // which is started by gont.Node.{Start,StartWith,Run} +var ( + ErrTracingAlreadyEnabled = errors.New("tracing already enabled") + ErrTracingNotSupported = errors.New("tracing not supported") + ErrTracingNotRunning = errors.New("tracing not running") +) + //nolint:gochecknoglobals var ( eventCallback EventCallback @@ -25,12 +32,12 @@ var ( func Start(bufsize int) error { if eventWriter != nil { - return fmt.Errorf("tracing already enabled") + return ErrTracingAlreadyEnabled } traceFileName := os.Getenv("GONT_TRACEFILE") if traceFileName == "" { - return fmt.Errorf("tracing not supported. Missing GONT_TRACEFILE environment variable") + return fmt.Errorf("%w: Missing GONT_TRACEFILE environment variable", ErrTracingNotSupported) } var err error @@ -53,7 +60,7 @@ func StartWithCallback(cb EventCallback) { func Stop() error { if eventWriter == nil { - return fmt.Errorf("tracing not running") + return ErrTracingNotRunning } if bufferedTraceWriter, ok := eventWriter.(*bufio.Writer); ok { diff --git a/pkg/trace_test.go b/pkg/trace_test.go index 705fafc7..5417726e 100644 --- a/pkg/trace_test.go +++ b/pkg/trace_test.go @@ -136,7 +136,7 @@ func TestTraceLog(t *testing.T) { "number": int64(1234), // zap is adding zap.Int() as an int64 internally } - _, filename, _, _ := runtime.Caller(0) + _, filename, _, _ := runtime.Caller(0) //nolint:dogsled function := "cunicu.li/gont/v2/pkg_test.TestTraceLog" diff --git a/test/tracee1/main.go b/test/tracee1/main.go index a50b5f17..f72f0616 100644 --- a/test/tracee1/main.go +++ b/test/tracee1/main.go @@ -13,7 +13,6 @@ func main() { if err := trace.Start(0); err != nil { log.Fatalf("Failed to start tracer: %s", err) } - defer trace.Stop() //nolint:errcheck myData := map[string]any{ "Hello": "World", @@ -22,4 +21,8 @@ func main() { if err := trace.PrintfWithData(myData, "This is my first trace message: %s", "Hurra"); err != nil { log.Fatalf("Failed to write trace: %s", err) } + + if err := trace.Stop(); err != nil { + log.Fatalf("Failed to stop trace: %s", err) + } }