From 04459e75d280d317726c52365c4b0a13ddbbcff3 Mon Sep 17 00:00:00 2001 From: zirain Date: Fri, 19 Apr 2024 16:38:27 +0800 Subject: [PATCH 1/7] ignore goland Signed-off-by: zirain --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index d660211d..0f4557e1 100644 --- a/.gitignore +++ b/.gitignore @@ -26,3 +26,6 @@ go.work # GolangCI-Lint cache /cache + +# goland +.idea From 3a0505fe8a7d505cedc8804dd8408b7f08cc5a6b Mon Sep 17 00:00:00 2001 From: zirain Date: Fri, 19 Apr 2024 16:46:59 +0800 Subject: [PATCH 2/7] fix vs hosts with Signed-off-by: zirain --- pkg/i2gw/providers/istio/converter.go | 4 + pkg/i2gw/providers/istio/converter_test.go | 90 ++++++++++++++++++++++ 2 files changed, 94 insertions(+) diff --git a/pkg/i2gw/providers/istio/converter.go b/pkg/i2gw/providers/istio/converter.go index d2ecbc08..79a5f01d 100644 --- a/pkg/i2gw/providers/istio/converter.go +++ b/pkg/i2gw/providers/istio/converter.go @@ -583,6 +583,10 @@ func (c *converter) convertVsHTTPRoutes(virtualService metav1.ObjectMeta, istioH // set istio hostnames as is, without extra filters. If it's not a fqdn, it would be rejected by K8S API implementation hostnames := make([]gatewayv1.Hostname, 0, len(allowedHostnames)) for _, host := range allowedHostnames { + // '*' is valid in istio, but not in HTTPRoute + if host == "*" { + continue + } hostnames = append(hostnames, gatewayv1.Hostname(host)) } diff --git a/pkg/i2gw/providers/istio/converter_test.go b/pkg/i2gw/providers/istio/converter_test.go index 6d8200b5..1a2bc31d 100644 --- a/pkg/i2gw/providers/istio/converter_test.go +++ b/pkg/i2gw/providers/istio/converter_test.go @@ -1245,6 +1245,96 @@ func Test_converter_convertVsHTTPRoutes(t *testing.T) { }, }, }, + { + name: "hosts with '*'", + args: args{ + virtualService: metav1.ObjectMeta{ + Name: "test", + Namespace: "ns", + }, + istioHTTPRoutes: []*istiov1beta1.HTTPRoute{ + { + Headers: &istiov1beta1.Headers{ + Request: &istiov1beta1.Headers_HeaderOperations{ + Set: map[string]string{ + "h1": "v1", + }, + Add: map[string]string{ + "h2": "v2", + }, + Remove: []string{"h3"}, + }, + Response: &istiov1beta1.Headers_HeaderOperations{ + Set: map[string]string{ + "h4": "v4", + }, + Add: map[string]string{ + "h5": "v5", + }, + Remove: []string{"h6"}, + }, + }, + }, + }, + allowedHostnames: []string{"*"}, + }, + want: []*gatewayv1.HTTPRoute{ + { + TypeMeta: metav1.TypeMeta{ + Kind: "HTTPRoute", + APIVersion: "gateway.networking.k8s.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-idx-0", + Namespace: "ns", + }, + Spec: gatewayv1.HTTPRouteSpec{ + Rules: []gatewayv1.HTTPRouteRule{ + { + Filters: []gatewayv1.HTTPRouteFilter{ + { + Type: gatewayv1.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: &gatewayv1.HTTPHeaderFilter{ + Set: []gatewayv1.HTTPHeader{ + { + Name: "h1", + Value: "v1", + }, + }, + Add: []gatewayv1.HTTPHeader{ + { + Name: "h2", + Value: "v2", + }, + }, + Remove: []string{"h3"}, + }, + }, + { + Type: gatewayv1.HTTPRouteFilterResponseHeaderModifier, + ResponseHeaderModifier: &gatewayv1.HTTPHeaderFilter{ + Set: []gatewayv1.HTTPHeader{ + { + Name: "h4", + Value: "v4", + }, + }, + Add: []gatewayv1.HTTPHeader{ + { + Name: "h5", + Value: "v5", + }, + }, + Remove: []string{"h6"}, + }, + }, + }, + }, + }, + }, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From f7ed997dc23188b5b56a251e682dde71ffaf62f3 Mon Sep 17 00:00:00 2001 From: zirain Date: Sun, 21 Apr 2024 18:09:17 +0800 Subject: [PATCH 3/7] Revert "ignore goland" This reverts commit 04459e75d280d317726c52365c4b0a13ddbbcff3. Signed-off-by: zirain --- .gitignore | 3 --- 1 file changed, 3 deletions(-) diff --git a/.gitignore b/.gitignore index 0f4557e1..d660211d 100644 --- a/.gitignore +++ b/.gitignore @@ -26,6 +26,3 @@ go.work # GolangCI-Lint cache /cache - -# goland -.idea From ac90aa6bbc95842bc7a13165a89d208450216041 Mon Sep 17 00:00:00 2001 From: zirain Date: Sun, 21 Apr 2024 18:19:55 +0800 Subject: [PATCH 4/7] split to convertHostnames Signed-off-by: zirain --- pkg/i2gw/providers/istio/converter.go | 31 ++++++++++++++++----------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/pkg/i2gw/providers/istio/converter.go b/pkg/i2gw/providers/istio/converter.go index 79a5f01d..eb32356b 100644 --- a/pkg/i2gw/providers/istio/converter.go +++ b/pkg/i2gw/providers/istio/converter.go @@ -289,10 +289,27 @@ func (c *converter) convertGateway(gw *istioclientv1beta1.Gateway, fieldPath *fi }, nil } -func (c *converter) convertVsHTTPRoutes(virtualService metav1.ObjectMeta, istioHTTPRoutes []*istiov1beta1.HTTPRoute, allowedHostnames []string, fieldPath *field.Path) ([]*gatewayv1.HTTPRoute, field.ErrorList) { +// convertHostnames set istio hostnames as is, without extra filters. +// If it's not a fqdn, it would be rejected by K8S API implementation +func (c *converter) convertHostnames(hosts []string) []gatewayv1.Hostname { + var resHostnames []gatewayv1.Hostname + for _, host := range hosts { + // '*' is valid in istio, but not in HTTPRoute + if host == "*" { + klog.Infof("ignoring host '*', which is not allowed in Gateway API HTTPRoute") + continue + } + resHostnames = append(resHostnames, gatewayv1.Hostname(host)) + } + return resHostnames +} + +func (c *converter) convertVsHTTPRoutes(virtualService metav1.ObjectMeta, istioHTTPRoutes []*istiov1beta1.HTTPRoute, istioHTTPHosts []string, fieldPath *field.Path) ([]*gatewayv1.HTTPRoute, field.ErrorList) { var errList field.ErrorList var resHTTPRoutes []*gatewayv1.HTTPRoute + allowedHostnames := c.convertHostnames(istioHTTPHosts) + for i, httpRoute := range istioHTTPRoutes { httpRouteFieldName := fmt.Sprintf("%v", i) if httpRoute.GetName() != "" { @@ -580,16 +597,6 @@ func (c *converter) convertVsHTTPRoutes(virtualService metav1.ObjectMeta, istioH } } - // set istio hostnames as is, without extra filters. If it's not a fqdn, it would be rejected by K8S API implementation - hostnames := make([]gatewayv1.Hostname, 0, len(allowedHostnames)) - for _, host := range allowedHostnames { - // '*' is valid in istio, but not in HTTPRoute - if host == "*" { - continue - } - hostnames = append(hostnames, gatewayv1.Hostname(host)) - } - routeName := fmt.Sprintf("%v-idx-%v", virtualService.Name, i) if httpRoute.GetName() != "" { routeName = fmt.Sprintf("%v-%v", virtualService.Name, httpRoute.GetName()) @@ -604,7 +611,7 @@ func (c *converter) convertVsHTTPRoutes(virtualService metav1.ObjectMeta, istioH OwnerReferences: virtualService.OwnerReferences, Finalizers: virtualService.Finalizers, }, - hostnames: hostnames, + hostnames: allowedHostnames, matches: gwHTTPRouteMatches, filters: gwHTTPRouteFilters, backendRefs: backendRefs, From 2450a69f6eaf274a6a5b9cc1388cf806e8041f36 Mon Sep 17 00:00:00 2001 From: zirain Date: Mon, 22 Apr 2024 09:35:11 +0800 Subject: [PATCH 5/7] simplify testcase Signed-off-by: zirain --- pkg/i2gw/providers/istio/converter_test.go | 38 ---------------------- 1 file changed, 38 deletions(-) diff --git a/pkg/i2gw/providers/istio/converter_test.go b/pkg/i2gw/providers/istio/converter_test.go index 1a2bc31d..2c67327d 100644 --- a/pkg/i2gw/providers/istio/converter_test.go +++ b/pkg/i2gw/providers/istio/converter_test.go @@ -1256,22 +1256,9 @@ func Test_converter_convertVsHTTPRoutes(t *testing.T) { { Headers: &istiov1beta1.Headers{ Request: &istiov1beta1.Headers_HeaderOperations{ - Set: map[string]string{ - "h1": "v1", - }, Add: map[string]string{ "h2": "v2", }, - Remove: []string{"h3"}, - }, - Response: &istiov1beta1.Headers_HeaderOperations{ - Set: map[string]string{ - "h4": "v4", - }, - Add: map[string]string{ - "h5": "v5", - }, - Remove: []string{"h6"}, }, }, }, @@ -1295,37 +1282,12 @@ func Test_converter_convertVsHTTPRoutes(t *testing.T) { { Type: gatewayv1.HTTPRouteFilterRequestHeaderModifier, RequestHeaderModifier: &gatewayv1.HTTPHeaderFilter{ - Set: []gatewayv1.HTTPHeader{ - { - Name: "h1", - Value: "v1", - }, - }, Add: []gatewayv1.HTTPHeader{ { Name: "h2", Value: "v2", }, }, - Remove: []string{"h3"}, - }, - }, - { - Type: gatewayv1.HTTPRouteFilterResponseHeaderModifier, - ResponseHeaderModifier: &gatewayv1.HTTPHeaderFilter{ - Set: []gatewayv1.HTTPHeader{ - { - Name: "h4", - Value: "v4", - }, - }, - Add: []gatewayv1.HTTPHeader{ - { - Name: "h5", - Value: "v5", - }, - }, - Remove: []string{"h6"}, }, }, }, From b67c8699222b5b278762215ec78622017d302472 Mon Sep 17 00:00:00 2001 From: zirain Date: Mon, 22 Apr 2024 09:53:28 +0800 Subject: [PATCH 6/7] improve convertHostnames Signed-off-by: zirain --- pkg/i2gw/providers/istio/converter.go | 19 ++++++++--- pkg/i2gw/providers/istio/converter_test.go | 38 ++++++++++++++++++++++ 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/pkg/i2gw/providers/istio/converter.go b/pkg/i2gw/providers/istio/converter.go index eb32356b..d46592e0 100644 --- a/pkg/i2gw/providers/istio/converter.go +++ b/pkg/i2gw/providers/istio/converter.go @@ -18,6 +18,8 @@ package istio import ( "fmt" + "net" + "regexp" "strings" "github.com/kubernetes-sigs/ingress2gateway/pkg/i2gw" @@ -289,16 +291,25 @@ func (c *converter) convertGateway(gw *istioclientv1beta1.Gateway, fieldPath *fi }, nil } +var hostnameRegexp = regexp.MustCompile(`^(\*\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`) + // convertHostnames set istio hostnames as is, without extra filters. // If it's not a fqdn, it would be rejected by K8S API implementation -func (c *converter) convertHostnames(hosts []string) []gatewayv1.Hostname { +func convertHostnames(hosts []string) []gatewayv1.Hostname { var resHostnames []gatewayv1.Hostname for _, host := range hosts { // '*' is valid in istio, but not in HTTPRoute - if host == "*" { - klog.Infof("ignoring host '*', which is not allowed in Gateway API HTTPRoute") + if !hostnameRegexp.MatchString(host) { + klog.Infof("ignoring host %s, which is not allowed in Gateway API HTTPRoute", host) continue } + + // IP addresses are not allowed in Gateway API + if net.ParseIP(host) != nil { + klog.Infof("ignoring host %s, which is an IP address", host) + continue + } + resHostnames = append(resHostnames, gatewayv1.Hostname(host)) } return resHostnames @@ -308,7 +319,7 @@ func (c *converter) convertVsHTTPRoutes(virtualService metav1.ObjectMeta, istioH var errList field.ErrorList var resHTTPRoutes []*gatewayv1.HTTPRoute - allowedHostnames := c.convertHostnames(istioHTTPHosts) + allowedHostnames := convertHostnames(istioHTTPHosts) for i, httpRoute := range istioHTTPRoutes { httpRouteFieldName := fmt.Sprintf("%v", i) diff --git a/pkg/i2gw/providers/istio/converter_test.go b/pkg/i2gw/providers/istio/converter_test.go index 2c67327d..229fa6de 100644 --- a/pkg/i2gw/providers/istio/converter_test.go +++ b/pkg/i2gw/providers/istio/converter_test.go @@ -1997,3 +1997,41 @@ func Test_converter_generateReferences(t *testing.T) { }) } } + +func Test_convertHostnames(t *testing.T) { + cases := []struct { + name string + hostnames []string + expected []gatewayv1alpha2.Hostname + }{ + { + name: "default", + hostnames: []string{"*.com", "test.net", "*.example.com"}, + expected: []gatewayv1alpha2.Hostname{"*.com", "test.net", "*.example.com"}, + }, + { + name: "* is not allowed", + hostnames: []string{"*"}, + expected: []gatewayv1alpha2.Hostname{}, + }, + { + name: "IP is not allowed", + hostnames: []string{"192.0.2.1", "2001:db8::68", "::ffff:192.0.2.1"}, + expected: []gatewayv1alpha2.Hostname{}, + }, + { + name: "The wildcard label must appear by itself as the first label", + hostnames: []string{"example*.com"}, + expected: []gatewayv1alpha2.Hostname{}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + actual := convertHostnames(tc.hostnames) + if !apiequality.Semantic.DeepEqual(actual, tc.expected) { + t.Errorf("convertHostnames() = %v, want %v", actual, tc.expected) + } + }) + } +} From 484e0545d41aeaab7130659052076b2fa1f39203 Mon Sep 17 00:00:00 2001 From: zirain Date: Mon, 22 Apr 2024 22:49:12 +0800 Subject: [PATCH 7/7] address comments Signed-off-by: zirain --- pkg/i2gw/providers/istio/converter.go | 4 ++-- pkg/i2gw/providers/istio/converter_test.go | 7 ++++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/pkg/i2gw/providers/istio/converter.go b/pkg/i2gw/providers/istio/converter.go index d46592e0..7cf0d6e3 100644 --- a/pkg/i2gw/providers/istio/converter.go +++ b/pkg/i2gw/providers/istio/converter.go @@ -300,13 +300,13 @@ func convertHostnames(hosts []string) []gatewayv1.Hostname { for _, host := range hosts { // '*' is valid in istio, but not in HTTPRoute if !hostnameRegexp.MatchString(host) { - klog.Infof("ignoring host %s, which is not allowed in Gateway API HTTPRoute", host) + klog.Warningf("ignoring host %s, which is not allowed in Gateway API HTTPRoute", host) continue } // IP addresses are not allowed in Gateway API if net.ParseIP(host) != nil { - klog.Infof("ignoring host %s, which is an IP address", host) + klog.Warningf("ignoring host %s, which is an IP address", host) continue } diff --git a/pkg/i2gw/providers/istio/converter_test.go b/pkg/i2gw/providers/istio/converter_test.go index 229fa6de..2fce66f9 100644 --- a/pkg/i2gw/providers/istio/converter_test.go +++ b/pkg/i2gw/providers/istio/converter_test.go @@ -2020,10 +2020,15 @@ func Test_convertHostnames(t *testing.T) { expected: []gatewayv1alpha2.Hostname{}, }, { - name: "The wildcard label must appear by itself as the first label", + name: "The wildcard label must appear by itself as the first character", hostnames: []string{"example*.com"}, expected: []gatewayv1alpha2.Hostname{}, }, + { + name: "mix", + hostnames: []string{"192.0.2.1", "2001:db8::68", "::ffff:192.0.2.1", "*", "*.com", "test.net", "*.example.com"}, + expected: []gatewayv1alpha2.Hostname{"*.com", "test.net", "*.example.com"}, + }, } for _, tc := range cases {