fix: reject invalid L4RoutePolicy plugin configs

internal/adc/translator/l4route_test.go

@@ -43,6 +43,7 @@ func TestTranslateTCPRouteWithL4RoutePolicy(t *testing.T) {
 		policy        *v1alpha1.L4RoutePolicy
 		wantPlugins   []string
 		wantNoPlugins bool
+		wantErr       bool
 	}{
 		{
 			name: "attaches plugins from matching L4RoutePolicy",
@@ -52,6 +53,13 @@ func TestTranslateTCPRouteWithL4RoutePolicy(t *testing.T) {
 			}),
 			wantPlugins: []string{"limit-conn", "ip-restriction"},
 		},
+		{
+			name: "rejects a policy with a non-object plugin config",
+			policy: makeL4RoutePolicy("default", "tcp-policy", "TCPRoute", "my-tcp", []v1alpha1.Plugin{
+				{Name: "ip-restriction", Config: mustJSON([]string{"10.0.0.0/8"})},
+			}),
+			wantErr: true,
+		},
 		{
 			name: "does not attach plugins from policy targeting different route kind",
 			policy: makeL4RoutePolicy("default", "udp-policy", "UDPRoute", "my-tcp", []v1alpha1.Plugin{
@@ -96,6 +104,11 @@ func TestTranslateTCPRouteWithL4RoutePolicy(t *testing.T) {
 			}
 
 			result, err := translator.TranslateTCPRoute(tctx, route)
+			if tt.wantErr {
+				require.Error(t, err)
+				assert.Nil(t, result)
+				return
+			}
 			require.NoError(t, err)
 			require.Len(t, result.Services, 1)
 			require.NotEmpty(t, result.Services[0].StreamRoutes)
@@ -118,6 +131,7 @@ func TestTranslateUDPRouteWithL4RoutePolicy(t *testing.T) {
 		policy        *v1alpha1.L4RoutePolicy
 		wantPlugins   []string
 		wantNoPlugins bool
+		wantErr       bool
 	}{
 		{
 			name: "attaches plugins from matching L4RoutePolicy",
@@ -126,6 +140,13 @@ func TestTranslateUDPRouteWithL4RoutePolicy(t *testing.T) {
 			}),
 			wantPlugins: []string{"limit-conn"},
 		},
+		{
+			name: "rejects a policy with a non-object plugin config",
+			policy: makeL4RoutePolicy("default", "udp-policy", "UDPRoute", "my-udp", []v1alpha1.Plugin{
+				{Name: "ip-restriction", Config: mustJSON("10.0.0.0/8")},
+			}),
+			wantErr: true,
+		},
 		{
 			name: "does not attach plugins from policy targeting TCPRoute",
 			policy: makeL4RoutePolicy("default", "tcp-policy", "TCPRoute", "my-udp", []v1alpha1.Plugin{
@@ -163,6 +184,11 @@ func TestTranslateUDPRouteWithL4RoutePolicy(t *testing.T) {
 			}
 
 			result, err := translator.TranslateUDPRoute(tctx, route)
+			if tt.wantErr {
+				require.Error(t, err)
+				assert.Nil(t, result)
+				return
+			}
 			require.NoError(t, err)
 			require.Len(t, result.Services, 1)
 			require.NotEmpty(t, result.Services[0].StreamRoutes)
@@ -186,6 +212,7 @@ func TestTranslateTLSRouteWithL4RoutePolicy(t *testing.T) {
 		hostnames     []string
 		wantPlugins   []string
 		wantNoPlugins bool
+		wantErr       bool
 	}{
 		{
 			name: "attaches plugins from matching L4RoutePolicy",
@@ -195,6 +222,14 @@ func TestTranslateTLSRouteWithL4RoutePolicy(t *testing.T) {
 			hostnames:   []string{"example.com"},
 			wantPlugins: []string{"ip-restriction"},
 		},
+		{
+			name: "rejects a policy with a non-object plugin config",
+			policy: makeL4RoutePolicy("default", "tls-policy", "TLSRoute", "my-tls", []v1alpha1.Plugin{
+				{Name: "ip-restriction", Config: mustJSON(true)},
+			}),
+			hostnames: []string{"example.com"},
+			wantErr:   true,
+		},
 		{
 			name: "plugins attached once per rule even with multiple SNI hostnames",
 			policy: makeL4RoutePolicy("default", "tls-policy", "TLSRoute", "my-tls", []v1alpha1.Plugin{
@@ -248,6 +283,11 @@ func TestTranslateTLSRouteWithL4RoutePolicy(t *testing.T) {
 			}
 
 			result, err := translator.TranslateTLSRoute(tctx, route)
+			if tt.wantErr {
+				require.Error(t, err)
+				assert.Nil(t, result)
+				return
+			}
 			require.NoError(t, err)
 			require.Len(t, result.Services, 1)
 

internal/adc/translator/l4routepolicy_test.go

@@ -74,7 +74,7 @@ func TestAttachL4RoutePolicyPlugins_AttachesMatchingPolicy(t *testing.T) {
 	}
 
 	plugins := adctypes.Plugins{}
-	tr.AttachL4RoutePolicyPlugins(policies, "default", "my-tcp-route", "TCPRoute", plugins, nil)
+	assert.NoError(t, tr.AttachL4RoutePolicyPlugins(policies, "default", "my-tcp-route", "TCPRoute", plugins, nil))
 
 	assert.Len(t, plugins, 2)
 	assert.Contains(t, plugins, "limit-conn")
@@ -97,7 +97,7 @@ func TestAttachL4RoutePolicyPlugins_NoMatchOnKind(t *testing.T) {
 
 	plugins := adctypes.Plugins{}
 	// Looking for TCPRoute, but policy targets UDPRoute — should not match.
-	tr.AttachL4RoutePolicyPlugins(policies, "default", "my-udp-route", "TCPRoute", plugins, nil)
+	assert.NoError(t, tr.AttachL4RoutePolicyPlugins(policies, "default", "my-udp-route", "TCPRoute", plugins, nil))
 
 	assert.Empty(t, plugins)
 }
@@ -115,7 +115,7 @@ func TestAttachL4RoutePolicyPlugins_NoMatchOnNamespace(t *testing.T) {
 
 	plugins := adctypes.Plugins{}
 	// Route is in "default" namespace, policy is in "other-ns" — should not match.
-	tr.AttachL4RoutePolicyPlugins(policies, "default", "my-tcp-route", "TCPRoute", plugins, nil)
+	assert.NoError(t, tr.AttachL4RoutePolicyPlugins(policies, "default", "my-tcp-route", "TCPRoute", plugins, nil))
 
 	assert.Empty(t, plugins)
 }
@@ -130,14 +130,14 @@ func TestAttachL4RoutePolicyPlugins_EmptyPlugins(t *testing.T) {
 	}
 
 	plugins := adctypes.Plugins{}
-	tr.AttachL4RoutePolicyPlugins(policies, "default", "my-tcp-route", "TCPRoute", plugins, nil)
+	assert.NoError(t, tr.AttachL4RoutePolicyPlugins(policies, "default", "my-tcp-route", "TCPRoute", plugins, nil))
 
 	assert.Empty(t, plugins)
 }
 
 func TestAttachL4RoutePolicyPlugins_EmptyPolicies(t *testing.T) {
 	tr := NewTranslator(logr.Discard(), "")
 	plugins := adctypes.Plugins{}
-	tr.AttachL4RoutePolicyPlugins(nil, "default", "my-tcp-route", "TCPRoute", plugins, nil)
+	assert.NoError(t, tr.AttachL4RoutePolicyPlugins(nil, "default", "my-tcp-route", "TCPRoute", plugins, nil))
 	assert.Empty(t, plugins)
 }

internal/adc/translator/plugin.go

@@ -18,40 +18,16 @@
 package translator
 
 import (
-	"encoding/json"
-	"fmt"
-
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 
 	"github.com/apache/apisix-ingress-controller/api/v1alpha1"
-	pkgutils "github.com/apache/apisix-ingress-controller/pkg/utils"
+	"github.com/apache/apisix-ingress-controller/internal/pluginconfig"
 )
 
 // renderPluginConfig renders the configuration of an apisix.apache.org/v1alpha1 Plugin.
 // The data of the referenced Secret is merged over spec.config, with each Secret key
 // read as a dot separated path so that `session.secret` nests under `session`.
 func renderPluginConfig(plugin v1alpha1.Plugin, namespace string, secrets map[types.NamespacedName]*corev1.Secret) (map[string]any, error) {
-	config := make(map[string]any)
-	if len(plugin.Config.Raw) > 0 {
-		if err := json.Unmarshal(plugin.Config.Raw, &config); err != nil {
-			return nil, fmt.Errorf("failed to unmarshal config of plugin %s: %w", plugin.Name, err)
-		}
-	}
-	// A literal `config: null` unmarshals to a nil map, which serializes back to
-	// null and is rejected by most APISIX plugins; normalize it to an empty object.
-	if config == nil {
-		config = make(map[string]any)
-	}
-	if plugin.SecretRef == nil || plugin.SecretRef.Name == "" {
-		return config, nil
-	}
-	secret, ok := secrets[types.NamespacedName{Namespace: namespace, Name: plugin.SecretRef.Name}]
-	if !ok || secret == nil {
-		return nil, fmt.Errorf("secret %s/%s referenced by plugin %s not found", namespace, plugin.SecretRef.Name, plugin.Name)
-	}
-	for key, value := range secret.Data {
-		pkgutils.InsertKeyInMap(key, string(value), config)
-	}
-	return config, nil
+	return pluginconfig.Render(plugin, namespace, secrets)
 }

internal/adc/translator/policies.go

@@ -18,6 +18,8 @@
 package translator
 
 import (
+	"fmt"
+
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/ptr"
@@ -229,9 +231,9 @@ func (t *Translator) AttachL4RoutePolicyPlugins(
 	routeNamespace, routeName, routeKind string,
 	plugins adctypes.Plugins,
 	secrets map[types.NamespacedName]*corev1.Secret,
-) {
+) error {
 	if len(policies) == 0 {
-		return
+		return nil
 	}
 	for _, policy := range policies {
 		if policy.Namespace != routeNamespace {
@@ -252,19 +254,19 @@ func (t *Translator) AttachL4RoutePolicyPlugins(
 			if ref.SectionName != nil && *ref.SectionName != "" {
 				continue
 			}
-			t.mergeL4PolicyPlugins(policy, plugins, secrets)
-			return
+			return t.mergeL4PolicyPlugins(policy, plugins, secrets)
 		}
 	}
+	return nil
 }
 
-func (t *Translator) mergeL4PolicyPlugins(policy *v1alpha1.L4RoutePolicy, plugins adctypes.Plugins, secrets map[types.NamespacedName]*corev1.Secret) {
+func (t *Translator) mergeL4PolicyPlugins(policy *v1alpha1.L4RoutePolicy, plugins adctypes.Plugins, secrets map[types.NamespacedName]*corev1.Secret) error {
 	for _, plugin := range policy.Spec.Plugins {
 		cfg, err := renderPluginConfig(plugin, policy.Namespace, secrets)
 		if err != nil {
-			t.Log.Error(err, "failed to render L4RoutePolicy plugin config", "plugin", plugin.Name, "policy", policy.Name)
-			continue
+			return fmt.Errorf("failed to render plugin %q from L4RoutePolicy %s/%s: %w", plugin.Name, policy.Namespace, policy.Name, err)
 		}
 		plugins[plugin.Name] = cfg
 	}
+	return nil
 }

internal/adc/translator/tcproute.go

@@ -67,7 +67,7 @@ func listenerPortSet(tctx *provider.TranslateContext) map[int32]struct{} {
 // the match to work, so injection is opt-in (explicit sectionName/port targeting,
 // or more than one listener port). When it is not injected we keep the previous
 // single portless StreamRoute, preserving backward compatibility.
-func (t *Translator) buildL4StreamRoutes(tctx *provider.TranslateContext, namespace, name string, ruleIndex int, typ, routeKind string, labels map[string]string) []*adctypes.StreamRoute {
+func (t *Translator) buildL4StreamRoutes(tctx *provider.TranslateContext, namespace, name string, ruleIndex int, typ, routeKind string, labels map[string]string) ([]*adctypes.StreamRoute, error) {
 	var ports []int32
 	if portSet := listenerPortSet(tctx); t.shouldInjectServerPortVars(tctx.HasExplicitListenerMatch, portSet) {
 		ports = make([]int32, 0, len(portSet))
@@ -98,10 +98,12 @@ func (t *Translator) buildL4StreamRoutes(tctx *provider.TranslateContext, namesp
 		// Attach L4RoutePolicy plugins at the stream_route level: the APISIX stream proxy
 		// applies plugins from the stream_route, not from the service.
 		streamRoute.Plugins = make(adctypes.Plugins)
-		t.AttachL4RoutePolicyPlugins(tctx.L4RoutePolicies, namespace, name, routeKind, streamRoute.Plugins, tctx.Secrets)
+		if err := t.AttachL4RoutePolicyPlugins(tctx.L4RoutePolicies, namespace, name, routeKind, streamRoute.Plugins, tctx.Secrets); err != nil {
+			return nil, err
+		}
 		streamRoutes = append(streamRoutes, streamRoute)
 	}
-	return streamRoutes
+	return streamRoutes, nil
 }
 
 func (t *Translator) TranslateTCPRoute(tctx *provider.TranslateContext, tcpRoute *gatewayv1.TCPRoute) (*TranslateResult, error) {
@@ -212,7 +214,11 @@ func (t *Translator) TranslateTCPRoute(tctx *provider.TranslateContext, tcpRoute
 			}
 		}
 		// TODO: support remote_addr, server_addr, sni
-		service.StreamRoutes = t.buildL4StreamRoutes(tctx, tcpRoute.Namespace, tcpRoute.Name, ruleIndex, "TCP", "TCPRoute", labels)
+		streamRoutes, err := t.buildL4StreamRoutes(tctx, tcpRoute.Namespace, tcpRoute.Name, ruleIndex, "TCP", "TCPRoute", labels)
+		if err != nil {
+			return nil, err
+		}
+		service.StreamRoutes = streamRoutes
 
 		result.Services = append(result.Services, service)
 	}

internal/adc/translator/tlsroute.go

@@ -144,7 +144,10 @@ func (t *Translator) TranslateTLSRoute(tctx *provider.TranslateContext, tlsRoute
 		}
 
 		for _, host := range hosts {
-			streamRoutes := t.buildL4StreamRoutes(tctx, tlsRoute.Namespace, tlsRoute.Name, ruleIndex, "TLS", "TLSRoute", labels)
+			streamRoutes, err := t.buildL4StreamRoutes(tctx, tlsRoute.Namespace, tlsRoute.Name, ruleIndex, "TLS", "TLSRoute", labels)
+			if err != nil {
+				return nil, err
+			}
 			for _, streamRoute := range streamRoutes {
 				streamRoute.SNI = host
 			}

internal/adc/translator/udproute.go

@@ -139,7 +139,11 @@ func (t *Translator) TranslateUDPRoute(tctx *provider.TranslateContext, udpRoute
 			}
 		}
 		// TODO: support remote_addr, server_addr, sni
-		service.StreamRoutes = t.buildL4StreamRoutes(tctx, udpRoute.Namespace, udpRoute.Name, ruleIndex, "UDP", "UDPRoute", labels)
+		streamRoutes, err := t.buildL4StreamRoutes(tctx, udpRoute.Namespace, udpRoute.Name, ruleIndex, "UDP", "UDPRoute", labels)
+		if err != nil {
+			return nil, err
+		}
+		service.StreamRoutes = streamRoutes
 
 		result.Services = append(result.Services, service)
 	}

internal/controller/l4routepolicy_test.go

@@ -0,0 +1,82 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package controller
+
+import (
+	"context"
+	"testing"
+
+	"github.com/go-logr/logr"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	k8stypes "k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
+
+	"github.com/apache/apisix-ingress-controller/api/v1alpha1"
+	"github.com/apache/apisix-ingress-controller/internal/controller/indexer"
+	"github.com/apache/apisix-ingress-controller/internal/provider"
+)
+
+func TestProcessL4RoutePolicy_InvalidPluginConfigSetsRejectedStatus(t *testing.T) {
+	policy := &v1alpha1.L4RoutePolicy{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace:  "default",
+			Name:       "tcp-policy",
+			Generation: 3,
+		},
+		Spec: v1alpha1.L4RoutePolicySpec{
+			TargetRefs: []gatewayv1.LocalPolicyTargetReferenceWithSectionName{{
+				LocalPolicyTargetReference: gatewayv1.LocalPolicyTargetReference{
+					Group: gatewayv1.GroupName,
+					Kind:  "TCPRoute",
+					Name:  "tcp-route",
+				},
+			}},
+			Plugins: []v1alpha1.Plugin{{
+				Name:   "ip-restriction",
+				Config: apiextensionsv1.JSON{Raw: []byte(`["10.0.0.0/8"]`)},
+			}},
+		},
+	}
+	scheme := runtime.NewScheme()
+	require.NoError(t, v1alpha1.AddToScheme(scheme))
+	cli := fake.NewClientBuilder().WithScheme(scheme).WithObjects(policy).
+		WithIndex(&v1alpha1.L4RoutePolicy{}, indexer.PolicyTargetRefs, indexer.L4RoutePolicyIndexFunc).
+		Build()
+	tctx := provider.NewDefaultTranslateContext(context.Background())
+	tctx.RouteParentRefs = []gatewayv1.ParentReference{{Name: "gateway"}}
+
+	ProcessL4RoutePolicy(cli, logr.Discard(), tctx, "default", "tcp-route", "TCPRoute")
+
+	key := k8stypes.NamespacedName{Namespace: "default", Name: "tcp-policy"}
+	require.NotNil(t, tctx.L4RoutePolicies[key], "the policy must reach translation so rendering stops the update")
+	require.Len(t, tctx.StatusUpdaters, 1)
+	mutated := tctx.StatusUpdaters[0].Mutator.Mutate(&v1alpha1.L4RoutePolicy{}).(*v1alpha1.L4RoutePolicy)
+	require.Len(t, mutated.Status.Ancestors, 1)
+	require.Len(t, mutated.Status.Ancestors[0].Conditions, 1)
+	condition := mutated.Status.Ancestors[0].Conditions[0]
+	assert.Equal(t, string(gatewayv1.PolicyConditionAccepted), condition.Type)
+	assert.Equal(t, metav1.ConditionFalse, condition.Status)
+	assert.Equal(t, string(gatewayv1.PolicyReasonInvalid), condition.Reason)
+	assert.Equal(t, int64(3), condition.ObservedGeneration)
+	assert.Equal(t, `plugin "ip-restriction" has invalid configuration`, condition.Message)
+}