From fab59f1278ffbbde52701e8e471461478f3038fa Mon Sep 17 00:00:00 2001 From: Mustafa Abdelrahman Date: Fri, 1 Nov 2024 15:53:10 +0100 Subject: [PATCH 1/4] host2regex: doesn't take in consideration `*` Signed-off-by: Mustafa Abdelrahman --- dataclients/kubernetes/hosts_test.go | 35 ++++++++++++++++++++++++++ predicates/forwarded/forwarded_test.go | 13 +++++++++- 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 dataclients/kubernetes/hosts_test.go diff --git a/dataclients/kubernetes/hosts_test.go b/dataclients/kubernetes/hosts_test.go new file mode 100644 index 0000000000..00335d133f --- /dev/null +++ b/dataclients/kubernetes/hosts_test.go @@ -0,0 +1,35 @@ +package kubernetes + +import ( + "regexp" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestHostsToRegex(t *testing.T) { + for _, ti := range []struct { + msg string + host string + regex string + }{ + { + msg: "simple", + host: "simple.example.org", + regex: "^(simple[.]example[.]org[.]?(:[0-9]+)?)$", + }, + { + msg: "wildcard", + host: "*.example.org", + regex: "^(*[.]example[.]org[.]?(:[0-9]+)?)$", + }, + } { + t.Run(ti.msg, func(t *testing.T) { + regex := createHostRx(ti.host) + require.Equal(t, ti.regex, regex) + // maybe we should validate the generated regex and maybe add + _, err := regexp.Compile(regex) + require.NoError(t, err) + }) + } +} diff --git a/predicates/forwarded/forwarded_test.go b/predicates/forwarded/forwarded_test.go index a8dc3a7313..c53b086fac 100644 --- a/predicates/forwarded/forwarded_test.go +++ b/predicates/forwarded/forwarded_test.go @@ -162,6 +162,17 @@ func TestForwardedHost(t *testing.T) { }, matches: true, isError: false, + }, { + msg: "wildcard host should match", + host: "^(*[.]example[.]org[.]?(:[0-9]+)?)$", // *.example.org + r: request{ + url: "https://test.example.com/index.html", + headers: http.Header{ + "Forwarded": []string{`host="test.example.org"`}, + }, + }, + matches: true, + isError: false, }} for _, tc := range testCases { @@ -173,7 +184,7 @@ func TestForwardedHost(t *testing.T) { hasError := err != nil if hasError || tc.isError { if !tc.isError { - t.Fatal("Predicate creation failed") + t.Fatalf("Predicate creation failed, %s", err) } if !hasError { From 8027b998f44ed1112251a4fecb87d5936baf4767 Mon Sep 17 00:00:00 2001 From: Mustafa Abdelrahman Date: Fri, 1 Nov 2024 16:14:45 +0100 Subject: [PATCH 2/4] another test case to k8s ingress Signed-off-by: Mustafa Abdelrahman --- .../ingress-data/wildcard-ing-prefix.eskip | 3 ++ .../ingress-data/wildcard-ing-prefix.yaml | 49 +++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 dataclients/kubernetes/testdata/ingressV1/ingress-data/wildcard-ing-prefix.eskip create mode 100644 dataclients/kubernetes/testdata/ingressV1/ingress-data/wildcard-ing-prefix.yaml diff --git a/dataclients/kubernetes/testdata/ingressV1/ingress-data/wildcard-ing-prefix.eskip b/dataclients/kubernetes/testdata/ingressV1/ingress-data/wildcard-ing-prefix.eskip new file mode 100644 index 0000000000..cae3c7182b --- /dev/null +++ b/dataclients/kubernetes/testdata/ingressV1/ingress-data/wildcard-ing-prefix.eskip @@ -0,0 +1,3 @@ +kube_foo__qux____example_org_____qux: + Host("^(*[.]example[.]org[.]?(:[0-9]+)?)$") && PathSubtree("/") + -> ; diff --git a/dataclients/kubernetes/testdata/ingressV1/ingress-data/wildcard-ing-prefix.yaml b/dataclients/kubernetes/testdata/ingressV1/ingress-data/wildcard-ing-prefix.yaml new file mode 100644 index 0000000000..9c3c914786 --- /dev/null +++ b/dataclients/kubernetes/testdata/ingressV1/ingress-data/wildcard-ing-prefix.yaml @@ -0,0 +1,49 @@ +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: qux + namespace: foo +spec: + rules: + - host: "*.example.org" + http: + paths: + - path: "/" + pathType: Prefix + backend: + service: + name: qux + port: + name: baz +--- +apiVersion: v1 +kind: Service +metadata: + name: qux + namespace: foo +spec: + clusterIP: 10.3.190.97 + ports: + - name: baz + port: 8181 + protocol: TCP + targetPort: 8080 + selector: + application: myapp + type: ClusterIP +--- +apiVersion: v1 +kind: Endpoints +metadata: + labels: + application: myapp + name: qux + namespace: foo +subsets: +- addresses: + - ip: 10.2.9.103 + - ip: 10.2.9.104 + ports: + - name: baz + port: 8080 + protocol: TCP From 31d49227755bb483640437cd50dad5a680003f0f Mon Sep 17 00:00:00 2001 From: Mustafa Abdelrahman Date: Fri, 1 Nov 2024 16:55:28 +0100 Subject: [PATCH 3/4] support wildcard for the first segment only in ingress Signed-off-by: Mustafa Abdelrahman --- dataclients/kubernetes/hosts.go | 3 +++ dataclients/kubernetes/hosts_test.go | 6 +---- .../ingress-data/wildcard-ing-prefix.eskip | 2 +- predicates/forwarded/forwarded_test.go | 24 ++++++++++++++++++- 4 files changed, 28 insertions(+), 7 deletions(-) diff --git a/dataclients/kubernetes/hosts.go b/dataclients/kubernetes/hosts.go index 60dfd4a37a..cb6e201ac3 100644 --- a/dataclients/kubernetes/hosts.go +++ b/dataclients/kubernetes/hosts.go @@ -15,6 +15,9 @@ func createHostRx(hosts ...string) string { hrx := make([]string, len(hosts)) for i, host := range hosts { + if strings.HasPrefix(host, "*.") { + host = strings.Replace(host, "*", "[a-z0-9]+(-[a-z0-9]+)?", 1) + } // trailing dots and port are not allowed in kube // ingress spec, so we can append optional setting // without check diff --git a/dataclients/kubernetes/hosts_test.go b/dataclients/kubernetes/hosts_test.go index 00335d133f..677e99fc81 100644 --- a/dataclients/kubernetes/hosts_test.go +++ b/dataclients/kubernetes/hosts_test.go @@ -1,7 +1,6 @@ package kubernetes import ( - "regexp" "testing" "github.com/stretchr/testify/require" @@ -21,15 +20,12 @@ func TestHostsToRegex(t *testing.T) { { msg: "wildcard", host: "*.example.org", - regex: "^(*[.]example[.]org[.]?(:[0-9]+)?)$", + regex: "^([a-z0-9]+(-[a-z0-9]+)?[.]example[.]org[.]?(:[0-9]+)?)$", }, } { t.Run(ti.msg, func(t *testing.T) { regex := createHostRx(ti.host) require.Equal(t, ti.regex, regex) - // maybe we should validate the generated regex and maybe add - _, err := regexp.Compile(regex) - require.NoError(t, err) }) } } diff --git a/dataclients/kubernetes/testdata/ingressV1/ingress-data/wildcard-ing-prefix.eskip b/dataclients/kubernetes/testdata/ingressV1/ingress-data/wildcard-ing-prefix.eskip index cae3c7182b..345f474ae2 100644 --- a/dataclients/kubernetes/testdata/ingressV1/ingress-data/wildcard-ing-prefix.eskip +++ b/dataclients/kubernetes/testdata/ingressV1/ingress-data/wildcard-ing-prefix.eskip @@ -1,3 +1,3 @@ kube_foo__qux____example_org_____qux: - Host("^(*[.]example[.]org[.]?(:[0-9]+)?)$") && PathSubtree("/") + Host("^([a-z0-9]+(-[a-z0-9]+)?[.]example[.]org[.]?(:[0-9]+)?)$") && PathSubtree("/") -> ; diff --git a/predicates/forwarded/forwarded_test.go b/predicates/forwarded/forwarded_test.go index c53b086fac..a7ed04d94e 100644 --- a/predicates/forwarded/forwarded_test.go +++ b/predicates/forwarded/forwarded_test.go @@ -164,7 +164,7 @@ func TestForwardedHost(t *testing.T) { isError: false, }, { msg: "wildcard host should match", - host: "^(*[.]example[.]org[.]?(:[0-9]+)?)$", // *.example.org + host: "^([a-z0-9]+(-[a-z0-9]+)?[.]example[.]org[.]?(:[0-9]+)?)$", // *.example.org r: request{ url: "https://test.example.com/index.html", headers: http.Header{ @@ -173,6 +173,28 @@ func TestForwardedHost(t *testing.T) { }, matches: true, isError: false, + }, { + msg: "wildcard 2 host should match", + host: "^([a-z0-9]+(-[a-z0-9]+)?[.]example[.]org[.]?(:[0-9]+)?)$", // *.example.org + r: request{ + url: "https://test-v2.example.com/index.html", + headers: http.Header{ + "Forwarded": []string{`host="test-v2.example.org"`}, + }, + }, + matches: true, + isError: false, + }, { + msg: "wildcard 3 host shouldn't match", + host: "^([a-z0-9]+(-[a-z0-9]+)?[.]example[.]org[.]?(:[0-9]+)?)$", // *.example.org + r: request{ + url: "https://test-.example.com/index.html", + headers: http.Header{ + "Forwarded": []string{`host="test-.example.com"`}, + }, + }, + matches: false, + isError: false, }} for _, tc := range testCases { From 8e6c3651874a24ddddcd4d8ab4cb3f5e35bb00e9 Mon Sep 17 00:00:00 2001 From: Mustafa Abdelrahman Date: Mon, 4 Nov 2024 10:31:08 +0100 Subject: [PATCH 4/4] allow multiple `-` through the hostname regex Signed-off-by: Mustafa Abdelrahman --- dataclients/kubernetes/hosts.go | 2 +- dataclients/kubernetes/hosts_test.go | 2 +- .../ingress-data/wildcard-ing-prefix.eskip | 2 +- predicates/forwarded/forwarded_test.go | 27 +++++++++++++------ 4 files changed, 22 insertions(+), 11 deletions(-) diff --git a/dataclients/kubernetes/hosts.go b/dataclients/kubernetes/hosts.go index cb6e201ac3..c6cbb04af7 100644 --- a/dataclients/kubernetes/hosts.go +++ b/dataclients/kubernetes/hosts.go @@ -16,7 +16,7 @@ func createHostRx(hosts ...string) string { hrx := make([]string, len(hosts)) for i, host := range hosts { if strings.HasPrefix(host, "*.") { - host = strings.Replace(host, "*", "[a-z0-9]+(-[a-z0-9]+)?", 1) + host = strings.Replace(host, "*", "[a-z0-9]+((-[a-z0-9]+)?)*", 1) } // trailing dots and port are not allowed in kube // ingress spec, so we can append optional setting diff --git a/dataclients/kubernetes/hosts_test.go b/dataclients/kubernetes/hosts_test.go index 677e99fc81..3e2f668ae0 100644 --- a/dataclients/kubernetes/hosts_test.go +++ b/dataclients/kubernetes/hosts_test.go @@ -20,7 +20,7 @@ func TestHostsToRegex(t *testing.T) { { msg: "wildcard", host: "*.example.org", - regex: "^([a-z0-9]+(-[a-z0-9]+)?[.]example[.]org[.]?(:[0-9]+)?)$", + regex: "^([a-z0-9]+((-[a-z0-9]+)?)*[.]example[.]org[.]?(:[0-9]+)?)$", }, } { t.Run(ti.msg, func(t *testing.T) { diff --git a/dataclients/kubernetes/testdata/ingressV1/ingress-data/wildcard-ing-prefix.eskip b/dataclients/kubernetes/testdata/ingressV1/ingress-data/wildcard-ing-prefix.eskip index 345f474ae2..99c886544e 100644 --- a/dataclients/kubernetes/testdata/ingressV1/ingress-data/wildcard-ing-prefix.eskip +++ b/dataclients/kubernetes/testdata/ingressV1/ingress-data/wildcard-ing-prefix.eskip @@ -1,3 +1,3 @@ kube_foo__qux____example_org_____qux: - Host("^([a-z0-9]+(-[a-z0-9]+)?[.]example[.]org[.]?(:[0-9]+)?)$") && PathSubtree("/") + Host("^([a-z0-9]+((-[a-z0-9]+)?)*[.]example[.]org[.]?(:[0-9]+)?)$") && PathSubtree("/") -> ; diff --git a/predicates/forwarded/forwarded_test.go b/predicates/forwarded/forwarded_test.go index a7ed04d94e..7b05341af6 100644 --- a/predicates/forwarded/forwarded_test.go +++ b/predicates/forwarded/forwarded_test.go @@ -164,9 +164,9 @@ func TestForwardedHost(t *testing.T) { isError: false, }, { msg: "wildcard host should match", - host: "^([a-z0-9]+(-[a-z0-9]+)?[.]example[.]org[.]?(:[0-9]+)?)$", // *.example.org + host: "^([a-z0-9]+((-[a-z0-9]+)?)*[.]example[.]org[.]?(:[0-9]+)?)$", // *.example.org r: request{ - url: "https://test.example.com/index.html", + url: "https://test.example.org/index.html", headers: http.Header{ "Forwarded": []string{`host="test.example.org"`}, }, @@ -175,9 +175,9 @@ func TestForwardedHost(t *testing.T) { isError: false, }, { msg: "wildcard 2 host should match", - host: "^([a-z0-9]+(-[a-z0-9]+)?[.]example[.]org[.]?(:[0-9]+)?)$", // *.example.org + host: "^([a-z0-9]+((-[a-z0-9]+)?)*[.]example[.]org[.]?(:[0-9]+)?)$", // *.example.org r: request{ - url: "https://test-v2.example.com/index.html", + url: "https://test-v2.example.org/index.html", headers: http.Header{ "Forwarded": []string{`host="test-v2.example.org"`}, }, @@ -185,12 +185,23 @@ func TestForwardedHost(t *testing.T) { matches: true, isError: false, }, { - msg: "wildcard 3 host shouldn't match", - host: "^([a-z0-9]+(-[a-z0-9]+)?[.]example[.]org[.]?(:[0-9]+)?)$", // *.example.org + msg: "wildcard 3 host should match", + host: "^([a-z0-9]+((-[a-z0-9]+)?)*[.]example[.]org[.]?(:[0-9]+)?)$", // *.example.org r: request{ - url: "https://test-.example.com/index.html", + url: "https://test-v2-v3.example.org/index.html", headers: http.Header{ - "Forwarded": []string{`host="test-.example.com"`}, + "Forwarded": []string{`host="test-v2-v3.example.org"`}, + }, + }, + matches: true, + isError: false, + }, { + msg: "wildcard 4 host shouldn't match", + host: "^([a-z0-9]+((-[a-z0-9]+)?)*[.]example[.]org[.]?(:[0-9]+)?)$", // *.example.org + r: request{ + url: "https://test-.example.org/index.html", + headers: http.Header{ + "Forwarded": []string{`host="test-.example.org"`}, }, }, matches: false,