fix: previous error log is too strict (#788)

Signed-off-by: spacewander <[email protected]>

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)
+						}
 					}
 				}
 			}

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)
+		})
+	}
+}

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{})
 }

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