From b1443474e80894eb8e94c10cc0a8f5a022a91162 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Wed, 16 Sep 2026 17:27:40 +0545 Subject: [PATCH] fix: reject invalid GatewayProxy plugin objects --- internal/adc/translator/gateway.go | 38 +++++-- .../translator/gatewayproxy_render_test.go | 102 ++++++++++++++++++ internal/adc/translator/ingressclass.go | 8 +- 3 files changed, 136 insertions(+), 12 deletions(-) create mode 100644 internal/adc/translator/gatewayproxy_render_test.go diff --git a/internal/adc/translator/gateway.go b/internal/adc/translator/gateway.go index fd4d4dffd..43102b7ce 100644 --- a/internal/adc/translator/gateway.go +++ b/internal/adc/translator/gateway.go @@ -67,8 +67,12 @@ func (t *Translator) TranslateGateway(tctx *provider.TranslateContext, obj *gate globalRules := make(adctypes.GlobalRule) pluginMetadata := make(adctypes.PluginMetadata) // apply plugins from GatewayProxy to global rules - t.fillPluginsFromGatewayProxy(globalRules, &gatewayProxy) - t.fillPluginMetadataFromGatewayProxy(pluginMetadata, &gatewayProxy) + if err := t.fillPluginsFromGatewayProxy(globalRules, &gatewayProxy); err != nil { + return nil, err + } + if err := t.fillPluginMetadataFromGatewayProxy(pluginMetadata, &gatewayProxy); err != nil { + return nil, err + } result.GlobalRules = globalRules result.PluginMetadata = pluginMetadata @@ -364,11 +368,12 @@ func (t *Translator) translateFrontendValidation(tctx *provider.TranslateContext } // fillPluginsFromGatewayProxy fill plugins from GatewayProxy to given plugins -func (t *Translator) fillPluginsFromGatewayProxy(plugins adctypes.GlobalRule, gatewayProxy *v1alpha1.GatewayProxy) { +func (t *Translator) fillPluginsFromGatewayProxy(plugins adctypes.GlobalRule, gatewayProxy *v1alpha1.GatewayProxy) error { if gatewayProxy == nil { - return + return nil } + translated := make(adctypes.GlobalRule) for _, plugin := range gatewayProxy.Spec.Plugins { // only apply enabled plugins if !plugin.Enabled { @@ -379,26 +384,39 @@ func (t *Translator) fillPluginsFromGatewayProxy(plugins adctypes.GlobalRule, ga pluginConfig := map[string]any{} if len(plugin.Config.Raw) > 0 { if err := json.Unmarshal(plugin.Config.Raw, &pluginConfig); err != nil { - t.Log.Error(err, "gateway proxy plugin config unmarshal failed", "plugin", pluginName) - continue + return fmt.Errorf("failed to unmarshal config of GatewayProxy plugin %q: %w", pluginName, err) + } + if pluginConfig == nil { + return fmt.Errorf("config of GatewayProxy plugin %q must be a JSON object", pluginName) } } + translated[pluginName] = pluginConfig + } + for pluginName, pluginConfig := range translated { plugins[pluginName] = pluginConfig } t.Log.V(1).Info("fill plugins for gateway proxy", "plugins", plugins) + return nil } -func (t *Translator) fillPluginMetadataFromGatewayProxy(pluginMetadata adctypes.PluginMetadata, gatewayProxy *v1alpha1.GatewayProxy) { +func (t *Translator) fillPluginMetadataFromGatewayProxy(pluginMetadata adctypes.PluginMetadata, gatewayProxy *v1alpha1.GatewayProxy) error { if gatewayProxy == nil { - return + return nil } + translated := make(adctypes.PluginMetadata) for pluginName, plugin := range gatewayProxy.Spec.PluginMetadata { var pluginConfig map[string]any if err := json.Unmarshal(plugin.Raw, &pluginConfig); err != nil { - t.Log.Error(err, "gateway proxy plugin_metadata unmarshal failed", "plugin", pluginName, "config", string(plugin.Raw)) - continue + return fmt.Errorf("failed to unmarshal GatewayProxy plugin metadata for %q: %w", pluginName, err) + } + if pluginConfig == nil { + return fmt.Errorf("GatewayProxy plugin metadata for %q must be a JSON object", pluginName) } t.Log.V(1).Info("fill plugin_metadata for gateway proxy", "plugin", pluginName, "config", pluginConfig) + translated[pluginName] = pluginConfig + } + for pluginName, pluginConfig := range translated { pluginMetadata[pluginName] = pluginConfig } + return nil } diff --git a/internal/adc/translator/gatewayproxy_render_test.go b/internal/adc/translator/gatewayproxy_render_test.go new file mode 100644 index 000000000..f197fa3bf --- /dev/null +++ b/internal/adc/translator/gatewayproxy_render_test.go @@ -0,0 +1,102 @@ +// 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/require" + networkingv1 "k8s.io/api/networking/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + + "github.com/apache/apisix-ingress-controller/api/v1alpha1" + "github.com/apache/apisix-ingress-controller/internal/provider" + "github.com/apache/apisix-ingress-controller/internal/utils" +) + +func TestGatewayProxyPluginRenderErrors(t *testing.T) { + badJSON := apiextensionsv1.JSON{Raw: []byte(`["not-an-object"]`)} + + objects := []struct { + name string + object client.Object + translate func(*Translator, *provider.TranslateContext, client.Object) (*TranslateResult, error) + }{ + { + name: "Gateway", + object: &gatewayv1.Gateway{ + TypeMeta: metav1.TypeMeta{APIVersion: gatewayv1.GroupVersion.String(), Kind: "Gateway"}, + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "gateway"}, + }, + translate: func(tr *Translator, tctx *provider.TranslateContext, obj client.Object) (*TranslateResult, error) { + return tr.TranslateGateway(tctx, obj.(*gatewayv1.Gateway)) + }, + }, + { + name: "IngressClass", + object: &networkingv1.IngressClass{ + TypeMeta: metav1.TypeMeta{APIVersion: networkingv1.SchemeGroupVersion.String(), Kind: "IngressClass"}, + ObjectMeta: metav1.ObjectMeta{Name: "ingress-class"}, + }, + translate: func(tr *Translator, tctx *provider.TranslateContext, obj client.Object) (*TranslateResult, error) { + return tr.TranslateIngressClass(tctx, obj.(*networkingv1.IngressClass)) + }, + }, + } + + tests := []struct { + name string + proxy v1alpha1.GatewayProxy + wantError string + }{ + { + name: "plugin config", + proxy: v1alpha1.GatewayProxy{Spec: v1alpha1.GatewayProxySpec{ + Plugins: []v1alpha1.GatewayProxyPlugin{{ + Name: "response-rewrite", Enabled: true, Config: badJSON, + }}, + }}, + wantError: `failed to unmarshal config of GatewayProxy plugin "response-rewrite"`, + }, + { + name: "plugin metadata", + proxy: v1alpha1.GatewayProxy{Spec: v1alpha1.GatewayProxySpec{ + PluginMetadata: map[string]apiextensionsv1.JSON{"key-auth": badJSON}, + }}, + wantError: `failed to unmarshal GatewayProxy plugin metadata for "key-auth"`, + }, + } + + for _, object := range objects { + for _, tt := range tests { + t.Run(object.name+"/"+tt.name, func(t *testing.T) { + tctx := provider.NewDefaultTranslateContext(context.Background()) + tctx.GatewayProxies[utils.NamespacedNameKind(object.object)] = tt.proxy + + result, err := object.translate(&Translator{Log: logr.Discard()}, tctx, object.object) + require.Nil(t, result) + require.ErrorContains(t, err, tt.wantError) + }) + } + } +} diff --git a/internal/adc/translator/ingressclass.go b/internal/adc/translator/ingressclass.go index 064bd77d9..9052fc56b 100644 --- a/internal/adc/translator/ingressclass.go +++ b/internal/adc/translator/ingressclass.go @@ -38,8 +38,12 @@ func (t *Translator) TranslateIngressClass(tctx *provider.TranslateContext, obj globalRules := make(adctypes.GlobalRule) pluginMetadata := make(adctypes.PluginMetadata) // apply plugins from GatewayProxy to global rules - t.fillPluginsFromGatewayProxy(globalRules, &gatewayProxy) - t.fillPluginMetadataFromGatewayProxy(pluginMetadata, &gatewayProxy) + if err := t.fillPluginsFromGatewayProxy(globalRules, &gatewayProxy); err != nil { + return nil, err + } + if err := t.fillPluginMetadataFromGatewayProxy(pluginMetadata, &gatewayProxy); err != nil { + return nil, err + } result.GlobalRules = globalRules result.PluginMetadata = pluginMetadata