From 304837f183850c89d3cc4d7a64664bed3d0656d9 Mon Sep 17 00:00:00 2001 From: Shaun Crampton Date: Fri, 11 Sep 2026 19:43:34 -0700 Subject: [PATCH 1/3] Avoid string round-trip in NET IP set lookups on the policy match path The match functions hold the flow's net.IP but IPSet.Contains only takes a string, so NET sets re-parse the address on every rule that references an IP set. Add an IPAddrSet fast path (ContainsIP) implemented by ipNetSet, memoize the parsed per-flow IPs in requestCache, and route address lookups through it. The parsed-IP memoization tracks fetched-ness explicitly rather than using nil as its "not fetched yet" sentinel: nil is also a valid value (non-IP connections, e.g. pipes), and such flows would otherwise call back into the Flow implementation on every access. BenchmarkEvaluateBaselinePolicyScale/MissingSets (-cpu 1), measured before cheapest-first criterion ordering (#13408) landed: 3.95ms -> 3.25ms per evaluation, 5247 -> 499 allocs/op, 108KB -> 33KB/op. Squash of the first, fourth and fifth commits of #13267 (15f7ca048bd, 441839302cb, 9ac0d63951b), rebased onto master after #13408, #13410 and #13416. --- app-policy/checker/match.go | 37 +++++++++++++++++++--------- app-policy/checker/requestcache.go | 35 ++++++++++++++++++++++---- app-policy/policystore/ipset.go | 12 +++++++++ app-policy/policystore/ipset_test.go | 15 +++++++++++ 4 files changed, 82 insertions(+), 17 deletions(-) diff --git a/app-policy/checker/match.go b/app-policy/checker/match.go index 9bb063b6286..58e9537c86b 100644 --- a/app-policy/checker/match.go +++ b/app-policy/checker/match.go @@ -503,9 +503,9 @@ func matchSrcIPSets(r *proto.Rule, req *requestCache) bool { if len(r.SrcIpSetIds) == 0 && len(r.NotSrcIpSetIds) == 0 { return true } - srcIP := req.getSrcIPStr() - return matchIPSetsAll(r.SrcIpSetIds, req.getIPSet, srcIP) && - matchIPSetsNotAny(r.NotSrcIpSetIds, req.getIPSet, srcIP) + srcIP, srcIPStr := req.getSrcIP(), req.getSrcIPStr() + return matchIPSetsAll(r.SrcIpSetIds, req.getIPSet, srcIP, srcIPStr) && + matchIPSetsNotAny(r.NotSrcIpSetIds, req.getIPSet, srcIP, srcIPStr) } // matchDstIPPortSetIds checks if the destination IP, protocol and port is within the IP sets. It @@ -519,8 +519,9 @@ func matchDstIPPortSetIds(r *proto.Rule, req *requestCache) bool { if len(r.GetDstIpPortSetIds()) == 0 { return true } - // The values compared against are of the form "ip,protocol:port". - return matchIPSetsAll(r.GetDstIpPortSetIds(), req.getIPSet, req.getDstIPProtoPortStr()) + // The values compared against are of the form "ip,protocol:port", not a plain + // address, so there is no parsed-IP fast path. + return matchIPSetsAll(r.GetDstIpPortSetIds(), req.getIPSet, nil, req.getDstIPProtoPortStr()) } // matchDstIPSets checks if the destination IP is within the IP sets and not in the not IP sets. It @@ -535,16 +536,16 @@ func matchDstIPSets(r *proto.Rule, req *requestCache) bool { if len(r.GetDstIpSetIds()) == 0 && len(r.GetNotDstIpSetIds()) == 0 { return true } - destIP := req.getDstIPStr() - return matchIPSetsAll(r.GetDstIpSetIds(), req.getIPSet, destIP) && - matchIPSetsNotAny(r.GetNotDstIpSetIds(), req.getIPSet, destIP) + dstIP, dstIPStr := req.getDstIP(), req.getDstIPStr() + return matchIPSetsAll(r.GetDstIpSetIds(), req.getIPSet, dstIP, dstIPStr) && + matchIPSetsNotAny(r.GetNotDstIpSetIds(), req.getIPSet, dstIP, dstIPStr) } // matchIPSetsAll returns true if the address matches all of the IP set ids, false otherwise. // The value is either an IP address or an IP address protocol and port. -func matchIPSetsAll(ids []string, ipsSetFunc func(string) policystore.IPSet, value string) bool { +func matchIPSetsAll(ids []string, ipsSetFunc func(string) policystore.IPSet, ip net.IP, value string) bool { for _, id := range ids { - if s := ipsSetFunc(id); s != nil && !s.Contains(value) { + if s := ipsSetFunc(id); s != nil && !ipSetContains(s, ip, value) { return false } } @@ -554,15 +555,27 @@ func matchIPSetsAll(ids []string, ipsSetFunc func(string) policystore.IPSet, val // matchIPSetsNotAny returns true if the address does not match any of the ipset ids, false // otherwise. The value is either an IP address or an IP address protocol and port. -func matchIPSetsNotAny(ids []string, ipsSetFunc func(string) policystore.IPSet, value string) bool { +func matchIPSetsNotAny(ids []string, ipsSetFunc func(string) policystore.IPSet, ip net.IP, value string) bool { for _, id := range ids { - if s := ipsSetFunc(id); s != nil && s.Contains(value) { + if s := ipsSetFunc(id); s != nil && ipSetContains(s, ip, value) { return false } } return true } +// ipSetContains tests set membership using the parsed-IP fast path when the value is an +// IP address and the set supports it (NET sets re-parse a string value on every call). +// Callers matching a non-address value (e.g. an "ip,protocol:port" key) pass a nil ip. +func ipSetContains(s policystore.IPSet, ip net.IP, value string) bool { + if ip != nil { + if as, ok := s.(policystore.IPAddrSet); ok { + return as.ContainsIP(ip) + } + } + return s.Contains(value) +} + // matchDstPort checks if the destination port is within the port ranges and named port sets. It // also checks if the destination port is not within the not port ranges and named port sets. func matchDstPort(r *proto.Rule, req *requestCache) bool { diff --git a/app-policy/checker/requestcache.go b/app-policy/checker/requestcache.go index 40a572b14d3..0e236b23dc4 100644 --- a/app-policy/checker/requestcache.go +++ b/app-policy/checker/requestcache.go @@ -17,6 +17,7 @@ package checker import ( "fmt" "maps" + "net" "regexp" "sync" @@ -44,9 +45,14 @@ type requestCache struct { Flow store *policystore.PolicyStore - // Memoized string forms of per-flow values. The match functions need these for - // every rule that carries an IP set reference; recomputing them per rule dominates - // allocation when many policies apply to an endpoint. + // Memoized per-flow values. The match functions need these for every rule that + // carries an IP set reference; recomputing them per rule dominates allocation + // when many policies apply to an endpoint. The IPs need explicit "fetched" + // flags because nil is a valid value (non-IP connections, e.g. pipes). + srcIP net.IP + dstIP net.IP + srcIPFetched bool + dstIPFetched bool srcIPStr string dstIPStr string srcIPProtoPort string @@ -142,10 +148,29 @@ func (r *requestCache) getIdentity(side flowSide) identity { return *id } +// getSrcIP returns the source IP, memoized across the request (the Flow +// implementations construct a fresh net.IP per call). +func (r *requestCache) getSrcIP() net.IP { + if !r.srcIPFetched { + r.srcIP = r.GetSourceIP() + r.srcIPFetched = true + } + return r.srcIP +} + +// getDstIP returns the destination IP, memoized across the request. +func (r *requestCache) getDstIP() net.IP { + if !r.dstIPFetched { + r.dstIP = r.GetDestIP() + r.dstIPFetched = true + } + return r.dstIP +} + // getSrcIPStr returns the source IP in string form, memoized across the request. func (r *requestCache) getSrcIPStr() string { if r.srcIPStr == "" { - r.srcIPStr = r.GetSourceIP().String() + r.srcIPStr = r.getSrcIP().String() } return r.srcIPStr } @@ -153,7 +178,7 @@ func (r *requestCache) getSrcIPStr() string { // getDstIPStr returns the destination IP in string form, memoized across the request. func (r *requestCache) getDstIPStr() string { if r.dstIPStr == "" { - r.dstIPStr = r.GetDestIP().String() + r.dstIPStr = r.getDstIP().String() } return r.dstIPStr } diff --git a/app-policy/policystore/ipset.go b/app-policy/policystore/ipset.go index 1f434ff46a3..581fd0eeb1d 100644 --- a/app-policy/policystore/ipset.go +++ b/app-policy/policystore/ipset.go @@ -58,6 +58,14 @@ type IPSet interface { Members() []string } +// IPAddrSet is implemented by IPSet types that can test membership of an +// already-parsed IP address. Hot callers that hold a net.IP should use this +// in preference to Contains, which (for NET sets) re-parses the string on +// every call. +type IPAddrSet interface { + ContainsIP(ip net.IP) bool +} + // We'll use golang's map type under the covers here because it is simple to implement. type ipMapSet map[string]bool type ipPortMapSet map[string]bool @@ -178,6 +186,10 @@ func (m ipNetSet) Contains(addr string) bool { rlogBadAddr.Warnf("could not parse IP: %s", addr) return false } + return m.ContainsIP(ip) +} + +func (m ipNetSet) ContainsIP(ip net.IP) bool { ip4 := ip.To4() if ip4 != nil { return m.v4.containsIP(ip4, 0) diff --git a/app-policy/policystore/ipset_test.go b/app-policy/policystore/ipset_test.go index d2bfa911dea..f87c2048b03 100644 --- a/app-policy/policystore/ipset_test.go +++ b/app-policy/policystore/ipset_test.go @@ -15,6 +15,7 @@ package policystore import ( + "net" "testing" . "github.com/onsi/gomega" @@ -146,6 +147,20 @@ func TestIPNet(t *testing.T) { Expect(uut.Contains(addr192_168_20_1)).To(BeFalse()) } +func TestIPNetContainsIP(t *testing.T) { + RegisterTestingT(t) + + uut := NewIPSet(proto.IPSetUpdate_NET) + uut.AddString("192.168.8.0/24") + uut.AddString("fd5f::/64") + + addrSet := uut.(IPAddrSet) + Expect(addrSet.ContainsIP(net.ParseIP("192.168.8.1"))).To(BeTrue()) + Expect(addrSet.ContainsIP(net.ParseIP("192.168.9.1"))).To(BeFalse()) + Expect(addrSet.ContainsIP(net.ParseIP("fd5f::1"))).To(BeTrue()) + Expect(addrSet.ContainsIP(net.ParseIP("fd5e::1"))).To(BeFalse()) +} + func TestIPNetAddIP(t *testing.T) { RegisterTestingT(t) From f32d89e3ed1b9cbd5e8fe52be4c2a58df08d5154 Mon Sep 17 00:00:00 2001 From: Shaun Crampton Date: Fri, 11 Sep 2026 19:43:35 -0700 Subject: [PATCH 2/3] Compile dikastes/collector policies once instead of interpreting per flow checker.Evaluate walked every criterion of every rule for every flow, re-reading the large heap-scattered proto.Rule structs and recomputing rule-constant values each time (action enum, namespace-match inputs, selector/CIDR/protocol parses, IP set ID lookups). At the scale seen in a large production deployment (294 policies x 68 rules, ~3.7k IP sets) one evaluation cost milliseconds and re-warned 'IPSet not found' per rule per flow, saturating the felix collector. Compile each policy/profile once, when the policy store applies its update: per-rule slices of matcher closures covering only the criteria the rule uses, over pre-resolved values (IP set objects, parsed selectors and CIDRs, flattened port ranges, resolved protocol numbers, precomputed namespace matches, parsed actions). The matchers are emitted in the same cheapest-first order the interpreted match() evaluates its criteria in (#13408): protocol, ports, CIDRs, IP sets, identity, HTTP last so that a malformed request path only fails rules that otherwise apply. The store owns the compiled artifacts, so they die with the double-buffered store on resync; a reverse index (IP set ID -> referencing policies) recompiles just the affected policies when a full IPSetUpdate replaces a set's object (deltas mutate the object in place and need nothing). The compiler is injected into the store by the two wiring sites (dikastes and the felix collector) to keep the package dependency checker -> policystore one-way. The interpreted path remains the reference implementation and the fallback: policies with no compiled entry (no compiler configured, compile failure, or the CALICO_DISABLE_POLICY_COMPILATION kill switch) are interpreted per flow as before, and the compiled path defers to it at debug log level so per-criterion debug logging is unchanged. The tier walk is shared by both engines, so the staged-policy scope (#13416) applies identically: under EnforcedOnly staged policies are skipped and a staged-only tier contributes no end-of-tier action, and a policy missing from the store fails the evaluation closed whichever engine would have evaluated it. TestCompiledPolicyEquivalence asserts both engines return identical (action, index) across the criterion matrix, and the existing checkStore tests run against both engines. A missing IP set now warns once at compile time instead of once per rule per flow. Endpoints are compiled too: their tiers' policy references and their profile references become slices, index-parallel to the endpoint's own, that evaluation indexes directly instead of hashing a three-string policy ID per policy per flow. A PolicySlot indirection sits between the slice and the compiled policy so that recompiling a policy publishes through the slot and leaves compiled endpoints untouched; without it one update to an all-endpoints policy would rebuild every endpoint on the node. Endpoints are keyed by the identity of the endpoint object, which is what evaluation has to hand; a stale copy misses and falls back to the by-ID lookup. The remaining per-evaluation allocations are removed as well: the log level is tested once per Evaluate rather than per policy (boxing an int above 255 allocates); the tier default RuleID is precomputed per tier and direction and the 'no active profiles' RuleIDs built once; actionFromString and ruleActionFromStr compare with EqualFold instead of building a map and lowercasing; the requestCache is recycled through a sync.Pool (it escapes because the compiled matchers take it through a func value); Evaluate appends the trace to a caller-supplied slice, which the felix collector reuses across flows; and the matched rule's trace entry is memoized on the compiled rule on first use (building one per rule eagerly would cost megabytes for entries that are almost never reached), published atomically since evaluations read it concurrently under the store's read lock. Squash of the second, third and sixth to eleventh commits of #13267 (c855bb3fefb, e6b5b7ee874, e6710837b26, c3fb6cae62e, 16b8eb8e968, 26d50b88510, 64970c9c06a, 411f5f43c87), rebased onto master after #13408, #13410 and #13416. --- app-policy/checker/bench_egress_test.go | 4 +- app-policy/checker/bench_test.go | 63 +- app-policy/checker/check.go | 238 ++++++-- app-policy/checker/check_test.go | 54 +- app-policy/checker/compile.go | 765 ++++++++++++++++++++++++ app-policy/checker/compile_test.go | 593 ++++++++++++++++++ app-policy/checker/requestcache.go | 21 + app-policy/pkg/dikastes/dikastes.go | 4 +- app-policy/policystore/compiler.go | 311 ++++++++++ app-policy/policystore/compiler_test.go | 452 ++++++++++++++ app-policy/policystore/process.go | 30 +- app-policy/policystore/store.go | 63 +- felix/collector/collector.go | 15 +- 13 files changed, 2516 insertions(+), 97 deletions(-) create mode 100644 app-policy/checker/compile.go create mode 100644 app-policy/checker/compile_test.go create mode 100644 app-policy/policystore/compiler.go create mode 100644 app-policy/policystore/compiler_test.go diff --git a/app-policy/checker/bench_egress_test.go b/app-policy/checker/bench_egress_test.go index bb37194784f..c8888a8a4a4 100644 --- a/app-policy/checker/bench_egress_test.go +++ b/app-policy/checker/bench_egress_test.go @@ -163,7 +163,7 @@ func benchEvaluateEgressAllowList(b *testing.B, caseFor egressCaseFunc) { // Pre-flight outside the timed loop: prove the walk is the one the case intends, so that // a fixture change cannot silently turn a full walk into an early exit. - trace, err := Evaluate(EnforcedOnly, rules.RuleDirEgress, store, ep, c.flow) + trace, err := Evaluate(EnforcedOnly, rules.RuleDirEgress, store, ep, c.flow, nil) if err != nil { b.Fatalf("evaluation failed: %v", err) } @@ -178,7 +178,7 @@ func benchEvaluateEgressAllowList(b *testing.B, caseFor egressCaseFunc) { b.ReportAllocs() b.ResetTimer() for i := 0; i < b.N; i++ { - benchTraceSink, _ = Evaluate(EnforcedOnly, rules.RuleDirEgress, store, ep, c.flow) + benchTraceSink, _ = Evaluate(EnforcedOnly, rules.RuleDirEgress, store, ep, c.flow, benchTraceSink[:0]) } b.StopTimer() b.ReportMetric(float64(c.rulesWalked), "rules/op") diff --git a/app-policy/checker/bench_test.go b/app-policy/checker/bench_test.go index dcb5e510d5d..4b94e7431fe 100644 --- a/app-policy/checker/bench_test.go +++ b/app-policy/checker/bench_test.go @@ -78,6 +78,16 @@ func defaultBaselinePolicyScaleParams() baselinePolicyScaleParams { } } +// oneRulePerPolicyScaleParams is the same total rule count spread over many +// single-rule policies, a common shape where per-policy work (resolving the +// policy from the endpoint's tier) costs as much as evaluating its one rule. +func oneRulePerPolicyScaleParams() baselinePolicyScaleParams { + p := defaultBaselinePolicyScaleParams() + p.numPolicies = 2000 + p.rulesPerPolicy = 1 + return p +} + // ipSetSizeHistogram is the per-node IP set size distribution measured in the same // deployment: 3,708 sets, ~256k members in total, dominated by tiny sets with a long // tail of large ones. @@ -127,9 +137,43 @@ func BenchmarkEvaluateBaselinePolicyScale(b *testing.B) { b.Run("MatchEarly", func(b *testing.B) { benchEvaluateBaselinePolicyScale(b, defaultBaselinePolicyScaleParams(), log.WarnLevel, true) }) + // Same rule count spread over single-rule policies, so per-policy work is + // not amortized over 68 rules. + b.Run("OneRulePerPolicy", func(b *testing.B) { + benchEvaluateBaselinePolicyScale(b, oneRulePerPolicyScaleParams(), log.WarnLevel, false) + }) +} + +// BenchmarkEvaluateBaselinePolicyScaleCompiled is BenchmarkEvaluateBaselinePolicyScale +// with the store's policies compiled, as when a PolicyCompiler is configured. Missing +// IP sets warn at compile time (outside the timed loop) rather than per flow, so there +// is no MissingSetsLogsOff variant to distinguish. +func BenchmarkEvaluateBaselinePolicyScaleCompiled(b *testing.B) { + b.Run("AllSetsPresent", func(b *testing.B) { + benchEvaluateBaselinePolicyScaleCompiled(b, defaultBaselinePolicyScaleParams(), log.WarnLevel, false) + }) + b.Run("MissingSets", func(b *testing.B) { + p := defaultBaselinePolicyScaleParams() + p.numMissingIPSets = 8 + benchEvaluateBaselinePolicyScaleCompiled(b, p, log.WarnLevel, false) + }) + b.Run("MatchEarly", func(b *testing.B) { + benchEvaluateBaselinePolicyScaleCompiled(b, defaultBaselinePolicyScaleParams(), log.WarnLevel, true) + }) + b.Run("OneRulePerPolicy", func(b *testing.B) { + benchEvaluateBaselinePolicyScaleCompiled(b, oneRulePerPolicyScaleParams(), log.WarnLevel, false) + }) } func benchEvaluateBaselinePolicyScale(b *testing.B, p baselinePolicyScaleParams, level log.Level, matchEarly bool) { + benchEvaluateBaselinePolicyScaleImpl(b, p, level, matchEarly, false) +} + +func benchEvaluateBaselinePolicyScaleCompiled(b *testing.B, p baselinePolicyScaleParams, level log.Level, matchEarly bool) { + benchEvaluateBaselinePolicyScaleImpl(b, p, level, matchEarly, true) +} + +func benchEvaluateBaselinePolicyScaleImpl(b *testing.B, p baselinePolicyScaleParams, level log.Level, matchEarly, compiled bool) { logger := log.StandardLogger() counter, restoreLogging := withBenchLogging(level) defer restoreLogging() @@ -138,6 +182,17 @@ func benchEvaluateBaselinePolicyScale(b *testing.B, p baselinePolicyScaleParams, if matchEarly { addMatchEarlyPolicy(store, ep) } + if compiled { + // Compiling moves the missing-set warnings to compile time (once per + // missing reference), off the per-flow path entirely. + compileStoreForTest(store) + if logger.IsLevelEnabled(log.WarnLevel) && counter.count.Load() != int64(expectedWarns) { + b.Fatalf("expected %d 'IPSet not found' warnings at compile time, got %d", + expectedWarns, counter.count.Load()) + } + counter.count.Store(0) + expectedWarns = 0 + } flow := &MockFlow{ SourceIP: net.ParseIP(benchSourceIP), DestIP: net.ParseIP(benchDestIP), @@ -148,7 +203,7 @@ func benchEvaluateBaselinePolicyScale(b *testing.B, p baselinePolicyScaleParams, // Pre-flight outside the timed loop: prove the walk is the intended one and that // the warning count matches the analytic count, so that warnings/op is exact. - trace, _ := Evaluate(EnforcedOnly, rules.RuleDirIngress, store, ep, flow) + trace, _ := Evaluate(EnforcedOnly, rules.RuleDirIngress, store, ep, flow, nil) if matchEarly { if len(trace) != 1 || trace[0].Action != rules.RuleActionAllow || trace[0].Index != 0 { b.Fatalf("expected an immediate allow from the match-early policy, got %v", trace) @@ -167,7 +222,8 @@ func benchEvaluateBaselinePolicyScale(b *testing.B, p baselinePolicyScaleParams, b.ReportAllocs() b.ResetTimer() for i := 0; i < b.N; i++ { - benchTraceSink, _ = Evaluate(EnforcedOnly, rules.RuleDirIngress, store, ep, flow) + // Reuse the trace buffer, as the felix collector does. + benchTraceSink, _ = Evaluate(EnforcedOnly, rules.RuleDirIngress, store, ep, flow, benchTraceSink[:0]) } b.StopTimer() b.ReportMetric(float64(counter.count.Load())/float64(b.N), "warnings/op") @@ -237,7 +293,10 @@ func buildBaselinePolicyStore(p baselinePolicyScaleParams) (*policystore.PolicyS expectedWarns += refCount[id] } + // The endpoint goes into the store, as dikastes' per-pod store holds it: + // evaluation resolves an endpoint's compiled form by identity. ep := &proto.WorkloadEndpoint{Tiers: []*proto.TierInfo{tier}} + store.Endpoint = ep return store, ep, expectedWarns } diff --git a/app-policy/checker/check.go b/app-policy/checker/check.go index c4c68767277..ce797df40ec 100644 --- a/app-policy/checker/check.go +++ b/app-policy/checker/check.go @@ -119,11 +119,14 @@ const ( // decides whether staged policies take part: pass StagedAsEnforced for the pending trace, or // EnforcedOnly for the trace the dataplane enforces. // +// The trace is appended to traceBuf. Callers that evaluate repeatedly should pass a scratch slice +// (as buf[:0]) so that a trace costs no allocation; pass nil for a freshly allocated one. +// // It returns an error if the evaluation could not be completed, in which case the trace is nil and // the caller should hold on to whatever trace it already had: an empty trace would say the flow has // no policy, which is a stronger claim than "we could not work it out". -func Evaluate(scope PolicyScope, dir rules.RuleDir, store *policystore.PolicyStore, ep *proto.WorkloadEndpoint, flow Flow) ([]*calc.RuleID, error) { - s, trace := checkTiers(scope, store, ep, dir, flow) +func Evaluate(scope PolicyScope, dir rules.RuleDir, store *policystore.PolicyStore, ep *proto.WorkloadEndpoint, flow Flow, traceBuf []*calc.RuleID) ([]*calc.RuleID, error) { + s, trace := checkTiers(scope, store, ep, dir, flow, traceBuf) if s.Code == INTERNAL || s.Code == INVALID_ARGUMENT { // The evaluation stopped part way through, so the trace stops short of a verdict. Drop it // and report why it stopped. @@ -165,29 +168,57 @@ func ipToEndpointKeys(store *policystore.PolicyStore, addr ip.Addr) []proto.Work // checkStore applies the tiered policy plus any config based corrections and returns OK if the // check passes or PERMISSION_DENIED if the check fails. func checkStore(scope PolicyScope, store *policystore.PolicyStore, ep *proto.WorkloadEndpoint, dir rules.RuleDir, req Flow) (s status.Status) { - // Check using the configured policy - s, _ = checkTiers(scope, store, ep, dir, req) + // Check using the configured policy. Dikastes wants the verdict, not the + // trace, so it passes no slice to append it to. + s, _ = checkTiers(scope, store, ep, dir, req, nil) return } // checkTiers applies the tiered policy in the given store and returns OK if the check passes, or PERMISSION_DENIED if // the check fails. Note, if no policy matches, the default is PERMISSION_DENIED. It returns the trace of rules that -// were evaluated. -func checkTiers(scope PolicyScope, store *policystore.PolicyStore, ep *proto.WorkloadEndpoint, dir rules.RuleDir, flow Flow) (s status.Status, trace []*calc.RuleID) { +// were evaluated, appended to traceBuf. +// +// The walk is shared by the compiled and the interpreted engine: each policy is evaluated by its +// compiled form when it has one and interpreted otherwise (see checkTierPolicy), so the scope, +// the end-of-tier action and the fail-closed handling of a policy missing from the store apply +// identically whichever engine evaluates a given policy. +func checkTiers(scope PolicyScope, store *policystore.PolicyStore, ep *proto.WorkloadEndpoint, dir rules.RuleDir, flow Flow, traceBuf []*calc.RuleID) (s status.Status, trace []*calc.RuleID) { s = status.Status{Code: PERMISSION_DENIED} + trace = traceBuf if ep == nil { return } - request := NewRequestCache(store, flow) + // The request cache is scratch space for one evaluation. It cannot live on + // the stack — the compiled matchers take it through a func value, so escape + // analysis has to assume it leaks — so it is pooled rather than allocated + // per flow. A single shared one would race: dikastes evaluates concurrently + // under the store's read lock. + request := getRequestCache(store, flow) + defer putRequestCache(request) defer handlePanic(&s) - for _, tier := range ep.Tiers { - log.Debugf("Checking tier %s", tier.GetName()) + // The endpoint's compiled form, if it has one, holds its policies' compiled + // forms in slices parallel to its tiers, so the walk below indexes a slice + // instead of hashing each policy ID. A nil compiledEndpoint just means + // every policy is looked up by ID, as before. + ce, _ := store.CompiledEndpoints[ep].(*compiledEndpoint) + + // The walk below logs per tier, per policy and per profile, so — as in the + // match functions — it tests the level once rather than paying the + // argument boxing on every iteration with debug logging switched off. + debugEnabled := log.IsLevelEnabled(log.DebugLevel) + + for ti, tier := range ep.Tiers { + if debugEnabled { + log.Debugf("Checking tier %s", tier.GetName()) + } policies := getPoliciesByDirection(dir, tier) if len(policies) == 0 { continue } + td := ce.tierDirFor(ti, dir, len(policies)) + slots := td.policySlots() var ( ruleIndex int @@ -201,18 +232,21 @@ func checkTiers(scope PolicyScope, store *policystore.PolicyStore, ep *proto.Wor Policy: for i, pID := range policies { if scope == EnforcedOnly && model.KindIsStaged(pID.Kind) { - log.Debugf("Staged policy, not enforced, skipping (ordinal=%d, Id=%+v)", i, pID) + if debugEnabled { + log.Debugf("Staged policy, not enforced, skipping (ordinal=%d, Id=%+v)", i, pID) + } continue Policy } policiesInScope++ - policyID := ftypes.ProtoToPolicyID(pID) - policy := store.PolicyByID[policyID] - if policy == nil { + var found bool + action, ruleIndex, found = checkTierPolicy(store, slotAt(slots, i), pID, dir, request) + if !found { // The endpoint's tier names this policy but the store does not have it, so we cannot // know its verdict. We should never get here: a policy is sent before the endpoints // that reference it. Fail closed rather than apply the rest of the tier to a request // this policy may govern. + policyID := ftypes.ProtoToPolicyID(pID) rlogMissingPolicy.Errorf("Policy named in tier is missing from the store, failing evaluation (ordinal=%d, policy=%s, tier=%s)", i, policyID.ID(), tier.GetName()) s.Code = INTERNAL @@ -220,38 +254,40 @@ func checkTiers(scope PolicyScope, store *policystore.PolicyStore, ep *proto.Wor policyID.ID(), tier.GetName()) return } - - action, ruleIndex = checkPolicy(policy, dir, request) - log.Debugf("Policy checked (ordinal=%d, Id=%+v, action=%v)", i, pID, action) + if debugEnabled { + log.Debugf("Policy checked (ordinal=%d, Id=%+v, action=%v)", i, pID, action) + } switch action { case NO_MATCH: if tierDefaultActionRuleID == nil { - tierDefaultActionRuleID = calc.NewRuleID(pID.Kind, tier.GetName(), pID.Name, pID.Namespace, tierDefaultActionIndex, dir, ruleActionFromStr(tier.DefaultAction)) + tierDefaultActionRuleID = td.tierDefaultRuleID(pID, tier, dir) } continue Policy // If the Policy matches, end evaluation (skipping profiles, if any) case ALLOW: s.Code = OK - trace = append(trace, calc.NewRuleID(pID.Kind, tier.GetName(), pID.Name, pID.Namespace, ruleIndex, dir, rules.RuleActionAllow)) + trace = append(trace, policyRuleID(store, slotAt(slots, i), dir, ruleIndex, pID, tier, rules.RuleActionAllow)) return case DENY: s.Code = PERMISSION_DENIED - trace = append(trace, calc.NewRuleID(pID.Kind, tier.GetName(), pID.Name, pID.Namespace, ruleIndex, dir, rules.RuleActionDeny)) + trace = append(trace, policyRuleID(store, slotAt(slots, i), dir, ruleIndex, pID, tier, rules.RuleActionDeny)) return case PASS: - trace = append(trace, calc.NewRuleID(pID.Kind, tier.GetName(), pID.Name, pID.Namespace, ruleIndex, dir, rules.RuleActionPass)) + trace = append(trace, policyRuleID(store, slotAt(slots, i), dir, ruleIndex, pID, tier, rules.RuleActionPass)) // Pass means end evaluation of policies and proceed to next tier (or profiles), if any. break Policy case LOG: log.Debug("policy should never return LOG action") s.Code = INVALID_ARGUMENT - s.Message = fmt.Sprintf("policy %s returned a LOG action", policyID.ID()) + s.Message = fmt.Sprintf("policy %s returned a LOG action", ftypes.ProtoToPolicyID(pID).ID()) return } } // Done evaluating policies in the tier. If no policy rules have matched, apply tier's default action. if policiesInScope > 0 && action == NO_MATCH { - log.Debugf("No policy matched. Tier default action %v applies.", tier.DefaultAction) + if debugEnabled { + log.Debugf("No policy matched. Tier default action %v applies.", tier.DefaultAction) + } trace = append(trace, tierDefaultActionRuleID) // If the default action is anything beside Pass, then apply tier default deny action. // Otherwise, continue to next tier or profiles. @@ -264,21 +300,23 @@ func checkTiers(scope PolicyScope, store *policystore.PolicyStore, ep *proto.Wor // If we reach here, there were either no tiers, or a policy PASSed the request. if len(ep.ProfileIds) > 0 { + slots := ce.profileSlotsFor(len(ep.ProfileIds)) for i, name := range ep.ProfileIds { pID := proto.ProfileID{Name: name} - profile := store.ProfileByID[ftypes.ProtoToProfileID(&pID)] - action, ruleIndex := checkProfile(profile, dir, request) - log.Debugf("Profile checked (ordinal=%d, profileId=%v, action=%v)", i, &pID, action) + action, ruleIndex := checkEndpointProfile(store, slotAt(slots, i), &pID, dir, request) + if debugEnabled { + log.Debugf("Profile checked (ordinal=%d, profileId=%v, action=%v)", i, &pID, action) + } switch action { case NO_MATCH: continue case ALLOW: s.Code = OK - trace = append(trace, calc.NewRuleID(v3.KindProfile, profileStr, name, "", ruleIndex, dir, rules.RuleActionAllow)) + trace = append(trace, profileRuleID(store, slotAt(slots, i), dir, ruleIndex, &pID, rules.RuleActionAllow)) return case DENY, PASS: s.Code = PERMISSION_DENIED - trace = append(trace, calc.NewRuleID(v3.KindProfile, profileStr, name, "", ruleIndex, dir, rules.RuleActionDeny)) + trace = append(trace, profileRuleID(store, slotAt(slots, i), dir, ruleIndex, &pID, rules.RuleActionDeny)) return case LOG: log.Debug("profile should never return LOG action") @@ -289,11 +327,116 @@ func checkTiers(scope PolicyScope, store *policystore.PolicyStore, ep *proto.Wor } else { log.Debug("0 active profiles, deny request.") s.Code = PERMISSION_DENIED - trace = append(trace, calc.NewRuleID(v3.KindProfile, profileStr, profileStr, "", tierDefaultActionIndex, dir, rules.RuleActionDeny)) + trace = append(trace, noProfilesDenyRuleID(dir)) } return } +// The RuleID recording that an endpoint with no profiles denied the flow +// depends only on the direction, so both are built once. RuleIDs are read-only +// once constructed (as the calc package's own interning of them relies on). +var ( + noProfilesDenyIngress = calc.NewRuleID(v3.KindProfile, profileStr, profileStr, "", tierDefaultActionIndex, rules.RuleDirIngress, rules.RuleActionDeny) + noProfilesDenyEgress = calc.NewRuleID(v3.KindProfile, profileStr, profileStr, "", tierDefaultActionIndex, rules.RuleDirEgress, rules.RuleActionDeny) +) + +func noProfilesDenyRuleID(dir rules.RuleDir) *calc.RuleID { + if dir == rules.RuleDirEgress { + return noProfilesDenyEgress + } + return noProfilesDenyIngress +} + +// slotAt returns the precomputed slot at index i, or nil if the caller has no +// precomputed slots (slots is nil unless the endpoint has a compiled form). +func slotAt(slots []*policystore.PolicySlot, i int) *policystore.PolicySlot { + if slots == nil { + return nil + } + return slots[i] +} + +// checkTierPolicy checks one of a tier's policies against the request: its +// compiled form when it has one, otherwise the stored policy interpreted per +// flow (no compiler configured, or the policy failed to compile). slot is the +// precomputed slot for this policy, or nil when the endpoint has no compiled +// form and the slot must be looked up by ID. +// +// found is false when the store holds no such policy at all, compiled or not. +// The caller fails the evaluation closed: the policy's verdict is unknowable, +// so neither engine may guess at it. +func checkTierPolicy( + store *policystore.PolicyStore, slot *policystore.PolicySlot, pID *proto.PolicyID, + dir rules.RuleDir, req *requestCache, +) (action Action, index int, found bool) { + if cp := compiledPolicyFor(store, slot, pID); cp != nil { + action, index = cp.check(dir, req) + return action, index, true + } + policy := store.PolicyByID[ftypes.ProtoToPolicyID(pID)] + if policy == nil { + return Action(INTERNAL), unknownIndex, false + } + action, index = checkPolicy(policy, dir, req) + return action, index, true +} + +// checkEndpointProfile is checkTierPolicy for one of an endpoint's profiles. +// A profile missing from the store keeps checkProfile's nil semantics. +func checkEndpointProfile( + store *policystore.PolicyStore, slot *policystore.PolicySlot, pID *proto.ProfileID, + dir rules.RuleDir, req *requestCache, +) (Action, int) { + if cp := compiledProfileFor(store, slot, pID); cp != nil { + return cp.check(dir, req) + } + return checkProfile(store.ProfileByID[ftypes.ProtoToProfileID(pID)], dir, req) +} + +// compiledPolicyFor resolves a tier policy's compiled form: the endpoint's +// precomputed slot when it has one, otherwise a lookup by ID. +func compiledPolicyFor(store *policystore.PolicyStore, slot *policystore.PolicySlot, pID *proto.PolicyID) *compiledPolicy { + if slot == nil { + slot = store.CompiledPolicyByID[ftypes.ProtoToPolicyID(pID)] + } + cp, _ := slot.Compiled().(*compiledPolicy) + return cp +} + +func compiledProfileFor(store *policystore.PolicyStore, slot *policystore.PolicySlot, pID *proto.ProfileID) *compiledPolicy { + if slot == nil { + slot = store.CompiledProfileByID[ftypes.ProtoToProfileID(pID)] + } + cp, _ := slot.Compiled().(*compiledPolicy) + return cp +} + +// policyRuleID returns the trace entry for the rule a policy matched, taken +// from the compiled rule's memo when the policy was compiled. It resolves the +// compiled policy again rather than having the walk carry it along: a walk +// traces at most one rule, so this runs once per evaluation, where the walk +// runs once per policy. +func policyRuleID( + store *policystore.PolicyStore, slot *policystore.PolicySlot, dir rules.RuleDir, index int, + pID *proto.PolicyID, tier *proto.TierInfo, action rules.RuleAction, +) *calc.RuleID { + if cp := compiledPolicyFor(store, slot, pID); cp != nil { + return cp.ruleID(dir, index, pID.Kind, tier.GetName(), pID.Name, pID.Namespace, action) + } + return calc.NewRuleID(pID.Kind, tier.GetName(), pID.Name, pID.Namespace, index, dir, action) +} + +// profileRuleID is policyRuleID for a rule a profile matched. +func profileRuleID( + store *policystore.PolicyStore, slot *policystore.PolicySlot, dir rules.RuleDir, index int, + pID *proto.ProfileID, action rules.RuleAction, +) *calc.RuleID { + if cp := compiledProfileFor(store, slot, pID); cp != nil { + return cp.ruleID(dir, index, v3.KindProfile, profileStr, pID.Name, "", action) + } + return calc.NewRuleID(v3.KindProfile, profileStr, pID.Name, "", index, dir, action) +} + // checkPolicy checks the policy against the request and returns the action to take. func checkPolicy(policy *proto.Policy, dir rules.RuleDir, req *requestCache) (action Action, index int) { if policy == nil { @@ -336,39 +479,38 @@ func checkRules(rules []*proto.Rule, req *requestCache, policyNamespace string) } // actionFromString converts a string to an Action. It panics if the string is not a valid action. -// The string is case-insensitive. +// The string is case-insensitive. EqualFold compares without allocating, where lowercasing the +// input would allocate on every call. func actionFromString(s string) Action { // Felix currently passes us the v1 resource types where the "pass" action is called "next-tier". // Here we support both the v1 and v3 action names. - m := map[string]Action{ - "allow": ALLOW, - "deny": DENY, - "pass": PASS, - "next-tier": PASS, - "log": LOG, - } - a, found := m[strings.ToLower(s)] - if !found { - log.Errorf("Got bad action %v", s) - panic(&InvalidDataFromDataPlane{"got bad action"}) + switch { + case strings.EqualFold(s, "allow"): + return ALLOW + case strings.EqualFold(s, "deny"): + return DENY + case strings.EqualFold(s, "pass"), strings.EqualFold(s, "next-tier"): + return PASS + case strings.EqualFold(s, "log"): + return LOG } - return a + log.Errorf("Got bad action %v", s) + panic(&InvalidDataFromDataPlane{"got bad action"}) } // ruleActionFromStr converts a string to a rules.RuleAction. It panics if the string is not a // valid action. func ruleActionFromStr(s string) rules.RuleAction { - switch strings.ToLower(s) { - case "allow": + switch { + case strings.EqualFold(s, "allow"): return rules.RuleActionAllow - case "deny": + case strings.EqualFold(s, "deny"): return rules.RuleActionDeny - case "pass": + case strings.EqualFold(s, "pass"): return rules.RuleActionPass - default: - log.Errorf("Got bad action %v", s) - panic(&InvalidDataFromDataPlane{"got bad action"}) } + log.Errorf("Got bad action %v", s) + panic(&InvalidDataFromDataPlane{"got bad action"}) } // handlePanic recovers from a panic and sets the status to INVALID_ARGUMENT if the panic was due diff --git a/app-policy/checker/check_test.go b/app-policy/checker/check_test.go index 518695d858b..9bf79e37ce9 100644 --- a/app-policy/checker/check_test.go +++ b/app-policy/checker/check_test.go @@ -38,7 +38,7 @@ func TestEvaluateNoEndpoint(t *testing.T) { store := policystore.NewPolicyStore() flow := &MockFlow{} - trace, _ := Evaluate(EnforcedOnly, rules.RuleDirIngress, store, nil, flow) + trace, _ := Evaluate(EnforcedOnly, rules.RuleDirIngress, store, nil, flow, nil) Expect(trace).To(BeNil()) } @@ -49,7 +49,7 @@ func TestEvaluateEndpointNoTiersNoProfiles(t *testing.T) { ep := &proto.WorkloadEndpoint{} flow := &MockFlow{} - trace, _ := Evaluate(EnforcedOnly, rules.RuleDirIngress, store, ep, flow) + trace, _ := Evaluate(EnforcedOnly, rules.RuleDirIngress, store, ep, flow, nil) Expect(trace).To(HaveLen(1)) Expect(trace[0].Action).To(Equal(rules.RuleActionDeny)) Expect(trace[0].Direction).To(Equal(rules.RuleDirIngress)) @@ -85,7 +85,7 @@ func TestEvaluateEndpointWithMatchingPolicy(t *testing.T) { Protocol: 6, DestPort: 80, } - trace, _ := Evaluate(EnforcedOnly, rules.RuleDirIngress, store, ep, flow) + trace, _ := Evaluate(EnforcedOnly, rules.RuleDirIngress, store, ep, flow, nil) Expect(trace).To(HaveLen(1)) Expect(trace[0].Action).To(Equal(rules.RuleActionAllow)) Expect(trace[0].Direction).To(Equal(rules.RuleDirIngress)) @@ -151,7 +151,7 @@ func TestEvaluateEndpointWithNonMatchingPolicyTierDefaultAction(t *testing.T) { store.PolicyByID[types.PolicyID{Name: "policy2", Kind: v3.KindGlobalNetworkPolicy}] = &proto.Policy{Tier: "default"} flow := &MockFlow{Protocol: 6, DestPort: 443} - trace, _ := Evaluate(EnforcedOnly, rules.RuleDirIngress, store, ep, flow) + trace, _ := Evaluate(EnforcedOnly, rules.RuleDirIngress, store, ep, flow, nil) Expect(trace).To(HaveLen(tt.expLen)) for i, act := range tt.expActs { @@ -181,7 +181,7 @@ func TestEvaluateEndpointWithMatchingProfile(t *testing.T) { Protocol: 6, DestPort: 80, } - trace, _ := Evaluate(EnforcedOnly, rules.RuleDirIngress, store, ep, flow) + trace, _ := Evaluate(EnforcedOnly, rules.RuleDirIngress, store, ep, flow, nil) Expect(trace).To(HaveLen(1)) Expect(trace[0].Action).To(Equal(rules.RuleActionAllow)) Expect(trace[0].Direction).To(Equal(rules.RuleDirIngress)) @@ -238,7 +238,7 @@ func TestEvaluateEndpointWithNonMatchingProfile(t *testing.T) { SourceIP: ip_10_0_0_1, DestIP: ip_192_168_1_1, } - trace, _ := Evaluate(EnforcedOnly, rules.RuleDirEgress, store, ep, flow1) + trace, _ := Evaluate(EnforcedOnly, rules.RuleDirEgress, store, ep, flow1, nil) Expect(trace).To(HaveLen(1)) Expect(trace[0].Action).To(Equal(rules.RuleActionDeny)) Expect(trace[0].Direction).To(Equal(rules.RuleDirEgress)) @@ -255,7 +255,7 @@ func TestEvaluateEndpointWithNonMatchingProfile(t *testing.T) { SourceIP: ip_10_0_0_1, DestIP: ip_192_168_1_1, } - trace, _ = Evaluate(EnforcedOnly, rules.RuleDirEgress, store, ep, flow2) + trace, _ = Evaluate(EnforcedOnly, rules.RuleDirEgress, store, ep, flow2, nil) Expect(trace).To(HaveLen(1)) Expect(trace[0].Action).To(Equal(rules.RuleActionAllow)) Expect(trace[0].Direction).To(Equal(rules.RuleDirEgress)) @@ -272,7 +272,7 @@ func TestEvaluateEndpointWithNonMatchingProfile(t *testing.T) { SourceIP: ip_192_168_1_2, DestIP: ip_10_0_0_2, } - trace, _ = Evaluate(EnforcedOnly, rules.RuleDirEgress, store, ep, flow3) + trace, _ = Evaluate(EnforcedOnly, rules.RuleDirEgress, store, ep, flow3, nil) Expect(trace).To(HaveLen(1)) Expect(trace[0].Action).To(Equal(rules.RuleActionDeny)) Expect(trace[0].Direction).To(Equal(rules.RuleDirEgress)) @@ -425,7 +425,7 @@ func TestCheckNoIngressPolicyRulesInTier(t *testing.T) { }, }} flow := NewCheckRequestToFlowAdapter(req) - status, _ := checkTiers(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) + status, _ := checkTiersBothEngines(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) expectedStatus := rpc.Status{Code: OK} Expect(status.Code).To(Equal(expectedStatus.Code)) Expect(status.Message).To(Equal(expectedStatus.Message)) @@ -449,7 +449,7 @@ func TestCheckStoreNoEndpoint(t *testing.T) { }, }} flow := NewCheckRequestToFlowAdapter(req) - status := checkStore(EnforcedOnly, store, nil, rules.RuleDirIngress, flow) + status := checkStoreBothEngines(EnforcedOnly, store, nil, rules.RuleDirIngress, flow) Expect(status.Code).To(Equal(PERMISSION_DENIED)) } @@ -473,7 +473,7 @@ func TestCheckStoreNoTiers(t *testing.T) { }, }} flow := NewCheckRequestToFlowAdapter(req) - status := checkStore(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) + status := checkStoreBothEngines(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) Expect(status.Code).To(Equal(PERMISSION_DENIED)) } @@ -523,13 +523,13 @@ func TestCheckStorePolicyMatch(t *testing.T) { }} flow := NewCheckRequestToFlowAdapter(req) - status := checkStore(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) + status := checkStoreBothEngines(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) Expect(status.Code).To(Equal(OK)) http := req.GetAttributes().GetRequest().GetHttp() http.Method = "HEAD" - status = checkStore(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) + status = checkStoreBothEngines(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) Expect(status.Code).To(Equal(PERMISSION_DENIED)) } @@ -572,13 +572,13 @@ func TestCheckStoreProfileOnly(t *testing.T) { }} flow := NewCheckRequestToFlowAdapter(req) - status := checkStore(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) + status := checkStoreBothEngines(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) Expect(status.Code).To(Equal(OK)) http := req.GetAttributes().GetRequest().GetHttp() http.Method = "HEAD" - status = checkStore(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) + status = checkStoreBothEngines(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) Expect(status.Code).To(Equal(PERMISSION_DENIED)) } @@ -628,7 +628,7 @@ func TestCheckStorePolicyDefaultDeny(t *testing.T) { }} flow := NewCheckRequestToFlowAdapter(req) - status := checkStore(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) + status := checkStoreBothEngines(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) Expect(status.Code).To(Equal(PERMISSION_DENIED)) } @@ -689,7 +689,7 @@ func TestCheckStorePass(t *testing.T) { }} flow := NewCheckRequestToFlowAdapter(req) - status := checkStore(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) + status := checkStoreBothEngines(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) Expect(status.Code).To(Equal(OK)) } @@ -718,7 +718,7 @@ func TestCheckStoreInitFails(t *testing.T) { // The tier names policies the store does not have, so their verdict is unknowable and // evaluation fails closed rather than guessing. - status := checkStore(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) + status := checkStoreBothEngines(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) Expect(status.Code).To(Equal(INTERNAL)) } @@ -760,7 +760,7 @@ func TestCheckStoreWithInvalidData(t *testing.T) { }, }} flow := NewCheckRequestToFlowAdapter(req) - status := checkStore(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) + status := checkStoreBothEngines(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) Expect(status.Code).To(Equal(INVALID_ARGUMENT)) } @@ -845,20 +845,20 @@ func TestCheckStorePolicyMultiTierMatch(t *testing.T) { }} flow := NewCheckRequestToFlowAdapter(req) - status := checkStore(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) + status := checkStoreBothEngines(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) Expect(status.Code).To(Equal(OK)) // Change to a bad path, and check that we get PERMISSION_DENIED http := req.GetAttributes().GetRequest().GetHttp() http.Path = "/bad" - status = checkStore(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) + status = checkStoreBothEngines(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) Expect(status.Code).To(Equal(PERMISSION_DENIED)) // Change to a path that hits tier2 default Pass action, and then is allowed in tier3 http.Path = "/bar" - status = checkStore(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) + status = checkStoreBothEngines(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) Expect(status.Code).To(Equal(OK)) } @@ -924,13 +924,13 @@ func TestCheckStorePolicyMultiTierDiffTierMatch(t *testing.T) { }, }} flow := NewCheckRequestToFlowAdapter(req) - status := checkStore(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) + status := checkStoreBothEngines(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) Expect(status.Code).To(Equal(PERMISSION_DENIED)) http := req.GetAttributes().GetRequest().GetHttp() http.Method = "GET" - status = checkStore(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) + status = checkStoreBothEngines(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, flow) Expect(status.Code).To(Equal(OK)) } @@ -1221,7 +1221,7 @@ func TestEvaluateReportsFailureInsteadOfPartialTrace(t *testing.T) { ep := &proto.WorkloadEndpoint{Tiers: tierInfos(policyIDs(passes), policyIDs(notInStore))} for _, scope := range []PolicyScope{EnforcedOnly, StagedAsEnforced} { - trace, err := Evaluate(scope, rules.RuleDirIngress, store, ep, &MockFlow{Protocol: 6, DestPort: 80}) + trace, err := Evaluate(scope, rules.RuleDirIngress, store, ep, &MockFlow{Protocol: 6, DestPort: 80}, nil) Expect(err).To(MatchError(ContainSubstring("not-in-store")), "scope %v", scope) Expect(trace).To(BeNil(), "scope %v", scope) } @@ -1247,14 +1247,14 @@ func TestEvaluateRecordsStagedPolicyInPendingTraceOnly(t *testing.T) { ep := &proto.WorkloadEndpoint{Tiers: tierInfos(policyIDs(stagedDeny), policyIDs(enforcedAllow))} flow := &MockFlow{Protocol: 6, DestPort: 80} - pending, err := Evaluate(StagedAsEnforced, rules.RuleDirIngress, store, ep, flow) + pending, err := Evaluate(StagedAsEnforced, rules.RuleDirIngress, store, ep, flow, nil) Expect(err).ToNot(HaveOccurred()) Expect(pending).To(Equal([]*calc.RuleID{ calc.NewRuleID(v3.KindStagedGlobalNetworkPolicy, "tier1", "staged-deny", "", 0, rules.RuleDirIngress, rules.RuleActionDeny), })) - enforced, err := Evaluate(EnforcedOnly, rules.RuleDirIngress, store, ep, flow) + enforced, err := Evaluate(EnforcedOnly, rules.RuleDirIngress, store, ep, flow, nil) Expect(err).ToNot(HaveOccurred()) Expect(enforced).To(Equal([]*calc.RuleID{ calc.NewRuleID(v3.KindGlobalNetworkPolicy, "tier2", "allow", "", diff --git a/app-policy/checker/compile.go b/app-policy/checker/compile.go new file mode 100644 index 00000000000..9cc924e14d1 --- /dev/null +++ b/app-policy/checker/compile.go @@ -0,0 +1,765 @@ +// Copyright (c) 2026 Tigera, Inc. All rights reserved. + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package checker + +import ( + "net" + "os" + "strings" + "sync/atomic" + + log "github.com/sirupsen/logrus" + + "github.com/projectcalico/calico/app-policy/policystore" + "github.com/projectcalico/calico/felix/calc" + "github.com/projectcalico/calico/felix/proto" + "github.com/projectcalico/calico/felix/rules" + ftypes "github.com/projectcalico/calico/felix/types" + "github.com/projectcalico/calico/libcalico-go/lib/selector" +) + +// This file compiles policies into a form that is cheap to evaluate per flow. +// A proto.Rule has ~20 possible criteria but a typical rule uses one or two; +// the uncompiled match path walks every criterion for every rule for every +// flow, re-reading the large heap-scattered proto.Rule structs each time and +// recomputing rule-constant values (the action enum, the namespace-match +// inputs, selector, CIDR and protocol parses, IP set ID lookups). Compiling +// once per policy update reduces per-flow work to just the criteria each rule +// actually uses, evaluated over compact pre-resolved values. +// +// Compilation is driven by the policy store: it invokes the PolicyCompiler as +// policy/profile updates are applied, and checkTiers picks up the compiled +// form from the store's CompiledPolicyByID/CompiledProfileByID maps. A policy +// without a compiled entry is evaluated by the uncompiled path (checkPolicy / +// checkProfile), which remains the reference implementation: the two paths +// must give identical results (TestCompiledPolicyEquivalence), and the +// compiled path defers to the uncompiled one at debug log level so that every +// criterion logs as it is checked. +// +// IP set references are resolved to the IPSet objects at compile time. The +// calc graph guarantees an IP set is present in the store before any policy +// that references it arrives (see "Flush order is the dependency contract" in +// felix/design/calc-graph.md), so resolution can only fail if felix and the +// store are out of sync (in which case the miss is logged once per compile, +// not once per flow, and the missing set keeps the semantics of the +// uncompiled path). Membership deltas mutate the resolved IPSet objects in +// place; a full IPSetUpdate replaces the object, and the store recompiles +// the policies that reference it (see policystore/compiler.go). + +// disableCompilationEnvVar disables policy compilation when set to "true": +// every flow is then evaluated by the uncompiled path. Kill switch only; the +// compiled and uncompiled paths are equivalence-tested against each other. +const disableCompilationEnvVar = "CALICO_DISABLE_POLICY_COMPILATION" + +// NewPolicyCompiler returns the PolicyCompiler to plumb into +// policystore.WithPolicyCompiler, or nil (compilation disabled) if the +// CALICO_DISABLE_POLICY_COMPILATION environment variable is set to "true". +func NewPolicyCompiler() policystore.PolicyCompiler { + if strings.EqualFold(os.Getenv(disableCompilationEnvVar), "true") { + log.Warnf("Policy compilation disabled by %s", disableCompilationEnvVar) + return nil + } + return policyCompiler{} +} + +type policyCompiler struct{} + +func (policyCompiler) CompilePolicy(store *policystore.PolicyStore, policy *proto.Policy) policystore.CompiledPolicy { + if cp := compilePolicy(store, policy.InboundRules, policy.OutboundRules, policy.Namespace); cp != nil { + return cp + } + return nil +} + +func (policyCompiler) CompileProfile(store *policystore.PolicyStore, profile *proto.Profile) policystore.CompiledPolicy { + if cp := compilePolicy(store, profile.InboundRules, profile.OutboundRules, ""); cp != nil { + return cp + } + return nil +} + +func (policyCompiler) CompileEndpoint(store *policystore.PolicyStore, ep *proto.WorkloadEndpoint) policystore.CompiledEndpoint { + if ce := compileEndpoint(store, ep); ce != nil { + return ce + } + return nil +} + +// compiledEndpoint resolves an endpoint's policy and profile references to the +// store's slots once, so that evaluating a flow walks compact slices instead +// of hashing every policy ID. Each slice is index-parallel to the endpoint +// field it was built from (ep.Tiers, TierInfo.IngressPolicies/EgressPolicies, +// ep.ProfileIds), which is safe because the store treats the endpoint object +// as immutable — a changed endpoint arrives as a new object and is compiled +// afresh. +type compiledEndpoint struct { + tiers []compiledTier + profiles []*policystore.PolicySlot +} + +type compiledTier struct { + ingress, egress compiledTierDir +} + +// compiledTierDir is one direction of one tier: its policies' slots, plus the +// RuleID recording that the tier's default action applied. That RuleID is +// constant, but the walk attributes it to the first policy that did not match, +// so it is precomputed for the tier's first policy and used only for that one +// (see tierDefaultRuleID). +type compiledTierDir struct { + slots []*policystore.PolicySlot + defaultRuleID *calc.RuleID + defaultRuleIDFor *proto.PolicyID +} + +func compileEndpoint(store *policystore.PolicyStore, ep *proto.WorkloadEndpoint) (ce *compiledEndpoint) { + if ep == nil { + return nil + } + defer func() { + if r := recover(); r != nil { + if _, ok := r.(*InvalidDataFromDataPlane); !ok { + panic(r) + } + // A bad tier default action; the interpreted path panics on it per + // flow, as before. + log.Warn("Endpoint failed to compile; its policies will be resolved per flow instead") + ce = nil + } + }() + ce = &compiledEndpoint{tiers: make([]compiledTier, len(ep.Tiers))} + for i, tier := range ep.Tiers { + ce.tiers[i] = compiledTier{ + ingress: compileTierDir(store, tier, tier.GetIngressPolicies(), rules.RuleDirIngress), + egress: compileTierDir(store, tier, tier.GetEgressPolicies(), rules.RuleDirEgress), + } + } + for _, name := range ep.ProfileIds { + id := proto.ProfileID{Name: name} + ce.profiles = append(ce.profiles, store.CompiledProfileByID[ftypes.ProtoToProfileID(&id)]) + } + return ce +} + +func compileTierDir(store *policystore.PolicyStore, tier *proto.TierInfo, ids []*proto.PolicyID, dir rules.RuleDir) compiledTierDir { + if len(ids) == 0 { + return compiledTierDir{} + } + td := compiledTierDir{slots: make([]*policystore.PolicySlot, len(ids))} + for i, id := range ids { + td.slots[i] = store.CompiledPolicyByID[ftypes.ProtoToPolicyID(id)] + } + td.defaultRuleIDFor = ids[0] + td.defaultRuleID = newTierDefaultRuleID(ids[0], tier, dir) + return td +} + +// tierDirFor returns the precomputed form of one direction of the tier at +// index ti, or nil if there is none to use — no compiled endpoint, or a slot +// slice that does not line up with the endpoint being evaluated (which would +// mean the compiled form was built from a different object). The walk then +// falls back to looking each policy up by ID. +func (ce *compiledEndpoint) tierDirFor(ti int, dir rules.RuleDir, numPolicies int) *compiledTierDir { + if ce == nil || ti >= len(ce.tiers) { + return nil + } + td := &ce.tiers[ti].ingress + if dir == rules.RuleDirEgress { + td = &ce.tiers[ti].egress + } + if len(td.slots) != numPolicies { + return nil + } + return td +} + +func (td *compiledTierDir) policySlots() []*policystore.PolicySlot { + if td == nil { + return nil + } + return td.slots +} + +// tierDefaultRuleID returns the RuleID recording that the tier's default +// action applied, attributed to pID — the first policy of the tier that did +// not match. That is the tier's first policy unless earlier policies are +// missing from the store (which makes them neither match nor not-match), so +// the precomputed one is used only when it was built for this policy. +func (td *compiledTierDir) tierDefaultRuleID(pID *proto.PolicyID, tier *proto.TierInfo, dir rules.RuleDir) *calc.RuleID { + if td != nil && td.defaultRuleIDFor == pID { + return td.defaultRuleID + } + return newTierDefaultRuleID(pID, tier, dir) +} + +func newTierDefaultRuleID(pID *proto.PolicyID, tier *proto.TierInfo, dir rules.RuleDir) *calc.RuleID { + return calc.NewRuleID(pID.Kind, tier.GetName(), pID.Name, pID.Namespace, + tierDefaultActionIndex, dir, ruleActionFromStr(tier.DefaultAction)) +} + +// profileSlotsFor is policySlotsFor for the endpoint's profiles. +func (ce *compiledEndpoint) profileSlotsFor(numProfiles int) []*policystore.PolicySlot { + if ce == nil || len(ce.profiles) != numProfiles { + return nil + } + return ce.profiles +} + +// compiledPolicy is a policy (or profile) reduced to per-rule slices of +// compiled criteria. It is the concrete type behind the store's +// policystore.CompiledPolicy entries. +type compiledPolicy struct { + inbound []compiledRule + outbound []compiledRule + + // The uncompiled rules and the policy's namespace, for the debug-logging + // path, which interprets the rules so that every criterion logs as it is + // checked. + rawInbound []*proto.Rule + rawOutbound []*proto.Rule + namespace string +} + +// compiledRule is a rule reduced to its active criteria plus its pre-parsed +// action. Every matcher must return true for the rule to match a flow. +type compiledRule struct { + action Action + matchers []ruleMatcher + + // traced is this rule's trace entry, built the first time a flow matches + // the rule rather than for all of them up front: a walk only ever traces + // the one rule it stops at, and a RuleID costs 164 bytes and six + // allocations to build, so building one per rule eagerly would cost + // megabytes for entries that are almost all never used. Read by concurrent + // evaluations under the store's read lock, so it is published atomically; + // writing it twice is harmless, as every write stores an equivalent value. + traced atomic.Pointer[tracedRuleID] +} + +// tracedRuleID is a memoized trace entry, together with the identity it was +// built for. A rule's entry depends on its position, its action and the policy +// the endpoint named — and one policy can be named by many endpoints — so a +// memoized entry is reused only for the identity that produced it. +type tracedRuleID struct { + kind, tier, name, namespace string + action rules.RuleAction + id *calc.RuleID +} + +func (t *tracedRuleID) isFor(kind, tier, name, namespace string, action rules.RuleAction) bool { + return t != nil && t.action == action && + t.kind == kind && t.tier == tier && t.name == name && t.namespace == namespace +} + +// ruleID returns the trace entry for the rule at index in the given direction, +// building and memoizing it on first use. Callers pass the identity of the +// policy (or profile) the endpoint named, which is what the entry records +// alongside the rule's own position and action. +func (cp *compiledPolicy) ruleID( + dir rules.RuleDir, index int, kind, tier, name, namespace string, action rules.RuleAction, +) *calc.RuleID { + crs := cp.inbound + if dir == rules.RuleDirEgress { + crs = cp.outbound + } + if index < 0 || index >= len(crs) { + // Not a rule of this policy (the tier's default action, say). + return calc.NewRuleID(kind, tier, name, namespace, index, dir, action) + } + cr := &crs[index] + if t := cr.traced.Load(); t.isFor(kind, tier, name, namespace, action) { + return t.id + } + t := &tracedRuleID{ + kind: kind, tier: tier, name: name, namespace: namespace, action: action, + id: calc.NewRuleID(kind, tier, name, namespace, index, dir, action), + } + cr.traced.Store(t) + return t.id +} + +// ruleMatcher is a single compiled criterion of a rule. +type ruleMatcher func(req *requestCache) bool + +// compilePolicy compiles the rules of a policy or profile (profiles have no +// namespace). It returns nil if the rules cannot be compiled (e.g. a bad +// action string); the caller then keeps no compiled entry and evaluation +// falls back to the uncompiled path, which preserves that case's semantics +// (panic at evaluate time, recovered into INVALID_ARGUMENT). +func compilePolicy(store *policystore.PolicyStore, inbound, outbound []*proto.Rule, namespace string) (cp *compiledPolicy) { + defer func() { + if r := recover(); r != nil { + if _, ok := r.(*InvalidDataFromDataPlane); !ok { + panic(r) + } + log.Warn("Policy failed to compile; it will be interpreted per flow instead") + cp = nil + } + }() + return &compiledPolicy{ + inbound: compileRules(store, inbound, namespace), + outbound: compileRules(store, outbound, namespace), + rawInbound: inbound, + rawOutbound: outbound, + namespace: namespace, + } +} + +// check evaluates the compiled policy against the flow, mirroring checkRules: +// first matching rule wins (LOG rules match but evaluation continues), no +// matching rule means NO_MATCH. +func (cp *compiledPolicy) check(dir rules.RuleDir, req *requestCache) (Action, int) { + if log.IsLevelEnabled(log.DebugLevel) { + // Use the uncompiled path so each criterion logs as it is checked. + if dir == rules.RuleDirEgress { + return checkRules(cp.rawOutbound, req, cp.namespace) + } + return checkRules(cp.rawInbound, req, cp.namespace) + } + + // matchL4Protocol rejects an out-of-range protocol value no matter what + // the rule says, but compiled rules only include a protocol matcher when + // the rule constrains the protocol. Replicate the validity check once per + // policy; requestCache.GetProtocol has already warned about the value, + // once per request. + if !validL4Protocol(req.GetProtocol()) { + return NO_MATCH, tierDefaultActionIndex + } + + crs := cp.inbound + if dir == rules.RuleDirEgress { + crs = cp.outbound + } + for i := range crs { + cr := &crs[i] + if cr.matches(req) { + if cr.action != LOG { + return cr.action, i + } + } + } + return NO_MATCH, tierDefaultActionIndex +} + +func (cr *compiledRule) matches(req *requestCache) bool { + for _, m := range cr.matchers { + if !m(req) { + return false + } + } + return true +} + +// compileRules compiles each rule's active criteria, building all the rules' +// matcher slices as views into one shared backing array (one policy-sized +// allocation instead of one per rule). +func compileRules(store *policystore.PolicyStore, rs []*proto.Rule, policyNamespace string) []compiledRule { + out := make([]compiledRule, len(rs)) + var all []ruleMatcher + starts := make([]int, len(rs)+1) + for i, r := range rs { + starts[i] = len(all) + all = appendRuleMatchers(all, store, r, policyNamespace) + } + starts[len(rs)] = len(all) + for i, r := range rs { + // Field by field: a compiledRule carries an atomic, so it must not be + // copied. + out[i].action = actionFromString(r.Action) + out[i].matchers = all[starts[i]:starts[i+1]:starts[i+1]] + } + return out +} + +// appendRuleMatchers appends a matcher per criterion the rule actually uses, +// in the same cheapest-first order the uncompiled path (match) checks them: +// protocol, ports, CIDRs, IP sets, identity, HTTP. A rule's criteria are ANDed +// pure predicates, so the verdict is the same in any order; the order matters +// because most rules are rejected by exactly one criterion and the walk stops +// at it, and because the HTTP matcher panics on a malformed request path, so +// last position confines that to requests a rule otherwise fully matches, +// exactly as the uncompiled path does. A criterion whose fields are empty +// always matches, so it is omitted. +func appendRuleMatchers(ms []ruleMatcher, store *policystore.PolicyStore, r *proto.Rule, policyNamespace string) []ruleMatcher { + add := func(m ruleMatcher) { + if m != nil { + ms = append(ms, m) + } + } + + // L4 header: integer comparisons against the flow. + add(compileProtocolMatcher(r.GetProtocol(), r.GetNotProtocol())) + add(compilePortsMatcher(r.GetSrcPorts(), r.GetNotSrcPorts(), + resolveIPSets(store, r.GetSrcNamedPortIpSetIds()), resolveIPSets(store, r.GetNotSrcNamedPortIpSetIds()), + (*requestCache).GetSourcePort, (*requestCache).getSrcIPProtoPortStr)) + add(compilePortsMatcher(r.GetDstPorts(), r.GetNotDstPorts(), + resolveIPSets(store, r.GetDstNamedPortIpSetIds()), resolveIPSets(store, r.GetNotDstNamedPortIpSetIds()), + (*requestCache).GetDestPort, (*requestCache).getDstIPProtoPortStr)) + + // Addresses: CIDRs (parsed once, here), then IP set lookups. + add(compileNetsMatcher(r.GetSrcNet(), r.GetNotSrcNet(), (*requestCache).getSrcIP)) + add(compileNetsMatcher(r.GetDstNet(), r.GetNotDstNet(), (*requestCache).getDstIP)) + add(compileSrcIPSetsMatcher(store, r)) + add(compileDstIPSetsMatcher(store, r)) + add(compileDstIPPortSetsMatcher(store, r)) + + // Identity: service account then namespace, source side then destination + // (matchSrcIdentity, matchDstIdentity). The namespace match depends only on + // the rule and the policy's namespace, so it is computed once here rather + // than per flow. + srcSA := r.GetSrcServiceAccountMatch() + add(compileServiceAccountsMatcher(srcSA, (*requestCache).getSrcPeer)) + srcNSMatch := computeNamespaceMatch( + policyNamespace, + r.GetOriginalSrcNamespaceSelector(), + r.GetOriginalSrcSelector(), + r.GetOriginalNotSrcSelector(), + srcSA) + add(compileNamespaceMatcher(srcNSMatch, (*requestCache).getSrcNamespace)) + dstSA := r.GetDstServiceAccountMatch() + add(compileServiceAccountsMatcher(dstSA, (*requestCache).getDstPeer)) + dstNSMatch := computeNamespaceMatch( + policyNamespace, + r.GetOriginalDstNamespaceSelector(), + r.GetOriginalDstSelector(), + r.GetOriginalNotDstSelector(), + dstSA) + add(compileNamespaceMatcher(dstNSMatch, (*requestCache).getDstNamespace)) + + // HTTP last; see above. + if r.GetHttpMatch() != nil { + add(func(req *requestCache) bool { return matchRequest(r, req) }) + } + + return ms +} + +// compileServiceAccountsMatcher mirrors matchServiceAccounts, with the +// selector parsed at compile time instead of per flow. A nil peer or an empty +// peer name (plain text traffic carries no service account) matches; a +// selector that fails to parse can never match. +func compileServiceAccountsMatcher(saMatch *proto.ServiceAccountMatch, getPeer func(*requestCache) *peer) ruleMatcher { + names := saMatch.GetNames() + if len(names) == 0 && saMatch.GetSelector() == "" { + return nil + } + sel := parseSelector(saMatch.GetSelector()) + return func(req *requestCache) bool { + p := getPeer(req) + if p == nil || p.Name == "" { + return true + } + return matchName(names, p.Name) && sel != nil && sel.Evaluate(p.Labels) + } +} + +// compileNamespaceMatcher mirrors matchNamespace, with the selector parsed at +// compile time instead of per flow. A nil namespace or an empty namespace +// name (plain text traffic carries no namespace) matches; a selector that +// fails to parse can never match. +func compileNamespaceMatcher(nsMatch namespaceMatch, getNamespace func(*requestCache) *namespace) ruleMatcher { + if len(nsMatch.Names) == 0 && nsMatch.Selector == "" { + return nil + } + sel := parseSelector(nsMatch.Selector) + return func(req *requestCache) bool { + ns := getNamespace(req) + if ns == nil || ns.Name == "" { + return true + } + return matchName(nsMatch.Names, ns.Name) && sel != nil && sel.Evaluate(ns.Labels) + } +} + +// parseSelector parses a label selector at compile time, returning nil (can +// never match, as in matchLabels) if it does not parse. Note the empty +// selector parses successfully and matches everything, as in matchLabels. +func parseSelector(selectorStr string) *selector.Selector { + sel, err := selector.Parse(selectorStr) + if err != nil { + log.Warnf("Could not parse label selector %v, %v", selectorStr, err) + return nil + } + return sel +} + +// resolvedIPSet is an IP set resolved to its object at compile time, with the +// parsed-IP fast path pre-asserted. A missing set resolves to a zero +// resolvedIPSet, keeping the uncompiled path's missing-set semantics: skipped +// by the all/not-any matchers. +type resolvedIPSet struct { + set policystore.IPSet + addrSet policystore.IPAddrSet // non-nil if the set supports parsed-IP lookup +} + +func (s resolvedIPSet) containsSrcIP(req *requestCache) bool { + if s.addrSet != nil { + // The flow's IP can be nil (non-IP connections, e.g. pipes); as in + // ipSetContains, only a non-nil IP may take the parsed-IP fast path. + if ip := req.getSrcIP(); ip != nil { + return s.addrSet.ContainsIP(ip) + } + } + return s.set.Contains(req.getSrcIPStr()) +} + +func (s resolvedIPSet) containsDstIP(req *requestCache) bool { + if s.addrSet != nil { + if ip := req.getDstIP(); ip != nil { + return s.addrSet.ContainsIP(ip) + } + } + return s.set.Contains(req.getDstIPStr()) +} + +// resolveIPSets looks up IP set IDs in the store. The calc graph sends IP +// sets before the policies that reference them, so a miss means the store is +// out of sync with felix; it is logged once here rather than once per flow. +func resolveIPSets(store *policystore.PolicyStore, ids []string) []resolvedIPSet { + if len(ids) == 0 { + return nil + } + out := make([]resolvedIPSet, len(ids)) + for i, id := range ids { + s, ok := store.IPSetByID[id] + if !ok { + log.WithField("ipset", id).Warn("IPSet not found") + continue + } + addrSet, _ := s.(policystore.IPAddrSet) + out[i] = resolvedIPSet{set: s, addrSet: addrSet} + } + return out +} + +// compileSrcIPSetsMatcher mirrors matchSrcIPSets: the source IP must be in +// all of the SrcIpSetIds sets and none of the NotSrcIpSetIds sets. +func compileSrcIPSetsMatcher(store *policystore.PolicyStore, r *proto.Rule) ruleMatcher { + if len(r.GetSrcIpSetIds()) == 0 && len(r.GetNotSrcIpSetIds()) == 0 { + return nil + } + sets := resolveIPSets(store, r.GetSrcIpSetIds()) + notSets := resolveIPSets(store, r.GetNotSrcIpSetIds()) + return func(req *requestCache) bool { + for _, s := range sets { + if s.set != nil && !s.containsSrcIP(req) { + return false + } + } + for _, s := range notSets { + if s.set != nil && s.containsSrcIP(req) { + return false + } + } + return true + } +} + +// compileDstIPSetsMatcher mirrors matchDstIPSets: the destination IP must be +// in all of the DstIpSetIds sets and none of the NotDstIpSetIds sets. +func compileDstIPSetsMatcher(store *policystore.PolicyStore, r *proto.Rule) ruleMatcher { + if len(r.GetDstIpSetIds()) == 0 && len(r.GetNotDstIpSetIds()) == 0 { + return nil + } + sets := resolveIPSets(store, r.GetDstIpSetIds()) + notSets := resolveIPSets(store, r.GetNotDstIpSetIds()) + return func(req *requestCache) bool { + for _, s := range sets { + if s.set != nil && !s.containsDstIP(req) { + return false + } + } + for _, s := range notSets { + if s.set != nil && s.containsDstIP(req) { + return false + } + } + return true + } +} + +// compileDstIPPortSetsMatcher mirrors matchDstIPPortSetIds: the flow's +// ",:" key must be in all of the DstIpPortSetIds sets. +func compileDstIPPortSetsMatcher(store *policystore.PolicyStore, r *proto.Rule) ruleMatcher { + if len(r.GetDstIpPortSetIds()) == 0 { + return nil + } + sets := resolveIPSets(store, r.GetDstIpPortSetIds()) + return func(req *requestCache) bool { + for _, s := range sets { + if s.set != nil && !s.set.Contains(req.getDstIPProtoPortStr()) { + return false + } + } + return true + } +} + +// portRange is a compact copy of proto.PortRange, so that port matching walks +// a contiguous slice instead of dereferencing per-range proto structs. +type portRange struct{ first, last int32 } + +func flattenPortRanges(ranges []*proto.PortRange) []portRange { + if len(ranges) == 0 { + return nil + } + out := make([]portRange, len(ranges)) + for i, r := range ranges { + out[i] = portRange{first: r.GetFirst(), last: r.GetLast()} + } + return out +} + +// compilePortsMatcher mirrors matchSrcPort/matchDstPort: the port must be in +// one of the ranges or named port sets (if any are specified), and not in any +// of the not-ranges or not-named-port sets. A named port set holds +// ",:" members rather than bare port numbers (see +// matchPort); namedPortKey supplies that key for the leg being tested, and is +// only consulted once the ranges have failed to decide the criterion. +func compilePortsMatcher( + ports, notPorts []*proto.PortRange, namedSets, notNamedSets []resolvedIPSet, + getPort func(*requestCache) int, namedPortKey func(*requestCache) string, +) ruleMatcher { + if len(ports) == 0 && len(notPorts) == 0 && len(namedSets) == 0 && len(notNamedSets) == 0 { + return nil + } + ranges := flattenPortRanges(ports) + notRanges := flattenPortRanges(notPorts) + return func(req *requestCache) bool { + port := int32(getPort(req)) + if len(ranges) > 0 || len(namedSets) > 0 { + if !portMatches(port, ranges, namedSets, req, namedPortKey) { + return false + } + } + if len(notRanges) > 0 || len(notNamedSets) > 0 { + if portMatches(port, notRanges, notNamedSets, req, namedPortKey) { + return false + } + } + return true + } +} + +func portMatches(port int32, ranges []portRange, namedSets []resolvedIPSet, req *requestCache, namedPortKey func(*requestCache) string) bool { + for _, r := range ranges { + if r.first <= port && port <= r.last { + return true + } + } + if len(namedSets) > 0 { + key := namedPortKey(req) + for _, s := range namedSets { + if s.set != nil && s.set.Contains(key) { + return true + } + } + } + return false +} + +// compileNetsMatcher mirrors matchSrcNet/matchDstNet with the CIDRs parsed at +// compile time: the IP must be in one of the nets (if any are specified) and +// not in any of the not-nets. Malformed CIDRs (which validation should have +// weeded out long before here) are logged once here rather than per flow, and +// keep the uncompiled path's in-order semantics: matchNet checks CIDRs in +// order and fails when it reaches a malformed one, so nets before it can +// still match; matchNotNet can never return true once a malformed not-net is +// present, which makes the whole criterion false. +func compileNetsMatcher(nets, notNets []string, getIP func(*requestCache) net.IP) ruleMatcher { + if len(nets) == 0 && len(notNets) == 0 { + return nil + } + if _, notNetsOK := parseCIDRs(notNets); !notNetsOK { + return func(req *requestCache) bool { return false } + } + parsed, _ := parseCIDRs(nets) + notParsed, _ := parseCIDRs(notNets) + hasNets := len(nets) > 0 + return func(req *requestCache) bool { + ip := getIP(req) + if hasNets { + any := false + for _, n := range parsed { + if n.Contains(ip) { + any = true + break + } + } + if !any { + return false + } + } + for _, n := range notParsed { + if n.Contains(ip) { + return false + } + } + return true + } +} + +// parseCIDRs parses up to the first malformed CIDR, returning the parsed +// prefixes and whether the whole list was well-formed. +func parseCIDRs(nets []string) ([]*net.IPNet, bool) { + out := make([]*net.IPNet, 0, len(nets)) + for _, n := range nets { + _, ipn, err := net.ParseCIDR(n) + if err != nil { + log.WithField("cidr", n).Warn("unable to parse CIDR") + return out, false + } + out = append(out, ipn) + } + return out, true +} + +// compileProtocolMatcher mirrors matchL4Protocol with the rule's protocol +// name/number resolved at compile time. (The flow protocol's range check +// lives in compiledPolicy.check.) +func compileProtocolMatcher(p, notP *proto.Protocol) ruleMatcher { + if p == nil && notP == nil { + return nil + } + resolve := func(p *proto.Protocol) (int32, bool) { + if name := p.GetName(); name != "" { + n, ok := stringToProto[strings.ToLower(name)] + // Narrowing to the proto's int32: every value in the map fits. + return int32(n), ok + } + return p.GetNumber(), true + } + if p != nil { + n, ok := resolve(p) + if !ok { + // Unknown protocol name: the rule can never match. + return func(req *requestCache) bool { return false } + } + if notP == nil { + return func(req *requestCache) bool { return int32(req.GetProtocol()) == n } + } + notN, notOK := resolve(notP) + return func(req *requestCache) bool { + proto := int32(req.GetProtocol()) + return proto == n && (!notOK || proto != notN) + } + } + notN, notOK := resolve(notP) + if !notOK { + // Unknown not-protocol name never excludes anything. + return nil + } + return func(req *requestCache) bool { return int32(req.GetProtocol()) != notN } +} diff --git a/app-policy/checker/compile_test.go b/app-policy/checker/compile_test.go new file mode 100644 index 00000000000..dda34155b35 --- /dev/null +++ b/app-policy/checker/compile_test.go @@ -0,0 +1,593 @@ +// Copyright (c) 2026 Tigera, Inc. All rights reserved. + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package checker + +import ( + "fmt" + "net" + "reflect" + "runtime" + "sync" + "testing" + + . "github.com/onsi/gomega" + v3 "github.com/projectcalico/api/pkg/apis/projectcalico/v3" + "google.golang.org/genproto/googleapis/rpc/status" + googleproto "google.golang.org/protobuf/proto" + + "github.com/projectcalico/calico/app-policy/policystore" + "github.com/projectcalico/calico/felix/calc" + "github.com/projectcalico/calico/felix/proto" + "github.com/projectcalico/calico/felix/rules" + "github.com/projectcalico/calico/felix/types" +) + +// compileStoreForTest compiles every policy, profile and endpoint already +// present in the store, as the store itself does when it applies updates with +// a compiler configured. Tests build stores by direct map assignment, which +// bypasses the store's compile-on-update hooks. Endpoints must be compiled +// last, so their slot slices see the compiled policies. +func compileStoreForTest(store *policystore.PolicyStore) { + store.SetPolicyCompiler(policyCompiler{}) +} + +func clearCompiledForTest(store *policystore.PolicyStore) { + store.SetPolicyCompiler(nil) + clear(store.CompiledPolicyByID) + clear(store.CompiledProfileByID) + clear(store.CompiledEndpoints) +} + +// checkStoreBothEngines runs checkStore with the store's policies +// interpreted, then again with them all compiled, asserts the two engines +// agree, and returns the (interpreted) result. It is a drop-in replacement +// for checkStore in tests, making every checkStore-level test case an +// equivalence test between the two engines. +func checkStoreBothEngines(scope PolicyScope, store *policystore.PolicyStore, ep *proto.WorkloadEndpoint, dir rules.RuleDir, req Flow) *status.Status { + s, _ := checkTiersBothEngines(scope, store, ep, dir, req) + return s +} + +// checkTiersBothEngines is checkStoreBothEngines for callers that also want +// the rule trace. +func checkTiersBothEngines(scope PolicyScope, store *policystore.PolicyStore, ep *proto.WorkloadEndpoint, dir rules.RuleDir, req Flow) (*status.Status, []*calc.RuleID) { + s, trace := checkTiers(scope, store, ep, dir, req, nil) + compileStoreForTest(store) + compiledS, compiledTrace := checkTiers(scope, store, ep, dir, req, nil) + clearCompiledForTest(store) + ExpectWithOffset(2, googleproto.Equal(&compiledS, &s)).To(BeTrue(), + "compiled and interpreted engines returned different statuses: %v vs %v", &compiledS, &s) + ExpectWithOffset(2, compiledTrace).To(Equal(trace), "compiled and interpreted engines returned different traces") + return &s, trace +} + +// TestCompiledPolicyEquivalence verifies that the compiled evaluation path returns the same +// (action, index) as the uncompiled checkRules path, for every criterion type a rule can carry, +// with flows chosen to exercise both the matching and non-matching side of each criterion. +func TestCompiledPolicyEquivalence(t *testing.T) { + RegisterTestingT(t) + + store := policystore.NewPolicyStore() + addIPSet(store, "ipset-hit", "10.0.0.1") + netSet := policystore.NewIPSet(proto.IPSetUpdate_NET) + netSet.AddString("10.0.0.0/24") + store.IPSetByID["netset-hit"] = netSet + // A named port set holds ",:" members: the first flow's source leg. + namedPortSet := policystore.NewIPSet(proto.IPSetUpdate_IP_AND_PORT) + namedPortSet.AddString("10.0.0.1,tcp:1234") + store.IPSetByID["portset-src"] = namedPortSet + ipPortSet := policystore.NewIPSet(proto.IPSetUpdate_IP_AND_PORT) + ipPortSet.AddString("192.168.1.1,tcp:80") + store.IPSetByID["ipportset-hit"] = ipPortSet + // "ipset-missing" is deliberately absent from the store. + + spiffe := func(ns, sa string) *string { + s := fmt.Sprintf("spiffe://cluster.local/ns/%s/sa/%s", ns, sa) + return &s + } + httpGet := "GET" + httpPath := "/foo/bar" + + flows := []Flow{ + // Plain L4 flow, matching the IP sets above on the source side. + &MockFlow{ + SourceIP: net.ParseIP("10.0.0.1"), + DestIP: net.ParseIP("192.168.1.1"), + SourcePort: 1234, + DestPort: 80, + Protocol: 6, + }, + // Reversed flow: matches the IP sets on the destination side, UDP. + &MockFlow{ + SourceIP: net.ParseIP("192.168.1.1"), + DestIP: net.ParseIP("10.0.0.1"), + SourcePort: 80, + DestPort: 1234, + Protocol: 17, + }, + // Flow with peer identities, labels and HTTP data. + &MockFlow{ + SourceIP: net.ParseIP("10.0.0.1"), + DestIP: net.ParseIP("10.0.0.2"), + SourcePort: 1234, + DestPort: 80, + Protocol: 6, + SourcePrincipal: spiffe("ns-src", "sa-src"), + DestPrincipal: spiffe("ns-dst", "sa-dst"), + SourceLabels: map[string]string{"app": "client"}, + HttpMethod: &httpGet, + HttpPath: &httpPath, + }, + // Out-of-range protocol: the uncompiled path fails every rule on it. + &MockFlow{ + SourceIP: net.ParseIP("10.0.0.1"), + DestIP: net.ParseIP("192.168.1.1"), + SourcePort: 1234, + DestPort: 80, + Protocol: 0, + }, + // Nil IPs, as seen for non-IP connections (e.g. pipes): the parsed-IP + // fast path must not be taken (ContainsIP would panic on a nil IP). + &MockFlow{ + SourcePort: 1234, + DestPort: 80, + Protocol: 6, + }, + } + + ruleVariants := []*proto.Rule{ + {}, // Empty rule: matches everything. + {SrcIpSetIds: []string{"ipset-hit"}}, + {SrcIpSetIds: []string{"netset-hit"}}, + {SrcIpSetIds: []string{"ipset-missing"}}, + {NotSrcIpSetIds: []string{"netset-hit"}}, + {DstIpSetIds: []string{"netset-hit"}}, + {DstIpSetIds: []string{"ipset-missing"}}, + {DstIpSetIds: []string{"netset-hit", "ipset-missing"}}, + {NotDstIpSetIds: []string{"netset-hit"}}, + {DstIpPortSetIds: []string{"ipportset-hit"}}, + {DstIpPortSetIds: []string{"ipset-missing"}}, + {SrcPorts: []*proto.PortRange{{First: 1234, Last: 1234}}}, + {SrcPorts: []*proto.PortRange{{First: 65001, Last: 65001}}}, + {NotSrcPorts: []*proto.PortRange{{First: 1234, Last: 1234}}}, + {DstPorts: []*proto.PortRange{{First: 80, Last: 80}}}, + {NotDstPorts: []*proto.PortRange{{First: 80, Last: 80}}}, + {SrcNet: []string{"10.0.0.0/8"}}, + {SrcNet: []string{"172.16.0.0/12"}}, + {NotSrcNet: []string{"10.0.0.0/8"}}, + {DstNet: []string{"192.168.0.0/16"}}, + {NotDstNet: []string{"192.168.0.0/16"}}, + {SrcServiceAccountMatch: &proto.ServiceAccountMatch{Names: []string{"sa-src"}}}, + {SrcServiceAccountMatch: &proto.ServiceAccountMatch{Names: []string{"other-sa"}}}, + {SrcServiceAccountMatch: &proto.ServiceAccountMatch{Selector: "app == 'client'"}}, + {SrcServiceAccountMatch: &proto.ServiceAccountMatch{Selector: "&& not a selector"}}, + {DstServiceAccountMatch: &proto.ServiceAccountMatch{Names: []string{"sa-dst"}}}, + {OriginalSrcNamespaceSelector: "name == 'ns-src'"}, + {OriginalSrcNamespaceSelector: "&& not a selector"}, + {OriginalSrcSelector: "app == 'client'"}, + {OriginalDstNamespaceSelector: "name == 'ns-dst'"}, + {HttpMatch: &proto.HTTPMatch{Methods: []string{"GET"}}}, + {HttpMatch: &proto.HTTPMatch{Methods: []string{"POST"}}}, + {HttpMatch: &proto.HTTPMatch{Paths: []*proto.HTTPMatch_PathMatch{ + {PathMatch: &proto.HTTPMatch_PathMatch_Prefix{Prefix: "/foo"}}, + }}}, + {HttpMatch: &proto.HTTPMatch{Paths: []*proto.HTTPMatch_PathMatch{ + {PathMatch: &proto.HTTPMatch_PathMatch_Exact{Exact: "/nope"}}, + }}}, + {Protocol: &proto.Protocol{NumberOrName: &proto.Protocol_Name{Name: "tcp"}}}, + {Protocol: &proto.Protocol{NumberOrName: &proto.Protocol_Number{Number: 17}}}, + {NotProtocol: &proto.Protocol{NumberOrName: &proto.Protocol_Name{Name: "tcp"}}}, + {Protocol: &proto.Protocol{NumberOrName: &proto.Protocol_Name{Name: "no-such-protocol"}}}, + {NotProtocol: &proto.Protocol{NumberOrName: &proto.Protocol_Name{Name: "no-such-protocol"}}}, + { + Protocol: &proto.Protocol{NumberOrName: &proto.Protocol_Name{Name: "tcp"}}, + NotProtocol: &proto.Protocol{NumberOrName: &proto.Protocol_Number{Number: 17}}, + }, + { + Protocol: &proto.Protocol{NumberOrName: &proto.Protocol_Name{Name: "tcp"}}, + NotProtocol: &proto.Protocol{NumberOrName: &proto.Protocol_Number{Number: 6}}, + }, + // Malformed CIDRs: matchNet checks CIDRs in order, so an earlier matching + // net wins before the malformed one is reached. + {SrcNet: []string{"10.0.0.0/8", "not-a-cidr"}}, + {SrcNet: []string{"not-a-cidr", "10.0.0.0/8"}}, + {SrcNet: []string{"172.16.0.0/12", "not-a-cidr"}}, + {NotSrcNet: []string{"not-a-cidr"}}, + {DstNet: []string{"192.168.0.0/16"}, NotDstNet: []string{"not-a-cidr"}}, + // Named port sets. + {SrcNamedPortIpSetIds: []string{"portset-src"}}, + {NotSrcNamedPortIpSetIds: []string{"portset-src"}}, + {DstNamedPortIpSetIds: []string{"portset-src"}}, + {DstNamedPortIpSetIds: []string{"ipset-missing"}}, + // Combined criteria, mirroring the baseline-policy benchmark's rule shape. + {SrcIpSetIds: []string{"ipset-missing"}, SrcPorts: []*proto.PortRange{{First: 65001, Last: 65001}}}, + {DstIpSetIds: []string{"netset-hit"}, DstPorts: []*proto.PortRange{{First: 80, Last: 80}}}, + } + + actions := []string{"allow", "deny", "pass", "log"} + + // Every rule variant becomes a policy of three rules (log rules exercise the + // "matched but keep going" path), evaluated standalone against each flow, in both + // namespaced and global form and in both directions. + for _, namespace := range []string{"", "policy-ns"} { + for vi, variant := range ruleVariants { + for _, action := range actions { + rule := googleproto.Clone(variant).(*proto.Rule) + rule.Action = action + ruleSet := []*proto.Rule{ + {Action: "log"}, // Always matches; evaluation must continue past it. + rule, + {Action: "allow"}, // Backstop so a non-matching variant still yields index 2. + } + cp := compilePolicy(store, ruleSet, ruleSet, namespace) + Expect(cp).NotTo(BeNil()) + for fi, flow := range flows { + for _, dir := range []rules.RuleDir{rules.RuleDirIngress, rules.RuleDirEgress} { + req := NewRequestCache(store, flow) + wantAction, wantIndex := checkRules(ruleSet, req, namespace) + + req = NewRequestCache(store, flow) + gotAction, gotIndex := cp.check(dir, req) + + desc := fmt.Sprintf("variant=%d action=%s namespace=%q flow=%d dir=%v", vi, action, namespace, fi, dir) + Expect(gotAction).To(Equal(wantAction), desc) + Expect(gotIndex).To(Equal(wantIndex), desc) + } + } + } + } + } +} + +// TestCompilePolicyBadAction verifies the compile-failure path: a rule with an +// invalid action makes CompilePolicy return nil (instead of panicking), and a +// store built through ProcessUpdate keeps no compiled entry for the policy, so +// evaluation falls back to the interpreted path and preserves its semantics +// (panic at evaluate time, recovered into INVALID_ARGUMENT). +func TestCompilePolicyBadAction(t *testing.T) { + RegisterTestingT(t) + + store := policystore.NewPolicyStoreWithCompiler(policyCompiler{}) + policyID := &proto.PolicyID{Name: "bad-action"} + store.ProcessUpdate("", &proto.ToDataplane{Payload: &proto.ToDataplane_ActivePolicyUpdate{ + ActivePolicyUpdate: &proto.ActivePolicyUpdate{ + Id: policyID, + Policy: &proto.Policy{InboundRules: []*proto.Rule{{Action: "not-an-action"}}}, + }, + }}) + Expect(store.PolicyByID).To(HaveLen(1)) + Expect(store.CompiledPolicyByID).To(BeEmpty()) + + ep := &proto.WorkloadEndpoint{ + Tiers: []*proto.TierInfo{{ + Name: "tier1", + IngressPolicies: []*proto.PolicyID{policyID}, + DefaultAction: "Deny", + }}, + } + flow := &MockFlow{ + SourceIP: net.ParseIP("10.0.0.1"), + DestIP: net.ParseIP("10.0.0.2"), + Protocol: 6, + } + s := checkStore(EnforcedOnly, store, ep, rules.RuleDirIngress, flow) + Expect(s.Code).To(Equal(INVALID_ARGUMENT)) +} + +// TestCompiledIPSetReplacement verifies end to end that a full IPSetUpdate +// (which replaces the IPSet object held by compiled matchers) recompiles the +// referencing policy so the compiled path picks up the new set. +func TestCompiledIPSetReplacement(t *testing.T) { + RegisterTestingT(t) + + store := policystore.NewPolicyStoreWithCompiler(policyCompiler{}) + ipSetUpdate := func(member string) *proto.ToDataplane { + return &proto.ToDataplane{Payload: &proto.ToDataplane_IpsetUpdate{ + IpsetUpdate: &proto.IPSetUpdate{Id: "set-a", Type: proto.IPSetUpdate_NET, Members: []string{member}}, + }} + } + store.ProcessUpdate("", ipSetUpdate("10.0.0.1/32")) + policyID := &proto.PolicyID{Name: "policy-a"} + store.ProcessUpdate("", &proto.ToDataplane{Payload: &proto.ToDataplane_ActivePolicyUpdate{ + ActivePolicyUpdate: &proto.ActivePolicyUpdate{ + Id: policyID, + Policy: &proto.Policy{InboundRules: []*proto.Rule{{Action: "allow", SrcIpSetIds: []string{"set-a"}}}}, + }, + }}) + Expect(store.CompiledPolicyByID).To(HaveLen(1)) + + ep := &proto.WorkloadEndpoint{ + Tiers: []*proto.TierInfo{{ + Name: "tier1", + IngressPolicies: []*proto.PolicyID{policyID}, + DefaultAction: "Deny", + }}, + } + flow := &MockFlow{ + SourceIP: net.ParseIP("10.0.0.2"), + DestIP: net.ParseIP("192.168.1.1"), + Protocol: 6, + } + s := checkStore(EnforcedOnly, store, ep, rules.RuleDirIngress, flow) + Expect(s.Code).To(Equal(PERMISSION_DENIED)) + + // Replace the set with one that contains the flow's source IP. + store.ProcessUpdate("", ipSetUpdate("10.0.0.2/32")) + s = checkStore(EnforcedOnly, store, ep, rules.RuleDirIngress, flow) + Expect(s.Code).To(Equal(OK)) +} + +// TestCompiledEndpointFollowsPolicyUpdates verifies end to end that an +// endpoint's precomputed policy slice keeps up with later policy updates and +// removals without the endpoint being re-sent — the slots it holds are +// published through by the store. +func TestCompiledEndpointFollowsPolicyUpdates(t *testing.T) { + RegisterTestingT(t) + + store := policystore.NewPolicyStoreWithCompiler(policyCompiler{}) + policyID := &proto.PolicyID{Name: "policy1"} + setPolicyAction := func(action string) { + store.ProcessUpdate("", &proto.ToDataplane{Payload: &proto.ToDataplane_ActivePolicyUpdate{ + ActivePolicyUpdate: &proto.ActivePolicyUpdate{ + Id: policyID, + Policy: &proto.Policy{InboundRules: []*proto.Rule{{Action: action}}}, + }, + }}) + } + setPolicyAction("allow") + + ep := &proto.WorkloadEndpoint{ + Tiers: []*proto.TierInfo{{ + Name: "tier1", + IngressPolicies: []*proto.PolicyID{policyID}, + DefaultAction: "Deny", + }}, + } + store.ProcessUpdate("", &proto.ToDataplane{Payload: &proto.ToDataplane_WorkloadEndpointUpdate{ + WorkloadEndpointUpdate: &proto.WorkloadEndpointUpdate{ + Id: &proto.WorkloadEndpointID{OrchestratorId: "k8s", WorkloadId: "wep1", EndpointId: "eth0"}, + Endpoint: ep, + }, + }}) + Expect(store.CompiledEndpoints).To(HaveKey(ep)) + + flow := &MockFlow{ + SourceIP: net.ParseIP("10.0.0.1"), + DestIP: net.ParseIP("10.0.0.2"), + Protocol: 6, + } + Expect(checkStore(EnforcedOnly, store, ep, rules.RuleDirIngress, flow).Code).To(Equal(OK)) + + // Update the policy in place: the endpoint is not re-sent, so it must see + // the new action through the slot it already holds. + setPolicyAction("deny") + Expect(checkStore(EnforcedOnly, store, ep, rules.RuleDirIngress, flow).Code).To(Equal(PERMISSION_DENIED)) + setPolicyAction("allow") + Expect(checkStore(EnforcedOnly, store, ep, rules.RuleDirIngress, flow).Code).To(Equal(OK)) + + // Removing the policy must stop the endpoint evaluating it: a policy + // missing from the store fails the evaluation closed (INTERNAL), whichever + // engine would have evaluated it. + store.ProcessUpdate("", &proto.ToDataplane{Payload: &proto.ToDataplane_ActivePolicyRemove{ + ActivePolicyRemove: &proto.ActivePolicyRemove{Id: policyID}, + }}) + Expect(checkStore(EnforcedOnly, store, ep, rules.RuleDirIngress, flow).Code).To(Equal(INTERNAL)) + // ...and matches what the interpreted path does for the same store. + clearCompiledForTest(store) + Expect(checkStore(EnforcedOnly, store, ep, rules.RuleDirIngress, flow).Code).To(Equal(INTERNAL)) +} + +// TestCompiledRuleIDMemo covers the memoized trace entries: the entry is +// reused across evaluations, but only for the policy identity it was built +// for, so two endpoints naming the same policy under differently-named tiers +// each get their own. +func TestCompiledRuleIDMemo(t *testing.T) { + RegisterTestingT(t) + + store := policystore.NewPolicyStore() + policyID := &proto.PolicyID{Name: "policy1", Kind: v3.KindGlobalNetworkPolicy} + store.PolicyByID[types.ProtoToPolicyID(policyID)] = &proto.Policy{ + InboundRules: []*proto.Rule{{Action: "allow"}}, + } + endpointInTier := func(tierName string) *proto.WorkloadEndpoint { + return &proto.WorkloadEndpoint{Tiers: []*proto.TierInfo{{ + Name: tierName, + IngressPolicies: []*proto.PolicyID{policyID}, + DefaultAction: "Deny", + }}} + } + epA, epB := endpointInTier("tier-a"), endpointInTier("tier-b") + store.Endpoints[types.WorkloadEndpointID{WorkloadId: "a"}] = epA + store.Endpoints[types.WorkloadEndpointID{WorkloadId: "b"}] = epB + compileStoreForTest(store) + + flow := &MockFlow{ + SourceIP: net.ParseIP("10.0.0.1"), + DestIP: net.ParseIP("10.0.0.2"), + Protocol: 6, + } + traceFor := func(ep *proto.WorkloadEndpoint) []*calc.RuleID { + trace, err := Evaluate(EnforcedOnly, rules.RuleDirIngress, store, ep, flow, nil) + Expect(err).NotTo(HaveOccurred()) + return trace + } + + traceA := traceFor(epA) + Expect(traceA).To(HaveLen(1)) + Expect(traceA[0].Tier).To(Equal("tier-a")) + + // Same rule, different tier: the memoized entry must not be reused. + traceB := traceFor(epB) + Expect(traceB).To(HaveLen(1)) + Expect(traceB[0].Tier).To(Equal("tier-b")) + + // Repeat evaluations hit the memo and must still be correct, and the entry + // for an unchanged identity is the very same object. + traceB2 := traceFor(epB) + Expect(traceB2[0]).To(BeIdenticalTo(traceB[0])) + Expect(traceFor(epA)[0].Tier).To(Equal("tier-a")) +} + +// TestCompiledRuleIDMemoConcurrent evaluates one policy from several +// goroutines, as dikastes does under the store's read lock: memoizing a trace +// entry must be safe (run under -race to mean anything). +func TestCompiledRuleIDMemoConcurrent(t *testing.T) { + RegisterTestingT(t) + + store := policystore.NewPolicyStore() + policyID := &proto.PolicyID{Name: "policy1", Kind: v3.KindGlobalNetworkPolicy} + store.PolicyByID[types.ProtoToPolicyID(policyID)] = &proto.Policy{ + InboundRules: []*proto.Rule{{Action: "allow"}}, + } + ep := &proto.WorkloadEndpoint{Tiers: []*proto.TierInfo{{ + Name: "tier1", + IngressPolicies: []*proto.PolicyID{policyID}, + DefaultAction: "Deny", + }}} + store.Endpoint = ep + compileStoreForTest(store) + + var wg sync.WaitGroup + for i := 0; i < 8; i++ { + wg.Add(1) + go func() { + defer wg.Done() + flow := &MockFlow{ + SourceIP: net.ParseIP("10.0.0.1"), + DestIP: net.ParseIP("10.0.0.2"), + Protocol: 6, + } + for j := 0; j < 100; j++ { + trace, err := Evaluate(EnforcedOnly, rules.RuleDirIngress, store, ep, flow, nil) + if err != nil || len(trace) != 1 || trace[0].Tier != "tier1" || trace[0].Action != rules.RuleActionAllow { + t.Errorf("unexpected trace: %v", trace) + return + } + } + }() + } + wg.Wait() +} + +// TestCompiledStaleEndpointReference covers the deleted-endpoint race: the +// felix collector's endpoint cache can hold an endpoint whose TierInfo +// references a policy that has since been removed from the store. The +// compiled dispatch must preserve the interpreted path's behavior for that +// case: the evaluation fails closed. +func TestCompiledStaleEndpointReference(t *testing.T) { + RegisterTestingT(t) + + store := policystore.NewPolicyStore() + ep := &proto.WorkloadEndpoint{ + Tiers: []*proto.TierInfo{{ + Name: "tier1", + IngressPolicies: []*proto.PolicyID{{Name: "removed-policy"}}, + DefaultAction: "Deny", + }}, + } + flow := &MockFlow{ + SourceIP: net.ParseIP("10.0.0.1"), + DestIP: net.ParseIP("10.0.0.2"), + Protocol: 6, + } + checkStoreBothEngines(EnforcedOnly, store, ep, rules.RuleDirIngress, flow) +} + +// TestCompilerCoversRuleFields fails when proto.Rule grows a field the +// compiler was not written to handle. Every field must be listed either as +// compiled (appendRuleMatchers emits a matcher for it) or as ignored (the +// interpreted path does not evaluate it either, so the compiled path must not +// start doing so). A new field in neither list means the two engines may have +// diverged: extend appendRuleMatchers (and the equivalence test) or record it +// as ignored. +func TestCompilerCoversRuleFields(t *testing.T) { + compiledFields := map[string]bool{ + "Action": true, + "Protocol": true, + "NotProtocol": true, + "SrcNet": true, + "NotSrcNet": true, + "DstNet": true, + "NotDstNet": true, + "SrcPorts": true, + "NotSrcPorts": true, + "DstPorts": true, + "NotDstPorts": true, + "SrcIpSetIds": true, + "NotSrcIpSetIds": true, + "DstIpSetIds": true, + "NotDstIpSetIds": true, + "DstIpPortSetIds": true, + "SrcNamedPortIpSetIds": true, + "NotSrcNamedPortIpSetIds": true, + "DstNamedPortIpSetIds": true, + "NotDstNamedPortIpSetIds": true, + "SrcServiceAccountMatch": true, + "DstServiceAccountMatch": true, + "OriginalSrcSelector": true, + "OriginalNotSrcSelector": true, + "OriginalDstSelector": true, + "OriginalNotDstSelector": true, + "OriginalSrcNamespaceSelector": true, + "OriginalDstNamespaceSelector": true, + "HttpMatch": true, + } + ignoredFields := map[string]bool{ + // Not evaluated by the interpreted match path either. + "IpVersion": true, + "Icmp": true, + "NotIcmp": true, + "OriginalSrcService": true, + "OriginalSrcServiceNamespace": true, + "OriginalDstService": true, + "OriginalDstServiceNamespace": true, + "Metadata": true, + "RuleId": true, + } + + ruleType := reflect.TypeOf(proto.Rule{}) + for i := 0; i < ruleType.NumField(); i++ { + f := ruleType.Field(i) + if !f.IsExported() { + continue + } + if !compiledFields[f.Name] && !ignoredFields[f.Name] { + t.Errorf("proto.Rule field %s is not handled by the policy compiler: "+ + "extend appendRuleMatchers and TestCompiledPolicyEquivalence, or record it as ignored", f.Name) + } + } +} + +// TestCompileMemoryFootprint reports the heap cost of compiling the full +// baseline-scale policy set. Not an assertion, a measurement; run with -v to +// see the numbers. +func TestCompileMemoryFootprint(t *testing.T) { + store, _, _ := buildBaselinePolicyStore(defaultBaselinePolicyScaleParams()) + + var before, after runtime.MemStats + runtime.GC() + runtime.ReadMemStats(&before) + compiled := make([]*compiledPolicy, 0, len(store.PolicyByID)) + for _, p := range store.PolicyByID { + compiled = append(compiled, compilePolicy(store, p.InboundRules, p.OutboundRules, p.Namespace)) + } + runtime.GC() + runtime.ReadMemStats(&after) + t.Logf("compiled policy set: retained=%dKB totalAlloc=%dKB mallocs=%d", + (after.HeapAlloc-before.HeapAlloc)/1024, + (after.TotalAlloc-before.TotalAlloc)/1024, + after.Mallocs-before.Mallocs) + runtime.KeepAlive(compiled) +} diff --git a/app-policy/checker/requestcache.go b/app-policy/checker/requestcache.go index 0e236b23dc4..77c1e4968c2 100644 --- a/app-policy/checker/requestcache.go +++ b/app-policy/checker/requestcache.go @@ -105,6 +105,27 @@ func NewRequestCache(store *policystore.PolicyStore, request Flow) *requestCache } } +// requestCaches recycles the per-evaluation scratch space. Evaluation is on the +// felix collector's and dikastes' hot paths, and the cache cannot be stack +// allocated: compiled matchers receive it through a func value, so escape +// analysis must assume it leaks. +var requestCaches = sync.Pool{New: func() any { return &requestCache{} }} + +func getRequestCache(store *policystore.PolicyStore, request Flow) *requestCache { + r := requestCaches.Get().(*requestCache) + r.Flow = request + r.store = store + return r +} + +// putRequestCache returns the cache to the pool, clearing it so that no value +// from this flow can be read by the next one, and so that it holds on to +// neither the flow nor the store. +func putRequestCache(r *requestCache) { + *r = requestCache{} + requestCaches.Put(r) +} + // getSrcPeer returns the source peer. func (r *requestCache) getSrcPeer() *peer { return r.getIdentity(sourceSide).peer diff --git a/app-policy/pkg/dikastes/dikastes.go b/app-policy/pkg/dikastes/dikastes.go index 02d805fec22..f6b3df5b23d 100644 --- a/app-policy/pkg/dikastes/dikastes.go +++ b/app-policy/pkg/dikastes/dikastes.go @@ -70,7 +70,9 @@ func RunServer(listenPath, dialTarget string) { defer cancel() gs := grpc.NewServer() - storeManager := policystore.NewPolicyStoreManager() + storeManager := policystore.NewPolicyStoreManagerWithOpts( + policystore.WithPolicyCompiler(checker.NewPolicyCompiler()), + ) NewCheckServer(ctx, gs, storeManager) opts := uds.GetDialOptions() diff --git a/app-policy/policystore/compiler.go b/app-policy/policystore/compiler.go new file mode 100644 index 00000000000..33c81bef09e --- /dev/null +++ b/app-policy/policystore/compiler.go @@ -0,0 +1,311 @@ +// Copyright (c) 2026 Tigera, Inc. All rights reserved. + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package policystore + +import ( + "github.com/projectcalico/calico/felix/proto" + "github.com/projectcalico/calico/felix/types" +) + +// CompiledPolicy is the compiled form of a policy or profile, and +// CompiledEndpoint that of an endpoint's tier/profile structure, both produced +// by a PolicyCompiler. They are opaque to the policy store: the checker +// package both produces the values and consumes them at evaluate time +// (type-asserting back to its concrete types). The types live here rather than +// in checker so that the store can own the compiled artifacts' lifecycle +// without importing checker (checker imports policystore). +type ( + CompiledPolicy interface{} + CompiledEndpoint interface{} +) + +// PolicySlot holds a policy's (or profile's) compiled form behind a pointer +// that is stable for as long as the policy exists. A compiled endpoint holds +// the slots of the policies its tiers name, so recompiling a policy — on a +// policy update, or when an IP set it references is replaced — publishes +// through the slot and leaves every compiled endpoint referencing it +// untouched. Without the indirection, a single policy update would have to +// rebuild every endpoint that names the policy, which for an +// all-endpoints policy is every endpoint on the node. +type PolicySlot struct { + compiled CompiledPolicy +} + +// Compiled returns the policy's compiled form, or nil if it has none (not +// compiled yet, compilation failed, or the policy has been removed). It is +// nil-receiver-safe so that callers can treat "no slot" and "empty slot" +// alike. +func (s *PolicySlot) Compiled() CompiledPolicy { + if s == nil { + return nil + } + return s.compiled +} + +// PolicyCompiler compiles policies, profiles and endpoints into a form that is +// cheap to evaluate per flow. The store invokes it eagerly as updates are +// applied (off the flow evaluation hot path, under the store's write lock), +// and again for affected policies when an IP set they reference is replaced or +// removed. A nil return means the input could not be compiled; the store keeps +// no compiled entry for it and evaluation falls back to interpreting the +// uncompiled form. +type PolicyCompiler interface { + CompilePolicy(store *PolicyStore, policy *proto.Policy) CompiledPolicy + CompileProfile(store *PolicyStore, profile *proto.Profile) CompiledPolicy + // CompileEndpoint resolves an endpoint's tier and profile references to + // the store's PolicySlots, so that evaluation walks a slice instead of + // hashing every policy ID per flow. Felix sends policies and profiles + // before the endpoints that name them, so the slots exist by now; one + // that does not (an out-of-sync store) simply falls back to the by-ID + // lookup for that policy. + CompileEndpoint(store *PolicyStore, ep *proto.WorkloadEndpoint) CompiledEndpoint +} + +// SetPolicyCompiler configures the store's compiler and compiles everything +// already in it; updates applied afterwards are compiled as they arrive. It is +// for callers that populate a store directly rather than through ProcessUpdate +// — the store manager wires a compiler into the stores it creates with +// WithPolicyCompiler instead. +func (store *PolicyStore) SetPolicyCompiler(compiler PolicyCompiler) { + store.compiler = compiler + store.ipSetPolicyRefs = nil + store.ipSetProfileRefs = nil + if compiler == nil { + return + } + for id, p := range store.PolicyByID { + store.onPolicyUpdate(id, nil, p) + } + for id, p := range store.ProfileByID { + store.onProfileUpdate(id, nil, p) + } + // Endpoints last: their compiled form resolves the policy and profile + // slots compiled above. + store.onEndpointUpdate(nil, store.Endpoint) + for _, ep := range store.Endpoints { + store.onEndpointUpdate(nil, ep) + } +} + +// onPolicyUpdate maintains the compiled form and the IP set reverse index for +// a stored (or replaced) policy. old is the policy previously stored under +// the ID, or nil. +func (store *PolicyStore) onPolicyUpdate(id types.PolicyID, old, updated *proto.Policy) { + if store.compiler == nil { + return + } + if old != nil { + forEachIPSetRef(old, func(setID string) { + deleteRef(store.ipSetPolicyRefs, setID, id) + }) + } + if updated != nil { + forEachIPSetRef(updated, func(setID string) { + addRef(&store.ipSetPolicyRefs, setID, id) + }) + } + store.compilePolicy(id, updated) +} + +// onPolicyRemove drops a removed policy's compiled form and reverse-index +// entries. old is the policy previously stored under the ID, or nil. +func (store *PolicyStore) onPolicyRemove(id types.PolicyID, old *proto.Policy) { + if store.compiler == nil { + return + } + if old != nil { + forEachIPSetRef(old, func(setID string) { + deleteRef(store.ipSetPolicyRefs, setID, id) + }) + } + // Empty the slot as well as dropping it: a compiled endpoint that still + // names the removed policy holds the slot, and must stop evaluating it. + if slot, ok := store.CompiledPolicyByID[id]; ok { + slot.compiled = nil + delete(store.CompiledPolicyByID, id) + } +} + +// onProfileUpdate is onPolicyUpdate for profiles. +func (store *PolicyStore) onProfileUpdate(id types.ProfileID, old, updated *proto.Profile) { + if store.compiler == nil { + return + } + if old != nil { + forEachProfileIPSetRef(old, func(setID string) { + deleteRef(store.ipSetProfileRefs, setID, id) + }) + } + if updated != nil { + forEachProfileIPSetRef(updated, func(setID string) { + addRef(&store.ipSetProfileRefs, setID, id) + }) + } + store.compileProfile(id, updated) +} + +// onProfileRemove is onPolicyRemove for profiles. +func (store *PolicyStore) onProfileRemove(id types.ProfileID, old *proto.Profile) { + if store.compiler == nil { + return + } + if old != nil { + forEachProfileIPSetRef(old, func(setID string) { + deleteRef(store.ipSetProfileRefs, setID, id) + }) + } + if slot, ok := store.CompiledProfileByID[id]; ok { + slot.compiled = nil + delete(store.CompiledProfileByID, id) + } +} + +// onIPSetReplaced recompiles the policies and profiles that reference an IP +// set whose object was replaced (full IPSetUpdate) or removed. Compiled +// matchers hold the IPSet object itself, so a replaced object would otherwise +// leave them evaluating the stale set. Membership deltas mutate the set in +// place and do NOT come through here. During the initial resync this is free: +// felix sends IP sets before the policies that reference them, so the reverse +// index is empty when the sets arrive. +func (store *PolicyStore) onIPSetReplaced(setID string) { + if store.compiler == nil { + return + } + for policyID := range store.ipSetPolicyRefs[setID] { + store.compilePolicy(policyID, store.PolicyByID[policyID]) + } + for profileID := range store.ipSetProfileRefs[setID] { + store.compileProfile(profileID, store.ProfileByID[profileID]) + } +} + +// compilePolicy and compileProfile publish a policy's compiled form through +// its slot, reusing the slot when there is one so that compiled endpoints +// holding it see the new form. A nil policy/profile (a malformed update, or a +// stale reverse-index entry) is treated as not compilable: the slot is +// emptied and evaluation falls back to interpreting the stored value. +func (store *PolicyStore) compilePolicy(id types.PolicyID, policy *proto.Policy) { + var cp CompiledPolicy + if policy != nil { + cp = store.compiler.CompilePolicy(store, policy) + } + if slot, ok := store.CompiledPolicyByID[id]; ok { + slot.compiled = cp + return + } + if cp != nil { + store.CompiledPolicyByID[id] = &PolicySlot{compiled: cp} + } +} + +func (store *PolicyStore) compileProfile(id types.ProfileID, profile *proto.Profile) { + var cp CompiledPolicy + if profile != nil { + cp = store.compiler.CompileProfile(store, profile) + } + if slot, ok := store.CompiledProfileByID[id]; ok { + slot.compiled = cp + return + } + if cp != nil { + store.CompiledProfileByID[id] = &PolicySlot{compiled: cp} + } +} + +// onEndpointUpdate compiles a stored (or replaced) endpoint. old is the +// endpoint previously stored under the same ID, or nil; its compiled form is +// dropped, since compiled endpoints are keyed by the identity of the endpoint +// object they were built from. +func (store *PolicyStore) onEndpointUpdate(old, updated *proto.WorkloadEndpoint) { + if store.compiler == nil { + return + } + if old != nil { + delete(store.CompiledEndpoints, old) + } + if updated == nil { + return + } + if ce := store.compiler.CompileEndpoint(store, updated); ce != nil { + store.CompiledEndpoints[updated] = ce + } +} + +// onEndpointRemove drops a removed endpoint's compiled form. +func (store *PolicyStore) onEndpointRemove(old *proto.WorkloadEndpoint) { + if store.compiler == nil || old == nil { + return + } + delete(store.CompiledEndpoints, old) +} + +// forEachIPSetRef calls f once per IP set ID referenced by the policy's +// rules (duplicates included). The field list must cover every proto.Rule +// field holding IP set IDs so that a replaced IP set recompiles every policy +// whose compiled form resolved it; TestForEachIPSetRefCoversRuleFields fails +// if proto.Rule grows an IP set reference field that is missing here. +func forEachIPSetRef(policy *proto.Policy, f func(setID string)) { + forEachRuleIPSetRef(policy.InboundRules, f) + forEachRuleIPSetRef(policy.OutboundRules, f) +} + +func forEachProfileIPSetRef(profile *proto.Profile, f func(setID string)) { + forEachRuleIPSetRef(profile.InboundRules, f) + forEachRuleIPSetRef(profile.OutboundRules, f) +} + +func forEachRuleIPSetRef(rules []*proto.Rule, f func(setID string)) { + for _, r := range rules { + for _, ids := range [][]string{ + r.SrcIpSetIds, + r.NotSrcIpSetIds, + r.SrcNamedPortIpSetIds, + r.NotSrcNamedPortIpSetIds, + r.DstIpSetIds, + r.NotDstIpSetIds, + r.DstNamedPortIpSetIds, + r.NotDstNamedPortIpSetIds, + r.DstIpPortSetIds, + } { + for _, id := range ids { + f(id) + } + } + } +} + +// addRef and deleteRef maintain a reverse index from IP set ID to the +// policies (or profiles) whose compiled form references it. Reference counts +// are not needed: deleteRef is only called with every ref of a policy at +// once, and duplicate adds are idempotent. +func addRef[ID comparable](index *map[string]map[ID]struct{}, setID string, id ID) { + if *index == nil { + *index = make(map[string]map[ID]struct{}) + } + refs := (*index)[setID] + if refs == nil { + refs = make(map[ID]struct{}) + (*index)[setID] = refs + } + refs[id] = struct{}{} +} + +func deleteRef[ID comparable](index map[string]map[ID]struct{}, setID string, id ID) { + refs := index[setID] + delete(refs, id) + if len(refs) == 0 { + delete(index, setID) + } +} diff --git a/app-policy/policystore/compiler_test.go b/app-policy/policystore/compiler_test.go new file mode 100644 index 00000000000..dd5ed4a183e --- /dev/null +++ b/app-policy/policystore/compiler_test.go @@ -0,0 +1,452 @@ +// Copyright (c) 2026 Tigera, Inc. All rights reserved. + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package policystore + +import ( + "reflect" + "strings" + "testing" + + . "github.com/onsi/gomega" + + "github.com/projectcalico/calico/felix/proto" + "github.com/projectcalico/calico/felix/types" +) + +// fakeCompiler counts compilations and returns a fresh marker value per +// compile, so tests can tell recompilations apart. Policies whose first +// inbound rule has RuleId "fail" refuse to compile (return nil). +type fakeCompiler struct { + policyCompiles map[types.PolicyID]int + profileCompiles map[types.ProfileID]int + endpointCompiles int +} + +func newFakeCompiler() *fakeCompiler { + return &fakeCompiler{ + policyCompiles: map[types.PolicyID]int{}, + profileCompiles: map[types.ProfileID]int{}, + } +} + +type fakeCompiled struct{ generation int } + +// fakeCompiledEndpoint records the policy slots the endpoint resolved to, so +// tests can check that a slot outlives recompilation of its policy. +type fakeCompiledEndpoint struct{ slots []*PolicySlot } + +func (c *fakeCompiler) CompileEndpoint(store *PolicyStore, ep *proto.WorkloadEndpoint) CompiledEndpoint { + c.endpointCompiles++ + ce := &fakeCompiledEndpoint{} + for _, tier := range ep.Tiers { + for _, id := range tier.IngressPolicies { + ce.slots = append(ce.slots, store.CompiledPolicyByID[types.ProtoToPolicyID(id)]) + } + } + return ce +} + +func (c *fakeCompiler) CompilePolicy(store *PolicyStore, policy *proto.Policy) CompiledPolicy { + id := findPolicyID(store, policy) + c.policyCompiles[id]++ + if len(policy.InboundRules) > 0 && policy.InboundRules[0].RuleId == "fail" { + return nil + } + return &fakeCompiled{generation: c.policyCompiles[id]} +} + +func (c *fakeCompiler) CompileProfile(store *PolicyStore, profile *proto.Profile) CompiledPolicy { + id := findProfileID(store, profile) + c.profileCompiles[id]++ + return &fakeCompiled{generation: c.profileCompiles[id]} +} + +func findPolicyID(store *PolicyStore, policy *proto.Policy) types.PolicyID { + for id, p := range store.PolicyByID { + if p == policy { + return id + } + } + return types.PolicyID{} +} + +func findProfileID(store *PolicyStore, profile *proto.Profile) types.ProfileID { + for id, p := range store.ProfileByID { + if p == profile { + return id + } + } + return types.ProfileID{} +} + +func policyUpdate(name string, policy *proto.Policy) *proto.ToDataplane { + return &proto.ToDataplane{Payload: &proto.ToDataplane_ActivePolicyUpdate{ + ActivePolicyUpdate: &proto.ActivePolicyUpdate{Id: &proto.PolicyID{Name: name}, Policy: policy}, + }} +} + +func policyRemove(name string) *proto.ToDataplane { + return &proto.ToDataplane{Payload: &proto.ToDataplane_ActivePolicyRemove{ + ActivePolicyRemove: &proto.ActivePolicyRemove{Id: &proto.PolicyID{Name: name}}, + }} +} + +func profileUpdate(name string, profile *proto.Profile) *proto.ToDataplane { + return &proto.ToDataplane{Payload: &proto.ToDataplane_ActiveProfileUpdate{ + ActiveProfileUpdate: &proto.ActiveProfileUpdate{Id: &proto.ProfileID{Name: name}, Profile: profile}, + }} +} + +func profileRemove(name string) *proto.ToDataplane { + return &proto.ToDataplane{Payload: &proto.ToDataplane_ActiveProfileRemove{ + ActiveProfileRemove: &proto.ActiveProfileRemove{Id: &proto.ProfileID{Name: name}}, + }} +} + +func endpointUpdate(name string, ep *proto.WorkloadEndpoint) *proto.ToDataplane { + return &proto.ToDataplane{Payload: &proto.ToDataplane_WorkloadEndpointUpdate{ + WorkloadEndpointUpdate: &proto.WorkloadEndpointUpdate{ + Id: &proto.WorkloadEndpointID{OrchestratorId: "k8s", WorkloadId: name, EndpointId: "eth0"}, + Endpoint: ep, + }, + }} +} + +func endpointRemove(name string) *proto.ToDataplane { + return &proto.ToDataplane{Payload: &proto.ToDataplane_WorkloadEndpointRemove{ + WorkloadEndpointRemove: &proto.WorkloadEndpointRemove{ + Id: &proto.WorkloadEndpointID{OrchestratorId: "k8s", WorkloadId: name, EndpointId: "eth0"}, + }, + }} +} + +func ipSetUpdate(id string, members ...string) *proto.ToDataplane { + return &proto.ToDataplane{Payload: &proto.ToDataplane_IpsetUpdate{ + IpsetUpdate: &proto.IPSetUpdate{Id: id, Type: proto.IPSetUpdate_IP, Members: members}, + }} +} + +func ipSetDeltaUpdate(id string, added ...string) *proto.ToDataplane { + return &proto.ToDataplane{Payload: &proto.ToDataplane_IpsetDeltaUpdate{ + IpsetDeltaUpdate: &proto.IPSetDeltaUpdate{Id: id, AddedMembers: added}, + }} +} + +func ipSetRemove(id string) *proto.ToDataplane { + return &proto.ToDataplane{Payload: &proto.ToDataplane_IpsetRemove{ + IpsetRemove: &proto.IPSetRemove{Id: id}, + }} +} + +func pID(name string) types.PolicyID { + return types.ProtoToPolicyID(&proto.PolicyID{Name: name}) +} + +func prID(name string) types.ProfileID { + return types.ProtoToProfileID(&proto.ProfileID{Name: name}) +} + +// TestCompileOnUpdate verifies that policies and profiles are compiled as +// their updates are applied, recompiled when replaced, and dropped when +// removed. +func TestCompileOnUpdate(t *testing.T) { + RegisterTestingT(t) + + compiler := newFakeCompiler() + store := NewPolicyStoreWithCompiler(compiler) + + store.ProcessUpdate("", policyUpdate("policy1", &proto.Policy{})) + Expect(store.CompiledPolicyByID).To(HaveKey(pID("policy1"))) + Expect(compiler.policyCompiles[pID("policy1")]).To(Equal(1)) + + // Replacing the policy recompiles it, publishing through the same slot so + // that compiled endpoints holding the slot see the new form. + slot := store.CompiledPolicyByID[pID("policy1")] + first := slot.Compiled() + store.ProcessUpdate("", policyUpdate("policy1", &proto.Policy{})) + Expect(compiler.policyCompiles[pID("policy1")]).To(Equal(2)) + Expect(store.CompiledPolicyByID[pID("policy1")]).To(BeIdenticalTo(slot)) + Expect(slot.Compiled()).NotTo(BeIdenticalTo(first)) + + // Removing the policy empties the slot as well as dropping it, so an + // endpoint that still holds it stops evaluating the removed policy. + store.ProcessUpdate("", policyRemove("policy1")) + Expect(store.CompiledPolicyByID).To(BeEmpty()) + Expect(slot.Compiled()).To(BeNil()) + + store.ProcessUpdate("", profileUpdate("profile1", &proto.Profile{})) + Expect(store.CompiledProfileByID).To(HaveKey(prID("profile1"))) + Expect(compiler.profileCompiles[prID("profile1")]).To(Equal(1)) + + store.ProcessUpdate("", profileRemove("profile1")) + Expect(store.CompiledProfileByID).To(BeEmpty()) +} + +// TestCompileFailureLeavesNoEntry verifies that a policy the compiler cannot +// compile gets no compiled entry (so evaluation falls back to interpreting +// it), and that a later working replacement compiles again. +func TestCompileFailureLeavesNoEntry(t *testing.T) { + RegisterTestingT(t) + + compiler := newFakeCompiler() + store := NewPolicyStoreWithCompiler(compiler) + + store.ProcessUpdate("", policyUpdate("policy1", &proto.Policy{ + InboundRules: []*proto.Rule{{RuleId: "fail"}}, + })) + Expect(store.PolicyByID).To(HaveKey(pID("policy1"))) + Expect(store.CompiledPolicyByID).To(BeEmpty()) + + store.ProcessUpdate("", policyUpdate("policy1", &proto.Policy{})) + Expect(store.CompiledPolicyByID).To(HaveKey(pID("policy1"))) +} + +// TestNilPolicyUpdateCompilesNothing verifies that a malformed update +// carrying a nil policy/profile does not crash the compile hooks and leaves +// no compiled entry (evaluation falls back to interpreting the stored nil, +// as before). +func TestNilPolicyUpdateCompilesNothing(t *testing.T) { + RegisterTestingT(t) + + compiler := newFakeCompiler() + store := NewPolicyStoreWithCompiler(compiler) + + // Replace a healthy policy/profile with a nil one: the compiled entry + // (and the reverse-index entries) must be dropped. + store.ProcessUpdate("", ipSetUpdate("set-a", "10.0.0.1")) + store.ProcessUpdate("", policyUpdate("policy1", &proto.Policy{ + InboundRules: []*proto.Rule{{SrcIpSetIds: []string{"set-a"}}}, + })) + store.ProcessUpdate("", profileUpdate("profile1", &proto.Profile{})) + Expect(store.CompiledPolicyByID).To(HaveLen(1)) + Expect(store.CompiledProfileByID).To(HaveLen(1)) + + // The slots stay (the IDs are still in the store, holding nil policies) but + // must be emptied, so evaluation interprets the stored nil as before. + store.ProcessUpdate("", policyUpdate("policy1", nil)) + store.ProcessUpdate("", profileUpdate("profile1", nil)) + Expect(store.CompiledPolicyByID[pID("policy1")].Compiled()).To(BeNil()) + Expect(store.CompiledProfileByID[prID("profile1")].Compiled()).To(BeNil()) + Expect(store.ipSetPolicyRefs).To(BeEmpty()) +} + +// TestEndpointCompilation verifies that endpoints are compiled on update, +// dropped on replacement and removal, and — the point of the PolicySlot +// indirection — are NOT rebuilt when a policy they name is recompiled. +func TestEndpointCompilation(t *testing.T) { + RegisterTestingT(t) + + compiler := newFakeCompiler() + store := NewPolicyStoreWithCompiler(compiler) + + store.ProcessUpdate("", ipSetUpdate("set-a", "10.0.0.1")) + store.ProcessUpdate("", policyUpdate("policy1", &proto.Policy{ + InboundRules: []*proto.Rule{{SrcIpSetIds: []string{"set-a"}}}, + })) + Expect(compiler.endpointCompiles).To(Equal(0)) + + ep := &proto.WorkloadEndpoint{Tiers: []*proto.TierInfo{{ + Name: "tier1", + IngressPolicies: []*proto.PolicyID{{Name: "policy1"}}, + }}} + store.ProcessUpdate("", endpointUpdate("wep1", ep)) + Expect(compiler.endpointCompiles).To(Equal(1)) + ce, ok := store.CompiledEndpoints[ep].(*fakeCompiledEndpoint) + Expect(ok).To(BeTrue()) + slot := store.CompiledPolicyByID[pID("policy1")] + Expect(ce.slots).To(Equal([]*PolicySlot{slot})) + + // Recompiling the policy — directly, and via an IP set replacement — + // publishes through the slot the endpoint already holds. No endpoint + // rebuild: with an all-endpoints policy that would mean rebuilding every + // endpoint on the node. + store.ProcessUpdate("", policyUpdate("policy1", &proto.Policy{})) + store.ProcessUpdate("", ipSetUpdate("set-a", "10.0.0.2")) + Expect(compiler.endpointCompiles).To(Equal(1)) + Expect(store.CompiledEndpoints[ep]).To(BeIdenticalTo(ce)) + + // Replacing the endpoint compiles the new object and drops the old one's + // entry, so a stale endpoint copy cannot resolve to a compiled form. + ep2 := &proto.WorkloadEndpoint{Tiers: ep.Tiers} + store.ProcessUpdate("", endpointUpdate("wep1", ep2)) + Expect(compiler.endpointCompiles).To(Equal(2)) + Expect(store.CompiledEndpoints).To(HaveLen(1)) + Expect(store.CompiledEndpoints).To(HaveKey(ep2)) + + store.ProcessUpdate("", endpointRemove("wep1")) + Expect(store.CompiledEndpoints).To(BeEmpty()) +} + +// TestEndpointCompilationPerHost covers the felix collector's subscription +// type, where the store holds many endpoints rather than one. +func TestEndpointCompilationPerHost(t *testing.T) { + RegisterTestingT(t) + + compiler := newFakeCompiler() + store := NewPolicyStoreWithCompiler(compiler) + + ep1 := &proto.WorkloadEndpoint{Name: "ep1"} + ep2 := &proto.WorkloadEndpoint{Name: "ep2"} + store.ProcessUpdate("per-host-policies", endpointUpdate("wep1", ep1)) + store.ProcessUpdate("per-host-policies", endpointUpdate("wep2", ep2)) + Expect(store.CompiledEndpoints).To(HaveLen(2)) + + store.ProcessUpdate("per-host-policies", endpointRemove("wep1")) + Expect(store.CompiledEndpoints).To(HaveLen(1)) + Expect(store.CompiledEndpoints).To(HaveKey(ep2)) +} + +// TestIPSetInvalidation verifies the reverse index: replacing or removing an +// IP set recompiles exactly the policies and profiles that reference it, +// while membership deltas (which mutate the IPSet object in place) recompile +// nothing. +func TestIPSetInvalidation(t *testing.T) { + RegisterTestingT(t) + + compiler := newFakeCompiler() + store := NewPolicyStoreWithCompiler(compiler) + + store.ProcessUpdate("", ipSetUpdate("set-a", "10.0.0.1")) + store.ProcessUpdate("", ipSetUpdate("set-b", "10.0.0.2")) + store.ProcessUpdate("", policyUpdate("refs-a", &proto.Policy{ + InboundRules: []*proto.Rule{{SrcIpSetIds: []string{"set-a"}}}, + })) + store.ProcessUpdate("", policyUpdate("refs-b", &proto.Policy{ + OutboundRules: []*proto.Rule{{NotDstNamedPortIpSetIds: []string{"set-b"}}}, + })) + store.ProcessUpdate("", profileUpdate("profile-a", &proto.Profile{ + InboundRules: []*proto.Rule{{DstIpPortSetIds: []string{"set-a"}}}, + })) + Expect(compiler.policyCompiles).To(Equal(map[types.PolicyID]int{pID("refs-a"): 1, pID("refs-b"): 1})) + Expect(compiler.profileCompiles).To(Equal(map[types.ProfileID]int{prID("profile-a"): 1})) + + // Replacing set-a recompiles only its referrers. + store.ProcessUpdate("", ipSetUpdate("set-a", "10.0.0.3")) + Expect(compiler.policyCompiles).To(Equal(map[types.PolicyID]int{pID("refs-a"): 2, pID("refs-b"): 1})) + Expect(compiler.profileCompiles).To(Equal(map[types.ProfileID]int{prID("profile-a"): 2})) + + // A delta update mutates the set in place: no recompilation. + store.ProcessUpdate("", ipSetDeltaUpdate("set-a", "10.0.0.4")) + Expect(compiler.policyCompiles[pID("refs-a")]).To(Equal(2)) + Expect(compiler.profileCompiles[prID("profile-a")]).To(Equal(2)) + + // Removing set-b recompiles its referrer (defensively; felix should have + // removed the reference first). + store.ProcessUpdate("", ipSetRemove("set-b")) + Expect(compiler.policyCompiles).To(Equal(map[types.PolicyID]int{pID("refs-a"): 2, pID("refs-b"): 2})) + + // Replacing refs-a with a policy that no longer references set-a must + // drop the reverse-index entry: a further set-a replace recompiles only + // the profile. + store.ProcessUpdate("", policyUpdate("refs-a", &proto.Policy{})) + Expect(compiler.policyCompiles[pID("refs-a")]).To(Equal(3)) + store.ProcessUpdate("", ipSetUpdate("set-a", "10.0.0.5")) + Expect(compiler.policyCompiles[pID("refs-a")]).To(Equal(3)) + Expect(compiler.profileCompiles[prID("profile-a")]).To(Equal(3)) + + // Removing the policies and profile cleans up their index entries: a + // further replace recompiles nothing. + store.ProcessUpdate("", policyRemove("refs-a")) + store.ProcessUpdate("", policyRemove("refs-b")) + store.ProcessUpdate("", profileRemove("profile-a")) + store.ProcessUpdate("", ipSetUpdate("set-a", "10.0.0.6")) + Expect(compiler.policyCompiles[pID("refs-a")]).To(Equal(3)) + Expect(compiler.profileCompiles[prID("profile-a")]).To(Equal(3)) + Expect(store.ipSetPolicyRefs).To(BeEmpty()) + Expect(store.ipSetProfileRefs).To(BeEmpty()) +} + +// TestNilCompilerIsNoOp verifies that a store without a compiler applies +// updates as before and keeps no compiled state. +func TestNilCompilerIsNoOp(t *testing.T) { + RegisterTestingT(t) + + store := NewPolicyStore() + store.ProcessUpdate("", ipSetUpdate("set-a", "10.0.0.1")) + store.ProcessUpdate("", policyUpdate("policy1", &proto.Policy{ + InboundRules: []*proto.Rule{{SrcIpSetIds: []string{"set-a"}}}, + })) + store.ProcessUpdate("", profileUpdate("profile1", &proto.Profile{})) + store.ProcessUpdate("", ipSetUpdate("set-a", "10.0.0.2")) + store.ProcessUpdate("", ipSetRemove("set-a")) + + Expect(store.PolicyByID).To(HaveLen(1)) + Expect(store.ProfileByID).To(HaveLen(1)) + Expect(store.CompiledPolicyByID).To(BeEmpty()) + Expect(store.CompiledProfileByID).To(BeEmpty()) +} + +// TestManagerThreadsCompilerThroughReconnect verifies the manager gives every +// store it creates the compiler — in particular the fresh pending store built +// by OnReconnecting, where forgetting it would silently disable compilation +// after the first reconnect. +func TestManagerThreadsCompilerThroughReconnect(t *testing.T) { + RegisterTestingT(t) + + compiler := newFakeCompiler() + m := NewPolicyStoreManagerWithOpts(WithPolicyCompiler(compiler)) + + sync := func() { + m.DoWithLock(func(s *PolicyStore) { + s.ProcessUpdate("", policyUpdate("policy1", &proto.Policy{})) + }) + m.OnInSync() + } + + sync() + m.DoWithReadLock(func(s *PolicyStore) { + Expect(s.CompiledPolicyByID).To(HaveKey(pID("policy1"))) + }) + + // Reconnect: updates go to a fresh pending store, which must also + // compile; after the in-sync swap the compiled entries are visible. + m.OnReconnecting() + sync() + m.DoWithReadLock(func(s *PolicyStore) { + Expect(s.CompiledPolicyByID).To(HaveKey(pID("policy1"))) + }) + Expect(compiler.policyCompiles[pID("policy1")]).To(Equal(2)) +} + +// TestForEachIPSetRefCoversRuleFields fails when proto.Rule grows a new IP +// set reference field that forEachRuleIPSetRef does not walk: such a field +// would leave compiled policies holding a stale IPSet object after a full +// IPSetUpdate replaces it. Any []string field whose name mentions IP sets +// must be visited. +func TestForEachIPSetRefCoversRuleFields(t *testing.T) { + RegisterTestingT(t) + + rule := &proto.Rule{} + want := map[string]bool{} + v := reflect.ValueOf(rule).Elem() + for i := 0; i < v.NumField(); i++ { + f := v.Type().Field(i) + if !f.IsExported() || f.Type != reflect.TypeOf([]string(nil)) { + continue + } + if !strings.Contains(f.Name, "IpSetIds") && !strings.Contains(f.Name, "IpPortSetIds") { + continue + } + sentinel := "sentinel-" + f.Name + v.Field(i).Set(reflect.ValueOf([]string{sentinel})) + want[sentinel] = true + } + Expect(want).NotTo(BeEmpty()) + + got := map[string]bool{} + forEachRuleIPSetRef([]*proto.Rule{rule}, func(id string) { got[id] = true }) + Expect(got).To(Equal(want), + "forEachRuleIPSetRef missed an IP set reference field on proto.Rule; add it to the walker (and the compiler)") +} diff --git a/app-policy/policystore/process.go b/app-policy/policystore/process.go index 0836a1932b2..5fecea51882 100644 --- a/app-policy/policystore/process.go +++ b/app-policy/policystore/process.go @@ -80,6 +80,9 @@ func (store *PolicyStore) processIPSetUpdate(update *proto.IPSetUpdate) { s.AddString(addr) } store.IPSetByID[update.Id] = s + // Compiled policies hold the replaced IPSet object; recompile them + // against the new one. + store.onIPSetReplaced(update.Id) } } @@ -97,6 +100,8 @@ func (store *PolicyStore) processIPSetDeltaUpdate(update *proto.IPSetDeltaUpdate return // we shouldn't be getting a delta update before we've seen the IPSet } + // A delta mutates the IPSet object in place, so compiled policies holding + // the object see the new membership without recompilation. for _, addr := range update.AddedMembers { s.AddString(addr) } @@ -112,6 +117,11 @@ func (store *PolicyStore) processIPSetRemove(update *proto.IPSetRemove) { }).Debug("Processing IPSetRemove") } delete(store.IPSetByID, update.Id) + // Felix only removes an IP set once no policy references it, so normally + // no policy needs recompiling here. Recompile defensively in case the + // store is out of sync; affected policies keep the missing-set semantics + // of the uncompiled path. + store.onIPSetReplaced(update.Id) } func (store *PolicyStore) processActiveProfileUpdate(update *proto.ActiveProfileUpdate) { @@ -125,7 +135,9 @@ func (store *PolicyStore) processActiveProfileUpdate(update *proto.ActiveProfile return } id := types.ProtoToProfileID(update.GetId()) + old := store.ProfileByID[id] store.ProfileByID[id] = update.Profile + store.onProfileUpdate(id, old, update.Profile) } func (store *PolicyStore) processActiveProfileRemove(update *proto.ActiveProfileRemove) { @@ -139,7 +151,9 @@ func (store *PolicyStore) processActiveProfileRemove(update *proto.ActiveProfile return } id := types.ProtoToProfileID(update.GetId()) + old := store.ProfileByID[id] delete(store.ProfileByID, id) + store.onProfileRemove(id, old) } func (store *PolicyStore) processActivePolicyUpdate(update *proto.ActivePolicyUpdate) { @@ -153,7 +167,9 @@ func (store *PolicyStore) processActivePolicyUpdate(update *proto.ActivePolicyUp return } id := types.ProtoToPolicyID(update.GetId()) + old := store.PolicyByID[id] store.PolicyByID[id] = update.Policy + store.onPolicyUpdate(id, old, update.Policy) } func (store *PolicyStore) processActivePolicyRemove(update *proto.ActivePolicyRemove) { @@ -167,7 +183,9 @@ func (store *PolicyStore) processActivePolicyRemove(update *proto.ActivePolicyRe return } id := types.ProtoToPolicyID(update.GetId()) + old := store.PolicyByID[id] delete(store.PolicyByID, id) + store.onPolicyRemove(id, old) } func (store *PolicyStore) processWorkloadEndpointUpdate(subscriptionType string, update *proto.WorkloadEndpointUpdate) { @@ -180,9 +198,14 @@ func (store *PolicyStore) processWorkloadEndpointUpdate(subscriptionType string, } switch subscriptionType { case "per-pod-policies", "": + old := store.Endpoint store.Endpoint = update.Endpoint + store.onEndpointUpdate(old, update.Endpoint) case "per-host-policies": - store.Endpoints[types.ProtoToWorkloadEndpointID(update.Id)] = update.Endpoint + id := types.ProtoToWorkloadEndpointID(update.Id) + old := store.Endpoints[id] + store.Endpoints[id] = update.Endpoint + store.onEndpointUpdate(old, update.Endpoint) log.Debugf("%d endpoints received so far", len(store.Endpoints)) } } @@ -198,9 +221,12 @@ func (store *PolicyStore) processWorkloadEndpointRemove(subscriptionType string, switch subscriptionType { case "per-pod-policies", "": + store.onEndpointRemove(store.Endpoint) store.Endpoint = nil case "per-host-policies": - delete(store.Endpoints, types.ProtoToWorkloadEndpointID(update.Id)) + id := types.ProtoToWorkloadEndpointID(update.Id) + store.onEndpointRemove(store.Endpoints[id]) + delete(store.Endpoints, id) } } diff --git a/app-policy/policystore/store.go b/app-policy/policystore/store.go index 9f09731c8e8..45bd97a88c6 100644 --- a/app-policy/policystore/store.go +++ b/app-policy/policystore/store.go @@ -42,18 +42,46 @@ type PolicyStore struct { Endpoints map[types.WorkloadEndpointID]*proto.WorkloadEndpoint ServiceAccountByID map[types.ServiceAccountID]*proto.ServiceAccountUpdate NamespaceByID map[types.NamespaceID]*proto.NamespaceUpdate + + // Compiled forms of PolicyByID/ProfileByID, maintained as updates are + // applied when a PolicyCompiler is configured (see compiler.go). An empty + // or absent slot means the policy must be evaluated by interpreting the + // uncompiled policy. + CompiledPolicyByID map[types.PolicyID]*PolicySlot + CompiledProfileByID map[types.ProfileID]*PolicySlot + + // Compiled forms of the endpoints' tier/profile structure, keyed by the + // identity of the endpoint object they were built from — evaluation only + // ever has the endpoint pointer to hand, and a stale copy of a + // since-replaced endpoint must miss rather than match. + CompiledEndpoints map[*proto.WorkloadEndpoint]CompiledEndpoint + + compiler PolicyCompiler + // Reverse index from IP set ID to the policies/profiles whose compiled + // form references it, used to recompile them when the set's object is + // replaced. + ipSetPolicyRefs map[string]map[types.PolicyID]struct{} + ipSetProfileRefs map[string]map[types.ProfileID]struct{} } func NewPolicyStore() *PolicyStore { + return NewPolicyStoreWithCompiler(nil) +} + +func NewPolicyStoreWithCompiler(compiler PolicyCompiler) *PolicyStore { return &PolicyStore{ - IPToIndexes: apptypes.NewIPToEndpointsIndex(), - Endpoints: make(map[types.WorkloadEndpointID]*proto.WorkloadEndpoint), - RWMutex: sync.RWMutex{}, - IPSetByID: make(map[string]IPSet), - ProfileByID: make(map[types.ProfileID]*proto.Profile), - PolicyByID: make(map[types.PolicyID]*proto.Policy), - ServiceAccountByID: make(map[types.ServiceAccountID]*proto.ServiceAccountUpdate), - NamespaceByID: make(map[types.NamespaceID]*proto.NamespaceUpdate), + IPToIndexes: apptypes.NewIPToEndpointsIndex(), + Endpoints: make(map[types.WorkloadEndpointID]*proto.WorkloadEndpoint), + RWMutex: sync.RWMutex{}, + IPSetByID: make(map[string]IPSet), + ProfileByID: make(map[types.ProfileID]*proto.Profile), + PolicyByID: make(map[types.PolicyID]*proto.Policy), + ServiceAccountByID: make(map[types.ServiceAccountID]*proto.ServiceAccountUpdate), + NamespaceByID: make(map[types.NamespaceID]*proto.NamespaceUpdate), + CompiledPolicyByID: make(map[types.PolicyID]*PolicySlot), + CompiledProfileByID: make(map[types.ProfileID]*PolicySlot), + CompiledEndpoints: make(map[*proto.WorkloadEndpoint]CompiledEndpoint), + compiler: compiler, } } @@ -61,6 +89,7 @@ type policyStoreManager struct { current, pending *PolicyStore mu sync.RWMutex toActive bool + compiler PolicyCompiler } type PolicyStoreManager interface { @@ -81,18 +110,26 @@ type PolicyStoreManager interface { type PolicyStoreManagerOption func(*policyStoreManager) +// WithPolicyCompiler configures the manager to create every store (including +// the fresh pending store built on reconnect) with the given compiler, so +// policies are compiled as updates are applied. A nil compiler is a no-op. +func WithPolicyCompiler(compiler PolicyCompiler) PolicyStoreManagerOption { + return func(m *policyStoreManager) { + m.compiler = compiler + } +} + func NewPolicyStoreManager() PolicyStoreManager { return NewPolicyStoreManagerWithOpts() } func NewPolicyStoreManagerWithOpts(opts ...PolicyStoreManagerOption) *policyStoreManager { - psm := &policyStoreManager{ - current: NewPolicyStore(), - pending: NewPolicyStore(), - } + psm := &policyStoreManager{} for _, o := range opts { o(psm) } + psm.current = NewPolicyStoreWithCompiler(psm.compiler) + psm.pending = NewPolicyStoreWithCompiler(psm.compiler) return psm } @@ -140,7 +177,7 @@ func (m *policyStoreManager) OnReconnecting() { defer m.mu.Unlock() // create store - m.pending = NewPolicyStore() + m.pending = NewPolicyStoreWithCompiler(m.compiler) log.Tracef("storeManager OnReconnecting() created new pending store %p", m.pending) // route next writes to pending diff --git a/felix/collector/collector.go b/felix/collector/collector.go index d3b53b4cd82..fb8f2fba033 100644 --- a/felix/collector/collector.go +++ b/felix/collector/collector.go @@ -183,6 +183,11 @@ type collector struct { // policyEvalMinInterval is the minimum time between re-evaluations of one flow. Half the // ticker interval. policyEvalMinInterval time.Duration + // pendingTraceScratch is the buffer policy evaluation appends each rule + // trace to, reused across flows so that an unchanged trace — the common + // case — costs neither an allocation nor a copy. Only touched from the + // stats collection goroutine. + pendingTraceScratch []*calc.RuleID } // newCollector instantiates a new collector. The StartDataplaneStatsCollector function is the only public @@ -202,7 +207,9 @@ func newCollector(lc *calc.LookupsCache, cfg *Config) Collector { } if c.policyStoreManager == nil { - c.policyStoreManager = policystore.NewPolicyStoreManager() + c.policyStoreManager = policystore.NewPolicyStoreManagerWithOpts( + policystore.WithPolicyCompiler(checker.NewPolicyCompiler()), + ) } // Only run the re-evaluation sweep when pending policies are enabled; leaving the ticker nil @@ -1058,7 +1065,7 @@ func (c *collector) evaluatePendingRuleTraceForLocalEp(data *Data, reason policy func (c *collector) evaluatePendingRuleTrace(direction rules.RuleDir, store *policystore.PolicyStore, ep calc.EndpointData, flow TupleAsFlow, ruleIDs *[]*calc.RuleID) { // Get the proto.WorkloadEndpoint, needed for the evaluation, from the policy store. if protoEp := c.lookupProtoWorkloadEndpoint(store, ep.Key()); protoEp != nil { - trace, err := checker.Evaluate(checker.StagedAsEnforced, direction, store, protoEp, &flow) + trace, err := checker.Evaluate(checker.StagedAsEnforced, direction, store, protoEp, &flow, c.pendingTraceScratch[:0]) if err != nil { // Keep the trace we worked out last time: reporting no pending policy at all would be a // stronger claim than we are in a position to make. The checker logs the reason, rate @@ -1066,7 +1073,11 @@ func (c *collector) evaluatePendingRuleTrace(direction rules.RuleDir, store *pol log.WithError(err).Tracef("Pending %s evaluation failed, tuple: %v", direction, flow) return } + c.pendingTraceScratch = trace if !equal(*ruleIDs, trace) { + // Copy rather than hand over the scratch buffer: the Data's slice is + // passed on to the metric reporters, so its backing array must not + // be written to again. *ruleIDs = append([]*calc.RuleID(nil), trace...) log.Tracef("Updated pending %s, tuple: %v, rule trace: %v", direction, flow, ruleIDs) } From 06e5f33a2af7587954fea028162ad7e66521d13f Mon Sep 17 00:00:00 2001 From: Dimitri Nicolopoulos Date: Fri, 11 Sep 2026 19:57:58 -0700 Subject: [PATCH 3/3] app-policy: test the compiled engine against cheapest-first order and staged scope The compiled matchers were reconciled with #13408's cheapest-first criterion order and #13416's staged-policy scope while rebasing #13267. Pin both with tests that fail if either drifts again: - TestCompiledPolicyEquivalence gains an ICMP flow (port 0, so the IP+port keys carry "icmp:0"), an IPv6 flow through the NET set's v6 prefix, named port sets keyed on the destination leg's ",:" (#13174), port ranges combined with named sets, wide and multi-range negations, a protocol combined with ports and with HTTP criteria, and a rule that uses every criterion class at once. - TestCheckTiersPolicyScope, TestCheckStoreReportsWhichPolicyIsMissing, TestEvaluateRecordsStagedPolicyInPendingTraceOnly and TestMalformedHTTPPathOnlyFailsRulesThatReachIt run both engines and assert they agree, so the scope handling, the fail-closed missing policy and the protocol-before-HTTP ordering are checked on the compiled path too. - BenchmarkEvaluateEgressAllowListCompiled measures the egress allow-list fixture from #13408 with compiled policies, next to the interpreted one. --- app-policy/checker/bench_egress_test.go | 28 ++++++++++-- app-policy/checker/check_test.go | 44 +++++++++++------- app-policy/checker/compile_test.go | 60 ++++++++++++++++++++++++- 3 files changed, 110 insertions(+), 22 deletions(-) diff --git a/app-policy/checker/bench_egress_test.go b/app-policy/checker/bench_egress_test.go index c8888a8a4a4..002d3236f8b 100644 --- a/app-policy/checker/bench_egress_test.go +++ b/app-policy/checker/bench_egress_test.go @@ -97,16 +97,30 @@ var egressPortsPerRule = []struct { func BenchmarkEvaluateEgressAllowList(b *testing.B) { // A flow on a port few rules share: rejected on the port comparison nearly everywhere. b.Run("TailPort", func(b *testing.B) { - benchEvaluateEgressAllowList(b, egressTailPortFlow) + benchEvaluateEgressAllowList(b, egressTailPortFlow, false) }) // A flow on the most popular port: ~18% of rules share it and go on to the address check. b.Run("PopularPort", func(b *testing.B) { - benchEvaluateEgressAllowList(b, egressPopularPortFlow) + benchEvaluateEgressAllowList(b, egressPopularPortFlow, false) }) // No rule matches, so the walk covers the whole tier and ends in the tier default deny. // Ordering cannot reduce the scan depth here, only the cost of each rejected rule. b.Run("Denied", func(b *testing.B) { - benchEvaluateEgressAllowList(b, egressDeniedFlow) + benchEvaluateEgressAllowList(b, egressDeniedFlow, false) + }) +} + +// BenchmarkEvaluateEgressAllowListCompiled is BenchmarkEvaluateEgressAllowList with the store's +// policies compiled, as when a PolicyCompiler is configured. +func BenchmarkEvaluateEgressAllowListCompiled(b *testing.B) { + b.Run("TailPort", func(b *testing.B) { + benchEvaluateEgressAllowList(b, egressTailPortFlow, true) + }) + b.Run("PopularPort", func(b *testing.B) { + benchEvaluateEgressAllowList(b, egressPopularPortFlow, true) + }) + b.Run("Denied", func(b *testing.B) { + benchEvaluateEgressAllowList(b, egressDeniedFlow, true) }) } @@ -154,11 +168,14 @@ func egressFlow(destIP string, destPort int32) *MockFlow { } } -func benchEvaluateEgressAllowList(b *testing.B, caseFor egressCaseFunc) { +func benchEvaluateEgressAllowList(b *testing.B, caseFor egressCaseFunc, compiled bool) { _, restoreLogging := withBenchLogging(log.WarnLevel) defer restoreLogging() store, ep, target := buildEgressAllowListStore() + if compiled { + compileStoreForTest(store) + } c := caseFor(target) // Pre-flight outside the timed loop: prove the walk is the one the case intends, so that @@ -251,7 +268,10 @@ func buildEgressAllowListStore() (*policystore.PolicyStore, *proto.WorkloadEndpo tier.EgressPolicies = append(tier.EgressPolicies, policyID) } + // The endpoint goes into the store, as dikastes' per-pod store holds it: evaluation resolves + // an endpoint's compiled form by identity. ep := &proto.WorkloadEndpoint{Tiers: []*proto.TierInfo{tier}} + store.Endpoint = ep return store, ep, target } diff --git a/app-policy/checker/check_test.go b/app-policy/checker/check_test.go index 9bf79e37ce9..0d416d1abb3 100644 --- a/app-policy/checker/check_test.go +++ b/app-policy/checker/check_test.go @@ -1164,7 +1164,10 @@ func TestCheckTiersPolicyScope(t *testing.T) { InboundRules: []*proto.Rule{{Action: "allow"}}, } - st := checkStore(scope.PolicyScope, store, store.Endpoint, rules.RuleDirIngress, + // Both engines: the scope is applied by the tier walk they share, and the + // endpoint is in the store, so the compiled pass takes the compiled-endpoint + // path with the staged policies skipped out of its precomputed slots. + st := checkStoreBothEngines(scope.PolicyScope, store, store.Endpoint, rules.RuleDirIngress, &MockFlow{Protocol: 6, DestPort: 80}) Expect(st.Code).To(Equal(scope.want), "scope %v", scope.PolicyScope) } @@ -1198,7 +1201,7 @@ func TestCheckStoreReportsWhichPolicyIsMissing(t *testing.T) { store := policystore.NewPolicyStore() store.Endpoint = &proto.WorkloadEndpoint{Tiers: tierInfos(policyIDs(missing))} - st := checkStore(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, + st := checkStoreBothEngines(EnforcedOnly, store, store.Endpoint, rules.RuleDirIngress, &MockFlow{Protocol: 6, DestPort: 80}) Expect(st.Code).To(Equal(INTERNAL)) Expect(st.Message).To(Equal("policy np/ns1/policy1 of tier tier1 is missing from the policy store")) @@ -1247,19 +1250,24 @@ func TestEvaluateRecordsStagedPolicyInPendingTraceOnly(t *testing.T) { ep := &proto.WorkloadEndpoint{Tiers: tierInfos(policyIDs(stagedDeny), policyIDs(enforcedAllow))} flow := &MockFlow{Protocol: 6, DestPort: 80} - pending, err := Evaluate(StagedAsEnforced, rules.RuleDirIngress, store, ep, flow, nil) - Expect(err).ToNot(HaveOccurred()) - Expect(pending).To(Equal([]*calc.RuleID{ - calc.NewRuleID(v3.KindStagedGlobalNetworkPolicy, "tier1", "staged-deny", "", - 0, rules.RuleDirIngress, rules.RuleActionDeny), - })) - - enforced, err := Evaluate(EnforcedOnly, rules.RuleDirIngress, store, ep, flow, nil) - Expect(err).ToNot(HaveOccurred()) - Expect(enforced).To(Equal([]*calc.RuleID{ - calc.NewRuleID(v3.KindGlobalNetworkPolicy, "tier2", "allow", "", - 0, rules.RuleDirIngress, rules.RuleActionAllow), - })) + for _, compiled := range []bool{false, true} { + if compiled { + compileStoreForTest(store) + } + pending, err := Evaluate(StagedAsEnforced, rules.RuleDirIngress, store, ep, flow, nil) + Expect(err).ToNot(HaveOccurred()) + Expect(pending).To(Equal([]*calc.RuleID{ + calc.NewRuleID(v3.KindStagedGlobalNetworkPolicy, "tier1", "staged-deny", "", + 0, rules.RuleDirIngress, rules.RuleActionDeny), + }), "compiled=%v", compiled) + + enforced, err := Evaluate(EnforcedOnly, rules.RuleDirIngress, store, ep, flow, nil) + Expect(err).ToNot(HaveOccurred()) + Expect(enforced).To(Equal([]*calc.RuleID{ + calc.NewRuleID(v3.KindGlobalNetworkPolicy, "tier2", "allow", "", + 0, rules.RuleDirIngress, rules.RuleActionAllow), + }), "compiled=%v", compiled) + } } // A rule whose HTTP criteria would reject a malformed request path no longer decides the @@ -1300,13 +1308,15 @@ func TestMalformedHTTPPathOnlyFailsRulesThatReachIt(t *testing.T) { store.PolicyByID[types.ProtoToPolicyID(udpWithPaths)] = &proto.Policy{ InboundRules: []*proto.Rule{httpRule("UDP")}, } - Expect(checkStore(EnforcedOnly, store, ep, rules.RuleDirIngress, flow).Code).To(Equal(OK)) + // Both engines: the compiled rule's matchers are emitted in the same order, so the + // protocol matcher rejects before the HTTP matcher can panic. + Expect(checkStoreBothEngines(EnforcedOnly, store, ep, rules.RuleDirIngress, flow).Code).To(Equal(OK)) // The same rule on TCP does reach them, and the malformed path still fails the request. store.PolicyByID[types.ProtoToPolicyID(udpWithPaths)] = &proto.Policy{ InboundRules: []*proto.Rule{httpRule("TCP")}, } - st := checkStore(EnforcedOnly, store, ep, rules.RuleDirIngress, flow) + st := checkStoreBothEngines(EnforcedOnly, store, ep, rules.RuleDirIngress, flow) Expect(st.Code).To(Equal(INVALID_ARGUMENT)) Expect(st.Message).To(ContainSubstring(badPath)) } diff --git a/app-policy/checker/compile_test.go b/app-policy/checker/compile_test.go index dda34155b35..cff6a6d4ea4 100644 --- a/app-policy/checker/compile_test.go +++ b/app-policy/checker/compile_test.go @@ -83,11 +83,16 @@ func TestCompiledPolicyEquivalence(t *testing.T) { addIPSet(store, "ipset-hit", "10.0.0.1") netSet := policystore.NewIPSet(proto.IPSetUpdate_NET) netSet.AddString("10.0.0.0/24") + netSet.AddString("fd00::/64") store.IPSetByID["netset-hit"] = netSet - // A named port set holds ",:" members: the first flow's source leg. + // Named port sets hold ",:" members: the first flow's source leg, + // and its destination leg. namedPortSet := policystore.NewIPSet(proto.IPSetUpdate_IP_AND_PORT) namedPortSet.AddString("10.0.0.1,tcp:1234") store.IPSetByID["portset-src"] = namedPortSet + namedPortSetDst := policystore.NewIPSet(proto.IPSetUpdate_IP_AND_PORT) + namedPortSetDst.AddString("192.168.1.1,tcp:80") + store.IPSetByID["namedport-dst"] = namedPortSetDst ipPortSet := policystore.NewIPSet(proto.IPSetUpdate_IP_AND_PORT) ipPortSet.AddString("192.168.1.1,tcp:80") store.IPSetByID["ipportset-hit"] = ipPortSet @@ -145,8 +150,26 @@ func TestCompiledPolicyEquivalence(t *testing.T) { DestPort: 80, Protocol: 6, }, + // ICMP: no ports, so every port criterion sees 0, and the IP+port keys + // carry "icmp:0". + &MockFlow{ + SourceIP: net.ParseIP("10.0.0.1"), + DestIP: net.ParseIP("192.168.1.1"), + Protocol: 1, + }, + // IPv6, inside the NET set's v6 prefix: the parsed-IP fast path's v6 branch. + &MockFlow{ + SourceIP: net.ParseIP("fd00::1"), + DestIP: net.ParseIP("fd00::2"), + SourcePort: 1234, + DestPort: 80, + Protocol: 6, + }, } + tcp := &proto.Protocol{NumberOrName: &proto.Protocol_Name{Name: "tcp"}} + udp := &proto.Protocol{NumberOrName: &proto.Protocol_Name{Name: "udp"}} + ruleVariants := []*proto.Rule{ {}, // Empty rule: matches everything. {SrcIpSetIds: []string{"ipset-hit"}}, @@ -214,6 +237,41 @@ func TestCompiledPolicyEquivalence(t *testing.T) { // Combined criteria, mirroring the baseline-policy benchmark's rule shape. {SrcIpSetIds: []string{"ipset-missing"}, SrcPorts: []*proto.PortRange{{First: 65001, Last: 65001}}}, {DstIpSetIds: []string{"netset-hit"}, DstPorts: []*proto.PortRange{{First: 80, Last: 80}}}, + // Named port sets are keyed on the leg's ",:", so a set + // holding the destination leg never matches on the source leg. + {DstNamedPortIpSetIds: []string{"namedport-dst"}}, + {NotDstNamedPortIpSetIds: []string{"namedport-dst"}}, + {SrcNamedPortIpSetIds: []string{"namedport-dst"}}, + // Ranges and named sets on one leg: either side may match; a negated named + // set excludes even when the negated ranges do not. + {DstPorts: []*proto.PortRange{{First: 65001, Last: 65001}}, DstNamedPortIpSetIds: []string{"namedport-dst"}}, + {NotDstPorts: []*proto.PortRange{{First: 65001, Last: 65001}}, NotDstNamedPortIpSetIds: []string{"namedport-dst"}}, + {SrcPorts: []*proto.PortRange{{First: 1, Last: 1023}}, SrcNamedPortIpSetIds: []string{"portset-src"}}, + // Port ranges: several ranges, wide negations, a range excluding what the + // positive ranges admit, and port 0 (the ICMP flow). + {DstPorts: []*proto.PortRange{{First: 1, Last: 79}, {First: 81, Last: 65535}}}, + {NotDstPorts: []*proto.PortRange{{First: 1, Last: 1023}}}, + {SrcPorts: []*proto.PortRange{{First: 1000, Last: 2000}}, NotSrcPorts: []*proto.PortRange{{First: 1234, Last: 1234}}}, + {DstPorts: []*proto.PortRange{{First: 0, Last: 0}}}, + // Protocol with ports: the protocol is checked first, so a rule on the wrong + // protocol is rejected before its ports are consulted. + {Protocol: udp, DstPorts: []*proto.PortRange{{First: 80, Last: 80}}}, + {Protocol: tcp, NotDstPorts: []*proto.PortRange{{First: 80, Last: 80}}}, + {Protocol: &proto.Protocol{NumberOrName: &proto.Protocol_Name{Name: "icmp"}}}, + {Protocol: &proto.Protocol{NumberOrName: &proto.Protocol_Number{Number: 1}}, DstPorts: []*proto.PortRange{{First: 0, Last: 0}}}, + {NotProtocol: &proto.Protocol{NumberOrName: &proto.Protocol_Name{Name: "ICMP"}}}, // Names are case-insensitive. + // Protocol with HTTP criteria: a rule on the wrong protocol never reaches them. + {Protocol: udp, HttpMatch: &proto.HTTPMatch{Methods: []string{"POST"}}}, + {Protocol: tcp, HttpMatch: &proto.HTTPMatch{Methods: []string{"GET"}}}, + // Every criterion class at once. + { + Protocol: tcp, + DstPorts: []*proto.PortRange{{First: 80, Last: 80}}, + DstNet: []string{"192.168.0.0/16", "10.0.0.0/8"}, + SrcIpSetIds: []string{"netset-hit"}, + SrcServiceAccountMatch: &proto.ServiceAccountMatch{Names: []string{"sa-src"}}, + HttpMatch: &proto.HTTPMatch{Methods: []string{"GET"}}, + }, } actions := []string{"allow", "deny", "pass", "log"}