From 5f5f2435c6bfed208719ef68464c07d4970c8b59 Mon Sep 17 00:00:00 2001 From: Tao Yi Date: Tue, 4 Nov 2025 14:07:24 +0800 Subject: [PATCH] fix(sendconfig): Do not cleanup `null`s in plugin configuration (#7751) * remove cleanup of nulls in plugin config * update tests * update changelog * dump plugin config in tests * find plugin in multiple plugins * use different paths for plugin tests and fix workspace (cherry picked from commit 2bb2029577de2e30d3b0dea01e36baa7c82c3157) --- .../dataplane/sendconfig/inmemory_schema.go | 43 ------ .../sendconfig/inmemory_schema_test.go | 4 + test/integration/plugin_test.go | 139 ++++++++++++++++++ 3 files changed, 143 insertions(+), 43 deletions(-) diff --git a/internal/dataplane/sendconfig/inmemory_schema.go b/internal/dataplane/sendconfig/inmemory_schema.go index 7687143574..28bfb3e236 100644 --- a/internal/dataplane/sendconfig/inmemory_schema.go +++ b/internal/dataplane/sendconfig/inmemory_schema.go @@ -28,55 +28,12 @@ func (DefaultContentToDBLessConfigConverter) Convert(content *file.Content) DBLe // DBLess schema does not support decK's Info section. dblessConfig.Content.Info = nil - // DBLess schema does not support nulls in plugin configs. - cleanUpNullsInPluginConfigs(&dblessConfig.Content) - // DBLess schema does not 1-1 match decK's schema for ConsumerGroups. convertConsumerGroups(&dblessConfig) return dblessConfig } -// cleanUpNullsInPluginConfigs removes null values from plugins' configs. -func cleanUpNullsInPluginConfigs(state *file.Content) { - for _, s := range state.Services { - for _, p := range s.Plugins { - for k, v := range p.Config { - if v == nil { - delete(p.Config, k) - } - } - } - for _, r := range state.Routes { - for _, p := range r.Plugins { - for k, v := range p.Config { - if v == nil { - delete(p.Config, k) - } - } - } - } - } - - for _, c := range state.Consumers { - for _, p := range c.Plugins { - for k, v := range p.Config { - if v == nil { - delete(p.Config, k) - } - } - } - } - - for _, p := range state.Plugins { - for k, v := range p.Config { - if v == nil { - delete(p.Config, k) - } - } - } -} - // convertConsumerGroups drops consumer groups related fields that are not supported in DBLess schema: // - Content.Consumers[].Groups, // - Content.ConsumerGroups[].Plugins diff --git a/internal/dataplane/sendconfig/inmemory_schema_test.go b/internal/dataplane/sendconfig/inmemory_schema_test.go index a5aabb9d4a..6d77b59109 100644 --- a/internal/dataplane/sendconfig/inmemory_schema_test.go +++ b/internal/dataplane/sendconfig/inmemory_schema_test.go @@ -279,6 +279,7 @@ func TestDefaultContentToDBLessConfigConverter(t *testing.T) { Plugin: kong.Plugin{ Name: kong.String("p1"), Config: kong.Configuration{ + "config1": nil, "config2": "value2", }, }, @@ -294,6 +295,7 @@ func TestDefaultContentToDBLessConfigConverter(t *testing.T) { Plugin: kong.Plugin{ Name: kong.String("p1"), Config: kong.Configuration{ + "config1": nil, "config2": "value2", }, }, @@ -311,6 +313,7 @@ func TestDefaultContentToDBLessConfigConverter(t *testing.T) { Plugin: kong.Plugin{ Name: kong.String("p1"), Config: kong.Configuration{ + "config1": nil, "config2": "value2", }, }, @@ -328,6 +331,7 @@ func TestDefaultContentToDBLessConfigConverter(t *testing.T) { Plugin: kong.Plugin{ Name: kong.String("p1"), Config: kong.Configuration{ + "config1": nil, "config2": "value2", }, }, diff --git a/test/integration/plugin_test.go b/test/integration/plugin_test.go index 356709b121..a138a9d133 100644 --- a/test/integration/plugin_test.go +++ b/test/integration/plugin_test.go @@ -5,6 +5,7 @@ package integration import ( "bytes" "context" + "encoding/json" "fmt" "net/http" "strings" @@ -13,6 +14,7 @@ import ( "github.com/kong/go-kong/kong" "github.com/kong/kubernetes-testing-framework/pkg/clusters" "github.com/kong/kubernetes-testing-framework/pkg/utils/kubernetes/generators" + "github.com/samber/lo" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" @@ -26,6 +28,7 @@ import ( "github.com/kong/kubernetes-ingress-controller/v2/test" "github.com/kong/kubernetes-ingress-controller/v2/test/consts" "github.com/kong/kubernetes-ingress-controller/v2/test/internal/helpers" + "github.com/kong/kubernetes-ingress-controller/v2/test/internal/testenv" ) func TestPluginEssentials(t *testing.T) { @@ -304,3 +307,139 @@ func TestPluginOrdering(t *testing.T) { require.NoError(t, clusters.DeleteIngress(ctx, env.Cluster(), ns.Name, ingress)) helpers.EventuallyExpectHTTP404WithNoRoute(t, proxyURL, "/test_plugin_ordering", ingressWait, waitTick, nil) } + +func TestPluginNullInConfig(t *testing.T) { + ctx := context.Background() + t.Parallel() + ns, cleaner := helpers.Setup(ctx, t, env) + + t.Log("deploying a minimal HTTP container deployment to test Ingress routes") + container := generators.NewContainer("httpbin", test.HTTPBinImage, test.HTTPBinPort) + deployment := generators.NewDeploymentForContainer(container) + deployment, err := env.Cluster().Client().AppsV1().Deployments(ns.Name).Create(ctx, deployment, metav1.CreateOptions{}) + require.NoError(t, err) + cleaner.Add(deployment) + + t.Logf("exposing deployment %s via service", deployment.Name) + service := generators.NewServiceForDeployment(deployment, corev1.ServiceTypeLoadBalancer) + service, err = env.Cluster().Client().CoreV1().Services(ns.Name).Create(ctx, service, metav1.CreateOptions{}) + require.NoError(t, err) + cleaner.Add(service) + + t.Logf("creating an ingress for service %s with ingress.class %s", service.Name, consts.IngressClass) + ingress := generators.NewIngressForService("/test_plugin_null_in_config", map[string]string{ + "konghq.com/strip-path": "true", + }, service) + ingress.Spec.IngressClassName = kong.String(consts.IngressClass) + ingress, err = env.Cluster().Client().NetworkingV1().Ingresses(ns.Name).Create(ctx, ingress, metav1.CreateOptions{}) + require.NoError(t, err) + cleaner.Add(ingress) + + t.Log("waiting for routes from Ingress to be operational") + assert.Eventually(t, func() bool { + resp, err := helpers.DefaultHTTPClient().Get(fmt.Sprintf("%s/test_plugin_null_in_config", proxyURL)) + if err != nil { + t.Logf("WARNING: error while waiting for %s: %v", proxyURL, err) + return false + } + defer resp.Body.Close() + if resp.StatusCode == http.StatusOK { + // now that the ingress backend is routable, make sure the contents we're getting back are what we expect + // Expected: "httpbin.org" + b := new(bytes.Buffer) + n, err := b.ReadFrom(resp.Body) + require.NoError(t, err) + require.True(t, n > 0) + return strings.Contains(b.String(), "httpbin.org") + } + return false + }, ingressWait, waitTick) + + t.Log("Creating a plugin with `null` in its configuration") + + kongplugin := &kongv1.KongPlugin{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns.Name, + Name: "plugin-datadog", + }, + InstanceName: "plugin-with-null", + PluginName: "datadog", + Config: apiextensionsv1.JSON{ + Raw: []byte(`{"host":"localhost","port":8125,"prefix":null}`), + }, + } + c, err := clientset.NewForConfig(env.Cluster().Config()) + require.NoError(t, err) + kongplugin, err = c.ConfigurationV1().KongPlugins(ns.Name).Create(ctx, kongplugin, metav1.CreateOptions{}) + require.NoError(t, err) + cleaner.Add(kongplugin) + + t.Logf("Updating Ingress to use plugin %s", kongplugin.Name) + require.Eventually(t, func() bool { + ingress, err := env.Cluster().Client().NetworkingV1().Ingresses(ns.Name).Get(ctx, ingress.Name, metav1.GetOptions{}) + if err != nil { + return false + } + ingress.Annotations[annotations.AnnotationPrefix+annotations.PluginsKey] = kongplugin.Name + _, err = env.Cluster().Client().NetworkingV1().Ingresses(ns.Name).Update(ctx, ingress, metav1.UpdateOptions{}) + return err == nil + }, ingressWait, waitTick) + + t.Logf("Checking the configuration of the plugin %s in Kong", kongplugin.Name) + + kc, err := kong.NewClient(lo.ToPtr(proxyAdminURL.String()), + &http.Client{ + Transport: transportWithKongAdminToken{ + tr: &http.Transport{}, + token: consts.KongTestPassword, + }, + }) + require.NoError(t, err, "failed to create Kong client") + // For integration tests in enterprise edition and postgres DB backed Kong gateway, + // the tests are run in "notdefault" workspace of Kong. + if testenv.DBMode() != testenv.DBModeOff && testenv.KongEnterpriseEnabled() { + kc.SetWorkspace(consts.KongTestWorkspace) + } + require.Eventually(t, func() bool { + plugins, err := kc.Plugins.ListAll(ctx) + require.NoError(t, err, "failed to list plugins") + + datadogPlugin, found := lo.Find(plugins, func(p *kong.Plugin) bool { + return p.Name != nil && *p.Name == "datadog" + }) + if !found { + t.Logf("datadog plugin not found. %d plugins found: %s", + len(plugins), + strings.Join( + lo.Map(plugins, func(p *kong.Plugin, _ int) string { + return lo.FromPtrOr(p.Name, "_") + }), + ","), + ) + return false + } + + configJSON, err := json.Marshal(datadogPlugin.Config) + require.NoError(t, err) + t.Logf("Configuration of datadog plugin: %s", string(configJSON)) + + configPrefix, ok := datadogPlugin.Config["prefix"] + return ok && configPrefix == nil + }, ingressWait, waitTick, "failed to find 'datadog' plugin with null in config.prefix in Kong") +} + +// transportWithKongAdminToken is a transport to set `Kong-Admin-Token` header +// used to create Kong clients with admin tokens. +type transportWithKongAdminToken struct { + tr http.RoundTripper + token string +} + +var _ http.RoundTripper = transportWithKongAdminToken{} + +// RoundTrip implements the http.Transport interfacec. +// It adds the `Kong-Admin-Token` header to the request to authenticate the request to Kong admin APIs. +func (tr transportWithKongAdminToken) RoundTrip(req *http.Request) (*http.Response, error) { + req.Header.Set("Kong-Admin-Token", tr.token) + return tr.tr.RoundTrip(req) +}