From 64c398f2c2050a79ed6bf87ebd44b9a8f27d3107 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 18 May 2026 18:32:58 +0000 Subject: [PATCH] metrics, policy/v2: drop unused scaffolding + nil-error returns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two cleanups in the same package boundary: policy/v2.Policy.compileFilterRules and compileFilterRulesForNode returned (rules, error) where the error was always nil. Drop the error return and the dead error handling at 15 call sites in the compat tests. Delete code with no callers: hscontrol/metrics.go prometheusMiddleware + respWriterProm — custom HTTP timer middleware that was never registered. /metrics stays mounted via debug.go and noise.go; mapresponse_* counters keep emitting. httpDuration and httpCounter — backing metrics for the deleted middleware. hscontrol/policy/v2/filter.go resolvedAddrsToPrincipals — superseded by ipSetToPrincipals. ipSetToPrefixStringList — superseded by inline prefix formatting at the surviving callers. hscontrol/policy/v2/tailscale_{acl,grants}_data_compat_test.go setupACLCompatNodes / setupGrantsCompatNodes — the data-driven tests switched to per-scenario topology reconstruction. --- hscontrol/metrics.go | 63 ----- hscontrol/policy/v2/filter.go | 93 +++----- hscontrol/policy/v2/filter_test.go | 49 ++-- hscontrol/policy/v2/issue_3267_test.go | 3 +- .../v2/tailscale_acl_data_compat_test.go | 63 +---- .../policy/v2/tailscale_grants_compat_test.go | 221 +----------------- .../v2/tailscale_routes_data_compat_test.go | 9 +- 7 files changed, 52 insertions(+), 449 deletions(-) diff --git a/hscontrol/metrics.go b/hscontrol/metrics.go index 09cbc393..eb4fbf8c 100644 --- a/hscontrol/metrics.go +++ b/hscontrol/metrics.go @@ -1,10 +1,6 @@ package hscontrol import ( - "net/http" - "strconv" - - "github.com/gorilla/mux" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" "tailscale.com/envknob" @@ -42,63 +38,4 @@ var ( Name: "mapresponse_ended_total", Help: "total count of new mapsessions ended", }, []string{"reason"}) - httpDuration = promauto.NewHistogramVec(prometheus.HistogramOpts{ - Namespace: prometheusNamespace, - Name: "http_duration_seconds", - Help: "Duration of HTTP requests.", - }, []string{"path"}) - httpCounter = promauto.NewCounterVec(prometheus.CounterOpts{ - Namespace: prometheusNamespace, - Name: "http_requests_total", - Help: "Total number of http requests processed", - }, []string{"code", "method", "path"}, - ) ) - -// prometheusMiddleware implements mux.MiddlewareFunc. -func prometheusMiddleware(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - route := mux.CurrentRoute(r) - path, _ := route.GetPathTemplate() - - // Ignore streaming and noise sessions - // it has its own router further down. - if path == "/ts2021" || path == "/machine/map" || path == "/derp" || path == "/derp/probe" || path == "/derp/latency-check" || path == "/bootstrap-dns" { - next.ServeHTTP(w, r) - return - } - - rw := &respWriterProm{ResponseWriter: w} - - timer := prometheus.NewTimer(httpDuration.WithLabelValues(path)) - - next.ServeHTTP(rw, r) - timer.ObserveDuration() - httpCounter.WithLabelValues(strconv.Itoa(rw.status), r.Method, path).Inc() - }) -} - -type respWriterProm struct { - http.ResponseWriter - - status int - written int64 - wroteHeader bool -} - -func (r *respWriterProm) WriteHeader(code int) { - r.status = code - r.wroteHeader = true - r.ResponseWriter.WriteHeader(code) -} - -func (r *respWriterProm) Write(b []byte) (int, error) { - if !r.wroteHeader { - r.WriteHeader(http.StatusOK) - } - - n, err := r.ResponseWriter.Write(b) - r.written += int64(n) - - return n, err -} diff --git a/hscontrol/policy/v2/filter.go b/hscontrol/policy/v2/filter.go index 7a8dc628..1d2c4356 100644 --- a/hscontrol/policy/v2/filter.go +++ b/hscontrol/policy/v2/filter.go @@ -26,23 +26,23 @@ var ( // companionCaps maps certain well-known Tailscale capabilities to // their companion capability. When a grant includes one of these // capabilities, Tailscale automatically generates an additional -// FilterRule with the companion capability and a nil CapMap value. +// [tailcfg.FilterRule] with the companion capability and a nil CapMap value. var companionCaps = map[tailcfg.PeerCapability]tailcfg.PeerCapability{ tailcfg.PeerCapabilityTaildrive: tailcfg.PeerCapabilityTaildriveSharer, tailcfg.PeerCapabilityRelay: tailcfg.PeerCapabilityRelayTarget, } -// companionCapGrantRules returns additional FilterRules for any +// companionCapGrantRules returns additional [tailcfg.FilterRule]s for any // well-known capabilities that have companion caps. Companion rules // are **reversed**: SrcIPs come from the original destinations and // CapGrant Dsts come from the original sources. This allows -// ReduceFilterRules to distribute companion rules to source nodes -// (e.g. drive-sharer goes to the member nodes, not the destination). +// [policyutil.ReduceFilterRules] to distribute companion rules to source +// nodes (e.g. drive-sharer goes to the member nodes, not the destination). // Rules are ordered by the original capability name. // // dstIPStrings are the resolved destination IPs as strings (used as // companion SrcIPs). srcPrefixes are the resolved source IPs as -// netip.Prefix (used as companion CapGrant Dsts). +// [netip.Prefix] (used as companion CapGrant Dsts). func companionCapGrantRules( dstIPStrings []string, srcPrefixes []netip.Prefix, @@ -87,7 +87,7 @@ func companionCapGrantRules( // sourcesHaveWildcard returns true if any of the source aliases is // a wildcard (*). Used to determine whether approved subnet routes -// should be appended to SrcIPs. +// should be appended to [tailcfg.FilterRule.SrcIPs]. func sourcesHaveWildcard(srcs Aliases) bool { for _, src := range srcs { if _, ok := src.(Asterix); ok { @@ -99,8 +99,8 @@ func sourcesHaveWildcard(srcs Aliases) bool { } // sourcesHaveDangerAll returns true if any of the source aliases is -// autogroup:danger-all. When present, SrcIPs should be ["*"] to -// represent all IP addresses including non-Tailscale addresses. +// autogroup:danger-all. When present, [tailcfg.FilterRule.SrcIPs] should +// be ["*"] to represent all IP addresses including non-Tailscale addresses. func sourcesHaveDangerAll(srcs Aliases) bool { for _, src := range srcs { if ag, ok := src.(*AutoGroup); ok && ag.Is(AutoGroupDangerAll) { @@ -111,8 +111,8 @@ func sourcesHaveDangerAll(srcs Aliases) bool { return false } -// srcIPsWithRoutes returns the SrcIPs string slice, appending -// approved subnet routes when the sources include a wildcard. +// srcIPsWithRoutes returns the [tailcfg.FilterRule.SrcIPs] string slice, +// appending approved subnet routes when the sources include a wildcard. // When hasDangerAll is true, returns ["*"] to represent all IPs. func srcIPsWithRoutes( resolved ResolvedAddresses, @@ -132,17 +132,18 @@ func srcIPsWithRoutes( return ips } -// compileFilterRules takes a set of nodes and an ACLPolicy and generates a -// set of Tailscale compatible FilterRules used to allow traffic on clients. +// compileFilterRules takes a set of nodes and a [Policy] and generates a +// set of Tailscale compatible [tailcfg.FilterRule]s used to allow traffic +// on clients. func (pol *Policy) compileFilterRules( users types.Users, nodes views.Slice[types.NodeView], -) ([]tailcfg.FilterRule, error) { +) []tailcfg.FilterRule { if pol == nil || (pol.ACLs == nil && pol.Grants == nil) { - return tailcfg.FilterAllowAll, nil + return tailcfg.FilterAllowAll } - return globalFilterRules(pol.compileGrants(users, nodes)), nil + return globalFilterRules(pol.compileGrants(users, nodes)) } func (pol *Policy) destinationsToNetPortRange( @@ -202,15 +203,15 @@ func (pol *Policy) compileFilterRulesForNode( users types.Users, node types.NodeView, nodes views.Slice[types.NodeView], -) ([]tailcfg.FilterRule, error) { +) []tailcfg.FilterRule { if pol == nil { - return tailcfg.FilterAllowAll, nil + return tailcfg.FilterAllowAll } grants := pol.compileGrants(users, nodes) userIdx := buildUserNodeIndex(nodes) - return filterRulesForNode(grants, node, userIdx), nil + return filterRulesForNode(grants, node, userIdx) } var sshAccept = tailcfg.SSHAction{ @@ -221,11 +222,11 @@ var sshAccept = tailcfg.SSHAction{ AllowRemotePortForwarding: true, } -// checkPeriodFromRule extracts the check period duration from an SSH rule. -// Returns SSHCheckPeriodDefault if no checkPeriod is configured, +// checkPeriodFromRule extracts the check period duration from an [SSH] rule. +// Returns [SSHCheckPeriodDefault] if no checkPeriod is configured, // 0 if checkPeriod is "always", or the configured duration otherwise. -// This is used server-side by SSHCheckParams to resolve the real period -// when the client calls back; the wire format always sends 0. +// This is used server-side by [PolicyManager.SSHCheckParams] to resolve the +// real period when the client calls back; the wire format always sends 0. func checkPeriodFromRule(rule SSH) time.Duration { switch { case rule.CheckPeriod == nil: @@ -258,6 +259,7 @@ func sshCheck(baseURL string, _ time.Duration) tailcfg.SSHAction { } } +//nolint:gocyclo // SSH compilation walks per-rule branches with intertwined autogroup:self handling func (pol *Policy) compileSSHPolicy( baseURL string, users types.Users, @@ -477,24 +479,7 @@ func (pol *Policy) compileSSHPolicy( }, nil } -// resolvedAddrsToPrincipals converts ResolvedAddresses into SSH principals, one per address. -func resolvedAddrsToPrincipals(addrs ResolvedAddresses) []*tailcfg.SSHPrincipal { - if addrs == nil { - return nil - } - - var principals []*tailcfg.SSHPrincipal - - for addr := range addrs.Iter() { - principals = append(principals, &tailcfg.SSHPrincipal{ - NodeIP: addr.String(), - }) - } - - return principals -} - -// ipSetToPrincipals converts an IPSet into SSH principals, one per address. +// ipSetToPrincipals converts an [netipx.IPSet] into SSH principals, one per address. func ipSetToPrincipals(ipSet *netipx.IPSet) []*tailcfg.SSHPrincipal { if ipSet == nil { return nil @@ -622,21 +607,8 @@ func groupSourcesByUser( return userIDs, principalsByUser, taggedPrincipals } -func ipSetToPrefixStringList(ips *netipx.IPSet) []string { - var out []string - - if ips == nil { - return out - } - - for _, pref := range ips.Prefixes() { - out = append(out, pref.String()) - } - - return out -} - -// filterRuleKey generates a unique key for merging based on SrcIPs and IPProto. +// filterRuleKey generates a unique key for merging based on [tailcfg.FilterRule.SrcIPs] +// and [tailcfg.FilterRule.IPProto]. func filterRuleKey(rule tailcfg.FilterRule) string { srcKey := strings.Join(rule.SrcIPs, ",") @@ -648,10 +620,13 @@ func filterRuleKey(rule tailcfg.FilterRule) string { return srcKey + "|" + strings.Join(protoStrs, ",") } -// mergeFilterRules merges rules with identical SrcIPs and IPProto by combining -// their DstPorts. DstPorts are NOT deduplicated to match Tailscale behavior. -// CapGrant rules (which have no DstPorts) are passed through without merging -// since CapGrant and DstPorts are mutually exclusive in a FilterRule. +// mergeFilterRules merges rules with identical [tailcfg.FilterRule.SrcIPs] and +// [tailcfg.FilterRule.IPProto] by combining their [tailcfg.FilterRule.DstPorts]. +// DstPorts are NOT deduplicated to match Tailscale behavior. +// [tailcfg.CapGrant] rules (which have no [tailcfg.FilterRule.DstPorts]) are +// passed through without merging since [tailcfg.CapGrant] and +// [tailcfg.FilterRule.DstPorts] are mutually exclusive in a +// [tailcfg.FilterRule]. func mergeFilterRules(rules []tailcfg.FilterRule) []tailcfg.FilterRule { if len(rules) <= 1 { return rules diff --git a/hscontrol/policy/v2/filter_test.go b/hscontrol/policy/v2/filter_test.go index 81f78f41..b6afc8e0 100644 --- a/hscontrol/policy/v2/filter_test.go +++ b/hscontrol/policy/v2/filter_test.go @@ -368,7 +368,7 @@ func TestParsing(t *testing.T) { return } - rules, err := pol.compileFilterRules( + rules := pol.compileFilterRules( users, types.Nodes{ &types.Node{ @@ -381,12 +381,6 @@ func TestParsing(t *testing.T) { }, }.ViewSlice()) - if (err != nil) != tt.wantErr { - t.Errorf("parsing() error = %v, wantErr %v", err, tt.wantErr) - - return - } - if diff := cmp.Diff(tt.want, rules); diff != "" { t.Errorf("parsing() unexpected result (-want +got):\n%s", diff) } @@ -1340,10 +1334,7 @@ func TestCompileFilterRulesForNodeWithAutogroupSelf(t *testing.T) { // Test compilation for user1's first node node1 := nodes[0].View() - rules, err := policy2.compileFilterRulesForNode(users, node1, nodes.ViewSlice()) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } + rules := policy2.compileFilterRulesForNode(users, node1, nodes.ViewSlice()) if len(rules) != 1 { t.Fatalf("expected 1 rule, got %d", len(rules)) @@ -1501,8 +1492,7 @@ func TestTagUserMutualExclusivity(t *testing.T) { // matching the production pipeline in filterForNodeLocked. userNode := nodes[0].View() - compiled, err := pol.compileFilterRulesForNode(users, userNode, nodes.ViewSlice()) - require.NoError(t, err) + compiled := pol.compileFilterRulesForNode(users, userNode, nodes.ViewSlice()) userRules := policyutil.ReduceFilterRules(userNode, compiled) @@ -1524,8 +1514,7 @@ func TestTagUserMutualExclusivity(t *testing.T) { // Tag:database should receive the tag:server → tag:database rule after reduction. dbNode := nodes[3].View() - compiled, err = pol.compileFilterRulesForNode(users, dbNode, nodes.ViewSlice()) - require.NoError(t, err) + compiled = pol.compileFilterRulesForNode(users, dbNode, nodes.ViewSlice()) dbRules := policyutil.ReduceFilterRules(dbNode, compiled) @@ -1596,8 +1585,7 @@ func TestUserToTagCrossIdentityGrant(t *testing.T) { // user1's IP as source. taggedNode := nodes[2].View() - compiled, err := pol.compileFilterRulesForNode(users, taggedNode, nodes.ViewSlice()) - require.NoError(t, err) + compiled := pol.compileFilterRulesForNode(users, taggedNode, nodes.ViewSlice()) rules := policyutil.ReduceFilterRules(taggedNode, compiled) @@ -1735,8 +1723,7 @@ func TestAutogroupTagged(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - rules, err := policy.compileFilterRulesForNode(users, tt.sourceNode, nodes.ViewSlice()) - require.NoError(t, err) + rules := policy.compileFilterRulesForNode(users, tt.sourceNode, nodes.ViewSlice()) // Verify all expected destinations are reachable for _, expectedDest := range tt.shouldReach { @@ -1819,8 +1806,7 @@ func TestAutogroupSelfWithSpecificUserSource(t *testing.T) { // For user1's node: sources should be user1's devices node1 := nodes[0].View() - rules, err := policy.compileFilterRulesForNode(users, node1, nodes.ViewSlice()) - require.NoError(t, err) + rules := policy.compileFilterRulesForNode(users, node1, nodes.ViewSlice()) require.Len(t, rules, 1) expectedSourceIPs := []string{"100.64.0.1", "100.64.0.2"} @@ -1850,8 +1836,7 @@ func TestAutogroupSelfWithSpecificUserSource(t *testing.T) { assert.ElementsMatch(t, expectedDestIPs, actualDestIPs) node2 := nodes[2].View() - rules2, err := policy.compileFilterRulesForNode(users, node2, nodes.ViewSlice()) - require.NoError(t, err) + rules2 := policy.compileFilterRulesForNode(users, node2, nodes.ViewSlice()) assert.Empty(t, rules2, "user2's node should have no rules (user1@ devices can't match user2's self)") } @@ -1893,8 +1878,7 @@ func TestAutogroupSelfWithGroupSource(t *testing.T) { // (group:admins has user1+user2, but autogroup:self filters to same user) node1 := nodes[0].View() - rules, err := policy.compileFilterRulesForNode(users, node1, nodes.ViewSlice()) - require.NoError(t, err) + rules := policy.compileFilterRulesForNode(users, node1, nodes.ViewSlice()) require.Len(t, rules, 1) expectedSrcIPs := []string{"100.64.0.1", "100.64.0.2"} @@ -1916,8 +1900,7 @@ func TestAutogroupSelfWithGroupSource(t *testing.T) { } node3 := nodes[4].View() - rules3, err := policy.compileFilterRulesForNode(users, node3, nodes.ViewSlice()) - require.NoError(t, err) + rules3 := policy.compileFilterRulesForNode(users, node3, nodes.ViewSlice()) assert.Empty(t, rules3, "user3 should have no rules") } @@ -2366,8 +2349,7 @@ func TestAutogroupSelfWithNonExistentUserInGroup(t *testing.T) { // Test superadmin's device: should have rules with tag:common, tag:tech, tag:privileged destinations // and superadmin's IP should appear in sources (partial resolution of group:superadmin works) superadminNode := nodes[0].View() - superadminRules, err := policy.compileFilterRulesForNode(users, superadminNode, nodes.ViewSlice()) - require.NoError(t, err) + superadminRules := policy.compileFilterRulesForNode(users, superadminNode, nodes.ViewSlice()) assert.True(t, containsIP(superadminRules, "100.64.0.10"), "rules should include tag:common server") assert.True(t, containsIP(superadminRules, "100.64.0.11"), "rules should include tag:tech server") assert.True(t, containsIP(superadminRules, "100.64.0.12"), "rules should include tag:privileged server") @@ -2383,8 +2365,7 @@ func TestAutogroupSelfWithNonExistentUserInGroup(t *testing.T) { // partial result to be discarded via `continue`. With the fix, superadmin's IPs // from group:superadmin are retained alongside admin's IPs from group:admin. adminNode := nodes[1].View() - adminRules, err := policy.compileFilterRulesForNode(users, adminNode, nodes.ViewSlice()) - require.NoError(t, err) + adminRules := policy.compileFilterRulesForNode(users, adminNode, nodes.ViewSlice()) // Rule 1 sources: [group:superadmin, group:admin, group:direction] // Without fix: group:superadmin discarded -> only admin + direction IPs in sources @@ -2398,8 +2379,7 @@ func TestAutogroupSelfWithNonExistentUserInGroup(t *testing.T) { // Test direction's device: similar to admin, verifies group:direction sources work directionNode := nodes[2].View() - directionRules, err := policy.compileFilterRulesForNode(users, directionNode, nodes.ViewSlice()) - require.NoError(t, err) + directionRules := policy.compileFilterRulesForNode(users, directionNode, nodes.ViewSlice()) assert.True(t, containsIP(directionRules, "100.64.0.10"), "direction rules should include tag:common server") assert.True(t, containsSrcIP(directionRules, "100.64.0.3"), @@ -3593,8 +3573,7 @@ func TestFilterAllowAllFix(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - rules, err := tt.pol.compileFilterRules(users, nodes) - require.NoError(t, err) + rules := tt.pol.compileFilterRules(users, nodes) isFilterAllowAll := cmp.Diff(tailcfg.FilterAllowAll, rules) == "" assert.Equal(t, tt.wantFilterAllow, isFilterAllowAll, diff --git a/hscontrol/policy/v2/issue_3267_test.go b/hscontrol/policy/v2/issue_3267_test.go index bcfd05a5..007ae09e 100644 --- a/hscontrol/policy/v2/issue_3267_test.go +++ b/hscontrol/policy/v2/issue_3267_test.go @@ -93,8 +93,7 @@ func TestIssue3267ViaGrantBroaderDestination(t *testing.T) { t.Run("compileFilterRulesForNode_emits_rule_with_grant_dst", func(t *testing.T) { t.Parallel() - rules, err := pol.compileFilterRulesForNode(users, router.View(), nodes.ViewSlice()) - require.NoError(t, err) + rules := pol.compileFilterRulesForNode(users, router.View(), nodes.ViewSlice()) found := slices.ContainsFunc(rules, func(r tailcfg.FilterRule) bool { return slices.ContainsFunc(r.DstPorts, func(d tailcfg.NetPortRange) bool { diff --git a/hscontrol/policy/v2/tailscale_acl_data_compat_test.go b/hscontrol/policy/v2/tailscale_acl_data_compat_test.go index 7b0be709..ecf21294 100644 --- a/hscontrol/policy/v2/tailscale_acl_data_compat_test.go +++ b/hscontrol/policy/v2/tailscale_acl_data_compat_test.go @@ -51,60 +51,6 @@ func setupACLCompatUsers() types.Users { } } -// setupACLCompatNodes returns the 8 test nodes for ACL compatibility tests. -// Node GivenNames match the anonymized pokémon naming. -func setupACLCompatNodes(users types.Users) types.Nodes { - return types.Nodes{ - { - ID: 1, GivenName: "bulbasaur", - User: &users[0], UserID: &users[0].ID, - IPv4: ptrAddr("100.90.199.68"), IPv6: ptrAddr("fd7a:115c:a1e0::2d01:c747"), - Hostinfo: &tailcfg.Hostinfo{}, - }, - { - ID: 2, GivenName: "ivysaur", - User: &users[1], UserID: &users[1].ID, - IPv4: ptrAddr("100.110.121.96"), IPv6: ptrAddr("fd7a:115c:a1e0::1737:7960"), - Hostinfo: &tailcfg.Hostinfo{}, - }, - { - ID: 3, GivenName: "venusaur", - User: &users[2], UserID: &users[2].ID, - IPv4: ptrAddr("100.103.90.82"), IPv6: ptrAddr("fd7a:115c:a1e0::9e37:5a52"), - Hostinfo: &tailcfg.Hostinfo{}, - }, - { - ID: 4, GivenName: "beedrill", - IPv4: ptrAddr("100.108.74.26"), IPv6: ptrAddr("fd7a:115c:a1e0::b901:4a87"), - Tags: []string{"tag:server"}, Hostinfo: &tailcfg.Hostinfo{}, - }, - { - ID: 5, GivenName: "kakuna", - IPv4: ptrAddr("100.103.8.15"), IPv6: ptrAddr("fd7a:115c:a1e0::5b37:80f"), - Tags: []string{"tag:prod"}, Hostinfo: &tailcfg.Hostinfo{}, - }, - { - ID: 6, GivenName: "weedle", - IPv4: ptrAddr("100.83.200.69"), IPv6: ptrAddr("fd7a:115c:a1e0::c537:c845"), - Tags: []string{"tag:client"}, Hostinfo: &tailcfg.Hostinfo{}, - }, - { - ID: 7, GivenName: "squirtle", - IPv4: ptrAddr("100.92.142.61"), IPv6: ptrAddr("fd7a:115c:a1e0::3e37:8e3d"), - Tags: []string{"tag:router"}, - Hostinfo: &tailcfg.Hostinfo{ - RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.33.0.0/16")}, - }, - ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("10.33.0.0/16")}, - }, - { - ID: 8, GivenName: "charmander", - IPv4: ptrAddr("100.85.66.106"), IPv6: ptrAddr("fd7a:115c:a1e0::7c37:426a"), - Tags: []string{"tag:exit"}, Hostinfo: &tailcfg.Hostinfo{}, - }, - } -} - // findNodeByGivenName finds a node by its GivenName field. func findNodeByGivenName(nodes types.Nodes, name string) *types.Node { for _, n := range nodes { @@ -411,18 +357,11 @@ func testACLSuccess( } // Compile headscale filter rules for this node - compiledRules, err := pol.compileFilterRulesForNode( + compiledRules := pol.compileFilterRulesForNode( users, node.View(), nodes.ViewSlice(), ) - require.NoError( - t, - err, - "%s/%s: failed to compile filter rules", - tf.TestID, - nodeName, - ) gotRules := policyutil.ReduceFilterRules( node.View(), diff --git a/hscontrol/policy/v2/tailscale_grants_compat_test.go b/hscontrol/policy/v2/tailscale_grants_compat_test.go index e2e3e8fb..b23b14ff 100644 --- a/hscontrol/policy/v2/tailscale_grants_compat_test.go +++ b/hscontrol/policy/v2/tailscale_grants_compat_test.go @@ -45,218 +45,6 @@ func setupGrantsCompatUsers() types.Users { } } -// setupGrantsCompatNodes returns the 15 test nodes for grants compatibility tests. -// The node configuration matches the Tailscale test environment: -// - 3 user-owned nodes (bulbasaur, ivysaur, venusaur) -// - 12 tagged nodes (beedrill, kakuna, weedle, squirtle, charmander, -// pidgey, pidgeotto, rattata, raticate, spearow, fearow, blastoise) -func setupGrantsCompatNodes(users types.Users) types.Nodes { - nodeBulbasaur := &types.Node{ - ID: 1, - GivenName: "bulbasaur", - User: &users[0], - UserID: &users[0].ID, - IPv4: ptrAddr("100.90.199.68"), - IPv6: ptrAddr("fd7a:115c:a1e0::2d01:c747"), - Hostinfo: &tailcfg.Hostinfo{}, - } - - nodeIvysaur := &types.Node{ - ID: 2, - GivenName: "ivysaur", - User: &users[1], - UserID: &users[1].ID, - IPv4: ptrAddr("100.110.121.96"), - IPv6: ptrAddr("fd7a:115c:a1e0::1737:7960"), - Hostinfo: &tailcfg.Hostinfo{}, - } - - nodeVenusaur := &types.Node{ - ID: 3, - GivenName: "venusaur", - User: &users[2], - UserID: &users[2].ID, - IPv4: ptrAddr("100.103.90.82"), - IPv6: ptrAddr("fd7a:115c:a1e0::9e37:5a52"), - Hostinfo: &tailcfg.Hostinfo{}, - } - - nodeBeedrill := &types.Node{ - ID: 4, - GivenName: "beedrill", - IPv4: ptrAddr("100.108.74.26"), - IPv6: ptrAddr("fd7a:115c:a1e0::b901:4a87"), - Tags: []string{"tag:server"}, - Hostinfo: &tailcfg.Hostinfo{}, - } - - nodeKakuna := &types.Node{ - ID: 5, - GivenName: "kakuna", - IPv4: ptrAddr("100.103.8.15"), - IPv6: ptrAddr("fd7a:115c:a1e0::5b37:80f"), - Tags: []string{"tag:prod"}, - Hostinfo: &tailcfg.Hostinfo{}, - } - - nodeWeedle := &types.Node{ - ID: 6, - GivenName: "weedle", - IPv4: ptrAddr("100.83.200.69"), - IPv6: ptrAddr("fd7a:115c:a1e0::c537:c845"), - Tags: []string{"tag:client"}, - Hostinfo: &tailcfg.Hostinfo{}, - } - - nodeSquirtle := &types.Node{ - ID: 7, - GivenName: "squirtle", - IPv4: ptrAddr("100.92.142.61"), - IPv6: ptrAddr("fd7a:115c:a1e0::3e37:8e3d"), - Tags: []string{"tag:router"}, - Hostinfo: &tailcfg.Hostinfo{ - RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.33.0.0/16")}, - }, - ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("10.33.0.0/16")}, - } - - nodeCharmander := &types.Node{ - ID: 8, - GivenName: "charmander", - IPv4: ptrAddr("100.85.66.106"), - IPv6: ptrAddr("fd7a:115c:a1e0::7c37:426a"), - Tags: []string{"tag:exit"}, - Hostinfo: &tailcfg.Hostinfo{ - RoutableIPs: []netip.Prefix{ - netip.MustParsePrefix("0.0.0.0/0"), - netip.MustParsePrefix("::/0"), - }, - }, - ApprovedRoutes: []netip.Prefix{ - netip.MustParsePrefix("0.0.0.0/0"), - netip.MustParsePrefix("::/0"), - }, - } - - // --- New nodes for expanded via grant topology --- - - nodePidgey := &types.Node{ - ID: 9, - GivenName: "pidgey", - IPv4: ptrAddr("100.124.195.93"), - IPv6: ptrAddr("fd7a:115c:a1e0::7837:c35d"), - Tags: []string{"tag:exit-a"}, - Hostinfo: &tailcfg.Hostinfo{ - RoutableIPs: []netip.Prefix{ - netip.MustParsePrefix("0.0.0.0/0"), - netip.MustParsePrefix("::/0"), - }, - }, - ApprovedRoutes: []netip.Prefix{ - netip.MustParsePrefix("0.0.0.0/0"), - netip.MustParsePrefix("::/0"), - }, - } - - nodePidgeotto := &types.Node{ - ID: 10, - GivenName: "pidgeotto", - IPv4: ptrAddr("100.116.18.24"), - IPv6: ptrAddr("fd7a:115c:a1e0::ff37:1218"), - Tags: []string{"tag:exit-b"}, - Hostinfo: &tailcfg.Hostinfo{ - RoutableIPs: []netip.Prefix{ - netip.MustParsePrefix("0.0.0.0/0"), - netip.MustParsePrefix("::/0"), - }, - }, - ApprovedRoutes: []netip.Prefix{ - netip.MustParsePrefix("0.0.0.0/0"), - netip.MustParsePrefix("::/0"), - }, - } - - nodeRattata := &types.Node{ - ID: 11, - GivenName: "rattata", - IPv4: ptrAddr("100.107.162.14"), - IPv6: ptrAddr("fd7a:115c:a1e0::a237:a20e"), - Tags: []string{"tag:group-a"}, - Hostinfo: &tailcfg.Hostinfo{}, - } - - nodeRaticate := &types.Node{ - ID: 12, - GivenName: "raticate", - IPv4: ptrAddr("100.77.135.18"), - IPv6: ptrAddr("fd7a:115c:a1e0::4b37:8712"), - Tags: []string{"tag:group-b"}, - Hostinfo: &tailcfg.Hostinfo{}, - } - - nodeSpearow := &types.Node{ - ID: 13, - GivenName: "spearow", - IPv4: ptrAddr("100.109.43.124"), - IPv6: ptrAddr("fd7a:115c:a1e0::a537:2b7c"), - Tags: []string{"tag:router-a"}, - Hostinfo: &tailcfg.Hostinfo{ - RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.44.0.0/16")}, - }, - ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("10.44.0.0/16")}, - } - - nodeFearow := &types.Node{ - ID: 14, - GivenName: "fearow", - IPv4: ptrAddr("100.65.172.123"), - IPv6: ptrAddr("fd7a:115c:a1e0::5a37:ac7c"), - Tags: []string{"tag:router-b"}, - Hostinfo: &tailcfg.Hostinfo{ - RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.55.0.0/16")}, - }, - ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("10.55.0.0/16")}, - } - - nodeBlastoise := &types.Node{ - ID: 15, - GivenName: "blastoise", - IPv4: ptrAddr("100.105.127.107"), - IPv6: ptrAddr("fd7a:115c:a1e0::9537:7f6b"), - Tags: []string{"tag:exit", "tag:router"}, - Hostinfo: &tailcfg.Hostinfo{ - RoutableIPs: []netip.Prefix{ - netip.MustParsePrefix("10.33.0.0/16"), - netip.MustParsePrefix("0.0.0.0/0"), - netip.MustParsePrefix("::/0"), - }, - }, - ApprovedRoutes: []netip.Prefix{ - netip.MustParsePrefix("10.33.0.0/16"), - netip.MustParsePrefix("0.0.0.0/0"), - netip.MustParsePrefix("::/0"), - }, - } - - return types.Nodes{ - nodeBulbasaur, - nodeIvysaur, - nodeVenusaur, - nodeBeedrill, - nodeKakuna, - nodeWeedle, - nodeSquirtle, - nodeCharmander, - nodePidgey, - nodePidgeotto, - nodeRattata, - nodeRaticate, - nodeSpearow, - nodeFearow, - nodeBlastoise, - } -} - // findGrantsNode finds a node by its GivenName in the grants test environment. func findGrantsNode(nodes types.Nodes, name string) *types.Node { for _, n := range nodes { @@ -496,18 +284,11 @@ func testGrantSuccess( "golden node %s not found in test setup", nodeName) // Compile headscale filter rules for this node - gotRules, err := pol.compileFilterRulesForNode( + gotRules := pol.compileFilterRulesForNode( users, node.View(), nodes.ViewSlice(), ) - require.NoError( - t, - err, - "%s/%s: failed to compile filter rules", - tf.TestID, - nodeName, - ) gotRules = policyutil.ReduceFilterRules(node.View(), gotRules) diff --git a/hscontrol/policy/v2/tailscale_routes_data_compat_test.go b/hscontrol/policy/v2/tailscale_routes_data_compat_test.go index 660d90bc..97897794 100644 --- a/hscontrol/policy/v2/tailscale_routes_data_compat_test.go +++ b/hscontrol/policy/v2/tailscale_routes_data_compat_test.go @@ -236,18 +236,11 @@ func TestRoutesCompat(t *testing.T) { require.NotNilf(t, node, "golden node %s not found in topology", nodeName) - compiledRules, err := pol.compileFilterRulesForNode( + compiledRules := pol.compileFilterRulesForNode( users, node.View(), nodes.ViewSlice(), ) - require.NoError( - t, - err, - "%s/%s: failed to compile filter rules", - tf.TestID, - nodeName, - ) gotRules := policyutil.ReduceFilterRules( node.View(),