Skip to content

Commit

Permalink
fix: previous error log is too strict (#788)
Browse files Browse the repository at this point in the history
Signed-off-by: spacewander <spacewanderlzx@gmail.com>
  • Loading branch information
spacewander authored Oct 31, 2024
1 parent 81b54af commit d9a5c25
Show file tree
Hide file tree
Showing 6 changed files with 204 additions and 75 deletions.
12 changes: 7 additions & 5 deletions api/pkg/filtermanager/filtermanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
}
Expand Down
51 changes: 51 additions & 0 deletions api/tests/integration/consumer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}
43 changes: 43 additions & 0 deletions api/tests/integration/test_plugins.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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{})
}
169 changes: 101 additions & 68 deletions api/tests/integration/testdata/services/grpc/sample.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion site/content/en/docs/developer-guide/plugin_development.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading

0 comments on commit d9a5c25

Please sign in to comment.