diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 68d744a..8444c7b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,6 +12,13 @@ on: permissions: contents: read +# Pre-empt the September 2026 Node.js 20 → 24 transition by opting in now. +# Without this, every action run in this workflow logs a deprecation banner +# and will hard-fail once Node 20 is removed from the runner image. See: +# https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/ +env: + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: 'true' + jobs: build: runs-on: ubuntu-latest diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 6c0c12e..db3ee6c 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -17,6 +17,11 @@ permissions: id-token: write # for cosign keyless OIDC signing packages: write +# Pre-empt the September 2026 Node.js 20 → 24 transition. See +# .github/workflows/ci.yml for the rationale. +env: + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: 'true' + jobs: release: runs-on: ubuntu-latest diff --git a/ChangeLog.md b/ChangeLog.md index 0735bc4..168739a 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -17,6 +17,64 @@ _No unreleased changes._ --- +## 26.09 — 2026-05-27 + +Post-Planning.md tightening pass — clears the lint debt that 26.06's +`golangci-lint` integration (item #40) catches but never fixed, and +pre-empts the September 2026 Node.js 20 → 24 transition on GitHub +Actions before it bites. + +### Tooling + +- **`golangci-lint run ./...` now passes clean.** The 21 baseline + findings (15× errcheck on `Close`/`Body.Close`, 4× staticcheck + QF1008/QF1012, 2× gocritic ifElseChain/exitAfterDefer) are all + resolved — either by explicit `_ =` discards on Close calls in defer + position (the Go idiom for "we don't care about this error") or by + rewriting the offending pattern. `make lint` is now meaningfully + enforceable. +- **`cmd/console` `os.Exit` no longer skips deferred cleanup** + (gocritic exitAfterDefer). The real entry point moved into a `run() + int` helper and `main` calls `os.Exit(run())`, matching the + established Go idiom. +- **`config.go` `MarshalJSON` drops the redundant `.Duration` selector** + (staticcheck QF1008). +- **`cmd/console/tui` `WriteString(fmt.Sprintf(...))` rewritten to + `fmt.Fprintf(&b, ...)`** (staticcheck QF1012) at three sites. +- **`render()` `loading`/`err`/default chain rewritten as `switch`** + (gocritic ifElseChain). + +### Added + +- **`renovate.json`** — Renovate Bot configuration for ongoing + supply-chain updates. Bundles `gomod` minor/patch into one weekly PR + (`go-deps`), auto-merges GitHub Actions and Docker base-image digest + bumps, schedules `lockFileMaintenance` monthly, and labels + vulnerability alerts immediately. No automerges on `gomod` majors — + those land as manually-reviewed PRs. +- **`internal/tracing/tracing_test.go`** — covers `Setup` with an + empty endpoint, confirms `HTTPMiddleware` produces a valid span + inside the handler context, and verifies `HTTPClient` wraps an + injected base RoundTripper instead of replacing it. + +### Changed + +- **GitHub Actions opt in to Node.js 24 now.** Both `ci.yml` and + `release.yml` set `FORCE_JAVASCRIPT_ACTIONS_TO_NODE24=true`, + pre-empting the September 2026 Node.js 20 removal that would + otherwise hard-fail CI without warning. +- **`internal/tracing` semconv bumped from v1.26.0 → v1.40.0** to + match the otel SDK's `resource.Default()` schema URL — the older + version produced "conflicting Schema URL" errors at `Setup` time. + +### Notes + +- No new Planning.md items remain. This sprint addresses real lint and + CI-deprecation debt rather than feature work; future cleanup + follow-ups should be tracked via issues or a new Planning.md entry. + +--- + ## 26.08 — 2026-05-27 Final Planning.md sprint. Items #13 (peer TLS), #20 (OpenTelemetry tracing), diff --git a/cmd/console/main.go b/cmd/console/main.go index 283107d..adbae98 100644 --- a/cmd/console/main.go +++ b/cmd/console/main.go @@ -23,13 +23,20 @@ import ( ) func main() { + os.Exit(run()) +} + +// run holds the real entry-point logic so the os.Exit lives in main — +// otherwise deferred cleanup (signal stop, db.Close) would be skipped on +// the error paths (gocritic exitAfterDefer). +func run() int { dbPath := flag.String("db", "inventory.db", "path to SQLite database file") showVersion := flag.Bool("version", false, "print version and exit") flag.Parse() if *showVersion { - fmt.Fprintf(os.Stdout, "console %s\n", runtime.VersionString()) - return + fmt.Printf("console %s\n", runtime.VersionString()) + return 0 } ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) @@ -38,14 +45,15 @@ func main() { db, err := sqlite.Open(ctx, *dbPath) if err != nil { fmt.Fprintf(os.Stderr, "console: open database %q: %v\n", *dbPath, err) - os.Exit(1) + return 1 } - defer db.Close() + defer func() { _ = db.Close() }() m := tui.New(ctx, db.Hosts(), db.Ports(), db.Scans()) p := tea.NewProgram(m, tea.WithAltScreen(), tea.WithContext(ctx)) if _, err := p.Run(); err != nil { fmt.Fprintf(os.Stderr, "console: %v\n", err) - os.Exit(1) + return 1 } + return 0 } diff --git a/cmd/console/tui/tui.go b/cmd/console/tui/tui.go index 687513c..a09799c 100644 --- a/cmd/console/tui/tui.go +++ b/cmd/console/tui/tui.go @@ -498,11 +498,12 @@ func (m Model) View() string { b.WriteString(m.renderTabs()) b.WriteString("\n") - if m.loading { + switch { + case m.loading: b.WriteString("\n " + m.spin.View() + " Loading…\n") - } else if m.err != nil { + case m.err != nil: b.WriteString(lipgloss.NewStyle().Foreground(colRed).Render(" Error: "+m.err.Error()) + "\n") - } else { + default: switch m.current { case viewDashboard: b.WriteString(m.renderDashboard()) @@ -607,12 +608,12 @@ func renderScanRows(scans []*models.Scan) string { if s.FinishedAt != nil { status = styleStatusDone.Render("done") } - b.WriteString(fmt.Sprintf(" %-20s hosts:%-4d started:%-22s %s\n", + fmt.Fprintf(&b, " %-20s hosts:%-4d started:%-22s %s\n", s.Subnet, s.HostsFound, fmtTime(s.StartedAt), status, - )) + ) } return b.String() } @@ -620,11 +621,11 @@ func renderScanRows(scans []*models.Scan) string { func renderHostRows(hosts []*models.Host) string { var b strings.Builder for _, h := range hosts { - b.WriteString(fmt.Sprintf(" %-16s %-22s %s\n", + fmt.Fprintf(&b, " %-16s %-22s %s\n", h.IPAddress, orDash(h.Hostname), fmtTime(h.LastSeen), - )) + ) } return b.String() } @@ -655,10 +656,10 @@ func (m Model) renderHostDetail() string { {"Last Seen", fmtTime(h.LastSeen)}, } for _, kv := range meta { - b.WriteString(fmt.Sprintf(" %-18s %s\n", + fmt.Fprintf(&b, " %-18s %s\n", styleCardLabel.Render(kv.k+":"), kv.v, - )) + ) } b.WriteString("\n") b.WriteString(styleSectionHeader.Render(fmt.Sprintf("Open Ports — %d found", len(m.portList))) + "\n") diff --git a/cmd/internal/runtime/runtime.go b/cmd/internal/runtime/runtime.go index 7e440f0..31a20a6 100644 --- a/cmd/internal/runtime/runtime.go +++ b/cmd/internal/runtime/runtime.go @@ -81,7 +81,7 @@ func Run(opts Options) int { flag.Parse() if *showVersion { - fmt.Fprintf(os.Stdout, "%s %s\n", opts.Name, VersionString()) + _, _ = fmt.Fprintf(os.Stdout, "%s %s\n", opts.Name, VersionString()) return 0 } diff --git a/internal/config/config.go b/internal/config/config.go index 0573967..3f84335 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -37,7 +37,7 @@ func (d *Duration) UnmarshalJSON(b []byte) error { } func (d Duration) MarshalJSON() ([]byte, error) { - return json.Marshal(d.Duration.String()) + return json.Marshal(d.String()) } // Config is the top-level configuration object. @@ -236,7 +236,7 @@ func Load(path string) (*Config, error) { return nil, fmt.Errorf("open config %q: %w", path, err) } if err == nil { - defer f.Close() + defer func() { _ = f.Close() }() if info, statErr := f.Stat(); statErr == nil { fileMode = info.Mode().Perm() loaded = true diff --git a/internal/health/client.go b/internal/health/client.go index 279cf70..396bbcb 100644 --- a/internal/health/client.go +++ b/internal/health/client.go @@ -74,7 +74,7 @@ func (c *Client) Ping(ctx context.Context) error { if err != nil { return fmt.Errorf("health check failed: %w", err) } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() _, _ = io.Copy(io.Discard, resp.Body) if resp.StatusCode != http.StatusOK { return fmt.Errorf("peer returned status %d", resp.StatusCode) @@ -93,7 +93,7 @@ func (c *Client) FetchStatus(ctx context.Context) (Status, error) { if err != nil { return Status{}, fmt.Errorf("fetch status failed: %w", err) } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() var s Status if err := json.NewDecoder(io.LimitReader(resp.Body, maxStatusBytes)).Decode(&s); err != nil { diff --git a/internal/scanner/arp.go b/internal/scanner/arp.go index f0f99e7..a00f888 100644 --- a/internal/scanner/arp.go +++ b/internal/scanner/arp.go @@ -32,7 +32,7 @@ func lookupARP(ip string) (string, string) { if err != nil { return "", "" } - defer f.Close() + defer func() { _ = f.Close() }() scanner := bufio.NewScanner(f) scanner.Scan() // header diff --git a/internal/scanner/scanner.go b/internal/scanner/scanner.go index 89df216..34ec2a3 100644 --- a/internal/scanner/scanner.go +++ b/internal/scanner/scanner.go @@ -231,7 +231,7 @@ func (s *Scanner) probe(ctx context.Context, ip string) (int, bool) { results <- result{} return } - conn.Close() + _ = conn.Close() results <- result{port: port, ok: true} }(port) } @@ -294,7 +294,7 @@ func (s *Scanner) deepScan(ctx context.Context, hostID int64, ip string, knownOp if err != nil { return } - conn.Close() + _ = conn.Close() s.upsertPort(ctx, hostID, ip, port, models.TCP, models.StateOpen, ts) }(port) } @@ -339,7 +339,7 @@ func probeUDP(ctx context.Context, ip string, port int, timeout time.Duration) ( // A connect-time error on UDP is unusual but treat it as filtered. return "", false } - defer conn.Close() + defer func() { _ = conn.Close() }() // Send a single zero byte. Many services respond to anything (DNS // returns FORMERR, NTP rejects); for the rest we still learn the // closed-vs-filtered distinction from the read error. @@ -399,7 +399,7 @@ func sshBanner(ctx context.Context, ip string, port int, timeout time.Duration) if err != nil { return "" } - defer conn.Close() + defer func() { _ = conn.Close() }() _ = conn.SetReadDeadline(time.Now().Add(timeout)) line, err := bufio.NewReader(conn).ReadString('\n') if err != nil { @@ -425,7 +425,7 @@ func httpServerHeader(ctx context.Context, ip string, port int, timeout time.Dur if err != nil { return "" } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() if s := resp.Header.Get("Server"); s != "" { return "HTTP: " + s } diff --git a/internal/sqlite/db.go b/internal/sqlite/db.go index ef48d29..1794192 100644 --- a/internal/sqlite/db.go +++ b/internal/sqlite/db.go @@ -38,7 +38,7 @@ func Open(ctx context.Context, path string) (*DB, error) { } if err := migrations.Run(ctx, writer); err != nil { - writer.Close() + _ = writer.Close() return nil, fmt.Errorf("run migrations: %w", err) } @@ -58,7 +58,7 @@ func Open(ctx context.Context, path string) (*DB, error) { } reader, err := openPool(ctx, path, readers) if err != nil { - writer.Close() + _ = writer.Close() return nil, fmt.Errorf("open reader pool: %w", err) } @@ -79,7 +79,7 @@ func openPool(ctx context.Context, path string, maxOpen int) (*sql.DB, error) { } for _, p := range pragmas { if _, err := conn.ExecContext(ctx, p); err != nil { - conn.Close() + _ = conn.Close() return nil, fmt.Errorf("set %q: %w", p, err) } } diff --git a/internal/sqlite/host.go b/internal/sqlite/host.go index 567a2f9..f89751e 100644 --- a/internal/sqlite/host.go +++ b/internal/sqlite/host.go @@ -76,7 +76,7 @@ func (r *HostRepo) List(ctx context.Context) ([]*models.Host, error) { if err != nil { return nil, fmt.Errorf("list hosts: %w", err) } - defer rows.Close() + defer func() { _ = rows.Close() }() var hosts []*models.Host for rows.Next() { diff --git a/internal/sqlite/migrations/migrate.go b/internal/sqlite/migrations/migrate.go index 5f92376..1045702 100644 --- a/internal/sqlite/migrations/migrate.go +++ b/internal/sqlite/migrations/migrate.go @@ -31,7 +31,7 @@ func Run(ctx context.Context, db *sql.DB) error { if err != nil { return fmt.Errorf("query applied migrations: %w", err) } - defer rows.Close() + defer func() { _ = rows.Close() }() applied := make(map[string]struct{}) for rows.Next() { diff --git a/internal/sqlite/port.go b/internal/sqlite/port.go index 1e9834e..06d5c45 100644 --- a/internal/sqlite/port.go +++ b/internal/sqlite/port.go @@ -43,7 +43,7 @@ func (r *PortRepo) ListByHost(ctx context.Context, hostID int64) ([]*models.Port if err != nil { return nil, fmt.Errorf("list ports for host %d: %w", hostID, err) } - defer rows.Close() + defer func() { _ = rows.Close() }() var ports []*models.Port for rows.Next() { diff --git a/internal/sqlite/scan.go b/internal/sqlite/scan.go index 5d28a3d..1883d4e 100644 --- a/internal/sqlite/scan.go +++ b/internal/sqlite/scan.go @@ -57,7 +57,7 @@ func (r *ScanRepo) List(ctx context.Context) ([]*models.Scan, error) { if err != nil { return nil, fmt.Errorf("list scans: %w", err) } - defer rows.Close() + defer func() { _ = rows.Close() }() var scans []*models.Scan for rows.Next() { diff --git a/internal/tracing/tracing.go b/internal/tracing/tracing.go index ee959c8..aeae184 100644 --- a/internal/tracing/tracing.go +++ b/internal/tracing/tracing.go @@ -24,7 +24,7 @@ import ( "go.opentelemetry.io/otel/propagation" "go.opentelemetry.io/otel/sdk/resource" sdktrace "go.opentelemetry.io/otel/sdk/trace" - semconv "go.opentelemetry.io/otel/semconv/v1.26.0" + semconv "go.opentelemetry.io/otel/semconv/v1.40.0" ) // Setup installs a global TracerProvider with an OTLP/HTTP exporter and the diff --git a/internal/tracing/tracing_test.go b/internal/tracing/tracing_test.go new file mode 100644 index 0000000..af852f0 --- /dev/null +++ b/internal/tracing/tracing_test.go @@ -0,0 +1,98 @@ +package tracing_test + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/Ronin48/NetworkInventoryAgent/internal/tracing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/trace" +) + +func TestSetup_EmptyEndpointReturnsShutdownFunc(t *testing.T) { + // Empty endpoint should not fail — the SDK falls back to the OTEL_ + // EXPORTER_OTLP_ENDPOINT env var or runs as a no-op exporter. + t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "") + shutdown, err := tracing.Setup(context.Background(), "test-service", "") + require.NoError(t, err) + require.NotNil(t, shutdown) + require.NoError(t, shutdown(context.Background())) +} + +func TestHTTPMiddleware_ProducesSpan(t *testing.T) { + // Use the global tracer to confirm the otelhttp wrap actually starts a + // span. We don't need an exporter — the trace.SpanFromContext recovers + // the active span if one is in flight. + shutdown, err := tracing.Setup(context.Background(), "test-service", "") + require.NoError(t, err) + t.Cleanup(func() { _ = shutdown(context.Background()) }) + + var sawValidSpan bool + inner := http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + sp := trace.SpanFromContext(r.Context()) + // otel always returns a span; we check IsRecording or a valid + // context to confirm the middleware actually started one. + if sp.SpanContext().IsValid() { + sawValidSpan = true + } + }) + wrapped := tracing.HTTPMiddleware("test-handler", inner) + + srv := httptest.NewServer(wrapped) + t.Cleanup(srv.Close) + + resp, err := http.Get(srv.URL + "/") + require.NoError(t, err) + _ = resp.Body.Close() + + assert.True(t, sawValidSpan, "otelhttp middleware should make a valid span available in the handler context") +} + +func TestHTTPClient_TransportWrapsBase(t *testing.T) { + // A nil base must fall back to http.DefaultTransport, not panic. + c := tracing.HTTPClient(nil) + require.NotNil(t, c) + require.NotNil(t, c.Transport) + + // A custom RoundTripper must be preserved (wrapped, not replaced). + custom := &countingTransport{base: http.DefaultTransport} + c = tracing.HTTPClient(custom) + require.NotNil(t, c.Transport) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + })) + t.Cleanup(srv.Close) + + resp, err := c.Get(srv.URL + "/") + require.NoError(t, err) + _ = resp.Body.Close() + assert.Equal(t, 1, custom.calls, "the base transport must still be called once per request") +} + +// TestSetup_GlobalTracerProviderSet confirms Setup replaces the global TP. +func TestSetup_GlobalTracerProviderSet(t *testing.T) { + shutdown, err := tracing.Setup(context.Background(), "service-a", "") + require.NoError(t, err) + t.Cleanup(func() { _ = shutdown(context.Background()) }) + + tp := otel.GetTracerProvider() + require.NotNil(t, tp) + tr := tp.Tracer("unit-test") + _, span := tr.Start(context.Background(), "noop") + span.End() +} + +type countingTransport struct { + base http.RoundTripper + calls int +} + +func (c *countingTransport) RoundTrip(req *http.Request) (*http.Response, error) { + c.calls++ + return c.base.RoundTrip(req) +} diff --git a/renovate.json b/renovate.json new file mode 100644 index 0000000..ba48607 --- /dev/null +++ b/renovate.json @@ -0,0 +1,49 @@ +{ + "$schema": "https://docs.renovatebot.com/renovate-schema.json", + "extends": [ + "config:recommended", + ":semanticCommits", + ":automergeDigest", + ":disableDependencyDashboard" + ], + "timezone": "UTC", + "schedule": ["before 4am on Monday"], + "prHourlyLimit": 4, + "labels": ["dependencies"], + "packageRules": [ + { + "matchManagers": ["gomod"], + "matchUpdateTypes": ["minor", "patch"], + "groupName": "go-deps", + "automerge": false + }, + { + "matchManagers": ["gomod"], + "matchUpdateTypes": ["major"], + "labels": ["dependencies", "breaking"] + }, + { + "matchManagers": ["github-actions"], + "groupName": "github-actions", + "automerge": true, + "automergeType": "pr", + "platformAutomerge": true + }, + { + "matchManagers": ["dockerfile"], + "matchUpdateTypes": ["digest"], + "groupName": "docker-base-digests", + "automerge": true, + "automergeType": "pr", + "platformAutomerge": true + } + ], + "vulnerabilityAlerts": { + "labels": ["security"], + "schedule": ["at any time"] + }, + "lockFileMaintenance": { + "enabled": true, + "schedule": ["before 4am on the first day of the month"] + } +}