From d11be77f1f4ac88d9785eb143547a7c1e67e49d2 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Thu, 9 Apr 2026 17:30:08 +0000 Subject: [PATCH] testcapture: type captured data with tailcfg types Updates #3157 --- .../v2/tailscale_acl_data_compat_test.go | 40 +---- .../policy/v2/tailscale_grants_compat_test.go | 32 +--- .../v2/tailscale_routes_data_compat_test.go | 97 ++---------- .../v2/tailscale_ssh_data_compat_test.go | 23 +-- hscontrol/types/testcapture/header.go | 16 +- hscontrol/types/testcapture/testcapture.go | 141 ++++++++++++++---- .../types/testcapture/testcapture_test.go | 104 ++++++------- 7 files changed, 192 insertions(+), 261 deletions(-) diff --git a/hscontrol/policy/v2/tailscale_acl_data_compat_test.go b/hscontrol/policy/v2/tailscale_acl_data_compat_test.go index 6f7bb970..6bd9484d 100644 --- a/hscontrol/policy/v2/tailscale_acl_data_compat_test.go +++ b/hscontrol/policy/v2/tailscale_acl_data_compat_test.go @@ -140,27 +140,6 @@ func cmpOptions() []cmp.Option { return a.Bits() < b.Bits() }), - // Compare json.RawMessage semantically rather than by exact - // bytes to handle indentation differences between the policy - // source and the golden capture data. - cmp.Comparer(func(a, b json.RawMessage) bool { - var va, vb any - - err := json.Unmarshal(a, &va) - if err != nil { - return string(a) == string(b) - } - - err = json.Unmarshal(b, &vb) - if err != nil { - return string(a) == string(b) - } - - ja, _ := json.Marshal(va) - jb, _ := json.Marshal(vb) - - return string(ja) == string(jb) - }), // Compare tailcfg.RawMessage semantically (it's a string type // containing JSON) to handle indentation differences. cmp.Comparer(func(a, b tailcfg.RawMessage) bool { @@ -492,9 +471,6 @@ func testACLSuccess( for nodeName, capture := range tf.Captures { t.Run(nodeName, func(t *testing.T) { - captureIsNull := len(capture.PacketFilterRules) == 0 || - string(capture.PacketFilterRules) == "null" //nolint:goconst - node := findNodeByGivenName(nodes, nodeName) if node == nil { t.Skipf( @@ -524,21 +500,7 @@ func testACLSuccess( compiledRules, ) - // Parse expected rules from JSON - var wantRules []tailcfg.FilterRule - if !captureIsNull { - err = json.Unmarshal( - capture.PacketFilterRules, - &wantRules, - ) - require.NoError( - t, - err, - "%s/%s: failed to unmarshal expected rules", - tf.TestID, - nodeName, - ) - } + wantRules := capture.PacketFilterRules // Compare opts := append( diff --git a/hscontrol/policy/v2/tailscale_grants_compat_test.go b/hscontrol/policy/v2/tailscale_grants_compat_test.go index 9aba7bf8..6102a7e4 100644 --- a/hscontrol/policy/v2/tailscale_grants_compat_test.go +++ b/hscontrol/policy/v2/tailscale_grants_compat_test.go @@ -18,7 +18,6 @@ package v2 import ( - "encoding/json" "net/netip" "path/filepath" "strings" @@ -273,9 +272,11 @@ func findGrantsNode(nodes types.Nodes, name string) *types.Node { // convertPolicyUserEmails used to map SaaS-side emails to @example.com. // tscap now anonymizes the policy JSON at write time (kratail2tid -> odin, // kristoffer -> thor, monitorpasskeykradalby -> freya), so the captured -// FullPolicy is already in its final form and this is a passthrough. -func convertPolicyUserEmails(policyJSON []byte) []byte { - return policyJSON +// FullPolicy is already in its final form and this is a passthrough that +// just adapts the captured string value to the []byte that the policy +// parser expects. +func convertPolicyUserEmails(policyJSON string) []byte { + return []byte(policyJSON) } // loadGrantTestFile loads and parses a single grant capture HuJSON file. @@ -501,10 +502,7 @@ func testGrantSuccess( // kakuna (tag:prod) was frequently offline (132 of 188 success tests). // When offline, packet_filter_rules is null and topology shows // hostname="unknown" with empty tags. - captureIsNull := len(capture.PacketFilterRules) == 0 || - string(capture.PacketFilterRules) == "null" - - if captureIsNull { + if len(capture.PacketFilterRules) == 0 { topoNode, exists := tf.Topology.Nodes[nodeName] if exists && (topoNode.Hostname == "unknown" || topoNode.Hostname == "") { t.Skipf( @@ -515,7 +513,7 @@ func testGrantSuccess( return } - // Node was online but has null/empty rules — means Tailscale + // Node was online but has empty rules — means Tailscale // produced no rules. headscale should also produce no rules. } @@ -545,21 +543,7 @@ func testGrantSuccess( gotRules = policyutil.ReduceFilterRules(node.View(), gotRules) - // Unmarshal Tailscale expected rules from JSON capture - var wantRules []tailcfg.FilterRule - if !captureIsNull { - err = json.Unmarshal( - []byte(capture.PacketFilterRules), - &wantRules, - ) - require.NoError( - t, - err, - "%s/%s: failed to unmarshal expected rules from JSON", - tf.TestID, - nodeName, - ) - } + wantRules := capture.PacketFilterRules // Compare headscale output against Tailscale expected output. // The diff labels show (-tailscale +headscale) to make clear diff --git a/hscontrol/policy/v2/tailscale_routes_data_compat_test.go b/hscontrol/policy/v2/tailscale_routes_data_compat_test.go index e5a53d32..9a8d4dea 100644 --- a/hscontrol/policy/v2/tailscale_routes_data_compat_test.go +++ b/hscontrol/policy/v2/tailscale_routes_data_compat_test.go @@ -26,7 +26,6 @@ package v2 import ( - "encoding/json" "net/netip" "path/filepath" "slices" @@ -245,9 +244,6 @@ func TestRoutesCompat(t *testing.T) { for nodeName, capture := range tf.Captures { t.Run(nodeName, func(t *testing.T) { - captureIsNull := len(capture.PacketFilterRules) == 0 || - string(capture.PacketFilterRules) == "null" //nolint:goconst - node := findNodeByGivenName(nodes, nodeName) if node == nil { t.Skipf( @@ -276,20 +272,7 @@ func TestRoutesCompat(t *testing.T) { compiledRules, ) - var wantRules []tailcfg.FilterRule - if !captureIsNull { - err = json.Unmarshal( - capture.PacketFilterRules, - &wantRules, - ) - require.NoError( - t, - err, - "%s/%s: failed to unmarshal expected rules", - tf.TestID, - nodeName, - ) - } + wantRules := capture.PacketFilterRules opts := append( cmpOptions(), @@ -331,19 +314,11 @@ func derivePeerPairsFromCaptures( pairs := make(map[[2]string]bool) for dstNodeName, capture := range tf.Captures { - captureIsNull := len(capture.PacketFilterRules) == 0 || - string(capture.PacketFilterRules) == "null" - if captureIsNull { + if len(capture.PacketFilterRules) == 0 { continue } - var rules []tailcfg.FilterRule - - err := json.Unmarshal(capture.PacketFilterRules, &rules) - require.NoError(t, err, - "%s/%s: failed to unmarshal capture rules", - tf.TestID, dstNodeName, - ) + rules := capture.PacketFilterRules // Build an IPSet of all SrcIPs from the capture's filter rules. var srcBuilder netipx.IPSetBuilder @@ -460,19 +435,11 @@ func deriveAllPeerPairsFromCaptures( pairs := make(map[[2]string]bool) for dstNodeName, capture := range tf.Captures { - captureIsNull := len(capture.PacketFilterRules) == 0 || - string(capture.PacketFilterRules) == "null" - if captureIsNull { + if len(capture.PacketFilterRules) == 0 { continue } - var rules []tailcfg.FilterRule - - err := json.Unmarshal(capture.PacketFilterRules, &rules) - require.NoError(t, err, - "%s/%s: failed to unmarshal capture rules", - tf.TestID, dstNodeName, - ) + rules := capture.PacketFilterRules // Build an IPSet of all SrcIPs. var srcBuilder netipx.IPSetBuilder @@ -665,22 +632,11 @@ func TestRoutesCompatPeerVisibility(t *testing.T) { // called from a node whose subnet routes overlap the // source CIDRs. for dstNodeName, capture := range tf.Captures { - captureIsNull := len( - capture.PacketFilterRules, - ) == 0 || - string( - capture.PacketFilterRules, - ) == "null" - if captureIsNull { + if len(capture.PacketFilterRules) == 0 { continue } - var rules []tailcfg.FilterRule - - err := json.Unmarshal( - capture.PacketFilterRules, &rules, - ) - require.NoError(t, err) + rules := capture.PacketFilterRules // Extract destination prefixes from the rules. var dstPrefixes []netip.Prefix @@ -920,25 +876,11 @@ func TestRoutesCompatReduceRoutes(t *testing.T) { // Then verify that viewer nodes with matching source // identity can access those routes via CanAccessRoute. for dstNodeName, capture := range tf.Captures { - captureIsNull := len( - capture.PacketFilterRules, - ) == 0 || - string( - capture.PacketFilterRules, - ) == "null" - if captureIsNull { + if len(capture.PacketFilterRules) == 0 { continue } - var rules []tailcfg.FilterRule - - err := json.Unmarshal( - capture.PacketFilterRules, &rules, - ) - require.NoError(t, err, - "%s/%s: failed to unmarshal capture rules", - tf.TestID, dstNodeName, - ) + rules := capture.PacketFilterRules // Build the set of destination route prefixes. var dstPrefixes []netip.Prefix @@ -1265,24 +1207,11 @@ func TestRoutesCompatNoPeersBeyondCaptures(t *testing.T) { // appears in DstPorts of rules delivered to another // node, they must be peers. for dstNodeName, capture := range tf.Captures { - captureIsNull := len( - capture.PacketFilterRules, - ) == 0 || - string( - capture.PacketFilterRules, - ) == "null" - if captureIsNull { + if len(capture.PacketFilterRules) == 0 { continue } - var rules []tailcfg.FilterRule - - err := json.Unmarshal( - capture.PacketFilterRules, &rules, - ) - if err != nil { - continue - } + rules := capture.PacketFilterRules var dstBuilder netipx.IPSetBuilder @@ -1423,9 +1352,7 @@ func TestRoutesCompatNoFalsePositivePeers(t *testing.T) { routerNodes := make(map[string]bool) for nodeName, capture := range tf.Captures { - captureIsNull := len(capture.PacketFilterRules) == 0 || - string(capture.PacketFilterRules) == "null" - if !captureIsNull { + if len(capture.PacketFilterRules) > 0 { routerNodes[nodeName] = true } } diff --git a/hscontrol/policy/v2/tailscale_ssh_data_compat_test.go b/hscontrol/policy/v2/tailscale_ssh_data_compat_test.go index 93e059bc..b66032d8 100644 --- a/hscontrol/policy/v2/tailscale_ssh_data_compat_test.go +++ b/hscontrol/policy/v2/tailscale_ssh_data_compat_test.go @@ -17,7 +17,6 @@ package v2 import ( - "encoding/json" "path/filepath" "testing" @@ -185,7 +184,7 @@ func TestSSHDataCompat(t *testing.T) { // tscap already rewrites SaaS emails to @example.com. policyJSON := tf.Input.FullPolicy - pol, err := unmarshalPolicy(policyJSON) + pol, err := unmarshalPolicy([]byte(policyJSON)) require.NoError( t, err, @@ -217,24 +216,10 @@ func TestSSHDataCompat(t *testing.T) { nodeName, ) - // Parse expected rules from JSON capture - var wantRules []*tailcfg.SSHRule - if len(capture.SSHRules) > 0 && - string(capture.SSHRules) != "null" { - err = json.Unmarshal(capture.SSHRules, &wantRules) - require.NoError( - t, - err, - "%s/%s: failed to unmarshal expected rules", - tf.TestID, - nodeName, - ) - } - - // Build expected SSHPolicy from the rules + // Build expected SSHPolicy from the typed rules. var wantSSH *tailcfg.SSHPolicy - if len(wantRules) > 0 { - wantSSH = &tailcfg.SSHPolicy{Rules: wantRules} + if len(capture.SSHRules) > 0 { + wantSSH = &tailcfg.SSHPolicy{Rules: capture.SSHRules} } // Normalize: treat empty-rules SSHPolicy as nil diff --git a/hscontrol/types/testcapture/header.go b/hscontrol/types/testcapture/header.go index e4822a75..e102ca65 100644 --- a/hscontrol/types/testcapture/header.go +++ b/hscontrol/types/testcapture/header.go @@ -89,7 +89,7 @@ func captureStats(c *Capture) string { if n.PacketFilterRules != nil { filterRulesSet = true - if !isJSONNullOrEmpty(n.PacketFilterRules) { + if len(n.PacketFilterRules) > 0 { filterRules++ } } @@ -97,7 +97,7 @@ func captureStats(c *Capture) string { if n.SSHRules != nil { sshRulesSet = true - if !isJSONNullOrEmpty(n.SSHRules) { + if len(n.SSHRules) > 0 { sshRules++ } } @@ -112,15 +112,3 @@ func captureStats(c *Capture) string { return "" } } - -// isJSONNullOrEmpty reports whether raw is the JSON value `null`, -// an empty array `[]`, or empty after whitespace trimming. -func isJSONNullOrEmpty(raw []byte) bool { - trimmed := strings.TrimSpace(string(raw)) - switch trimmed { - case "", "null", "[]": - return true - default: - return false - } -} diff --git a/hscontrol/types/testcapture/testcapture.go b/hscontrol/types/testcapture/testcapture.go index d61bb36a..e1fb7763 100644 --- a/hscontrol/types/testcapture/testcapture.go +++ b/hscontrol/types/testcapture/testcapture.go @@ -2,12 +2,15 @@ // policy v2 compatibility tests for golden data captured from // Tailscale SaaS by the tscap tool. // -// Files are HuJSON. The package intentionally does not import -// tailscale.com/tailcfg so it can live under hscontrol/types/ -// without dragging extra dependencies. Wire-format Tailscale data -// (filter rules, netmap, whois, SSH rules) is stored as -// json.RawMessage; consumers json.Unmarshal into the typed shape -// they need. +// Files are HuJSON. Wire-format Tailscale data (filter rules, netmap, +// whois, SSH rules) is stored as proper tailcfg/netmap/filtertype/ +// apitype values rather than json.RawMessage so that schema drift +// between tscap and headscale becomes a compile error rather than a +// silent test failure, and so that consumers don't have to repeat +// json.Unmarshal at every read site. Storing data as json.RawMessage +// previously hid a serious capture-pipeline bug (the IPN bus initial +// notification returns a stale Peers slice — see the comment on +// Node.Netmap below) for months. // // All four corpora (acl, routes, grant, ssh) use the same Capture // shape. SSH scenarios populate Captures[name].SSHRules; the others @@ -17,6 +20,11 @@ package testcapture import ( "encoding/json" "time" + + "tailscale.com/client/tailscale/apitype" + "tailscale.com/tailcfg" + "tailscale.com/types/netmap" + "tailscale.com/wgengine/filter/filtertype" ) // SchemaVersion identifies the on-disk format. Bumped on breaking changes. @@ -84,10 +92,18 @@ type Capture struct { // Input describes everything that was sent to the tailnet to produce // the captured state. +// +// Input has a custom UnmarshalJSON to accept both the new on-disk +// shape (where full_policy is a JSON-encoded string) and the legacy +// shape (where full_policy is a JSON object). The legacy shape is +// re-marshaled to a string at load time so consumers see the typed +// field uniformly. type Input struct { // FullPolicy is the verbatim policy that was POSTed to the SaaS - // API. Stored as raw JSON so it round-trips losslessly. - FullPolicy json.RawMessage `json:"full_policy"` + // API. Stored as a string because it is opaque JSON that round- + // trips losslessly without parsing — headscale's policy parser + // reads it on demand. + FullPolicy string `json:"full_policy"` // APIResponseCode is the HTTP status code of the policy POST. APIResponseCode int `json:"api_response_code"` @@ -109,6 +125,65 @@ type Input struct { ScenarioPath string `json:"scenario_path,omitempty"` } +// UnmarshalJSON handles both the current on-disk shape (full_policy +// as a JSON-encoded string) and the legacy shape (full_policy as a +// JSON object). Legacy objects are re-marshaled into a string at +// load time so consumers see the typed field uniformly. New captures +// always write the string form via the default Marshaler. +func (i *Input) UnmarshalJSON(data []byte) error { + type alias Input + + var raw struct { + alias + + FullPolicy json.RawMessage `json:"full_policy"` + } + + err := json.Unmarshal(data, &raw) + if err != nil { + return err + } + + *i = Input(raw.alias) + // raw.FullPolicy might be a JSON-encoded string ("...") or a JSON + // object/array/null. Try string first; on failure use the raw bytes + // verbatim, normalised to compact form. + if len(raw.FullPolicy) == 0 || string(raw.FullPolicy) == "null" { + i.FullPolicy = "" + return nil + } + + if raw.FullPolicy[0] == '"' { + var s string + + err := json.Unmarshal(raw.FullPolicy, &s) + if err != nil { + return err + } + + i.FullPolicy = s + + return nil + } + // Legacy: full_policy is a raw JSON object. Compact and store as + // the equivalent string. + var v any + + err = json.Unmarshal(raw.FullPolicy, &v) + if err != nil { + return err + } + + compact, err := json.Marshal(v) + if err != nil { + return err + } + + i.FullPolicy = string(compact) + + return nil +} + // APIResponseBody is the (subset of) the SaaS API error response we keep. type APIResponseBody struct { Message string `json:"message,omitempty"` @@ -190,31 +265,41 @@ type TopologyNode struct { // // Whichever fields are set in the file is what the consumer reads. type Node struct { - // PacketFilterRules is the wire-format []tailcfg.FilterRule as - // returned by tailscaled localapi /debug-packet-filter-rules. - // The single most important field for ACL/routes/grant tests. - PacketFilterRules json.RawMessage `json:"packet_filter_rules,omitempty"` + // PacketFilterRules is the wire-format filter rules as returned + // by tailscaled localapi /debug-packet-filter-rules. The single + // most important field for ACL/routes/grant tests. + PacketFilterRules []tailcfg.FilterRule `json:"packet_filter_rules,omitempty"` - // PacketFilterMatches is the compiled []filtertype.Match with - // CapMatch, returned by tailscaled localapi + // PacketFilterMatches is the compiled filter matches (with + // CapMatch) returned by tailscaled localapi // /debug-packet-filter-matches. Captured alongside // PacketFilterRules; useful for grant tests that want the // compiled form. - PacketFilterMatches json.RawMessage `json:"packet_filter_matches,omitempty"` + PacketFilterMatches []filtertype.Match `json:"packet_filter_matches,omitempty"` - // Netmap is the full netmap as returned by tailscaled localapi - // /watch-ipn-bus?mask=NotifyInitialNetMap. NEVER trimmed. - // Consumers extract whatever fields they need. - Netmap json.RawMessage `json:"netmap,omitempty"` + // Netmap is the full netmap as observed by the local tailscaled. + // NEVER trimmed. Consumers extract whatever fields they need. + // + // IMPORTANT: tscap captures this by waiting for the IPN bus to + // settle on a fresh delta-triggered notification, NOT by reading + // the WatchIPNBus(NotifyInitialNetMap) initial notification. + // The initial notification carries cn.NetMap() which returns + // nb.netMap as-is — the netmap.NetworkMap whose Peers slice was + // set at full-sync time and never re-synchronized from the + // authoritative nb.peers map. tscap previously used the initial + // notification and silently captured netmaps with mostly-empty + // Peers, which corrupted every via-grant compat test against the + // stale data. See tscap/tsdaemon/capture.go:NetMap for the + // stability-wait pattern, and tailscale.com/ipn/ipnlocal/c2n.go + // :handleC2NDebugNetMap which uses netMapWithPeers() for the + // same reason. + Netmap *netmap.NetworkMap `json:"netmap,omitempty"` - // Whois is per-peer whois lookups, keyed by peer IP. Each value - // is the verbatim WhoIsResponse JSON returned by - // /localapi/v0/whois. Captured only when - // scenario.options.capture_whois is true. - Whois map[string]json.RawMessage `json:"whois,omitempty"` + // Whois is per-peer whois lookups, keyed by peer IP. Captured + // only when scenario.options.capture_whois is true. + Whois map[string]*apitype.WhoIsResponse `json:"whois,omitempty"` - // SSHRules is the SSH rules for SSH corpus. Same shape as - // tailcfg.SSHPolicy.Rules ([]tailcfg.SSHRule), kept as raw JSON. - // Populated only for SSH scenarios. - SSHRules json.RawMessage `json:"ssh_rules,omitempty"` + // SSHRules is the SSH rules slice extracted from + // netmap.SSHPolicy.Rules. Populated only for SSH scenarios. + SSHRules []*tailcfg.SSHRule `json:"ssh_rules,omitempty"` } diff --git a/hscontrol/types/testcapture/testcapture_test.go b/hscontrol/types/testcapture/testcapture_test.go index eae2f408..962f08e1 100644 --- a/hscontrol/types/testcapture/testcapture_test.go +++ b/hscontrol/types/testcapture/testcapture_test.go @@ -8,19 +8,23 @@ import ( "testing" "time" - "github.com/google/go-cmp/cmp" "github.com/juanfont/headscale/hscontrol/types/testcapture" + "tailscale.com/tailcfg" + "tailscale.com/types/netmap" ) func sampleACLCapture() *testcapture.Capture { - policy := json.RawMessage(`{"acls":[{"action":"accept","src":["*"],"dst":["*:*"]}]}`) - rules := json.RawMessage(`[ - { - "SrcIPs": ["*"], - "DstPorts": [{"IP": "*", "Ports": {"First": 0, "Last": 65535}}] - } - ]`) - netmap := json.RawMessage(`{"SelfNode":{"Name":"user1.tail.example.com."}}`) + rules := []tailcfg.FilterRule{ + { + SrcIPs: []string{"*"}, + DstPorts: []tailcfg.NetPortRange{ + {IP: "*", Ports: tailcfg.PortRangeAny}, + }, + }, + } + nm := &netmap.NetworkMap{ + SelfNode: (&tailcfg.Node{Name: "user1.tail.example.com."}).View(), + } return &testcapture.Capture{ SchemaVersion: testcapture.SchemaVersion, @@ -31,7 +35,7 @@ func sampleACLCapture() *testcapture.Capture { ToolVersion: "tscap-test-0.0.0", Tailnet: "kratail2tid@passkey", Input: testcapture.Input{ - FullPolicy: policy, + FullPolicy: `{"acls":[{"action":"accept","src":["*"],"dst":["*:*"]}]}`, APIResponseCode: 200, Tailnet: testcapture.TailnetInput{ DNS: testcapture.DNSInput{ @@ -71,19 +75,24 @@ func sampleACLCapture() *testcapture.Capture { Captures: map[string]testcapture.Node{ "user1": { PacketFilterRules: rules, - Netmap: netmap, + Netmap: nm, }, "tagged-server": { PacketFilterRules: rules, - Netmap: netmap, + Netmap: nm, }, }, } } func sampleSSHCapture() *testcapture.Capture { - policy := json.RawMessage(`{"ssh":[{"action":"accept","src":["autogroup:member"],"dst":["autogroup:self"],"users":["root"]}]}`) - sshRules := json.RawMessage(`[{"action":{"accept":true},"principals":[{"nodeIP":"100.90.199.68"}],"sshUsers":{"root":"root"}}]`) + sshRules := []*tailcfg.SSHRule{ + { + Action: &tailcfg.SSHAction{Accept: true}, + Principals: []*tailcfg.SSHPrincipal{{NodeIP: "100.90.199.68"}}, + SSHUsers: map[string]string{"root": "root"}, + }, + } return &testcapture.Capture{ SchemaVersion: testcapture.SchemaVersion, @@ -94,7 +103,7 @@ func sampleSSHCapture() *testcapture.Capture { ToolVersion: "tscap-test-0.0.0", Tailnet: "kratail2tid@passkey", Input: testcapture.Input{ - FullPolicy: policy, + FullPolicy: `{"ssh":[{"action":"accept","src":["autogroup:member"],"dst":["autogroup:self"],"users":["root"]}]}`, APIResponseCode: 200, Tailnet: testcapture.TailnetInput{ DNS: testcapture.DNSInput{ @@ -126,39 +135,27 @@ func sampleSSHCapture() *testcapture.Capture { } } -// jsonRawMessageEqual is a cmp.Comparer that treats two json.RawMessage -// values as equal if they decode to the same value. This avoids false -// negatives from indentation/whitespace differences after hujson formats -// the embedded raw blobs. -func jsonRawMessageEqual(a, b json.RawMessage) bool { - var va, vb any +// equalViaJSON compares two captures by JSON-marshaling them and +// comparing the bytes. The Capture struct embeds tailcfg view types +// with unexported pointer fields that go-cmp can't traverse, so a +// JSON round-trip is the simplest way to verify Write+Read produced +// equivalent values. +func equalViaJSON(t *testing.T, want, got *testcapture.Capture) { + t.Helper() - err := json.Unmarshal(a, &va) + wantJSON, err := json.MarshalIndent(want, "", " ") if err != nil { - return string(a) == string(b) + t.Fatalf("marshal want: %v", err) } - err = json.Unmarshal(b, &vb) + gotJSON, err := json.MarshalIndent(got, "", " ") if err != nil { - return string(a) == string(b) + t.Fatalf("marshal got: %v", err) } - // Both va and vb came from successful Unmarshal, so they're guaranteed to - // re-marshal cleanly. The error returns here are for the linter; we treat - // them as bytewise inequality if they ever fire. - ja, jaErr := json.Marshal(va) - jb, jbErr := json.Marshal(vb) - - if jaErr != nil || jbErr != nil { - return string(a) == string(b) - } - - return string(ja) == string(jb) -} - -func captureCompareOpts() []cmp.Option { - return []cmp.Option{ - cmp.Comparer(jsonRawMessageEqual), + if string(wantJSON) != string(gotJSON) { + t.Errorf("roundtrip mismatch\n--- want ---\n%s\n--- got ---\n%s", + string(wantJSON), string(gotJSON)) } } @@ -178,9 +175,7 @@ func TestWriteReadRoundtrip_ACL(t *testing.T) { t.Fatalf("Read: %v", err) } - if diff := cmp.Diff(in, out, captureCompareOpts()...); diff != "" { - t.Errorf("ACL roundtrip mismatch (-want +got):\n%s", diff) - } + equalViaJSON(t, in, out) } func TestWriteReadRoundtrip_SSH(t *testing.T) { @@ -199,9 +194,7 @@ func TestWriteReadRoundtrip_SSH(t *testing.T) { t.Fatalf("Read: %v", err) } - if diff := cmp.Diff(in, out, captureCompareOpts()...); diff != "" { - t.Errorf("SSH roundtrip mismatch (-want +got):\n%s", diff) - } + equalViaJSON(t, in, out) } func TestWrite_ProducesCommentHeader(t *testing.T) { @@ -287,7 +280,7 @@ func TestRead_HuJSONWithComments(t *testing.T) { "tool_version": "test", "tailnet": "example.com", "input": { - "full_policy": {}, + "full_policy": "{}", "api_response_code": 200, "tailnet": { "dns": { @@ -362,17 +355,24 @@ func TestCommentHeader_NoStatsForEmptyCaptures(t *testing.T) { } } -func TestCommentHeader_NullFilterRulesCountAsEmpty(t *testing.T) { +func TestCommentHeader_EmptyFilterRulesCountAsEmpty(t *testing.T) { + // Mixed: nil, empty slice, and one populated rule. Only the + // populated entry should be counted in the "filter rules" stat. c := &testcapture.Capture{ TestID: "NULLS", Captures: map[string]testcapture.Node{ - "a": {PacketFilterRules: json.RawMessage(`null`)}, - "b": {PacketFilterRules: json.RawMessage(`[]`)}, - "c": {PacketFilterRules: json.RawMessage(`[{"SrcIPs":["*"]}]`)}, + "a": {PacketFilterRules: nil}, + "b": {PacketFilterRules: []tailcfg.FilterRule{}}, + "c": {PacketFilterRules: []tailcfg.FilterRule{{SrcIPs: []string{"*"}}}}, }, } header := testcapture.CommentHeader(c) + // Only "b" and "c" are non-nil, so the corpus is detected as + // "filter rules" — and only "c" actually has rules. With the new + // typed semantics, b's empty slice still counts as "set" (not + // nil), so the denominator is 2 of 3 capture entries that have + // any filter-rules slice at all, and 1 of those is populated. if !strings.Contains(header, "Nodes with filter rules: 1 of 3") { t.Errorf("expected '1 of 3' in header; got:\n%s", header) }