From b5198d55e21bd966f5f26257bdc68f2788da199d Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Wed, 15 Jul 2026 16:41:25 +1000 Subject: [PATCH] chore: enable depguard to block log --- .golangci.yml | 13 +++++++++-- cmd/dhcptest/main.go | 26 ++++++++++++++++------ spinifex/gateway/ec2/instance/placement.go | 2 +- spinifex/network/external/dhcp/manager.go | 4 ++-- tests/e2e/ddil/scenarios/main_test.go | 10 ++++----- 5 files changed, 38 insertions(+), 17 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index ed080aa6e..f9835772e 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -15,6 +15,7 @@ linters: - bodyclose # checks whether HTTP response body is closed successfully - canonicalheader # checks whether net/http.Header uses canonical header keys - decorder # checks declaration order of types, consts, vars, and funcs + - depguard # blocks imports of packages with a preferred replacement - durationcheck # checks for two durations multiplied together - embeddedstructfieldcheck # checks embedded sync.Mutex/RWMutex in structs - errcheck # checks for unchecked errors in type assertions and I/O @@ -23,7 +24,6 @@ linters: - errorlint # catches ==/!= on errors, type assertions on errors, and %v instead of %w — all break on wrapped errors - exptostd # detects golang.org/x/exp/ functions replaceable by stdlib - fatcontext # detects nested contexts in loops - - forcetypeassert # flags unchecked type assertions that can panic - gocheckcompilerdirectives # validates go compiler directive comments (//go:) - godot # checks that declaration comments end in a period - goprintffuncname # checks that printf-like functions are named with f at the end @@ -57,8 +57,17 @@ linters: embeddedstructfieldcheck: forbid-mutex: true + depguard: + rules: + main: + deny: + - pkg: log$ + desc: Use log/slog instead, see https://go.dev/blog/slog + - pkg: math/rand$ + desc: Use math/rand/v2 instead, see https://go.dev/blog/randv2 + errcheck: - check-type-assertions: true + check-type-assertions: true # replaces forcetypeassert exclude-functions: - (*github.com/nats-io/nats.go.Subscription).Unsubscribe # cleanup-only, errors not actionable diff --git a/cmd/dhcptest/main.go b/cmd/dhcptest/main.go index 6ac423700..7e7a3ed03 100644 --- a/cmd/dhcptest/main.go +++ b/cmd/dhcptest/main.go @@ -14,7 +14,7 @@ import ( "context" "flag" "fmt" - "log" + "log/slog" "net" "os" "os/exec" @@ -46,18 +46,21 @@ func main() { case *spinifexID != "": hwAddr, err = net.ParseMAC(generateMAC(*spinifexID)) if err != nil { - log.Fatalf("generateMAC(%q): %v", *spinifexID, err) + slog.Error("Could not parse generated MAC", "spinifex-id", *spinifexID, "error", err) + os.Exit(1) } fmt.Printf("spinifex virtual MAC for %q: %s\n", *spinifexID, hwAddr) case *macStr != "": hwAddr, err = net.ParseMAC(*macStr) if err != nil { - log.Fatalf("parse --mac %q: %v", *macStr, err) + slog.Error("Could not parse --mac", "mac", *macStr, "error", err) + os.Exit(1) } default: hwAddr, err = ifaceMAC(*iface) if err != nil { - log.Fatalf("get MAC for %s: %v", *iface, err) + slog.Error("Could not get MAC for interface", "iface", *iface, "error", err) + os.Exit(1) } fmt.Printf("using real interface MAC: %s\n", hwAddr) } @@ -83,7 +86,7 @@ func main() { if *promisc { if err := setPromisc(*iface, true); err != nil { - log.Printf("WARNING: failed to set promisc on %s: %v", *iface, err) + slog.Warn("Failed to set promisc on interface", "iface", *iface, "error", err) } else { fmt.Printf("set %s promisc ON\n\n", *iface) } @@ -96,12 +99,13 @@ func main() { nclient4.WithRetry(*retries), } if *verbose { - opts = append(opts, nclient4.WithLogger(nclient4.ShortSummaryLogger{Printfer: log.New(os.Stderr, "[nclient4] ", 0)})) + opts = append(opts, nclient4.WithLogger(nclient4.ShortSummaryLogger{Printfer: slogPrintfer{}})) } client, err := nclient4.New(*iface, opts...) if err != nil { - log.Fatalf("nclient4.New(%s): %v", *iface, err) + slog.Error("Could not create nclient4 client", "iface", *iface, "error", err) + os.Exit(1) } defer client.Close() @@ -141,6 +145,14 @@ func main() { } } +// slogPrintfer adapts nclient4's Printfer to slog. nclient4 only emits through +// it under --verbose, so the packet summaries log at info level to stay visible. +type slogPrintfer struct{} + +func (slogPrintfer) Printf(format string, v ...any) { + slog.Info(fmt.Sprintf(format, v...), "component", "nclient4") +} + // generateMAC is the same deterministic hash used by vpcd's topology.go. // It produces a locally-administered MAC (02:00:00:xx:xx:xx) from a resource ID. func generateMAC(resourceID string) string { diff --git a/spinifex/gateway/ec2/instance/placement.go b/spinifex/gateway/ec2/instance/placement.go index 1f5245a12..62d0ca165 100644 --- a/spinifex/gateway/ec2/instance/placement.go +++ b/spinifex/gateway/ec2/instance/placement.go @@ -6,7 +6,7 @@ import ( "errors" "fmt" "log/slog" - "math/rand" + "math/rand/v2" "sort" "sync" "time" diff --git a/spinifex/network/external/dhcp/manager.go b/spinifex/network/external/dhcp/manager.go index ceb0c5886..4641b8008 100644 --- a/spinifex/network/external/dhcp/manager.go +++ b/spinifex/network/external/dhcp/manager.go @@ -6,7 +6,7 @@ import ( "errors" "fmt" "log/slog" - "math/rand" + "math/rand/v2" "net" "sync" "time" @@ -704,5 +704,5 @@ func jitter(span time.Duration) time.Duration { if span <= 0 { return 0 } - return time.Duration(rand.Int63n(int64(span*2))) - span //nolint:gosec // jitter, not cryptographic + return time.Duration(rand.Int64N(int64(span*2))) - span //nolint:gosec // jitter, not cryptographic } diff --git a/tests/e2e/ddil/scenarios/main_test.go b/tests/e2e/ddil/scenarios/main_test.go index 76a835fa8..d9ebbe90b 100644 --- a/tests/e2e/ddil/scenarios/main_test.go +++ b/tests/e2e/ddil/scenarios/main_test.go @@ -7,7 +7,7 @@ package scenarios import ( "context" - "log" + "log/slog" "os" "os/signal" "sync" @@ -64,12 +64,12 @@ func setupCluster() (*harness.Cluster, harness.SSH, error) { // DDIL_DRY_RUN=1 skips cluster init and leaves only TestCoverageDrift active. func TestMain(m *testing.M) { if dryRun() { - log.Printf("ddil: DDIL_DRY_RUN=1 — planned scenarios: %v (TestCoverageDrift will still execute)", scenarioLetters) + slog.Info("ddil: DDIL_DRY_RUN=1, TestCoverageDrift will still execute", "planned_scenarios", scenarioLetters) } c, s, err := setupCluster() if err != nil { - log.Printf("ddil: cluster setup failed (continuing, scenarios will skip): %v", err) + slog.Warn("ddil: cluster setup failed, continuing and scenarios will skip", "error", err) } stop := installSignalHandler(c, s) @@ -92,12 +92,12 @@ func installSignalHandler(c *harness.Cluster, s harness.SSH) func() { go func() { select { case sig := <-ch: - log.Printf("ddil: received %s, cleaning up before exit", sig) + slog.Info("ddil: received signal, cleaning up before exit", "signal", sig) if c != nil && s != nil { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() if err := harness.ResetAllNodes(ctx, c, s); err != nil { - log.Printf("ddil: ResetAllNodes on signal: %v", err) + slog.Error("ddil: ResetAllNodes on signal failed", "error", err) } _ = s.Close() }