From 63aabc94a947bee910a46eb7943ae0ba121ca5cf Mon Sep 17 00:00:00 2001 From: spacewander Date: Thu, 31 Oct 2024 12:13:57 +0800 Subject: [PATCH] fix: previous error log is too strict Signed-off-by: spacewander --- api/pkg/filtermanager/filtermanager.go | 12 +++-- api/tests/integration/consumer_test.go | 51 +++++++++++++++++++ api/tests/integration/test_plugins.go | 43 ++++++++++++++++ .../developer-guide/plugin_development.md | 2 +- .../developer-guide/plugin_development.md | 2 +- 5 files changed, 103 insertions(+), 7 deletions(-) diff --git a/api/pkg/filtermanager/filtermanager.go b/api/pkg/filtermanager/filtermanager.go index 862d305a..c86546f6 100644 --- a/api/pkg/filtermanager/filtermanager.go +++ b/api/pkg/filtermanager/filtermanager.go @@ -195,11 +195,13 @@ func FilterManagerFactory(c interface{}, cb capi.FilterCallbackHandler) (streamF api.LogErrorf("plugin %s has DecodeRequest but not DecodeHeaders. To run DecodeRequest, we need to return api.WaitAllData from DecodeHeaders", fc.Name) } - p := pkgPlugins.LoadPluginType(fc.Name) - if p != nil { - order := p.Order() - if order.Position <= pkgPlugins.OrderPositionAuthn { - api.LogErrorf("plugin %s has DecodeRequest which is not supported because the order of plugin", fc.Name) + if conf.consumerFiltersEndAt != 0 { + p := pkgPlugins.LoadPluginType(fc.Name) + if p != nil { + order := p.Order() + if order.Position <= pkgPlugins.OrderPositionAuthn { + api.LogErrorf("plugin %s has DecodeRequest which is not supported because the order of plugin", fc.Name) + } } } } diff --git a/api/tests/integration/consumer_test.go b/api/tests/integration/consumer_test.go index a8152d0f..a367fe68 100644 --- a/api/tests/integration/consumer_test.go +++ b/api/tests/integration/consumer_test.go @@ -225,3 +225,54 @@ func TestConsumerFilterNotAfterConsumerRunInLaterPhase(t *testing.T) { }) } } + +func TestConsumerFilterNotAfterConsumerRunDecodeRequest(t *testing.T) { + dp, err := dataplane.StartDataPlane(t, &dataplane.Option{ + Bootstrap: dataplane.Bootstrap().AddConsumer("marvin", map[string]interface{}{ + "auth": map[string]interface{}{ + "consumer": `{"name":"marvin"}`, + }, + }), + NoErrorLogCheck: true, + ExpectLogPattern: []string{ + `plugin beforeConsumerAndHasDecodeRequest has DecodeRequest which is not supported because the order of plugin`, + }, + }) + if err != nil { + t.Fatalf("failed to start data plane: %v", err) + return + } + defer dp.Stop() + + tests := []struct { + name string + config *filtermanager.FilterManagerConfig + run func(t *testing.T) + }{ + { + name: "authn & exec", + config: controlplane.NewPluginConfig([]*model.FilterConfig{ + { + Name: "beforeConsumerAndHasDecodeRequest", + Config: map[string]interface{}{}, + }, + { + Name: "consumer", + Config: map[string]interface{}{}, + }, + }), + run: func(t *testing.T) { + resp, _ := dp.Get("/echo", http.Header{"Authorization": []string{"marvin"}}) + assert.Equal(t, 200, resp.StatusCode) + assert.Equal(t, []string{"beforeConsumerAndHasDecodeRequest"}, resp.Header.Values("Echo-Run")) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + controlPlane.UseGoPluginConfig(t, tt.config, dp) + tt.run(t) + }) + } +} diff --git a/api/tests/integration/test_plugins.go b/api/tests/integration/test_plugins.go index 9fe9359d..7245af38 100644 --- a/api/tests/integration/test_plugins.go +++ b/api/tests/integration/test_plugins.go @@ -532,6 +532,48 @@ func (f *beforeConsumerAndHasOtherMethodFilter) EncodeHeaders(headers api.Respon return api.Continue } +type beforeConsumerAndHasDecodeRequestPlugin struct { + plugins.PluginMethodDefaultImpl +} + +func (p *beforeConsumerAndHasDecodeRequestPlugin) Order() plugins.PluginOrder { + return plugins.PluginOrder{ + Position: plugins.OrderPositionAccess, + } +} + +func (p *beforeConsumerAndHasDecodeRequestPlugin) Config() api.PluginConfig { + return &Config{} +} + +func (p *beforeConsumerAndHasDecodeRequestPlugin) Factory() api.FilterFactory { + return beforeConsumerAndHasDecodeRequestFactory +} + +func beforeConsumerAndHasDecodeRequestFactory(c interface{}, callbacks api.FilterCallbackHandler) api.Filter { + return &beforeConsumerAndHasDecodeRequestFilter{ + callbacks: callbacks, + config: c.(*Config), + } +} + +type beforeConsumerAndHasDecodeRequestFilter struct { + api.PassThroughFilter + + callbacks api.FilterCallbackHandler + config *Config +} + +func (f *beforeConsumerAndHasDecodeRequestFilter) DecodeHeaders(headers api.RequestHeaderMap, endStream bool) api.ResultAction { + headers.Add("run", "beforeConsumerAndHasDecodeRequest") + return api.Continue +} + +func (f *beforeConsumerAndHasDecodeRequestFilter) DecodeRequest(headers api.RequestHeaderMap, data api.BufferInstance, trailers api.RequestTrailerMap) api.ResultAction { + headers.Add("run", "beforeConsumerAndHasDecodeRequest:DecodeRequest") + return api.Continue +} + type onLogPlugin struct { plugins.PluginMethodDefaultImpl } @@ -587,5 +629,6 @@ func init() { plugins.RegisterPlugin("benchmark", &benchmarkPlugin{}) plugins.RegisterPlugin("benchmark2", &benchmarkPlugin{}) plugins.RegisterPlugin("beforeConsumerAndHasOtherMethod", &beforeConsumerAndHasOtherMethodPlugin{}) + plugins.RegisterPlugin("beforeConsumerAndHasDecodeRequest", &beforeConsumerAndHasDecodeRequestPlugin{}) plugins.RegisterPlugin("onLog", &onLogPlugin{}) } diff --git a/site/content/en/docs/developer-guide/plugin_development.md b/site/content/en/docs/developer-guide/plugin_development.md index c064321d..35f1c227 100644 --- a/site/content/en/docs/developer-guide/plugin_development.md +++ b/site/content/en/docs/developer-guide/plugin_development.md @@ -143,7 +143,7 @@ The same process applies to the Encode path in a reverse order, and the method i Note: `EncodeResponse` is only executed if `EncodeHeaders` returns `WaitAllData`. So if `EncodeResponse` is defined, `EncodeHeaders` must be defined as well. When both `EncodeResponse` and `EncodeData/EncodeTrailers` are defined in the plugin: if `EncodeHeaders` returns `WaitAllData`, only `EncodeResponse` is executed, otherwise, only `EncodeData/EncodeTrailers` is executed. -Currently, `DecodeRequest` is not supported by plugins whose order is `Access` or `Authn`. +Currently, if Consumer plugins are configured, `DecodeRequest` is not supported by plugins whose order is `Access` or `Authn`. ## Consumer Plugins diff --git a/site/content/zh-hans/docs/developer-guide/plugin_development.md b/site/content/zh-hans/docs/developer-guide/plugin_development.md index 75787ec5..26f3c4c9 100644 --- a/site/content/zh-hans/docs/developer-guide/plugin_development.md +++ b/site/content/zh-hans/docs/developer-guide/plugin_development.md @@ -138,7 +138,7 @@ filter manager 实现了以下特性： 注意：`EncodeResponse` 仅在 `EncodeHeaders` 返回 `WaitAllData` 时才被执行。所以如果定义了 `EncodeResponse`，一定要定义 `EncodeHeaders`。当插件里同时定义了 `EncodeResponse` 和 `EncodeData/EncodeTrailers`：如果 `EncodeHeaders` 返回 `WaitAllData`，只有 `EncodeResponse` 会运行，否则只有 `EncodeData/EncodeTrailers` 会运行。 -目前顺序为 `Access` 或 `Authn` 的插件不支持 `DecodeRequest` 方法。 +目前如果配置了消费者插件，顺序为 `Access` 或 `Authn` 的插件的 `DecodeRequest` 方法将不会被执行。 ## 消费者插件