Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: replace * in name of kong routes #3312

Merged
merged 3 commits into from
Jan 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion internal/dataplane/parser/translators/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
70 changes: 70 additions & 0 deletions internal/dataplane/parser/translators/ingress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
102 changes: 102 additions & 0 deletions test/integration/ingress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}