diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f464fdc7a..ad1fc1a4de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -157,6 +157,10 @@ Adding a new version? You'll need three changes: to `/status/ready`. Gateways will be considered ready only after an initial configuration is applied by the controller. [#4368](https://github.com/Kong/kubernetes-ingress-controller/pull/4368 +- When translating to expression based Kong routes, annotations to specify + protocols are translated to `protocols` field of the result Kong route, + instead of putting the conditions to match protocols inside expressions. + [#4422](https://github.com/Kong/kubernetes-ingress-controller/pull/4422) ### Fixed diff --git a/internal/dataplane/kongstate/route.go b/internal/dataplane/kongstate/route.go index f6ccbf560f..2b461b0dca 100644 --- a/internal/dataplane/kongstate/route.go +++ b/internal/dataplane/kongstate/route.go @@ -236,10 +236,10 @@ func (r *Route) overrideByAnnotation(log logrus.FieldLogger) { r.overridePreserveHost(r.Ingress.Annotations) r.overrideRequestBuffering(log, r.Ingress.Annotations) r.overrideResponseBuffering(log, r.Ingress.Annotations) + r.overrideProtocols(r.Ingress.Annotations) // skip the fields that are not supported when kong is using expression router: - // `protocols`, `regexPriority`, `methods`, `snis`, `hosts`, `headers`, `pathHandling`, + // `regexPriority`, `methods`, `snis`, `hosts`, `headers`, `pathHandling`, if !r.ExpressionRoutes { - r.overrideProtocols(r.Ingress.Annotations) r.overrideRegexPriority(r.Ingress.Annotations) r.overrideMethods(log, r.Ingress.Annotations) r.overrideSNIs(log, r.Ingress.Annotations) diff --git a/internal/dataplane/kongstate/route_test.go b/internal/dataplane/kongstate/route_test.go index 15faa322f6..318aa7b680 100644 --- a/internal/dataplane/kongstate/route_test.go +++ b/internal/dataplane/kongstate/route_test.go @@ -2,6 +2,7 @@ package kongstate import ( "reflect" + "strconv" "testing" "github.com/kong/go-kong/kong" @@ -225,6 +226,95 @@ func TestOverrideRoute(t *testing.T) { }) } +func TestOverrideExpressionRoute(t *testing.T) { + testCases := []struct { + name string + inRoute Route + outRoute Route + }{ + { + name: "protocols should be overridden, but hosts, method, headers, snis should not", + inRoute: Route{ + Route: kong.Route{ + Name: kong.String("expression-route-1"), + Expression: kong.String(`(http.host == "foo.com") && (http.path ^= "/v1/api")`), + }, + Ingress: util.K8sObjectInfo{ + Annotations: map[string]string{ + "konghq.com/protocols": "https", + "konghq.com/method": "GET", + "konghq.com/host-aliases": "bar.com", + "konghq.com/headers.foo": "bar", + "kohghq.com/snis": "foo.com,bar.com", + }, + }, + ExpressionRoutes: true, + }, + outRoute: Route{ + Route: kong.Route{ + Name: kong.String("expression-route-1"), + Expression: kong.String(`(http.host == "foo.com") && (http.path ^= "/v1/api")`), + Protocols: kong.StringSlice("https"), + }, + Ingress: util.K8sObjectInfo{ + Annotations: map[string]string{ + "konghq.com/protocols": "https", + "konghq.com/method": "GET", + "konghq.com/host-aliases": "bar.com", + "konghq.com/headers.foo": "bar", + "kohghq.com/snis": "foo.com,bar.com", + }, + }, + ExpressionRoutes: true, + }, + }, + { + name: "request_buffering, response_buffering should be overridden, but regex_priority, path_handling should not", + inRoute: Route{ + Route: kong.Route{ + Name: kong.String("expression-route-2"), + Expression: kong.String(`(http.host == "foo.com") && (http.path ^= "/v1/api")`), + }, + Ingress: util.K8sObjectInfo{ + Annotations: map[string]string{ + "konghq.com/request-buffering": "true", + "konghq.com/response-buffering": "true", + "konghq.com/regex-priority": "100", + "konghq.com/path-handling": "v1", + }, + }, + ExpressionRoutes: true, + }, + outRoute: Route{ + Route: kong.Route{ + Name: kong.String("expression-route-2"), + Expression: kong.String(`(http.host == "foo.com") && (http.path ^= "/v1/api")`), + RequestBuffering: kong.Bool(true), + ResponseBuffering: kong.Bool(true), + }, + Ingress: util.K8sObjectInfo{ + Annotations: map[string]string{ + "konghq.com/request-buffering": "true", + "konghq.com/response-buffering": "true", + "konghq.com/regex-priority": "100", + "konghq.com/path-handling": "v1", + }, + }, + ExpressionRoutes: true, + }, + }, + } + + for i, tc := range testCases { + indexStr := strconv.Itoa(i) + tc := tc + t.Run(indexStr+"-"+tc.name, func(t *testing.T) { + tc.inRoute.override(logrus.New(), nil) + assert.Equal(t, tc.outRoute, tc.inRoute, "should be the same as expected after overriding") + }) + } +} + func TestOverrideRoutePriority(t *testing.T) { assert := assert.New(t) diff --git a/internal/dataplane/parser/testdata/golden/httproute-example/expression-routes-on_golden.yaml b/internal/dataplane/parser/testdata/golden/httproute-example/expression-routes-on_golden.yaml index ab5d8b66fc..29fde19ff6 100644 --- a/internal/dataplane/parser/testdata/golden/httproute-example/expression-routes-on_golden.yaml +++ b/internal/dataplane/parser/testdata/golden/httproute-example/expression-routes-on_golden.yaml @@ -7,8 +7,7 @@ services: read_timeout: 60000 retries: 5 routes: - - expression: ((net.protocol == "http") || (net.protocol == "https")) && ((http.path - == "/httproute-testing") || (http.path ^= "/httproute-testing/")) + - expression: (http.path == "/httproute-testing") || (http.path ^= "/httproute-testing/") https_redirect_status_code: 426 name: httproute..httproute-testing._.0.0 preserve_host: true diff --git a/internal/dataplane/parser/testdata/golden/ingress-v1-empty-path/expression-routes-on_golden.yaml b/internal/dataplane/parser/testdata/golden/ingress-v1-empty-path/expression-routes-on_golden.yaml index d5be1d73dd..be6c47d147 100644 --- a/internal/dataplane/parser/testdata/golden/ingress-v1-empty-path/expression-routes-on_golden.yaml +++ b/internal/dataplane/parser/testdata/golden/ingress-v1-empty-path/expression-routes-on_golden.yaml @@ -9,8 +9,7 @@ services: read_timeout: 60000 retries: 5 routes: - - expression: (http.host == "example.com") && (http.path ^= "/") && ((net.protocol - == "http") || (net.protocol == "https")) + - expression: (http.host == "example.com") && (http.path ^= "/") https_redirect_status_code: 426 name: foo-namespace.foo.foo-svc.example.com.80 preserve_host: true diff --git a/internal/dataplane/parser/testdata/golden/ingress-v1-multiple-ports-for-one-service/expression-routes-on_golden.yaml b/internal/dataplane/parser/testdata/golden/ingress-v1-multiple-ports-for-one-service/expression-routes-on_golden.yaml index 29d7a6c26f..783bd201fa 100644 --- a/internal/dataplane/parser/testdata/golden/ingress-v1-multiple-ports-for-one-service/expression-routes-on_golden.yaml +++ b/internal/dataplane/parser/testdata/golden/ingress-v1-multiple-ports-for-one-service/expression-routes-on_golden.yaml @@ -9,8 +9,7 @@ services: read_timeout: 60000 retries: 5 routes: - - expression: (http.host == "example.net") && (http.path ^= "/") && ((net.protocol - == "http") || (net.protocol == "https")) + - expression: (http.host == "example.net") && (http.path ^= "/") https_redirect_status_code: 426 name: foo-namespace.foo.foo-svc.example.net.8000 preserve_host: true @@ -39,8 +38,7 @@ services: read_timeout: 60000 retries: 5 routes: - - expression: (http.host == "example.com") && (http.path ^= "/") && ((net.protocol - == "http") || (net.protocol == "https")) + - expression: (http.host == "example.com") && (http.path ^= "/") https_redirect_status_code: 426 name: foo-namespace.foo.foo-svc.example.com.80 preserve_host: true diff --git a/internal/dataplane/parser/testdata/golden/ingress-v1-ports-defined-by-name/expression-routes-on_golden.yaml b/internal/dataplane/parser/testdata/golden/ingress-v1-ports-defined-by-name/expression-routes-on_golden.yaml index c727407ce2..126bc64a71 100644 --- a/internal/dataplane/parser/testdata/golden/ingress-v1-ports-defined-by-name/expression-routes-on_golden.yaml +++ b/internal/dataplane/parser/testdata/golden/ingress-v1-ports-defined-by-name/expression-routes-on_golden.yaml @@ -9,8 +9,7 @@ services: read_timeout: 60000 retries: 5 routes: - - expression: (http.host == "example.com") && (http.path ^= "/") && ((net.protocol - == "http") || (net.protocol == "https")) + - expression: (http.host == "example.com") && (http.path ^= "/") https_redirect_status_code: 426 name: foo-namespace.regex-prefix.foo-svc.example.com.http preserve_host: true diff --git a/internal/dataplane/parser/testdata/golden/ingress-v1-regex-prefix-exact-rule/expression-routes-on_golden.yaml b/internal/dataplane/parser/testdata/golden/ingress-v1-regex-prefix-exact-rule/expression-routes-on_golden.yaml index 6acaacf23c..3d3cc454e4 100644 --- a/internal/dataplane/parser/testdata/golden/ingress-v1-regex-prefix-exact-rule/expression-routes-on_golden.yaml +++ b/internal/dataplane/parser/testdata/golden/ingress-v1-regex-prefix-exact-rule/expression-routes-on_golden.yaml @@ -9,8 +9,7 @@ services: read_timeout: 60000 retries: 5 routes: - - expression: (http.host == "example.com") && (http.path == "/whatever") && ((net.protocol - == "http") || (net.protocol == "https")) + - expression: (http.host == "example.com") && (http.path == "/whatever") https_redirect_status_code: 426 name: foo-namespace.foo.foo-svc.example.com.80 preserve_host: true diff --git a/internal/dataplane/parser/testdata/golden/ingress-v1-regex-prefixed-path/expression-routes-on_golden.yaml b/internal/dataplane/parser/testdata/golden/ingress-v1-regex-prefixed-path/expression-routes-on_golden.yaml index 8cf1085015..3ee102bb31 100644 --- a/internal/dataplane/parser/testdata/golden/ingress-v1-regex-prefixed-path/expression-routes-on_golden.yaml +++ b/internal/dataplane/parser/testdata/golden/ingress-v1-regex-prefixed-path/expression-routes-on_golden.yaml @@ -9,8 +9,7 @@ services: read_timeout: 60000 retries: 5 routes: - - expression: (http.host == "example.com") && (http.path ~ "^/foo/\\d{3}") && ((net.protocol - == "http") || (net.protocol == "https")) + - expression: (http.host == "example.com") && (http.path ~ "^/foo/\\d{3}") https_redirect_status_code: 426 name: foo-namespace.regex-prefix.foo-svc.example.com.80 preserve_host: true diff --git a/internal/dataplane/parser/testdata/golden/ingress-v1-rule-with-tls/expression-routes-on_golden.yaml b/internal/dataplane/parser/testdata/golden/ingress-v1-rule-with-tls/expression-routes-on_golden.yaml index ea97bfca2d..40d3594d2b 100644 --- a/internal/dataplane/parser/testdata/golden/ingress-v1-rule-with-tls/expression-routes-on_golden.yaml +++ b/internal/dataplane/parser/testdata/golden/ingress-v1-rule-with-tls/expression-routes-on_golden.yaml @@ -76,8 +76,7 @@ services: read_timeout: 60000 retries: 5 routes: - - expression: (http.host == "example.com") && (http.path == "/") && ((net.protocol - == "http") || (net.protocol == "https")) + - expression: (http.host == "example.com") && (http.path == "/") https_redirect_status_code: 426 name: bar-namespace.ing-with-tls.foo-svc.example.com.80 preserve_host: true diff --git a/internal/dataplane/parser/testdata/golden/ingress-v1-single-service-in-multiple-ingresses/expression-routes-on_golden.yaml b/internal/dataplane/parser/testdata/golden/ingress-v1-single-service-in-multiple-ingresses/expression-routes-on_golden.yaml index d816bc5543..e05be36c80 100644 --- a/internal/dataplane/parser/testdata/golden/ingress-v1-single-service-in-multiple-ingresses/expression-routes-on_golden.yaml +++ b/internal/dataplane/parser/testdata/golden/ingress-v1-single-service-in-multiple-ingresses/expression-routes-on_golden.yaml @@ -9,8 +9,7 @@ services: read_timeout: 60000 retries: 5 routes: - - expression: (http.host == "example.com") && (http.path ^= "/") && ((net.protocol - == "http") || (net.protocol == "https")) + - expression: (http.host == "example.com") && (http.path ^= "/") https_redirect_status_code: 426 name: foo-namespace.foo.foo-svc.example.com.80 preserve_host: true @@ -24,8 +23,7 @@ services: - k8s-kind:Ingress - k8s-group:networking.k8s.io - k8s-version:v1 - - expression: (http.host == "example.com") && (http.path ^= "/") && ((net.protocol - == "http") || (net.protocol == "https")) + - expression: (http.host == "example.com") && (http.path ^= "/") https_redirect_status_code: 426 name: foo-namespace.foo-2.foo-svc.example.com.80 preserve_host: true diff --git a/internal/dataplane/parser/testdata/golden/ingress-v1-with-acme-like-path/expression-routes-on_golden.yaml b/internal/dataplane/parser/testdata/golden/ingress-v1-with-acme-like-path/expression-routes-on_golden.yaml index 9bfe4c196e..d367a95235 100644 --- a/internal/dataplane/parser/testdata/golden/ingress-v1-with-acme-like-path/expression-routes-on_golden.yaml +++ b/internal/dataplane/parser/testdata/golden/ingress-v1-with-acme-like-path/expression-routes-on_golden.yaml @@ -10,7 +10,6 @@ services: retries: 5 routes: - expression: (http.host == "example.com") && (http.path ^= "/.well-known/acme-challenge/yolo") - && ((net.protocol == "http") || (net.protocol == "https")) https_redirect_status_code: 426 name: foo-namespace.foo.cert-manager-solver-pod.example.com.80 preserve_host: true diff --git a/internal/dataplane/parser/testdata/golden/ingress-v1-with-default-backend/expression-routes-on_golden.yaml b/internal/dataplane/parser/testdata/golden/ingress-v1-with-default-backend/expression-routes-on_golden.yaml index 5d3a6ba279..99dd0c15bf 100644 --- a/internal/dataplane/parser/testdata/golden/ingress-v1-with-default-backend/expression-routes-on_golden.yaml +++ b/internal/dataplane/parser/testdata/golden/ingress-v1-with-default-backend/expression-routes-on_golden.yaml @@ -9,8 +9,7 @@ services: read_timeout: 60000 retries: 5 routes: - - expression: (http.host == "example.com") && (http.path == "/") && ((net.protocol - == "http") || (net.protocol == "https")) + - expression: (http.host == "example.com") && (http.path == "/") https_redirect_status_code: 426 name: foo-namespace.foo.foo-svc.example.com.80 preserve_host: true diff --git a/internal/dataplane/parser/translate_httproute_test.go b/internal/dataplane/parser/translate_httproute_test.go index f868977bfd..dc457490db 100644 --- a/internal/dataplane/parser/translate_httproute_test.go +++ b/internal/dataplane/parser/translate_httproute_test.go @@ -1534,7 +1534,7 @@ func TestIngressRulesFromHTTPRoutesUsingExpressionRoutes(t *testing.T) { { Route: kong.Route{ Name: kong.String("httproute.default.httproute-1._.0.0"), - Expression: kong.String(`((net.protocol == "http") || (net.protocol == "https")) && (http.path == "/v1/foo")`), + Expression: kong.String(`http.path == "/v1/foo"`), PreserveHost: kong.Bool(true), }, Plugins: []kong.Plugin{}, @@ -1543,7 +1543,7 @@ func TestIngressRulesFromHTTPRoutesUsingExpressionRoutes(t *testing.T) { { Route: kong.Route{ Name: kong.String("httproute.default.httproute-1._.0.1"), - Expression: kong.String(`((net.protocol == "http") || (net.protocol == "https")) && (http.path == "/v1/barr")`), + Expression: kong.String(`http.path == "/v1/barr"`), PreserveHost: kong.Bool(true), }, Plugins: []kong.Plugin{}, @@ -1638,7 +1638,7 @@ func TestIngressRulesFromHTTPRoutesUsingExpressionRoutes(t *testing.T) { { Route: kong.Route{ Name: kong.String("httproute.default.httproute-1.foo.com.0.0"), - Expression: kong.String(`(http.host == "foo.com") && ((net.protocol == "http") || (net.protocol == "https")) && (http.path == "/v1/foo")`), + Expression: kong.String(`(http.host == "foo.com") && (http.path == "/v1/foo")`), PreserveHost: kong.Bool(true), }, Plugins: []kong.Plugin{}, @@ -1649,7 +1649,7 @@ func TestIngressRulesFromHTTPRoutesUsingExpressionRoutes(t *testing.T) { { Route: kong.Route{ Name: kong.String("httproute.default.httproute-1._.bar.com.0.0"), - Expression: kong.String(`(http.host =^ ".bar.com") && ((net.protocol == "http") || (net.protocol == "https")) && (http.path == "/v1/foo")`), + Expression: kong.String(`(http.host =^ ".bar.com") && (http.path == "/v1/foo")`), PreserveHost: kong.Bool(true), }, Plugins: []kong.Plugin{}, @@ -1660,7 +1660,7 @@ func TestIngressRulesFromHTTPRoutesUsingExpressionRoutes(t *testing.T) { { Route: kong.Route{ Name: kong.String("httproute.default.httproute-1.foo.com.1.0"), - Expression: kong.String(`(http.host == "foo.com") && ((net.protocol == "http") || (net.protocol == "https")) && (http.path == "/v1/barr")`), + Expression: kong.String(`(http.host == "foo.com") && (http.path == "/v1/barr")`), PreserveHost: kong.Bool(true), }, Plugins: []kong.Plugin{}, @@ -1671,7 +1671,64 @@ func TestIngressRulesFromHTTPRoutesUsingExpressionRoutes(t *testing.T) { { Route: kong.Route{ Name: kong.String("httproute.default.httproute-1._.bar.com.1.0"), - Expression: kong.String(`(http.host =^ ".bar.com") && ((net.protocol == "http") || (net.protocol == "https")) && (http.path == "/v1/barr")`), + Expression: kong.String(`(http.host =^ ".bar.com") && (http.path == "/v1/barr")`), + PreserveHost: kong.Bool(true), + }, + Plugins: []kong.Plugin{}, + ExpressionRoutes: true, + }, + }, + }, + }, + { + name: "single HTTPRoute with protocol and SNI annotations", + httpRoutes: []*gatewayv1beta1.HTTPRoute{ + { + TypeMeta: httpRouteTypeMeta, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "httproute-1", + Annotations: map[string]string{ + "konghq.com/protocols": "https", + "konghq.com/snis": "foo.com", + }, + }, + Spec: gatewayv1beta1.HTTPRouteSpec{ + Hostnames: []gatewayv1beta1.Hostname{ + "foo.com", + }, + Rules: []gatewayv1beta1.HTTPRouteRule{ + { + Matches: []gatewayv1beta1.HTTPRouteMatch{ + builder.NewHTTPRouteMatch().WithPathExact("/v1/foo").Build(), + }, + BackendRefs: []gatewayv1beta1.HTTPBackendRef{ + builder.NewHTTPBackendRef("service1").WithPort(80).Build(), + }, + }, + }, + }, + }, + }, + expectedKongServices: []kongstate.Service{ + { + Service: kong.Service{ + Name: kong.String("httproute.default.httproute-1.foo.com.0"), + }, + Backends: []kongstate.ServiceBackend{ + { + Name: "service1", + PortDef: kongstate.PortDef{Mode: kongstate.PortModeByNumber, Number: int32(80)}, + }, + }, + }, + }, + expectedKongRoutes: map[string][]kongstate.Route{ + "httproute.default.httproute-1.foo.com.0": { + { + Route: kong.Route{ + Name: kong.String("httproute.default.httproute-1.foo.com.0.0"), + Expression: kong.String(`(http.host == "foo.com") && (tls.sni == "foo.com") && (http.path == "/v1/foo")`), PreserveHost: kong.Bool(true), }, Plugins: []kong.Plugin{}, @@ -1734,7 +1791,7 @@ func TestIngressRulesFromHTTPRoutesUsingExpressionRoutes(t *testing.T) { { Route: kong.Route{ Name: kong.String("httproute.default.httproute-1.foo.com.0.0"), - Expression: kong.String(`(http.host == "foo.com") && ((net.protocol == "http") || (net.protocol == "https")) && (http.path == "/v1/foo")`), + Expression: kong.String(`(http.host == "foo.com") && (http.path == "/v1/foo")`), PreserveHost: kong.Bool(true), }, Plugins: []kong.Plugin{}, @@ -1781,7 +1838,7 @@ func TestIngressRulesFromHTTPRoutesUsingExpressionRoutes(t *testing.T) { routeName := expectedRoute.Name r, ok := kongRouteNameToRoute[*routeName] require.Truef(t, ok, "should find route %s", *routeName) - require.Equal(t, expectedRoute.Expression, r.Expression) + require.Equal(t, *expectedRoute.Expression, *r.Expression) } } // check translation failures @@ -1854,7 +1911,7 @@ func TestIngressRulesWithPriority(t *testing.T) { expectedKongRoute: kongstate.Route{ Route: kong.Route{ Name: kong.String("httproute.default.httproute-1._.0.0"), - Expression: kong.String(`((net.protocol == "http") || (net.protocol == "https")) && (http.path == "/v1/foo")`), + Expression: kong.String(`http.path == "/v1/foo"`), PreserveHost: kong.Bool(true), StripPath: kong.Bool(false), Priority: kong.Int(1024), @@ -1914,7 +1971,7 @@ func TestIngressRulesWithPriority(t *testing.T) { expectedKongRoute: kongstate.Route{ Route: kong.Route{ Name: kong.String("httproute.default.httproute-1.foo.com.0.1"), - Expression: kong.String(`(http.host == "foo.com") && ((net.protocol == "http") || (net.protocol == "https")) && (http.path == "/v1/foo")`), + Expression: kong.String(`(http.host == "foo.com") && (http.path == "/v1/foo")`), PreserveHost: kong.Bool(true), StripPath: kong.Bool(false), Priority: kong.Int(1024), @@ -1990,7 +2047,7 @@ func TestIngressRulesWithPriority(t *testing.T) { expectedKongRoute: kongstate.Route{ Route: kong.Route{ Name: kong.String("httproute.default.httproute-1._.foo.com.0.0"), - Expression: kong.String(`(http.host =^ ".foo.com") && ((net.protocol == "http") || (net.protocol == "https")) && (http.path == "/v1/foo")`), + Expression: kong.String(`(http.host =^ ".foo.com") && (http.path == "/v1/foo")`), PreserveHost: kong.Bool(true), StripPath: kong.Bool(false), Priority: kong.Int(1024), diff --git a/internal/dataplane/parser/translators/grpcroute_atc.go b/internal/dataplane/parser/translators/grpcroute_atc.go index e96c62579c..f0387a23e6 100644 --- a/internal/dataplane/parser/translators/grpcroute_atc.go +++ b/internal/dataplane/parser/translators/grpcroute_atc.go @@ -74,16 +74,6 @@ func generateMathcherFromGRPCMatch(match gatewayv1alpha2.GRPCRouteMatch, hostnam routeMatcher.And(hostMatcher) } - // override protocols from annotations. - // Because Kong expression based router extracts net.protocol field from scheme of request, - // GRPC over HTTP/2 requests could not be matched if protocol is set to grpc/grpcs since protocol could only be http or https. - // So we do not AND a protocol matcher if no protocol is specified in annotations. - protocols := annotations.ExtractProtocolNames(metaAnnotations) - if len(protocols) > 0 { - protocolMatcher := protocolMatcherFromProtocols(protocols) - routeMatcher.And(protocolMatcher) - } - snis, exist := annotations.ExtractSNIs(metaAnnotations) if exist && len(snis) > 0 { sniMatcher := sniMatcherFromSNIs(snis) diff --git a/internal/dataplane/parser/translators/grpcroute_atc_test.go b/internal/dataplane/parser/translators/grpcroute_atc_test.go index 12479b5f6e..fe270fcfa4 100644 --- a/internal/dataplane/parser/translators/grpcroute_atc_test.go +++ b/internal/dataplane/parser/translators/grpcroute_atc_test.go @@ -189,7 +189,7 @@ func TestGenerateKongExpressionRoutesFromGRPCRouteRule(t *testing.T) { ExpressionRoutes: true, Route: kong.Route{ Name: kong.String("grpcroute.default.single-match-with-annotations.0.0"), - Expression: kong.String(`(http.path == "/service0/method0") && (net.protocol == "https") && (tls.sni == "kong.foo.com")`), + Expression: kong.String(`(http.path == "/service0/method0") && (tls.sni == "kong.foo.com")`), Priority: kong.Int(1), }, }, diff --git a/internal/dataplane/parser/translators/httproute_atc.go b/internal/dataplane/parser/translators/httproute_atc.go index cbe57c3a54..65ca6cdf8d 100644 --- a/internal/dataplane/parser/translators/httproute_atc.go +++ b/internal/dataplane/parser/translators/httproute_atc.go @@ -59,7 +59,7 @@ func GenerateKongExpressionRoutesFromHTTPRouteMatches( // if we do not need to generate a kong route for each match, we OR matchers from all matches together. routeMatcher := atc.And(atc.Or(generateMatchersFromHTTPRouteMatches(translation.Matches)...)) - // add matcher from parent httproute (hostnames, protocols, SNIs) to be ANDed with the matcher from match. + // Add matcher from parent httproute (hostnames, SNIs) to be ANDed with the matcher from match. matchersFromParent := matchersFromParentHTTPRoute(hostnames, ingressObjectInfo.Annotations) for _, matcher := range matchersFromParent { routeMatcher.And(matcher) @@ -215,16 +215,6 @@ func matchersFromParentHTTPRoute(hostnames []string, metaAnnotations map[string] ret = append(ret, hostMatcher) } - // translate protocols. - protocols := []string{"http", "https"} - // override from "protocols" key in annotations. - annonationProtocols := annotations.ExtractProtocolNames(metaAnnotations) - if len(annonationProtocols) > 0 { - protocols = annonationProtocols - } - protocolMatcher := protocolMatcherFromProtocols(protocols) - ret = append(ret, protocolMatcher) - // translate SNIs. snis, exist := annotations.ExtractSNIs(metaAnnotations) if exist && len(snis) > 0 { diff --git a/internal/dataplane/parser/translators/httproute_atc_test.go b/internal/dataplane/parser/translators/httproute_atc_test.go index 0a8b3b1772..a52f272ade 100644 --- a/internal/dataplane/parser/translators/httproute_atc_test.go +++ b/internal/dataplane/parser/translators/httproute_atc_test.go @@ -68,7 +68,7 @@ func TestGenerateKongExpressionRoutesFromHTTPRouteMatches(t *testing.T) { Name: kong.String("prefix_path_match.defualt.0.0"), PreserveHost: kong.Bool(true), StripPath: kong.Bool(false), - Expression: kong.String(`((http.path == "/prefix") || (http.path ^= "/prefix/")) && ((net.protocol == "http") || (net.protocol == "https"))`), + Expression: kong.String(`(http.path == "/prefix") || (http.path ^= "/prefix/")`), Priority: kong.Int(1), }, Plugins: []kong.Plugin{}, @@ -90,7 +90,7 @@ func TestGenerateKongExpressionRoutesFromHTTPRouteMatches(t *testing.T) { Name: kong.String("multiple_matches.default.0.0"), PreserveHost: kong.Bool(true), StripPath: kong.Bool(false), - Expression: kong.String(`(((http.path == "/prefix") || (http.path ^= "/prefix/")) || ((http.path == "/exact") && (http.method == "GET"))) && ((net.protocol == "http") || (net.protocol == "https"))`), + Expression: kong.String(`((http.path == "/prefix") || (http.path ^= "/prefix/")) || ((http.path == "/exact") && (http.method == "GET"))`), Priority: kong.Int(1), }, Plugins: []kong.Plugin{}, @@ -119,7 +119,7 @@ func TestGenerateKongExpressionRoutesFromHTTPRouteMatches(t *testing.T) { Name: kong.String("request_redirect.default.0.0"), PreserveHost: kong.Bool(true), StripPath: kong.Bool(false), - Expression: kong.String(`(http.path == "/exact/0") && ((net.protocol == "http") || (net.protocol == "https"))`), + Expression: kong.String(`http.path == "/exact/0"`), Priority: kong.Int(1), }, Plugins: []kong.Plugin{ @@ -145,7 +145,7 @@ func TestGenerateKongExpressionRoutesFromHTTPRouteMatches(t *testing.T) { Name: kong.String("request_redirect.default.0.0"), PreserveHost: kong.Bool(true), StripPath: kong.Bool(false), - Expression: kong.String(`(http.path == "/exact/1") && ((net.protocol == "http") || (net.protocol == "https"))`), + Expression: kong.String(`http.path == "/exact/1"`), Priority: kong.Int(1), }, Plugins: []kong.Plugin{ @@ -187,7 +187,7 @@ func TestGenerateKongExpressionRoutesFromHTTPRouteMatches(t *testing.T) { Name: kong.String("request_header_mod.default.0.0"), PreserveHost: kong.Bool(true), StripPath: kong.Bool(false), - Expression: kong.String(`((http.path == "/exact/0") || (http.path ~ "^/regex/[a-z]+")) && ((net.protocol == "http") || (net.protocol == "https"))`), + Expression: kong.String(`(http.path == "/exact/0") || (http.path ~ "^/regex/[a-z]+")`), Priority: kong.Int(1), }, Plugins: []kong.Plugin{ @@ -233,7 +233,7 @@ func TestGenerateKongExpressionRoutesFromHTTPRouteMatches(t *testing.T) { Name: kong.String("annotations_protocol_sni.default.0.0"), PreserveHost: kong.Bool(true), StripPath: kong.Bool(false), - Expression: kong.String(`((http.path == "/prefix/0") || (http.path ^= "/prefix/0/")) && (http.host == "a.foo.com") && (net.protocol == "https") && (tls.sni == "a.foo.com")`), + Expression: kong.String(`((http.path == "/prefix/0") || (http.path ^= "/prefix/0/")) && (http.host == "a.foo.com") && (tls.sni == "a.foo.com")`), Priority: kong.Int(1), }, Plugins: []kong.Plugin{}, diff --git a/internal/dataplane/parser/translators/ingress_atc.go b/internal/dataplane/parser/translators/ingress_atc.go index b6253d9971..c61f1a1f3c 100644 --- a/internal/dataplane/parser/translators/ingress_atc.go +++ b/internal/dataplane/parser/translators/ingress_atc.go @@ -82,15 +82,6 @@ func (m *ingressTranslationMeta) translateIntoKongExpressionRoute() *kongstate.R } routeMatcher.And(atc.Or(pathMatchers...)) - // translate protocols. - protocols := []string{"http", "https"} - annonationProtocols := annotations.ExtractProtocolNames(ingressAnnotations) - if len(annonationProtocols) > 0 { - protocols = annonationProtocols - } - protocolMatcher := protocolMatcherFromProtocols(protocols) - routeMatcher.And(protocolMatcher) - // translate headers. headers, exist := annotations.ExtractHeaders(ingressAnnotations) if len(headers) > 0 && exist { @@ -161,18 +152,6 @@ func pathMatcherFromIngressPath(httpIngressPath netv1.HTTPIngressPath, regexPath return nil } -// protocolMatcherFromProtocols gernerates matchers from protocols. -func protocolMatcherFromProtocols(protocols []string) atc.Matcher { - matchers := []atc.Matcher{} - for _, protocol := range protocols { - if !util.ValidateProtocol(protocol) { - continue - } - matchers = append(matchers, atc.NewPredicateNetProtocol(atc.OpEqual, protocol)) - } - return atc.Or(matchers...) -} - // headerMatcherFromHeaders generates matcher to match headers in HTTP requests. func headerMatcherFromHeaders(headers map[string][]string) atc.Matcher { matchers := make([]atc.Matcher, 0, len(headers)) diff --git a/internal/dataplane/parser/translators/ingress_atc_test.go b/internal/dataplane/parser/translators/ingress_atc_test.go index ec6da61b39..783fb80ada 100644 --- a/internal/dataplane/parser/translators/ingress_atc_test.go +++ b/internal/dataplane/parser/translators/ingress_atc_test.go @@ -73,7 +73,7 @@ func TestTranslateIngressATC(t *testing.T) { }, Route: kong.Route{ Name: kong.String("default.test-ingress.test-service.konghq.com.80"), - Expression: kong.String(`(http.host == "konghq.com") && ((http.path == "/api") || (http.path ^= "/api/")) && ((net.protocol == "http") || (net.protocol == "https"))`), + Expression: kong.String(`(http.host == "konghq.com") && ((http.path == "/api") || (http.path ^= "/api/"))`), Priority: kong.Int(IngressRoutePriorityTraits{ MatchFields: 2, PlainHostOnly: true, @@ -150,7 +150,7 @@ func TestTranslateIngressATC(t *testing.T) { }, Route: kong.Route{ Name: kong.String("default.test-ingress.test-service.konghq.com.80"), - Expression: kong.String(`(http.host == "konghq.com") && (http.path ^= "/api/") && ((net.protocol == "http") || (net.protocol == "https"))`), + Expression: kong.String(`(http.host == "konghq.com") && (http.path ^= "/api/")`), Priority: kong.Int(IngressRoutePriorityTraits{ MatchFields: 2, PlainHostOnly: true, @@ -177,6 +177,105 @@ func TestTranslateIngressATC(t *testing.T) { }, }, }, + { + name: "an ingress with method, protocol, and header annotations", + ingress: &netv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ingress-annotations", + Namespace: corev1.NamespaceDefault, + Annotations: map[string]string{ + "konghq.com/methods": "GET", + "konghq.com/protocols": "http", + "konghq.com/headers.foo": "bar", + }, + }, + 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, + }, + }, + }, + }}, + }, + }, + }}, + }, + }, + expectedServices: map[string]kongstate.Service{ + "default.test-ingress-annotations.test-service.80": { + Namespace: corev1.NamespaceDefault, + Service: kong.Service{ + Name: kong.String("default.test-ingress-annotations.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-annotations", + Namespace: corev1.NamespaceDefault, + Annotations: map[string]string{ + "konghq.com/methods": "GET", + "konghq.com/protocols": "http", + "konghq.com/headers.foo": "bar", + }, + }, + Route: kong.Route{ + Name: kong.String("default.test-ingress-annotations.test-service.konghq.com.80"), + Expression: kong.String(`(http.host == "konghq.com") && (http.path ^= "/api/") && (http.headers.foo == "bar") && (http.method == "GET")`), + Priority: kong.Int(IngressRoutePriorityTraits{ + MatchFields: 4, + PlainHostOnly: true, + MaxPathLength: 5, + HasRegexPath: false, + HeaderCount: 1, + }.EncodeToPriority()), + PreserveHost: kong.Bool(true), + StripPath: kong.Bool(false), + ResponseBuffering: kong.Bool(true), + RequestBuffering: kong.Bool(true), + Tags: kong.StringSlice("k8s-name:test-ingress-annotations", "k8s-namespace:default"), + }, + ExpressionRoutes: true, + }}, + Backends: []kongstate.ServiceBackend{{ + Name: "test-service", + Namespace: corev1.NamespaceDefault, + PortDef: kongstate.PortDef{ + Mode: kongstate.PortModeByNumber, + Number: 80, + }, + }}, + Parent: &netv1.Ingress{ + TypeMeta: metav1.TypeMeta{Kind: "Ingress", APIVersion: netv1.SchemeGroupVersion.String()}, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ingress-annotations", + Namespace: corev1.NamespaceDefault, + Annotations: map[string]string{ + "konghq.com/methods": "GET", + "konghq.com/protocols": "http", + "konghq.com/headers.foo": "bar", + }, + }, + }, + }, + }, + }, } for _, tc := range testCases { @@ -518,36 +617,6 @@ func TestHeaderMatcherFromHeaders(t *testing.T) { } } -func TestProtocolMatcherFromProtocols(t *testing.T) { - testCases := []struct { - name string - protocols []string - expression string - }{ - { - name: "single protocol", - protocols: []string{"https"}, - expression: `net.protocol == "https"`, - }, - { - name: "multiple protocols", - protocols: []string{"http", "https"}, - expression: `(net.protocol == "http") || (net.protocol == "https")`, - }, - { - name: "multiple protocols including invalid protocol", - protocols: []string{"http", "ppp"}, - expression: `net.protocol == "http"`, - }, - } - - for _, tc := range testCases { - tc := tc - matcher := protocolMatcherFromProtocols(tc.protocols) - require.Equal(t, tc.expression, matcher.Expression()) - } -} - func TestMethodMatcherFromMethods(t *testing.T) { testCases := []struct { name string