Skip to content

Commit

Permalink
feat(expression router) move setting protocols back to kongstate over…
Browse files Browse the repository at this point in the history
…ride for expression based routes (#4422)

* move setting protocols back to kongstate override

* update unit tests and CHANGELOG

* address comments

* update golden test & fix lint

* update CHANGELOG
  • Loading branch information
randmonkey committed Jul 31, 2023
1 parent 04c465e commit b82308e
Show file tree
Hide file tree
Showing 20 changed files with 284 additions and 117 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,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

Expand Down
4 changes: 2 additions & 2 deletions internal/dataplane/kongstate/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
90 changes: 90 additions & 0 deletions internal/dataplane/kongstate/route_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package kongstate

import (
"reflect"
"strconv"
"testing"

"github.com/kong/go-kong/kong"
Expand Down Expand Up @@ -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)

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit b82308e

Please sign in to comment.