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 2e7f19ba..8b6a932b 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 { @@ -92,24 +95,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 @@ -120,33 +127,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 != "" { @@ -156,7 +171,7 @@ func (t *Translator) buildPluginConfig(plugin apiv2.ApisixRoutePlugin, namespace } } } - return config + return config, nil } func (t *Translator) addAuthenticationPlugins(rule apiv2.ApisixRouteHTTP, plugins adc.Plugins) { @@ -474,7 +489,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 8d7f981e..afc05e3a 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) +}