Skip to content

Commit

Permalink
feat: disable strip path by default to conform to Ingress spec
Browse files Browse the repository at this point in the history 
Ingress specification isn't clear but it is assumed that proxies do
transparent proxying, i.e. they don't alter or change the requests as
they are proxied to the Kubernetes service.

This is an unfortunate breaking change that will cause problems with
adoption of the new version. Strip path setting can be tweaked either
via KongIngress resource or via an annotation on the Ingress resource.

From #457
  • Loading branch information
@hbagdi
hbagdi authored Feb 19, 2020
1 parent eaf714c commit d45d1de
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 10 deletions.
6 changes: 2 additions & 4 deletions internal/ingress/controller/parser/parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -482,8 +482,6 @@ func (p *Parser) parseIngressRules(
for j, rule := range rule.HTTP.Paths {
path := rule.Path

isACMEChallenge := strings.HasPrefix(path, "/.well-known/acme-challenge/")

if path == "" {
path = "/"
}
Expand All @@ -500,7 +498,7 @@ func (p *Parser) parseIngressRules(
// order?
Name: kong.String(ingress.Namespace + "." + ingress.Name + "." + strconv.Itoa(i) + strconv.Itoa(j)),
Paths: kong.StringSlice(path),
StripPath: kong.Bool(!isACMEChallenge),
StripPath: kong.Bool(false),
PreserveHost: kong.Bool(true),
Protocols: kong.StringSlice("http", "https"),
RegexPriority: kong.Int(0),
Expand Down Expand Up @@ -661,7 +659,7 @@ func (p *Parser) parseIngressRules(
Route: kong.Route{
Name: kong.String(ingress.Namespace + "." + ingress.Name),
Paths: kong.StringSlice("/"),
StripPath: kong.Bool(true),
StripPath: kong.Bool(false),
PreserveHost: kong.Bool(true),
Protocols: kong.StringSlice("http", "https"),
RegexPriority: kong.Int(0),
Expand Down
89 changes: 83 additions & 6 deletions internal/ingress/controller/parser/parser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,84 @@ func TestServiceClientCertificate(t *testing.T) {

func TestKongRouteAnnotations(t *testing.T) {
assert := assert.New(t)
t.Run("strip-path annotation is correctly processed", func(t *testing.T) {
t.Run("strip-path annotation is correctly processed (true)", func(t *testing.T) {
ingresses := []*networking.Ingress{
{
ObjectMeta: metav1.ObjectMeta{
Name: "bar",
Namespace: "default",
Annotations: map[string]string{
"configuration.konghq.com/strip-path": "trUe",
},
},
Spec: networking.IngressSpec{
Rules: []networking.IngressRule{
{
Host: "example.com",
IngressRuleValue: networking.IngressRuleValue{
HTTP: &networking.HTTPIngressRuleValue{
Paths: []networking.HTTPIngressPath{
{
Path: "/",
Backend: networking.IngressBackend{
ServiceName: "foo-svc",
ServicePort: intstr.FromInt(80),
},
},
},
},
},
},
},
},
},
}

services := []*corev1.Service{
{
ObjectMeta: metav1.ObjectMeta{
Name: "foo-svc",
Namespace: "default",
},
},
}
store, err := store.NewFakeStore(store.FakeObjects{
Ingresses: ingresses,
Services: services,
})
assert.Nil(err)
parser := New(store)
state, err := parser.Build()
assert.Nil(err)
assert.NotNil(state)

assert.Equal(1, len(state.Services),
"expected one service to be rendered")
assert.Equal(kong.Service{
Name: kong.String("default.foo-svc.80"),
Host: kong.String("foo-svc.default.80.svc"),
Path: kong.String("/"),
Port: kong.Int(80),
ConnectTimeout: kong.Int(60000),
ReadTimeout: kong.Int(60000),
WriteTimeout: kong.Int(60000),
Retries: kong.Int(5),
Protocol: kong.String("http"),
}, state.Services[0].Service)

assert.Equal(1, len(state.Services[0].Routes),
"expected one route to be rendered")
assert.Equal(kong.Route{
Name: kong.String("default.bar.00"),
StripPath: kong.Bool(true),
Hosts: kong.StringSlice("example.com"),
PreserveHost: kong.Bool(true),
Paths: kong.StringSlice("/"),
Protocols: kong.StringSlice("http", "https"),
RegexPriority: kong.Int(0),
}, state.Services[0].Routes[0].Route)
})
t.Run("strip-path annotation is correctly processed (false)", func(t *testing.T) {
ingresses := []*networking.Ingress{
{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -427,7 +504,7 @@ func TestKongRouteAnnotations(t *testing.T) {
"expected one route to be rendered")
assert.Equal(kong.Route{
Name: kong.String("default.bar.00"),
StripPath: kong.Bool(true),
StripPath: kong.Bool(false),
HTTPSRedirectStatusCode: kong.Int(301),
Hosts: kong.StringSlice("example.com"),
PreserveHost: kong.Bool(true),
Expand Down Expand Up @@ -506,7 +583,7 @@ func TestKongRouteAnnotations(t *testing.T) {
"expected one route to be rendered")
assert.Equal(kong.Route{
Name: kong.String("default.bar.00"),
StripPath: kong.Bool(true),
StripPath: kong.Bool(false),
Hosts: kong.StringSlice("example.com"),
PreserveHost: kong.Bool(true),
Paths: kong.StringSlice("/"),
Expand Down Expand Up @@ -584,7 +661,7 @@ func TestKongRouteAnnotations(t *testing.T) {
"expected one route to be rendered")
assert.Equal(kong.Route{
Name: kong.String("default.bar.00"),
StripPath: kong.Bool(true),
StripPath: kong.Bool(false),
Hosts: kong.StringSlice("example.com"),
PreserveHost: kong.Bool(false),
Paths: kong.StringSlice("/"),
Expand Down Expand Up @@ -662,7 +739,7 @@ func TestKongRouteAnnotations(t *testing.T) {
"expected one route to be rendered")
assert.Equal(kong.Route{
Name: kong.String("default.bar.00"),
StripPath: kong.Bool(true),
StripPath: kong.Bool(false),
Hosts: kong.StringSlice("example.com"),
PreserveHost: kong.Bool(true),
Paths: kong.StringSlice("/"),
Expand Down Expand Up @@ -743,7 +820,7 @@ func TestKongServiceAnnotations(t *testing.T) {
"expected one route to be rendered")
assert.Equal(kong.Route{
Name: kong.String("default.bar.00"),
StripPath: kong.Bool(true),
StripPath: kong.Bool(false),
Hosts: kong.StringSlice("example.com"),
PreserveHost: kong.Bool(true),
Paths: kong.StringSlice("/"),
Expand Down

0 comments on commit d45d1de

Please sign in to comment.