diff --git a/.jules/bolt.md b/.jules/bolt.md index 8267c90..7f00bcc 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -5,3 +5,7 @@ Critical learnings and performance patterns discovered in this codebase. ## 2026-08-08 - Pre-parse IP Addresses for Comparator Functions **Learning:** `sort.Slice` calls comparator functions $O(N \log N)$ times. Calling `netip.ParseAddr` or other parsing/conversion functions inside a sort comparator creates severe CPU overhead ($2 \cdot N \log_2 N$ string parses) during large discovery sweeps (up to 65,536 hosts). **Action:** Always pre-parse IP strings into `netip.Addr` structs once into a temporary slice or wrapper struct before sorting. + +## 2026-08-13 - Use Switch Statements for Zero-Allocation Static Lookups +**Learning:** Defining static lookup map literals (such as `map[string]string{...}`) inside helper functions evaluated per-host (e.g., `osFamily` in `parseFacts`) causes Go to allocate and populate a new hash map on the heap on every invocation (~1.2 KB and 3 allocations per call). +**Action:** Prefer switch statements over map literals for fixed static lookups to achieve zero heap allocations, complete immutability, and zero race-condition risk. diff --git a/pkg/proxy/discovery/facts.go b/pkg/proxy/discovery/facts.go index 2a9dcca..2b15934 100644 --- a/pkg/proxy/discovery/facts.go +++ b/pkg/proxy/discovery/facts.go @@ -59,24 +59,32 @@ func parseOSRelease(s string) map[string]string { return kv } +// osFamilyByID maps distro IDs to package tooling families via a switch statement. +// Provides zero allocations and complete immutability. +func osFamilyByID(id string) (string, bool) { + switch id { + case "rhel", "centos", "rocky", "almalinux", "ol", "oracle", "amzn", "fedora": + return "rhel", true + case "debian", "ubuntu", "linuxmint", "raspbian": + return "debian", true + case "sles", "sled", "opensuse", "opensuse-leap", "opensuse-tumbleweed": + return "suse", true + case "alpine": + return "alpine", true + default: + return "", false + } +} + // osFamily maps a distro ID to the family whose package tooling it uses. // ID_LIKE is the fallback so derivatives we have never heard of still land in // the right family — the common case for the RHEL rebuilds these fleets run. func osFamily(id, idLike string) string { - byID := map[string]string{ - "rhel": "rhel", "centos": "rhel", "rocky": "rhel", "almalinux": "rhel", - "ol": "rhel", "oracle": "rhel", "amzn": "rhel", "fedora": "rhel", - "debian": "debian", "ubuntu": "debian", "linuxmint": "debian", "raspbian": "debian", - "sles": "suse", "sled": "suse", "opensuse": "suse", - "opensuse-leap": "suse", "opensuse-tumbleweed": "suse", - "alpine": "alpine", - } - - if fam, ok := byID[id]; ok { + if fam, ok := osFamilyByID(id); ok { return fam } for _, like := range strings.Fields(idLike) { - if fam, ok := byID[like]; ok { + if fam, ok := osFamilyByID(like); ok { return fam } // SUSE ships ID_LIKE="suse opensuse" on some releases. diff --git a/pkg/proxy/discovery/pack.go b/pkg/proxy/discovery/pack.go index a131be9..414c623 100644 --- a/pkg/proxy/discovery/pack.go +++ b/pkg/proxy/discovery/pack.go @@ -49,9 +49,11 @@ func ParseAndVerify(raw []byte, pubKey ed25519.PublicKey) (*Pack, error) { return nil, fmt.Errorf("pack too large: %d bytes (max %d)", len(raw), maxPackBytes) } - // Checked before parsing: the signature covers the document minus these - // lines, so their count is part of what makes the signature meaningful. - if n := countSignatureLines(raw); n != 1 { + // Checked before parsing: splitSignatureLine extracts the signed payload body + // and counts top-level signature lines in a single pass to avoid duplicate + // regex line parsing. + signedBody, n := splitSignatureLine(raw) + if n != 1 { return nil, fmt.Errorf("pack must contain exactly one top-level signature line, found %d", n) } @@ -70,7 +72,7 @@ func ParseAndVerify(raw []byte, pubKey ed25519.PublicKey) (*Pack, error) { if err != nil { return nil, fmt.Errorf("pack signature is not valid base64: %w", err) } - if !ed25519.Verify(pubKey, SignedBytes(raw), sig) { + if !ed25519.Verify(pubKey, signedBody, sig) { return nil, fmt.Errorf("pack signature verification failed") } diff --git a/pkg/proxy/discovery/pack_test.go b/pkg/proxy/discovery/pack_test.go index 82c1e5b..33de6f5 100644 --- a/pkg/proxy/discovery/pack_test.go +++ b/pkg/proxy/discovery/pack_test.go @@ -15,7 +15,7 @@ func signPack(t *testing.T, body string, priv ed25519.PrivateKey) string { return body + "\nsignature: " + base64.StdEncoding.EncodeToString(sig) + "\n" } -func testKeys(t *testing.T) (ed25519.PublicKey, ed25519.PrivateKey) { +func testKeys(t testing.TB) (ed25519.PublicKey, ed25519.PrivateKey) { t.Helper() pub, priv, err := ed25519.GenerateKey(nil) if err != nil { @@ -314,3 +314,23 @@ func TestParseAndVerify_SignatureLineInjectionIsRejected(t *testing.T) { }) } } + +func BenchmarkParseFacts(b *testing.B) { + probe := "NAME=\"Ubuntu\"\nVERSION=\"22.04.3 LTS\"\nID=ubuntu\nID_LIKE=debian\nVERSION_ID=\"22.04\"\n---\nx86_64" + b.ResetTimer() + for i := 0; i < b.N; i++ { + _ = parseFacts(probe) + } +} + +func BenchmarkParseAndVerify(b *testing.B) { + pub, priv := testKeys(b) + body := "version: 1\nkind: inventory\ncollectors:\n - id: a\n cmd: \"echo hi\"\n" + sig := ed25519.Sign(priv, SignedBytes([]byte(body))) + doc := []byte(body + "signature: " + base64.StdEncoding.EncodeToString(sig) + "\n") + + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, _ = ParseAndVerify(doc, pub) + } +}