From fd55add50373db6834a9783baffc16eb3c59f03e Mon Sep 17 00:00:00 2001 From: spacewander Date: Wed, 21 Feb 2024 11:45:03 +0800 Subject: [PATCH 1/2] filtermanager: skip method was broken when there are multi plugins This bug is introduced during optimization. Signed-off-by: spacewander --- pkg/filtermanager/filtermanager.go | 10 ++++++++-- pkg/filtermanager/filtermanager_test.go | 26 +++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/pkg/filtermanager/filtermanager.go b/pkg/filtermanager/filtermanager.go index 58af4adb..fcf42ca3 100644 --- a/pkg/filtermanager/filtermanager.go +++ b/pkg/filtermanager/filtermanager.go @@ -350,6 +350,11 @@ func FilterManagerFactory(c interface{}, cb capi.FilterCallbackHandler) capi.Str fm := conf.pool.Get().(*filterManager) fm.callbacks.FilterCallbackHandler = cb + canSkipMethod := fm.canSkipMethod + if canSkipMethod == nil { + canSkipMethod = newSkipMethodsMap() + } + filters := make([]*filterWrapper, len(newConfig)) for i, fc := range newConfig { factory := fc.Factory @@ -357,7 +362,6 @@ func FilterManagerFactory(c interface{}, cb capi.FilterCallbackHandler) capi.Str f := factory(config, fm.callbacks) // Technically, the factory might create different f for different calls. We don't support this edge case for now. if fm.canSkipMethod == nil { - canSkipMethod := newSkipMethodsMap() for meth := range canSkipMethod { ok, err := isMethodFromPassThroughFilter(f, meth) if err != nil { @@ -366,11 +370,13 @@ func FilterManagerFactory(c interface{}, cb capi.FilterCallbackHandler) capi.Str } canSkipMethod[meth] = canSkipMethod[meth] && ok } - fm.canSkipMethod = canSkipMethod } filters[i] = newFilterWrapper(fc.Name, f) } + if fm.canSkipMethod == nil { + fm.canSkipMethod = canSkipMethod + } fm.filters = filters if conf.consumerFiltersEndAt != 0 { diff --git a/pkg/filtermanager/filtermanager_test.go b/pkg/filtermanager/filtermanager_test.go index 1f0298bd..bc7d4220 100644 --- a/pkg/filtermanager/filtermanager_test.go +++ b/pkg/filtermanager/filtermanager_test.go @@ -343,6 +343,32 @@ func (f *addReqFilter) DecodeHeaders(headers api.RequestHeaderMap, endStream boo return api.Continue } +func TestSkipMethodWhenThereAreMultiFilters(t *testing.T) { + cb := envoy.NewCAPIFilterCallbackHandler() + config := initFilterManagerConfig("ns") + config.authnFiltersEndAt = 1 + config.current = []*model.ParsedFilterConfig{ + { + Name: "add_req", + Factory: addReqFactory, + ParsedConfig: addReqConf{ + hdrName: "x-htnn-route", + }, + }, + { + Name: "on_log", + Factory: onLogFactory, + }, + } + + for i := 0; i < 2; i++ { + m := FilterManagerFactory(config, cb).(*filterManager) + assert.Equal(t, false, m.canSkipOnLog) + assert.Equal(t, false, m.canSkipDecodeHeaders) + assert.Equal(t, true, m.canSkipDecodeData) + } +} + func TestFiltersFromConsumer(t *testing.T) { cb := envoy.NewCAPIFilterCallbackHandler() config := initFilterManagerConfig("ns") From 182833f3082ca98c4fcc5bf2a94bac7cfac22684 Mon Sep 17 00:00:00 2001 From: spacewander Date: Wed, 21 Feb 2024 11:57:57 +0800 Subject: [PATCH 2/2] fix test Signed-off-by: spacewander --- pkg/filtermanager/filtermanager_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/filtermanager/filtermanager_test.go b/pkg/filtermanager/filtermanager_test.go index bc7d4220..0bfc350c 100644 --- a/pkg/filtermanager/filtermanager_test.go +++ b/pkg/filtermanager/filtermanager_test.go @@ -346,7 +346,6 @@ func (f *addReqFilter) DecodeHeaders(headers api.RequestHeaderMap, endStream boo func TestSkipMethodWhenThereAreMultiFilters(t *testing.T) { cb := envoy.NewCAPIFilterCallbackHandler() config := initFilterManagerConfig("ns") - config.authnFiltersEndAt = 1 config.current = []*model.ParsedFilterConfig{ { Name: "add_req",