backport/2.0.x: Fix(translator): Remove the cleanup of nulls in plugin configuration in DBLess mode (#2475)

Co-authored-by: Tao Yi <tao.yi@konghq.com>

Author: tao12345666333

CHANGELOG.md

@@ -40,6 +40,10 @@
 
 - Fix `DataPlane`'s volumes and volume mounts patching when specified by user
   [#2425](https://github.com/Kong/kong-operator/pull/2425)
+- Do not cleanup `null`s in the configuration of plugins with Kong running in
+  DBLess mode in the translator of ingress-controller. This enables user to use
+  explicit `null`s in plugins.
+  [#2459](https://github.com/Kong/kong-operator/pull/2459)
 
 ## [v2.0.4]
 

ingress-controller/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.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

ingress-controller/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",
 										},
 									},

ingress-controller/test/integration/plugin_test.go

@@ -25,9 +25,11 @@ import (
 	configurationv1 "github.com/kong/kubernetes-configuration/v2/api/configuration/v1"
 	"github.com/kong/kubernetes-configuration/v2/pkg/clientset"
 
+	"github.com/kong/kong-operator/ingress-controller/internal/adminapi"
 	"github.com/kong/kong-operator/ingress-controller/internal/annotations"
 	"github.com/kong/kong-operator/ingress-controller/internal/gatewayapi"
 	"github.com/kong/kong-operator/ingress-controller/internal/labels"
+	managercfg "github.com/kong/kong-operator/ingress-controller/pkg/manager/config"
 	"github.com/kong/kong-operator/ingress-controller/test"
 	"github.com/kong/kong-operator/ingress-controller/test/consts"
 	"github.com/kong/kong-operator/ingress-controller/test/internal/helpers"
@@ -716,3 +718,99 @@ func TestPluginCrossNamespaceReference(t *testing.T) {
 		assert.True(c, resp.StatusCode == http.StatusTeapot)
 	}, ingressWait, waitTick)
 }
+
+func TestPluginNullInConfig(t *testing.T) {
+	ctx := t.Context()
+
+	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_essentials", 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_essentials", proxyHTTPURL))
+		if err != nil {
+			t.Logf("WARNING: error while waiting for %s: %v", proxyHTTPURL, 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: "<title>httpbin.org</title>"
+			b := new(bytes.Buffer)
+			n, err := b.ReadFrom(resp.Body)
+			require.NoError(t, err)
+			require.True(t, n > 0)
+			return strings.Contains(b.String(), "<title>httpbin.org</title>")
+		}
+		return false
+	}, ingressWait, waitTick)
+
+	t.Log("Creating a plugin with `null` in its configuration")
+
+	kongplugin := &configurationv1.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)
+	require.Eventually(t, func() bool {
+		kc, err := adminapi.NewKongAPIClient(proxyAdminURL.String(), managercfg.AdminAPIClientConfig{}, consts.KongTestPassword)
+		require.NoError(t, err, "failed to create Kong client")
+		plugins, err := kc.Plugins.ListAll(ctx)
+		require.NoError(t, err, "failed to list plugins")
+		if len(plugins) != 1 {
+			return false
+		}
+		plugin := plugins[0]
+		if plugin.Name == nil || *plugin.Name != "datadog" {
+			return false
+		}
+		configPrefix, ok := plugin.Config["prefix"]
+		return ok && configPrefix == nil
+	}, ingressWait, waitTick, "failed to find 'datadog' plugin with null in config.prefix in Kong")
+}