Skip to content

Commit

Permalink
internal/dag: allow routes with overlapping header conditions (projec…
Browse files Browse the repository at this point in the history 
…tcontour#2600)

Fixes a bug where an HTTPProxy with multiple routes with the same prefixes
and conditions on the same headers, differing only on the condition operators
or values, would result in only the last route being configured in Envoy and
all others being dropped.

Signed-off-by: Steve Kriss <krisss@vmware.com>
  • Loading branch information
@skriss
skriss authored Jun 19, 2020
1 parent dd76482 commit e764e03
Show file tree
Hide file tree
Showing 3 changed files with 277 additions and 1 deletion.
150 changes: 150 additions & 0 deletions internal/dag/builder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1762,6 +1762,106 @@ func TestDAGInsert(t *testing.T) {
},
}

// proxy2d is a proxy with two routes that have the same prefix and a Contains header
// condition on the same header, differing only in the value of the condition.
proxy2d := &projcontour.HTTPProxy{
ObjectMeta: metav1.ObjectMeta{
Name: "example-com",
Namespace: "default",
},
Spec: projcontour.HTTPProxySpec{
VirtualHost: &projcontour.VirtualHost{
Fqdn: "example.com",
},
Routes: []projcontour.Route{
{
Conditions: []projcontour.Condition{
{
Header: &projcontour.HeaderCondition{
Name: "e-tag",
Contains: "abc",
},
},
{
Prefix: "/",
},
},
Services: []projcontour.Service{{
Name: "kuard",
Port: 8080,
}},
},
{
Conditions: []projcontour.Condition{
{
Header: &projcontour.HeaderCondition{
Name: "e-tag",
Contains: "def",
},
},
{
Prefix: "/",
},
},
Services: []projcontour.Service{{
Name: "kuard",
Port: 8080,
}},
},
},
},
}

// proxy2e is a proxy with two routes that both have a condition on the same
// header, one using Contains and one using NotContains.
proxy2e := &projcontour.HTTPProxy{
ObjectMeta: metav1.ObjectMeta{
Name: "example-com",
Namespace: "default",
},
Spec: projcontour.HTTPProxySpec{
VirtualHost: &projcontour.VirtualHost{
Fqdn: "example.com",
},
Routes: []projcontour.Route{
{
Conditions: []projcontour.Condition{
{
Header: &projcontour.HeaderCondition{
Name: "e-tag",
Contains: "abc",
},
},
{
Prefix: "/",
},
},
Services: []projcontour.Service{{
Name: "kuard",
Port: 8080,
}},
},
{
Conditions: []projcontour.Condition{
{
Header: &projcontour.HeaderCondition{
Name: "e-tag",
NotContains: "abc",
},
},
{
Prefix: "/",
},
},
Services: []projcontour.Service{{
Name: "kuard",
Port: 8080,
}},
},
},
},
}

// proxy6 has TLS and does not specify min tls version
proxy6 := &projcontour.HTTPProxy{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -4231,6 +4331,56 @@ func TestDAGInsert(t *testing.T) {
},
),
},
"insert httproxy w/ multiple routes with a Contains condition on the same header": {
objs: []interface{}{
proxy2d, s1,
},
want: listeners(
&Listener{
Port: 80,
VirtualHosts: virtualhosts(
virtualhost("example.com", &Route{
PathCondition: &PrefixCondition{Prefix: "/"},
HeaderConditions: []HeaderCondition{
{Name: "e-tag", Value: "abc", MatchType: "contains"},
},
Clusters: clusters(service(s1)),
}, &Route{
PathCondition: &PrefixCondition{Prefix: "/"},
HeaderConditions: []HeaderCondition{
{Name: "e-tag", Value: "def", MatchType: "contains"},
},
Clusters: clusters(service(s1)),
}),
),
},
),
},
"insert httproxy w/ multiple routes with condition on the same header, one Contains and one NotContains": {
objs: []interface{}{
proxy2e, s1,
},
want: listeners(
&Listener{
Port: 80,
VirtualHosts: virtualhosts(
virtualhost("example.com", &Route{
PathCondition: &PrefixCondition{Prefix: "/"},
HeaderConditions: []HeaderCondition{
{Name: "e-tag", Value: "abc", MatchType: "contains"},
},
Clusters: clusters(service(s1)),
}, &Route{
PathCondition: &PrefixCondition{Prefix: "/"},
HeaderConditions: []HeaderCondition{
{Name: "e-tag", Value: "abc", MatchType: "contains", Invert: true},
},
Clusters: clusters(service(s1)),
}),
),
},
),
},
"insert httproxy w/ included conditions": {
objs: []interface{}{
proxy2a, proxy2b, s1,
Expand Down
10 changes: 9 additions & 1 deletion internal/dag/dag.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package dag

import (
"fmt"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -79,7 +80,14 @@ type HeaderCondition struct {
}

func (hc *HeaderCondition) String() string {
return "header: " + hc.Name
details := strings.Join([]string{
"name=" + hc.Name,
"value=" + hc.Value,
"matchtype=", hc.MatchType,
"invert=", strconv.FormatBool(hc.Invert),
}, "&")

return "header: " + details
}

// Route defines the properties of a route to a Cluster.
Expand Down
118 changes: 118 additions & 0 deletions internal/featuretests/headercondition_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -388,4 +388,122 @@ func TestConditions_ContainsHeader_HTTProxy(t *testing.T) {
),
TypeUrl: routeType,
})

// proxy with two routes that have the same prefix and a Contains header
// condition on the same header, differing only in the value of the condition.
proxy6 := fixture.NewProxy("simple").WithSpec(
projcontour.HTTPProxySpec{
VirtualHost: &projcontour.VirtualHost{Fqdn: "hello.world"},
Routes: []projcontour.Route{
{
Conditions: conditions(
prefixCondition("/"),
headerContainsCondition("x-header", "abc"),
),
Services: []projcontour.Service{{
Name: "svc1",
Port: 80,
}},
},
{
Conditions: conditions(
prefixCondition("/"),
headerContainsCondition("x-header", "def"),
),
Services: []projcontour.Service{{
Name: "svc2",
Port: 80,
}},
}},
},
)

rh.OnUpdate(proxy5, proxy6)

c.Request(routeType).Equals(&v2.DiscoveryResponse{
Resources: resources(t,
envoy.RouteConfiguration("ingress_http",
envoy.VirtualHost("hello.world",
&envoy_api_v2_route.Route{
Match: routePrefix("/", dag.HeaderCondition{
Name: "x-header",
Value: "abc",
MatchType: "contains",
Invert: false,
}),
Action: routeCluster("default/svc1/80/da39a3ee5e"),
},
&envoy_api_v2_route.Route{
Match: routePrefix("/", dag.HeaderCondition{
Name: "x-header",
Value: "def",
MatchType: "contains",
Invert: false,
}),
Action: routeCluster("default/svc2/80/da39a3ee5e"),
},
),
),
),
TypeUrl: routeType,
})

// proxy with two routes that both have a condition on the same
// header, one using Contains and one using NotContains.
proxy7 := fixture.NewProxy("simple").WithSpec(
projcontour.HTTPProxySpec{
VirtualHost: &projcontour.VirtualHost{Fqdn: "hello.world"},
Routes: []projcontour.Route{
{
Conditions: conditions(
prefixCondition("/"),
headerContainsCondition("x-header", "abc"),
),
Services: []projcontour.Service{{
Name: "svc1",
Port: 80,
}},
},
{
Conditions: conditions(
prefixCondition("/"),
headerNotContainsCondition("x-header", "abc"),
),
Services: []projcontour.Service{{
Name: "svc2",
Port: 80,
}},
}},
},
)

rh.OnUpdate(proxy6, proxy7)

c.Request(routeType).Equals(&v2.DiscoveryResponse{
Resources: resources(t,
envoy.RouteConfiguration("ingress_http",
envoy.VirtualHost("hello.world",
&envoy_api_v2_route.Route{
Match: routePrefix("/", dag.HeaderCondition{
Name: "x-header",
Value: "abc",
MatchType: "contains",
Invert: false,
}),
Action: routeCluster("default/svc1/80/da39a3ee5e"),
},
&envoy_api_v2_route.Route{
Match: routePrefix("/", dag.HeaderCondition{
Name: "x-header",
Value: "abc",
MatchType: "contains",
Invert: true,
}),
Action: routeCluster("default/svc2/80/da39a3ee5e"),
},
),
),
),
TypeUrl: routeType,
})
}

0 comments on commit e764e03

Please sign in to comment.