This is an automated email from the ASF dual-hosted git repository.
shreemaan-abhishek pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/apisix-ingress-controller.git
The following commit(s) were added to refs/heads/master by this push:
new 70e216be fix: reject invalid plugin config instead of applying an
empty one (#2814)
70e216be is described below
commit 70e216be8fd9f8d3f809d968575c934b6ac307ae
Author: Shreemaan Abhishek <[email protected]>
AuthorDate: Wed Aug 12 18:12:59 2026 +0800
fix: reject invalid plugin config instead of applying an empty one (#2814)
---
internal/adc/translator/apisixconsumer.go | 5 +-
internal/adc/translator/apisixroute.go | 47 +++++--
internal/adc/translator/globalrule.go | 5 +-
internal/adc/translator/ingress.go | 38 +++--
internal/adc/translator/pluginconfig_test.go | 198 +++++++++++++++++++++++++++
5 files changed, 264 insertions(+), 29 deletions(-)
diff --git a/internal/adc/translator/apisixconsumer.go
b/internal/adc/translator/apisixconsumer.go
index 51a98c73..3e802192 100644
--- a/internal/adc/translator/apisixconsumer.go
+++ b/internal/adc/translator/apisixconsumer.go
@@ -103,7 +103,10 @@ func (t *Translator) TranslateApisixConsumer(tctx
*provider.TranslateContext, ac
if !plugin.Enable {
continue
}
- config := t.buildPluginConfig(plugin, ac.Namespace,
tctx.Secrets)
+ config, err := t.buildPluginConfig(plugin, ac.Namespace,
tctx.Secrets)
+ if err != nil {
+ return nil, err
+ }
plugins[plugin.Name] = config
}
diff --git a/internal/adc/translator/apisixroute.go
b/internal/adc/translator/apisixroute.go
index 5fd15cbe..fd9e1112 100644
--- a/internal/adc/translator/apisixroute.go
+++ b/internal/adc/translator/apisixroute.go
@@ -64,7 +64,10 @@ func (t *Translator) TranslateApisixRoute(tctx
*provider.TranslateContext, ar *a
func (t *Translator) translateHTTPRule(tctx *provider.TranslateContext, ar
*apiv2.ApisixRoute, rule apiv2.ApisixRouteHTTP, ruleIndex int) (*adc.Service,
error) {
timeout := t.buildTimeout(rule)
- plugins := t.buildPlugins(tctx, ar, rule)
+ plugins, err := t.buildPlugins(tctx, ar, rule)
+ if err != nil {
+ return nil, err
+ }
vars, err := rule.Match.NginxVars.ToVars()
if err != nil {
@@ -90,24 +93,28 @@ func (t *Translator) buildTimeout(rule
apiv2.ApisixRouteHTTP) *adc.Timeout {
}
}
-func (t *Translator) buildPlugins(tctx *provider.TranslateContext, ar
*apiv2.ApisixRoute, rule apiv2.ApisixRouteHTTP) adc.Plugins {
+func (t *Translator) buildPlugins(tctx *provider.TranslateContext, ar
*apiv2.ApisixRoute, rule apiv2.ApisixRouteHTTP) (adc.Plugins, error) {
plugins := make(adc.Plugins)
// Load plugins from referenced PluginConfig
- t.loadPluginConfigPlugins(tctx, ar, rule, plugins)
+ if err := t.loadPluginConfigPlugins(tctx, ar, rule, plugins); err !=
nil {
+ return nil, err
+ }
// Apply plugins from the route itself
- t.loadRoutePlugins(tctx, ar, rule.Plugins, plugins)
+ if err := t.loadRoutePlugins(tctx, ar, rule.Plugins, plugins); err !=
nil {
+ return nil, err
+ }
// Add authentication plugins
t.addAuthenticationPlugins(rule, plugins)
- return plugins
+ return plugins, nil
}
-func (t *Translator) loadPluginConfigPlugins(tctx *provider.TranslateContext,
ar *apiv2.ApisixRoute, rule apiv2.ApisixRouteHTTP, plugins adc.Plugins) {
+func (t *Translator) loadPluginConfigPlugins(tctx *provider.TranslateContext,
ar *apiv2.ApisixRoute, rule apiv2.ApisixRouteHTTP, plugins adc.Plugins) error {
if rule.PluginConfigName == "" {
- return
+ return nil
}
pcNamespace := ar.Namespace
@@ -118,33 +125,41 @@ func (t *Translator) loadPluginConfigPlugins(tctx
*provider.TranslateContext, ar
pcKey := types.NamespacedName{Namespace: pcNamespace, Name:
rule.PluginConfigName}
pc, ok := tctx.ApisixPluginConfigs[pcKey]
if !ok || pc == nil {
- return
+ return nil
}
for _, plugin := range pc.Spec.Plugins {
if !plugin.Enable {
continue
}
- config := t.buildPluginConfig(plugin, pc.Namespace,
tctx.Secrets)
+ config, err := t.buildPluginConfig(plugin, pc.Namespace,
tctx.Secrets)
+ if err != nil {
+ return err
+ }
plugins[plugin.Name] = config
}
+ return nil
}
-func (t *Translator) loadRoutePlugins(tctx *provider.TranslateContext, ar
*apiv2.ApisixRoute, routePlugins []apiv2.ApisixRoutePlugin, plugins
adc.Plugins) {
+func (t *Translator) loadRoutePlugins(tctx *provider.TranslateContext, ar
*apiv2.ApisixRoute, routePlugins []apiv2.ApisixRoutePlugin, plugins
adc.Plugins) error {
for _, plugin := range routePlugins {
if !plugin.Enable {
continue
}
- config := t.buildPluginConfig(plugin, ar.Namespace,
tctx.Secrets)
+ config, err := t.buildPluginConfig(plugin, ar.Namespace,
tctx.Secrets)
+ if err != nil {
+ return err
+ }
plugins[plugin.Name] = config
}
+ return nil
}
-func (t *Translator) buildPluginConfig(plugin apiv2.ApisixRoutePlugin,
namespace string, secrets map[types.NamespacedName]*corev1.Secret)
map[string]any {
+func (t *Translator) buildPluginConfig(plugin apiv2.ApisixRoutePlugin,
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 {
- t.Log.Error(err, "failed to unmarshal plugin config")
+ return nil, fmt.Errorf("failed to unmarshal config of
plugin %s: %w", plugin.Name, err)
}
}
if plugin.SecretRef != "" {
@@ -154,7 +169,7 @@ func (t *Translator) buildPluginConfig(plugin
apiv2.ApisixRoutePlugin, namespace
}
}
}
- return config
+ return config, nil
}
func (t *Translator) addAuthenticationPlugins(rule apiv2.ApisixRouteHTTP,
plugins adc.Plugins) {
@@ -470,7 +485,9 @@ func (t *Translator)
translateApisixRouteBackendResolveGranularityEndpoint(tctx
func (t *Translator) translateStreamRule(tctx *provider.TranslateContext, ar
*apiv2.ApisixRoute, part apiv2.ApisixRouteStream) (*adc.Service, error) {
// add stream route plugins
plugins := make(adc.Plugins)
- t.loadRoutePlugins(tctx, ar, part.Plugins, plugins)
+ if err := t.loadRoutePlugins(tctx, ar, part.Plugins, plugins); err !=
nil {
+ return nil, err
+ }
sr := adc.NewDefaultStreamRoute()
sr.Name = adc.ComposeStreamRouteName(ar.Namespace, ar.Name, part.Name,
part.Protocol)
diff --git a/internal/adc/translator/globalrule.go
b/internal/adc/translator/globalrule.go
index b54e1283..a75260c9 100644
--- a/internal/adc/translator/globalrule.go
+++ b/internal/adc/translator/globalrule.go
@@ -37,7 +37,10 @@ func (t *Translator) TranslateApisixGlobalRule(tctx
*provider.TranslateContext,
continue
}
- pluginConfig := t.buildPluginConfig(plugin, obj.Namespace,
tctx.Secrets)
+ pluginConfig, err := t.buildPluginConfig(plugin, obj.Namespace,
tctx.Secrets)
+ if err != nil {
+ return nil, err
+ }
plugins[plugin.Name] = pluginConfig
}
diff --git a/internal/adc/translator/ingress.go
b/internal/adc/translator/ingress.go
index ae820330..f3e121fe 100644
--- a/internal/adc/translator/ingress.go
+++ b/internal/adc/translator/ingress.go
@@ -106,7 +106,11 @@ func (t *Translator) TranslateIngress(
for j, path := range rule.HTTP.Paths {
index := fmt.Sprintf("%d-%d", i, j)
- if svc := t.buildServiceFromIngressPath(tctx, obj,
config, &path, index, hosts, labels); svc != nil {
+ svc, err := t.buildServiceFromIngressPath(tctx, obj,
config, &path, index, hosts, labels)
+ if err != nil {
+ return nil, err
+ }
+ if svc != nil {
result.Services = append(result.Services, svc)
}
}
@@ -149,9 +153,9 @@ func (t *Translator) buildServiceFromIngressPath(
index string,
hosts []string,
labels map[string]string,
-) *adctypes.Service {
+) (*adctypes.Service, error) {
if path.Backend.Service == nil {
- return nil
+ return nil, nil
}
service := adctypes.NewDefaultService()
@@ -164,7 +168,10 @@ func (t *Translator) buildServiceFromIngressPath(
protocol := t.resolveIngressUpstream(tctx, obj, config,
path.Backend.Service, upstream)
service.Upstream = upstream
- route := t.buildRouteFromIngressPath(tctx, obj, path, config, index,
labels)
+ route, err := t.buildRouteFromIngressPath(tctx, obj, path, config,
index, labels)
+ if err != nil {
+ return nil, err
+ }
// Check if websocket is enabled via annotation first, then fall back
to appProtocol detection
if config != nil && config.EnableWebsocket {
route.EnableWebsocket = ptr.To(true)
@@ -174,7 +181,7 @@ func (t *Translator) buildServiceFromIngressPath(
service.Routes = []*adctypes.Route{route}
t.fillHTTPRoutePoliciesForIngress(tctx, service.Routes)
- return service
+ return service, nil
}
func (t *Translator) resolveIngressUpstream(
@@ -262,7 +269,7 @@ func (t *Translator) buildRouteFromIngressPath(
config *IngressConfig,
index string,
labels map[string]string,
-) *adctypes.Route {
+) (*adctypes.Route, error) {
route := adctypes.NewDefaultRoute()
route.Name = adctypes.ComposeRouteName(obj.Namespace, obj.Name, index)
route.ID = id.GenID(route.Name)
@@ -308,7 +315,11 @@ func (t *Translator) buildRouteFromIngressPath(
if config != nil {
// check if PluginConfig is specified
if config.PluginConfigName != "" {
- route.Plugins =
t.loadPluginConfigPluginsForIngress(tctx, obj.Namespace,
config.PluginConfigName)
+ plugins, err :=
t.loadPluginConfigPluginsForIngress(tctx, obj.Namespace,
config.PluginConfigName)
+ if err != nil {
+ return nil, err
+ }
+ route.Plugins = plugins
}
// apply plugins from annotations
@@ -323,10 +334,10 @@ func (t *Translator) buildRouteFromIngressPath(
}
route.Uris = uris
- return route
+ return route, nil
}
-func (t *Translator) loadPluginConfigPluginsForIngress(tctx
*provider.TranslateContext, namespace, pluginConfigName string)
adctypes.Plugins {
+func (t *Translator) loadPluginConfigPluginsForIngress(tctx
*provider.TranslateContext, namespace, pluginConfigName string)
(adctypes.Plugins, error) {
plugins := make(adctypes.Plugins)
pcKey := types.NamespacedName{
@@ -335,18 +346,21 @@ func (t *Translator)
loadPluginConfigPluginsForIngress(tctx *provider.TranslateC
}
pc, ok := tctx.ApisixPluginConfigs[pcKey]
if !ok || pc == nil {
- return plugins
+ return plugins, nil
}
for _, plugin := range pc.Spec.Plugins {
if !plugin.Enable {
continue
}
- config := t.buildPluginConfig(plugin, namespace, tctx.Secrets)
+ config, err := t.buildPluginConfig(plugin, namespace,
tctx.Secrets)
+ if err != nil {
+ return nil, err
+ }
plugins[plugin.Name] = config
}
- return plugins
+ return plugins, nil
}
// translateEndpointSliceForIngress create upstream nodes from EndpointSlice
diff --git a/internal/adc/translator/pluginconfig_test.go
b/internal/adc/translator/pluginconfig_test.go
new file mode 100644
index 00000000..b9fff683
--- /dev/null
+++ b/internal/adc/translator/pluginconfig_test.go
@@ -0,0 +1,198 @@
+// 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 translator
+
+import (
+ "context"
+ "testing"
+
+ "github.com/go-logr/logr"
+ "github.com/stretchr/testify/assert"
+ corev1 "k8s.io/api/core/v1"
+ apiextensionsv1
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+ "k8s.io/apimachinery/pkg/types"
+
+ apiv2 "github.com/apache/apisix-ingress-controller/api/v2"
+ "github.com/apache/apisix-ingress-controller/internal/provider"
+)
+
+func TestBuildPluginConfig_NonObjectConfigIsRejected(t *testing.T) {
+ translator := NewTranslator(logr.Discard(), "")
+
+ for _, raw := range []string{`["10.0.0.0/8"]`, `"whitelist"`, `42`} {
+ plugin := apiv2.ApisixRoutePlugin{
+ Name: "ip-restriction",
+ Enable: true,
+ Config: apiextensionsv1.JSON{Raw: []byte(raw)},
+ }
+ config, err := translator.buildPluginConfig(plugin, "default",
nil)
+ assert.Error(t, err, "config %s must be rejected", raw)
+ assert.ErrorContains(t, err, "ip-restriction")
+ assert.Nil(t, config)
+ }
+}
+
+func TestBuildPluginConfig_ValidConfigWithSecretRef(t *testing.T) {
+ translator := NewTranslator(logr.Discard(), "")
+
+ plugin := apiv2.ApisixRoutePlugin{
+ Name: "ip-restriction",
+ Enable: true,
+ Config: apiextensionsv1.JSON{Raw:
[]byte(`{"whitelist":["10.0.0.0/8"]}`)},
+ SecretRef: "cred",
+ }
+ secrets := map[types.NamespacedName]*corev1.Secret{
+ {Namespace: "default", Name: "cred"}: {
+ Data: map[string][]byte{"message": []byte("denied")},
+ },
+ }
+ config, err := translator.buildPluginConfig(plugin, "default", secrets)
+ assert.NoError(t, err)
+ assert.Equal(t, []any{"10.0.0.0/8"}, config["whitelist"])
+ assert.Equal(t, "denied", config["message"])
+}
+
+func TestBuildPlugins_MalformedRoutePluginFailsTranslation(t *testing.T) {
+ translator := NewTranslator(logr.Discard(), "")
+ tctx := provider.NewDefaultTranslateContext(context.Background())
+
+ ar := &apiv2.ApisixRoute{
+ ObjectMeta: metav1.ObjectMeta{Name: "test-route", Namespace:
"default"},
+ }
+ rule := apiv2.ApisixRouteHTTP{
+ Name: "rule1",
+ Plugins: []apiv2.ApisixRoutePlugin{{
+ Name: "ip-restriction",
+ Enable: true,
+ Config: apiextensionsv1.JSON{Raw:
[]byte(`["10.0.0.0/8"]`)},
+ }},
+ }
+
+ plugins, err := translator.buildPlugins(tctx, ar, rule)
+ assert.Error(t, err)
+ assert.Nil(t, plugins)
+}
+
+func TestBuildPlugins_MalformedReferencedPluginConfigFailsTranslation(t
*testing.T) {
+ translator := NewTranslator(logr.Discard(), "")
+ tctx := provider.NewDefaultTranslateContext(context.Background())
+ tctx.ApisixPluginConfigs[types.NamespacedName{Namespace: "default",
Name: "pc"}] = &apiv2.ApisixPluginConfig{
+ ObjectMeta: metav1.ObjectMeta{Name: "pc", Namespace: "default"},
+ Spec: apiv2.ApisixPluginConfigSpec{
+ Plugins: []apiv2.ApisixRoutePlugin{{
+ Name: "ip-restriction",
+ Enable: true,
+ Config: apiextensionsv1.JSON{Raw:
[]byte(`["10.0.0.0/8"]`)},
+ }},
+ },
+ }
+
+ ar := &apiv2.ApisixRoute{
+ ObjectMeta: metav1.ObjectMeta{Name: "test-route", Namespace:
"default"},
+ }
+ rule := apiv2.ApisixRouteHTTP{
+ Name: "rule1",
+ PluginConfigName: "pc",
+ }
+
+ plugins, err := translator.buildPlugins(tctx, ar, rule)
+ assert.Error(t, err)
+ assert.Nil(t, plugins)
+}
+
+func TestTranslateStreamRule_MalformedPluginConfigFailsTranslation(t
*testing.T) {
+ translator := NewTranslator(logr.Discard(), "")
+ tctx := provider.NewDefaultTranslateContext(context.Background())
+
+ ar := &apiv2.ApisixRoute{
+ ObjectMeta: metav1.ObjectMeta{Name: "test-route", Namespace:
"default"},
+ }
+ part := apiv2.ApisixRouteStream{
+ Name: "stream1",
+ Protocol: "TCP",
+ Plugins: []apiv2.ApisixRoutePlugin{{
+ Name: "ip-restriction",
+ Enable: true,
+ Config: apiextensionsv1.JSON{Raw:
[]byte(`["10.0.0.0/8"]`)},
+ }},
+ }
+
+ svc, err := translator.translateStreamRule(tctx, ar, part)
+ assert.Error(t, err)
+ assert.Nil(t, svc)
+}
+
+func TestTranslateApisixConsumer_MalformedPluginConfigFailsTranslation(t
*testing.T) {
+ translator := NewTranslator(logr.Discard(), "")
+ tctx := provider.NewDefaultTranslateContext(context.Background())
+
+ ac := &apiv2.ApisixConsumer{
+ ObjectMeta: metav1.ObjectMeta{Name: "test-consumer", Namespace:
"default"},
+ Spec: apiv2.ApisixConsumerSpec{
+ Plugins: []apiv2.ApisixRoutePlugin{{
+ Name: "ip-restriction",
+ Enable: true,
+ Config: apiextensionsv1.JSON{Raw:
[]byte(`["10.0.0.0/8"]`)},
+ }},
+ },
+ }
+
+ result, err := translator.TranslateApisixConsumer(tctx, ac)
+ assert.Error(t, err)
+ assert.Nil(t, result)
+}
+
+func TestTranslateApisixGlobalRule_MalformedPluginConfigFailsTranslation(t
*testing.T) {
+ translator := NewTranslator(logr.Discard(), "")
+ tctx := provider.NewDefaultTranslateContext(context.Background())
+
+ obj := &apiv2.ApisixGlobalRule{
+ ObjectMeta: metav1.ObjectMeta{Name: "test-global-rule",
Namespace: "default"},
+ Spec: apiv2.ApisixGlobalRuleSpec{
+ Plugins: []apiv2.ApisixRoutePlugin{{
+ Name: "ip-restriction",
+ Enable: true,
+ Config: apiextensionsv1.JSON{Raw:
[]byte(`["10.0.0.0/8"]`)},
+ }},
+ },
+ }
+
+ result, err := translator.TranslateApisixGlobalRule(tctx, obj)
+ assert.Error(t, err)
+ assert.Nil(t, result)
+}
+
+func
TestLoadPluginConfigPluginsForIngress_MalformedPluginConfigFailsTranslation(t
*testing.T) {
+ translator := NewTranslator(logr.Discard(), "")
+ tctx := provider.NewDefaultTranslateContext(context.Background())
+ tctx.ApisixPluginConfigs[types.NamespacedName{Namespace: "default",
Name: "pc"}] = &apiv2.ApisixPluginConfig{
+ ObjectMeta: metav1.ObjectMeta{Name: "pc", Namespace: "default"},
+ Spec: apiv2.ApisixPluginConfigSpec{
+ Plugins: []apiv2.ApisixRoutePlugin{{
+ Name: "ip-restriction",
+ Enable: true,
+ Config: apiextensionsv1.JSON{Raw:
[]byte(`["10.0.0.0/8"]`)},
+ }},
+ },
+ }
+
+ plugins, err := translator.loadPluginConfigPluginsForIngress(tctx,
"default", "pc")
+ assert.Error(t, err)
+ assert.Nil(t, plugins)
+}
