Fix traefik annotations as they weren't working properly for any path…

…s except root Signed-off-by: Aleksandr Fofanov <[email protected]> Update README.md Signed-off-by: Aleksandr Fofanov <[email protected]> Fix an issue with incorrect path prefix for traefik in case when custom path defined Signed-off-by: Aleksandr Fofanov <[email protected]> Fix an issue with incorrect path prefix for traefik in case when custom path defined
openfaas · Jun 18, 2020 · a88feb0 · a88feb0
1 parent c539556
commit a88feb0
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 5 deletions.
diff --git a/README.md b/README.md
@@ -229,7 +229,10 @@ spec:
 ```
 
 * Edit the `email` and take note of the `namespace`, you will want this to be `openfaas`.
-* If using `traefik` instead of `nginx`, then edit `class: nginx` and replace it as necessary
+* If using `traefik` instead of `nginx`, then edit `class: nginx` and replace it as necessary.
+  **Recommended version is v1.7.21 or above**, previous versions will incorrectly route requests
+  to your function (with duplicated path, see
+  [related issue](https://github.com/openfaas-incubator/ingress-operator/issues/30)).
 
 Save as `letsencrypt-issuer.yaml` then run `kubectl apply -f letsencrypt-issuer.yaml`.
 

diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go
@@ -429,7 +429,12 @@ func makeRules(fni *faasv1.FunctionIngress) []v1beta1.IngressRule {
 	}
 
 	if getClass(fni.Spec.IngressType) == "traefik" {
-		path = strings.TrimRight(path, "(.*)")
+		// We have to trim the regex and the trailing slash for Traefik,
+		// otherwise routing won't work
+		path = strings.TrimRight(path, "/(.*)")
+		if len(path) == 0 {
+			path = "/"
+		}
 	}
 
 	serviceHost := "gateway"
@@ -513,7 +518,7 @@ func makeAnnotations(fni *faasv1.FunctionIngress) map[string]string {
 			annotations["zalando.org/skipper-filter"] = `setPath("/function/` + fni.Spec.Function + `")`
 			break
 		case "traefik":
-			annotations["traefik.ingress.kubernetes.io/rewrite-target"] = "/function/" + fni.Spec.Function + "/$1"
+			annotations["traefik.ingress.kubernetes.io/rewrite-target"] = "/function/" + fni.Spec.Function
 			annotations["traefik.ingress.kubernetes.io/rule-type"] = `PathPrefix`
 			break
 		}

diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go
@@ -107,6 +107,36 @@ func TestMakeAnnotations_IngressClassAdditionalAnnotations(t *testing.T) {
 	}
 }
 
+func TestMakeAnnotations_TraefikAnnotationsAreCorrect(t *testing.T) {
+	wantIngressType := "traefik"
+	ingress := faasv1.FunctionIngress{
+		ObjectMeta: metav1.ObjectMeta{
+			Annotations: map[string]string{
+				"kubernetes.io/ingress.class": "traefik",
+			},
+		},
+		Spec: faasv1.FunctionIngressSpec{
+			IngressType:   wantIngressType,
+			Function:      "nodeinfo",
+			BypassGateway: false,
+			Domain:        "nodeinfo.example.com",
+		},
+	}
+
+	result := makeAnnotations(&ingress)
+	t.Log(result)
+
+	wantRewriteTarget := "/function/" + ingress.Spec.Function
+	if val, ok := result["traefik.ingress.kubernetes.io/rewrite-target"]; !ok || val != wantRewriteTarget {
+		t.Errorf("Failed to find expected rewrite target annotation. Expected '%s' but got '%s'", wantRewriteTarget, val)
+	}
+
+	wantRuleType := "PathPrefix"
+	if val, ok := result["traefik.ingress.kubernetes.io/rule-type"]; !ok || val != wantRuleType {
+		t.Errorf("Failed to find expected rule type annotation. Expected '%s' but got '%s'", wantRuleType, val)
+	}
+}
+
 func Test_makeRules_Nginx_RootPath_HasRegex(t *testing.T) {
 	ingress := faasv1.FunctionIngress{
 		ObjectMeta: metav1.ObjectMeta{
@@ -223,7 +253,7 @@ func Test_makeRules_Traefik_RootPath_TrimsRegex(t *testing.T) {
 	}
 }
 
-func Test_makeRules_Traefik_NestedPath_TrimsRegex(t *testing.T) {
+func Test_makeRules_Traefik_NestedPath_TrimsRegex_And_TrailingSlash(t *testing.T) {
 	ingress := faasv1.FunctionIngress{
 		ObjectMeta: metav1.ObjectMeta{
 			Annotations: map[string]string{},
@@ -241,7 +271,7 @@ func Test_makeRules_Traefik_NestedPath_TrimsRegex(t *testing.T) {
 		t.Fail()
 	}
 
-	wantPath := "/v1/profiles/view/"
+	wantPath := "/v1/profiles/view"
 	gotPath := rules[0].HTTP.Paths[0].Path
 	if gotPath != wantPath {
 		t.Errorf("want path %s, but got %s", wantPath, gotPath)