Skip to content

Commit 1403244

Browse files
committed
waf: calls WAF.Close()
Signed-off-by: Matteo Pace <pace.matteo96@gmail.com>
1 parent 0092747 commit 1403244

2 files changed

Lines changed: 123 additions & 6 deletions

File tree

extensions/composer/waf/waf.go

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
package waf
88

99
import (
10+
"fmt"
11+
"io"
1012
"net"
1113
"strconv"
1214
"strings"
@@ -26,18 +28,50 @@ const (
2628
metadataKeyBlockPhase = "block_phase"
2729
)
2830

31+
// wafRegistry tracks all WAF instances created for a single Envoy config version
32+
// (main config + per-route overrides) so they can be closed together in OnDestroy
33+
// to release memoized regex patterns from Coraza's global cache.
34+
type wafRegistry struct {
35+
wafs []coraza.WAF
36+
}
37+
38+
func (r *wafRegistry) add(w coraza.WAF) {
39+
if w != nil {
40+
r.wafs = append(r.wafs, w)
41+
}
42+
}
43+
44+
func (r *wafRegistry) closeAll(l *logger.Logger) {
45+
for _, w := range r.wafs {
46+
if c, ok := w.(io.Closer); ok {
47+
if err := c.Close(); err != nil {
48+
l.Error(fmt.Sprintf("waf: error closing WAF instance: %v", err))
49+
}
50+
}
51+
}
52+
}
53+
2954
type wafPluginFactory struct {
3055
shared.EmptyHttpFilterFactory
31-
config coraza.WAF
32-
mode waf.WAFMode
33-
metrics *metrics
56+
config coraza.WAF
57+
mode waf.WAFMode
58+
metrics *metrics
59+
registry *wafRegistry
3460
}
3561

3662
type perRouteWafPluginConfig struct {
3763
config coraza.WAF
3864
mode waf.WAFMode
3965
}
4066

67+
// OnDestroy releases memoized regex caches held by all WAF instances (main + per-route)
68+
// created by this factory. Called by Envoy when the factory is destroyed (e.g. config update).
69+
func (f *wafPluginFactory) OnDestroy() {
70+
l := logger.GetLogger()
71+
l.Sugar().Infof("waf: closing %d WAF instances", len(f.registry.wafs))
72+
f.registry.closeAll(l)
73+
}
74+
4175
func (f *wafPluginFactory) Create(handle shared.HttpFilterHandle) shared.HttpFilter {
4276
config := f.config
4377
mode := f.mode
@@ -66,6 +100,9 @@ func (f *wafPluginFactory) Create(handle shared.HttpFilterHandle) shared.HttpFil
66100

67101
type wafPluginConfigFactory struct {
68102
shared.EmptyHttpFilterConfigFactory
103+
// registry is shared between the config factory and the plugin factory it creates,
104+
// so per-route WAFs added after the factory is created can still be closed on destroy.
105+
registry *wafRegistry
69106
}
70107

71108
func (f *wafPluginConfigFactory) Create(
@@ -88,10 +125,14 @@ func (f *wafPluginConfigFactory) Create(
88125
handle.Log(shared.LogLevelInfo, "waf: empty filter config")
89126
}
90127

128+
f.registry = &wafRegistry{}
129+
f.registry.add(wafConfig)
130+
91131
return &wafPluginFactory{
92-
config: wafConfig,
93-
mode: mode,
94-
metrics: newMetrics(handle),
132+
config: wafConfig,
133+
mode: mode,
134+
metrics: newMetrics(handle),
135+
registry: f.registry,
95136
}, nil
96137
}
97138

@@ -100,6 +141,9 @@ func (f *wafPluginConfigFactory) CreatePerRoute(unparsedConfig []byte) (any, err
100141
if err != nil {
101142
return nil, err
102143
}
144+
if f.registry != nil {
145+
f.registry.add(wafConfig)
146+
}
103147
return &perRouteWafPluginConfig{
104148
config: wafConfig,
105149
mode: mode,

extensions/composer/waf/waf_test.go

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@ package waf
77

88
import (
99
"encoding/json"
10+
"fmt"
1011
"io"
12+
"runtime"
13+
"runtime/debug"
1114
"strconv"
1215
"testing"
1316

@@ -1683,3 +1686,73 @@ func Test_PerRouteConfigBlocksRequest(t *testing.T) {
16831686

16841687
wafPlugin.OnStreamComplete()
16851688
}
1689+
1690+
// Verifies that OnDestroy() keeps heap growth bounded across
1691+
// config reloads by releasing Coraza's memoized regex patterns.
1692+
func Test_OnDestroyReleasesMemory(t *testing.T) {
1693+
const (
1694+
reloads = 30
1695+
rulesPerWAF = 50
1696+
)
1697+
1698+
ruleID := 1
1699+
makeDirectives := func() []string {
1700+
directives := []string{"SecRuleEngine On"}
1701+
for range rulesPerWAF {
1702+
directives = append(directives, fmt.Sprintf(
1703+
`SecRule REQUEST_URI "@rx ^/reload/%d/path$" "id:%d,phase:1,pass,nolog"`, ruleID, ruleID,
1704+
))
1705+
ruleID++
1706+
}
1707+
return directives
1708+
}
1709+
1710+
heapAlloc := func() uint64 {
1711+
// GC twice then scavenge: the first GC collects most objects, the second
1712+
// collects anything promoted or finalized during the first pass, and
1713+
// FreeOSMemory forces the runtime to return freed pages to the OS so that
1714+
// HeapAlloc reflects only genuinely live allocations.
1715+
runtime.GC()
1716+
runtime.GC()
1717+
debug.FreeOSMemory()
1718+
var m runtime.MemStats
1719+
runtime.ReadMemStats(&m)
1720+
return m.HeapAlloc
1721+
}
1722+
1723+
baseline := heapAlloc()
1724+
1725+
for range reloads {
1726+
// Use a fresh controller per reload so gomock state doesn't accumulate
1727+
// and skew the heap measurement.
1728+
ctrl := gomock.NewController(t)
1729+
1730+
mainConfig, err := json.Marshal(map[string]any{"directives": makeDirectives(), "mode": "FULL"})
1731+
require.NoError(t, err)
1732+
perRouteConfig, err := json.Marshal(map[string]any{"directives": makeDirectives(), "mode": "FULL"})
1733+
require.NoError(t, err)
1734+
1735+
configHandle := mocks.NewMockHttpFilterConfigHandle(ctrl)
1736+
configHandle.EXPECT().DefineCounter("waf_tx_total").Return(shared.MetricID(1), shared.MetricsSuccess)
1737+
configHandle.EXPECT().DefineCounter("waf_tx_blocked", "authority", "phase", "rule_id").Return(shared.MetricID(2), shared.MetricsSuccess)
1738+
1739+
configFactory := &wafPluginConfigFactory{}
1740+
factory, err := configFactory.Create(configHandle, mainConfig)
1741+
require.NoError(t, err)
1742+
_, err = configFactory.CreatePerRoute(perRouteConfig)
1743+
require.NoError(t, err)
1744+
1745+
// Simulate Envoy destroying the old factory when a config reload arrives.
1746+
factory.OnDestroy()
1747+
ctrl.Finish()
1748+
}
1749+
1750+
const maxGrowthKB = int64(1024)
1751+
heapGrowth := (int64(heapAlloc()) - int64(baseline)) / 1024
1752+
1753+
// Without OnDestroy releasing memoized caches: 30 reloads × 2 WAFs × 50 unique
1754+
// regexes leaks ~11+ MB. With Close(), heap growth stays under 1 MB.
1755+
assert.LessOrEqual(t, heapGrowth, maxGrowthKB,
1756+
"heap grew %d KB after %d reloads, expected ≤ %d KB: OnDestroy must call Close() on all WAF instances to release Coraza's memoized regex cache",
1757+
heapGrowth, reloads, maxGrowthKB)
1758+
}

0 commit comments

Comments
 (0)