fix: replace * in name of kong routes (#3312)

* fix: replace * in name of kong routes

* lint & update CHANGELOG

* Update CHANGELOG.md

Co-authored-by: Patryk Małek <[email protected]>

Co-authored-by: Patryk Małek <[email protected]>

CHANGELOG.md

@@ -61,6 +61,17 @@ Adding a new version? You'll need three changes:
  - [0.0.5](#005)
  - [0.0.4 and prior](#004-and-prior)
 
+## [2.9.0]
+
+> Release date: TBD
+
+### Fixed
+
+- When `CombinedRoutes` is turned on, translator will replace each occurrence of
+ `*` in `Ingress`'s host to `_` in kong route names because `*` is not 
+  allowed in kong route names.
+  [#3312](https://github.com/Kong/kubernetes-ingress-controller/pull/3312)
+
 ## [2.8.0]
 
 > Release date: 2022-12-19

internal/dataplane/parser/translators/ingress.go

@@ -195,7 +195,12 @@ func (m *ingressTranslationMeta) translateIntoKongStateService(kongServiceName s
 }
 
 func (m *ingressTranslationMeta) translateIntoKongRoutes() *kongstate.Route {
-	routeName := fmt.Sprintf("%s.%s.%s.%s.%d", m.ingressNamespace, m.ingressName, m.serviceName, m.ingressHost, m.servicePort)
+	ingressHost := m.ingressHost
+	if strings.Contains(ingressHost, "*") {
+		// '_' is not allowed in host, so we use '_' to replace '*' since '*' is not allowed in Kong.
+		ingressHost = strings.ReplaceAll(ingressHost, "*", "_")
+	}
+	routeName := fmt.Sprintf("%s.%s.%s.%s.%d", m.ingressNamespace, m.ingressName, m.serviceName, ingressHost, m.servicePort)
 	route := &kongstate.Route{
 		Ingress: util.K8sObjectInfo{
 			Namespace:   m.ingressNamespace,

internal/dataplane/parser/translators/ingress_test.go

@@ -1008,6 +1008,76 @@ func TestTranslateIngress(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "* in host is replaced to _",
+			ingress: &netv1.Ingress{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-ingress",
+					Namespace: corev1.NamespaceDefault,
+				},
+				Spec: netv1.IngressSpec{
+					Rules: []netv1.IngressRule{{
+						Host: "*.konghq.com",
+						IngressRuleValue: netv1.IngressRuleValue{
+							HTTP: &netv1.HTTPIngressRuleValue{
+								Paths: []netv1.HTTPIngressPath{{
+									Path: "/api/",
+									Backend: netv1.IngressBackend{
+										Service: &netv1.IngressServiceBackend{
+											Name: "test-service",
+											Port: netv1.ServiceBackendPort{
+												Name:   "http",
+												Number: 80,
+											},
+										},
+									},
+								}},
+							},
+						},
+					}},
+				},
+			},
+			expected: []*kongstate.Service{{
+				Namespace: corev1.NamespaceDefault,
+				Service: kong.Service{
+					Name:           kong.String("default.test-ingress.test-service.80"),
+					Host:           kong.String("test-service.default.80.svc"),
+					ConnectTimeout: kong.Int(int(defaultServiceTimeout.Milliseconds())),
+					Path:           kong.String("/"),
+					Port:           kong.Int(80),
+					Protocol:       kong.String("http"),
+					Retries:        kong.Int(defaultRetries),
+					ReadTimeout:    kong.Int(int(defaultServiceTimeout.Milliseconds())),
+					WriteTimeout:   kong.Int(int(defaultServiceTimeout.Milliseconds())),
+				},
+				Routes: []kongstate.Route{{
+					Ingress: util.K8sObjectInfo{
+						Name:      "test-ingress",
+						Namespace: corev1.NamespaceDefault,
+					},
+					Route: kong.Route{
+						Name:              kong.String("default.test-ingress.test-service._.konghq.com.80"),
+						Hosts:             kong.StringSlice("*.konghq.com"),
+						Paths:             kong.StringSlice("/api/"), // default ImplementationSpecific
+						PreserveHost:      kong.Bool(true),
+						Protocols:         kong.StringSlice("http", "https"),
+						RegexPriority:     kong.Int(0),
+						StripPath:         kong.Bool(false),
+						ResponseBuffering: kong.Bool(true),
+						RequestBuffering:  kong.Bool(true),
+					},
+				}},
+				Backends: []kongstate.ServiceBackend{{
+					Name:      "test-service",
+					Namespace: corev1.NamespaceDefault,
+					PortDef: kongstate.PortDef{
+						Mode:   kongstate.PortModeByNumber,
+						Number: 80,
+					},
+				}},
+				Parent: expectedParentIngress(),
+			}},
+		},
 	}
 
 	for _, tt := range tts {

test/integration/ingress_test.go

@@ -1149,3 +1149,105 @@ func TestIngressRecoverFromInvalidPath(t *testing.T) {
 		return false
 	}, ingressWait, waitTick)
 }
+
+func TestIngressMatchByHost(t *testing.T) {
+	ns, cleaner := setup(t)
+	defer func() {
+		if t.Failed() {
+			output, err := cleaner.DumpDiagnostics(ctx, t.Name())
+			t.Logf("%s failed, dumped diagnostics to %s", t.Name(), output)
+			assert.NoError(t, err)
+		}
+		assert.NoError(t, cleaner.Cleanup(ctx))
+	}()
+
+	t.Log("deploying a minimal HTTP container deployment to test Ingress routes")
+	container := generators.NewContainer("httpbin", test.HTTPBinImage, 80)
+	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)
+	_, 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 fixed host", service.Name)
+	require.NoError(t, err)
+
+	ingress := generators.NewIngressForService("/", map[string]string{
+		"konghq.com/strip-path": "true",
+	}, service)
+	ingress.Spec.IngressClassName = kong.String(ingressClass)
+	for i := range ingress.Spec.Rules {
+		ingress.Spec.Rules[i].Host = "test.example"
+	}
+	_, err = env.Cluster().Client().NetworkingV1().Ingresses(ns.Name).Create(ctx, ingress, metav1.CreateOptions{})
+	require.NoError(t, err)
+	cleaner.Add(ingress)
+
+	t.Log("try to access the ingress by matching host")
+	req, err := http.NewRequest("GET", proxyURL.String(), nil)
+	require.NoError(t, err)
+	req.Host = "test.example"
+	require.Eventually(t, func() bool {
+		resp, err := httpc.Do(req)
+		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 {
+			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("try to access the ingress by unmatching host, should return 404")
+	req.Host = "foo.example"
+	resp, err := httpc.Do(req)
+	require.NoError(t, err)
+	defer resp.Body.Close()
+	require.Equal(t, resp.StatusCode, http.StatusNotFound)
+
+	t.Log("change the ingress to wildcard host")
+	for i := range ingress.Spec.Rules {
+		ingress.Spec.Rules[i].Host = "*.example"
+	}
+	_, err = env.Cluster().Client().NetworkingV1().Ingresses(ns.Name).Update(ctx, ingress, metav1.UpdateOptions{})
+	require.NoError(t, err)
+	cleaner.Add(ingress)
+
+	t.Log("try to access the ingress by matching host")
+
+	req.Host = "test0.example"
+	require.Eventually(t, func() bool {
+		resp, err := httpc.Do(req)
+		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 {
+			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("try to access the ingress by unmatching host, should return 404")
+	req.Host = "test.another"
+	resp, err = httpc.Do(req)
+	require.NoError(t, err)
+	defer resp.Body.Close()
+	require.Equal(t, resp.StatusCode, http.StatusNotFound)
+}