Skip to content

Commit 67de01f

Browse files
squeedaanm
squeed
authored and
aanm
committed
fqdn: clean up regex cache
Switch the regex cache to the maintained (and already imported) hashicorp/go-lru package. Initialize the cache in `init()`, rather than requiring every single test and binary to initialize it. One nice property of go-lru is that it doesn't initially allocate a buffer of the given size, so we don't need to restrict the size to 1 for one-off CLI programs. The default size is fine. This allows us to remove the initialization code from lots and lots and lots of unit tests. Hooray. Signed-off-by: Casey Callendrello <cdc@isovalent.com>
1 parent 886a511 commit 67de01f

24 files changed

Lines changed: 27 additions & 229 deletions

‎cilium-dbg/cmd/policy.go‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import (
1717

1818
"github.com/spf13/cobra"
1919

20-
"github.com/cilium/cilium/pkg/fqdn/re"
2120
"github.com/cilium/cilium/pkg/logging/logfields"
2221
"github.com/cilium/cilium/pkg/policy/api"
2322
"github.com/cilium/cilium/pkg/safeio"
@@ -37,14 +36,6 @@ var (
3736

3837
func init() {
3938
RootCmd.AddCommand(PolicyCmd)
40-
41-
// Initialize LRU here because the policy subcommands (validate, import)
42-
// will call down to sanitizing the FQDN rules which contains regexes.
43-
// Regexes need to be compiled as part of the FQDN rule validation.
44-
//
45-
// It's not necessary to pass a useful size here because it's not
46-
// necessary to cache regexes in a short-lived binary (CLI).
47-
re.InitRegexCompileLRU(log, 1)
4839
}
4940

5041
func getContext(content []byte, offset int64) (int, string, int) {

‎daemon/cmd/daemon_main.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,7 @@ func InitGlobalFlags(logger *slog.Logger, cmd *cobra.Command, vp *viper.Viper) {
734734
flags.Duration(option.FQDNProxyResponseMaxDelay, defaults.FQDNProxyResponseMaxDelay, "The maximum time the DNS proxy holds an allowed DNS response before sending it along. Responses are sent as soon as the datapath is updated with the new IP information.")
735735
option.BindEnv(vp, option.FQDNProxyResponseMaxDelay)
736736

737-
flags.Int(option.FQDNRegexCompileLRUSize, defaults.FQDNRegexCompileLRUSize, "Size of the FQDN regex compilation LRU. Useful for heavy but repeated DNS L7 rules with MatchName or MatchPattern")
737+
flags.Uint(option.FQDNRegexCompileLRUSize, defaults.FQDNRegexCompileLRUSize, "Size of the FQDN regex compilation LRU. Useful for heavy but repeated DNS L7 rules with MatchName or MatchPattern")
738738
flags.MarkHidden(option.FQDNRegexCompileLRUSize)
739739
option.BindEnv(vp, option.FQDNRegexCompileLRUSize)
740740

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ require (
4848
github.com/go-openapi/swag v0.23.1
4949
github.com/go-openapi/validate v0.24.0
5050
github.com/gogo/protobuf v1.3.2
51-
github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8
5251
github.com/google/cel-go v0.25.0
5352
github.com/google/go-cmp v0.7.0
5453
github.com/google/go-github/v73 v73.0.0
@@ -203,6 +202,7 @@ require (
203202
github.com/gobwas/glob v0.2.3 // indirect
204203
github.com/godbus/dbus/v5 v5.1.0 // indirect
205204
github.com/golang-jwt/jwt/v5 v5.2.2 // indirect
205+
github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8 // indirect
206206
github.com/golang/protobuf v1.5.4 // indirect
207207
github.com/google/btree v1.1.3 // indirect
208208
github.com/google/certificate-transparency-go v1.1.7 // indirect

‎operator/pkg/networkpolicy/cell.go‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
apierrors "k8s.io/apimachinery/pkg/api/errors"
2121
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2222

23-
"github.com/cilium/cilium/pkg/fqdn/re"
2423
cilium_api_v2 "github.com/cilium/cilium/pkg/k8s/apis/cilium.io/v2"
2524
k8s_client "github.com/cilium/cilium/pkg/k8s/client"
2625
"github.com/cilium/cilium/pkg/k8s/resource"
@@ -80,13 +79,6 @@ func registerPolicyValidator(params PolicyParams) {
8079
return
8180
}
8281

83-
// LRU size of 1 since we are only doing one-off validation of policies and
84-
// the FQDN regexes are not referenced again.
85-
if err := re.InitRegexCompileLRU(params.Logger, 1); err != nil {
86-
params.Logger.Error("CNP / CCNP validator can't run due to failure in initializing regex LRU cache.", logfields.Error, err)
87-
return
88-
}
89-
9082
pv := &policyValidator{
9183
params: &params,
9284
}

‎pkg/fqdn/bootstrap/dns_proxy.go‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,7 @@ type dnsProxyParams struct {
4545

4646
// newDNSProxy initializes the DNS l7 proxy.
4747
func newDNSProxy(params dnsProxyParams) (proxy.DNSProxier, error) {
48-
if err := re.InitRegexCompileLRU(params.Logger, option.Config.FQDNRegexCompileLRUSize); err != nil {
49-
// can only happen if LRUSize is negative
50-
return nil, err
51-
}
48+
re.Resize(params.Logger, option.Config.FQDNRegexCompileLRUSize)
5249

5350
// Do not start the proxy in dry mode or if L7 proxy is disabled.
5451
// The proxy would not get any traffic in the dry mode anyway, and some of the socket

‎pkg/fqdn/cache_test.go‎

Lines changed: 0 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,13 @@ import (
1818
"k8s.io/apimachinery/pkg/util/sets"
1919

2020
"github.com/cilium/cilium/pkg/defaults"
21-
"github.com/cilium/cilium/pkg/fqdn/re"
2221
"github.com/cilium/cilium/pkg/ip"
2322
)
2423

2524
// TestUpdateLookup tests that we can insert DNS data and retrieve it. We
2625
// iterate through time, ensuring that data is expired as appropriate. We also
2726
// insert redundant DNS entries that should not change the output.
2827
func TestUpdateLookup(t *testing.T) {
29-
logger := hivetest.Logger(t)
30-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
31-
3228
name := "test.com"
3329
now := time.Now()
3430
cache := NewDNSCache(0)
@@ -79,9 +75,6 @@ func TestUpdateLookup(t *testing.T) {
7975

8076
// TestDelete tests that we can forcibly clear parts of the cache.
8177
func TestDelete(t *testing.T) {
82-
logger := hivetest.Logger(t)
83-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
84-
8578
names := map[string]netip.Addr{
8679
"test1.com": netip.MustParseAddr("2.2.2.1"),
8780
"test2.com": netip.MustParseAddr("2.2.2.2"),
@@ -152,9 +145,6 @@ func TestDelete(t *testing.T) {
152145
}
153146

154147
func Test_forceExpiredByNames(t *testing.T) {
155-
logger := hivetest.Logger(t)
156-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
157-
158148
names := []string{"test1.com", "test2.com"}
159149
cache := NewDNSCache(0)
160150
for i := 1; i < 4; i++ {
@@ -171,9 +161,6 @@ func Test_forceExpiredByNames(t *testing.T) {
171161
}
172162

173163
func TestReverseUpdateLookup(t *testing.T) {
174-
logger := hivetest.Logger(t)
175-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
176-
177164
names := map[string]netip.Addr{
178165
"test1.com": netip.MustParseAddr("2.2.2.1"),
179166
"test2.com": netip.MustParseAddr("2.2.2.2"),
@@ -241,9 +228,6 @@ func TestReverseUpdateLookup(t *testing.T) {
241228
}
242229

243230
func TestJSONMarshal(t *testing.T) {
244-
logger := hivetest.Logger(t)
245-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
246-
247231
names := map[string]netip.Addr{
248232
"test1.com": netip.MustParseAddr("2.2.2.1"),
249233
"test2.com": netip.MustParseAddr("2.2.2.2"),
@@ -296,9 +280,6 @@ func TestJSONMarshal(t *testing.T) {
296280
}
297281

298282
func TestCountIPs(t *testing.T) {
299-
logger := hivetest.Logger(t)
300-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
301-
302283
names := map[string]netip.Addr{
303284
"test1.com": netip.MustParseAddr("1.1.1.1"),
304285
"test2.com": netip.MustParseAddr("2.2.2.2"),
@@ -386,9 +367,6 @@ func makeEntries(now time.Time, live, redundant, expired uint32) (entries []*cac
386367

387368
// Note: each "op" works on size things
388369
func BenchmarkGetIPs(b *testing.B) {
389-
logger := hivetest.Logger(b)
390-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
391-
392370
now := time.Now()
393371
cache := NewDNSCache(0)
394372
cache.Update(now, "test.com", []netip.Addr{netip.MustParseAddr("1.2.3.4")}, 60)
@@ -404,9 +382,6 @@ func BenchmarkGetIPs(b *testing.B) {
404382

405383
// Note: each "op" works on size things
406384
func BenchmarkUpdateIPs(b *testing.B) {
407-
logger := hivetest.Logger(b)
408-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
409-
410385
for b.Loop() {
411386
b.StopTimer()
412387
now := time.Now()
@@ -456,9 +431,6 @@ func BenchmarkMarshalJSON1000Repeat2(b *testing.B) {
456431
// Note: It assumes the JSON only uses data in DNSCache.forward when generating
457432
// the data. Changes to the implementation need to also change this benchmark.
458433
func benchmarkMarshalJSON(b *testing.B, numDNSEntries int) {
459-
logger := hivetest.Logger(b)
460-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
461-
462434
ips := makeIPs(uint32(numIPsPerEntry))
463435

464436
cache := NewDNSCache(0)
@@ -478,9 +450,6 @@ func benchmarkMarshalJSON(b *testing.B, numDNSEntries int) {
478450
// Note: It assumes the JSON only uses data in DNSCache.forward when generating
479451
// the data. Changes to the implementation need to also change this benchmark.
480452
func benchmarkUnmarshalJSON(b *testing.B, numDNSEntries int) {
481-
logger := hivetest.Logger(b)
482-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
483-
484453
ips := makeIPs(uint32(numIPsPerEntry))
485454

486455
cache := NewDNSCache(0)
@@ -505,9 +474,6 @@ func benchmarkUnmarshalJSON(b *testing.B, numDNSEntries int) {
505474
}
506475

507476
func TestTTLInsertWithMinValue(t *testing.T) {
508-
logger := hivetest.Logger(t)
509-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
510-
511477
now := time.Now()
512478
cache := NewDNSCache(60)
513479
cache.Update(now, "test.com", []netip.Addr{netip.MustParseAddr("1.2.3.4")}, 3)
@@ -529,9 +495,6 @@ func TestTTLInsertWithMinValue(t *testing.T) {
529495
}
530496

531497
func TestTTLInsertWithZeroValue(t *testing.T) {
532-
logger := hivetest.Logger(t)
533-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
534-
535498
now := time.Now()
536499
cache := NewDNSCache(0)
537500
cache.Update(now, "test.com", []netip.Addr{netip.MustParseAddr("1.2.3.4")}, 10)
@@ -553,9 +516,6 @@ func TestTTLInsertWithZeroValue(t *testing.T) {
553516
}
554517

555518
func TestTTLCleanupEntries(t *testing.T) {
556-
logger := hivetest.Logger(t)
557-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
558-
559519
cache := NewDNSCache(0)
560520
cache.Update(now, "test.com", []netip.Addr{netip.MustParseAddr("1.2.3.4")}, 3)
561521
require.Len(t, cache.cleanup, 1)
@@ -566,9 +526,6 @@ func TestTTLCleanupEntries(t *testing.T) {
566526
}
567527

568528
func TestTTLCleanupWithoutForward(t *testing.T) {
569-
logger := hivetest.Logger(t)
570-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
571-
572529
cache := NewDNSCache(0)
573530
now := time.Now()
574531
cache.cleanup[now.Unix()] = []string{"test.com"}
@@ -580,9 +537,6 @@ func TestTTLCleanupWithoutForward(t *testing.T) {
580537
}
581538

582539
func TestOverlimitEntriesWithValidLimit(t *testing.T) {
583-
logger := hivetest.Logger(t)
584-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
585-
586540
limit := 5
587541
cache := NewDNSCacheWithLimit(0, limit)
588542

@@ -603,9 +557,6 @@ func TestOverlimitEntriesWithValidLimit(t *testing.T) {
603557
}
604558

605559
func TestOverlimitEntriesWithoutLimit(t *testing.T) {
606-
logger := hivetest.Logger(t)
607-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
608-
609560
limit := 0
610561
cache := NewDNSCacheWithLimit(0, limit)
611562
for i := range 5 {
@@ -617,9 +568,6 @@ func TestOverlimitEntriesWithoutLimit(t *testing.T) {
617568
}
618569

619570
func TestGCOverlimitAfterTTLCleanup(t *testing.T) {
620-
logger := hivetest.Logger(t)
621-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
622-
623571
limit := 5
624572
cache := NewDNSCacheWithLimit(0, limit)
625573

@@ -641,9 +589,6 @@ func TestGCOverlimitAfterTTLCleanup(t *testing.T) {
641589
}
642590

643591
func TestOverlimitAfterDeleteForwardEntry(t *testing.T) {
644-
logger := hivetest.Logger(t)
645-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
646-
647592
// Validate if something delete the forward entry no invalid key access on
648593
// CG operation
649594
dnsCache := NewDNSCache(0)
@@ -653,9 +598,6 @@ func TestOverlimitAfterDeleteForwardEntry(t *testing.T) {
653598
}
654599

655600
func assertZombiesContain(t *testing.T, zombies []*DNSZombieMapping, expected map[string][]string) {
656-
logger := hivetest.Logger(t)
657-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
658-
659601
t.Helper()
660602
require.Lenf(t, zombies, len(expected), "Different number of zombies than expected: %+v", zombies)
661603

@@ -675,7 +617,6 @@ func assertZombiesContain(t *testing.T, zombies []*DNSZombieMapping, expected ma
675617

676618
func TestZombiesSiblingsGC(t *testing.T) {
677619
logger := hivetest.Logger(t)
678-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
679620

680621
now := time.Now()
681622
zombies := NewDNSZombieMappings(logger, defaults.ToFQDNsMaxDeferredConnectionDeletes, defaults.ToFQDNsMaxIPsPerHost)
@@ -706,7 +647,6 @@ func TestZombiesSiblingsGC(t *testing.T) {
706647

707648
func TestZombiesGC(t *testing.T) {
708649
logger := hivetest.Logger(t)
709-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
710650

711651
now := time.Now()
712652
zombies := NewDNSZombieMappings(logger, defaults.ToFQDNsMaxDeferredConnectionDeletes, defaults.ToFQDNsMaxIPsPerHost)
@@ -808,7 +748,6 @@ func TestZombiesGC(t *testing.T) {
808748

809749
func TestZombiesGCOverLimit(t *testing.T) {
810750
logger := hivetest.Logger(t)
811-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
812751

813752
now := time.Now()
814753
zombies := NewDNSZombieMappings(logger, defaults.ToFQDNsMaxDeferredConnectionDeletes, 1)
@@ -835,7 +774,6 @@ func TestZombiesGCOverLimit(t *testing.T) {
835774

836775
func TestZombiesGCOverLimitWithCTGC(t *testing.T) {
837776
logger := hivetest.Logger(t)
838-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
839777

840778
now := time.Now()
841779
afterNow := now.Add(1 * time.Nanosecond)
@@ -873,7 +811,6 @@ func TestZombiesGCOverLimitWithCTGC(t *testing.T) {
873811

874812
func TestZombiesGCDeferredDeletes(t *testing.T) {
875813
logger := hivetest.Logger(t)
876-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
877814

878815
now := time.Now()
879816
zombies := NewDNSZombieMappings(logger, defaults.ToFQDNsMaxDeferredConnectionDeletes, defaults.ToFQDNsMaxIPsPerHost)
@@ -935,7 +872,6 @@ func TestZombiesGCDeferredDeletes(t *testing.T) {
935872

936873
func TestZombiesForceExpire(t *testing.T) {
937874
logger := hivetest.Logger(t)
938-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
939875

940876
now := time.Now()
941877
zombies := NewDNSZombieMappings(logger, defaults.ToFQDNsMaxDeferredConnectionDeletes, defaults.ToFQDNsMaxIPsPerHost)
@@ -1008,7 +944,6 @@ func TestZombiesForceExpire(t *testing.T) {
1008944

1009945
func TestCacheToZombiesGCCascade(t *testing.T) {
1010946
logger := hivetest.Logger(t)
1011-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
1012947

1013948
now := time.Now()
1014949
cache := NewDNSCache(0)
@@ -1056,7 +991,6 @@ func TestCacheToZombiesGCCascade(t *testing.T) {
1056991

1057992
func TestZombiesDumpAlive(t *testing.T) {
1058993
logger := hivetest.Logger(t)
1059-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
1060994

1061995
now := time.Now()
1062996
zombies := NewDNSZombieMappings(logger, defaults.ToFQDNsMaxDeferredConnectionDeletes, defaults.ToFQDNsMaxIPsPerHost)
@@ -1134,7 +1068,6 @@ func TestZombiesDumpAlive(t *testing.T) {
11341068

11351069
func TestOverlimitPreferNewerEntries(t *testing.T) {
11361070
logger := hivetest.Logger(t)
1137-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
11381071

11391072
toFQDNsMinTTL := 100
11401073
toFQDNsMaxIPsPerHost := 5
@@ -1223,7 +1156,6 @@ func TestOverlimitPreferNewerEntries(t *testing.T) {
12231156
// the front).
12241157
func TestPerHostLimitBehaviourForS3(t *testing.T) {
12251158
logger := hivetest.Logger(t)
1226-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
12271159

12281160
someDomain := "s3.example.com"
12291161
maxIPs := 5
@@ -1376,9 +1308,6 @@ func validateZombieSort(t *testing.T, zombies []*DNSZombieMapping) {
13761308
}
13771309

13781310
func Test_sortZombieMappingSlice(t *testing.T) {
1379-
logger := hivetest.Logger(t)
1380-
re.InitRegexCompileLRU(logger, defaults.FQDNRegexCompileLRUSize)
1381-
13821311
// Create three moments in time, so we can have before, equal and after.
13831312
moments := []time.Time{
13841313
time.Date(2001, time.January, 1, 1, 1, 1, 0, time.Local),

0 commit comments

Comments
 (0)