Skip to content

Commit

Permalink
internal/dag: Filter route Kind by listener
Browse files Browse the repository at this point in the history 
Filters the route kind by listener so that only the specified route type
will be processed by a listener.

Fixes projectcontour#3529

Signed-off-by: Steve Sloka <slokas@vmware.com>
  • Loading branch information
@stevesloka
stevesloka committed May 26, 2021
1 parent a61c636 commit 2498d76
Show file tree
Hide file tree
Showing 3 changed files with 186 additions and 62 deletions.
37 changes: 33 additions & 4 deletions internal/dag/builder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,35 @@ func TestDAGInsertGatewayAPI(t *testing.T) {
},
}

gatewayWithTLSRouteSelector := &gatewayapi_v1alpha1.Gateway{
ObjectMeta: metav1.ObjectMeta{
Name: "contour",
Namespace: "projectcontour",
},
Spec: gatewayapi_v1alpha1.GatewaySpec{
Listeners: []gatewayapi_v1alpha1.Listener{{
Port: 80,
Protocol: gatewayapi_v1alpha1.HTTPProtocolType,
Routes: gatewayapi_v1alpha1.RouteBindingSelector{
Kind: KindTLSRoute,
Namespaces: &gatewayapi_v1alpha1.RouteNamespaces{
From: routeSelectTypePtr(gatewayapi_v1alpha1.RouteSelectSame),
},
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"app": "contour",
},
MatchExpressions: []metav1.LabelSelectorRequirement{{
Key: "type",
Operator: "In",
Values: []string{"controller"},
}},
},
},
}},
},
}

gatewayWithSameNamespace := &gatewayapi_v1alpha1.Gateway{
ObjectMeta: metav1.ObjectMeta{
Name: "contour",
Expand Down Expand Up @@ -2272,7 +2301,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) {
),
},
"basic TLSRoute": {
gateway: gatewayWithSelector,
gateway: gatewayWithTLSRouteSelector,
objs: []interface{}{
kuardService,
&gatewayapi_v1alpha1.TLSRoute{
Expand Down Expand Up @@ -2314,7 +2343,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) {
),
},
"TLSRoute with multiple SNIs": {
gateway: gatewayWithSelector,
gateway: gatewayWithTLSRouteSelector,
objs: []interface{}{
kuardService,
&gatewayapi_v1alpha1.TLSRoute{
Expand Down Expand Up @@ -2376,7 +2405,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) {
),
},
"TLSRoute with multiple SNIs, one is invalid": {
gateway: gatewayWithSelector,
gateway: gatewayWithTLSRouteSelector,
objs: []interface{}{
kuardService,
&gatewayapi_v1alpha1.TLSRoute{
Expand Down Expand Up @@ -2458,7 +2487,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) {
want: listeners(),
},
"TLSRoute without any hostnames specified results in '*' match all": {
gateway: gatewayWithSelector,
gateway: gatewayWithTLSRouteSelector,
objs: []interface{}{
kuardService,
&gatewayapi_v1alpha1.TLSRoute{
Expand Down
115 changes: 59 additions & 56 deletions internal/dag/gatewayapi_processor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,74 +114,77 @@ func (p *GatewayAPIProcessor) Run(dag *DAG, source *KubernetesCache) {
continue
}

for _, route := range p.source.httproutes {
switch listener.Routes.Kind {
case KindHTTPRoute:
for _, route := range p.source.httproutes {

// Filter the HTTPRoutes that match the gateway which Contour is configured to watch.
// RouteBindingSelector defines a schema for associating routes with the Gateway.
// If Namespaces and Selector are defined, only routes matching both selectors are associated with the Gateway.
// Filter the HTTPRoutes that match the gateway which Contour is configured to watch.
// RouteBindingSelector defines a schema for associating routes with the Gateway.
// If Namespaces and Selector are defined, only routes matching both selectors are associated with the Gateway.

// ## RouteBindingSelector ##
//
// Selector specifies a set of route labels used for selecting routes to associate
// with the Gateway. If this Selector is defined, only routes matching the Selector
// are associated with the Gateway. An empty Selector matches all routes.
// ## RouteBindingSelector ##
//
// Selector specifies a set of route labels used for selecting routes to associate
// with the Gateway. If this Selector is defined, only routes matching the Selector
// are associated with the Gateway. An empty Selector matches all routes.

nsMatches, err := p.namespaceMatches(listener.Routes.Namespaces, route.Namespace)
if err != nil {
p.Errorf("error validating namespaces against Listener.Routes.Namespaces: %s", err)
}

selMatches, err := selectorMatches(listener.Routes.Selector, route.Labels)
if err != nil {
p.Errorf("error validating routes against Listener.Routes.Selector: %s", err)
}

// If all the match criteria for this HTTPRoute match the Gateway, then add
// the route to the set of matchingRoutes.
if selMatches && nsMatches {

gatewayAllowMatches := p.gatewayMatches(route.Spec.Gateways, route.Namespace)
if (listener.Routes.Selector != nil || listener.Routes.Namespaces != nil) && !gatewayAllowMatches {

// If a label selector or namespace selector matches, but the gateway Allow doesn't
// then set the "Admitted: false" for the route.
routeAccessor, commit := p.dag.StatusCache.ConditionsAccessor(k8s.NamespacedNameOf(route), route.Generation, status.ResourceHTTPRoute, route.Status.Gateways)
routeAccessor.AddCondition(gatewayapi_v1alpha1.ConditionRouteAdmitted, metav1.ConditionFalse, status.ReasonGatewayAllowMismatch, "Gateway RouteSelector matches, but GatewayAllow has mismatch.")
commit()
continue
nsMatches, err := p.namespaceMatches(listener.Routes.Namespaces, route.Namespace)
if err != nil {
p.Errorf("error validating namespaces against Listener.Routes.Namespaces: %s", err)
}

if gatewayAllowMatches {
// Empty Selector matches all routes.
matchingHTTPRoutes = append(matchingHTTPRoutes, route)
selMatches, err := selectorMatches(listener.Routes.Selector, route.Labels)
if err != nil {
p.Errorf("error validating routes against Listener.Routes.Selector: %s", err)
}
}
}

for _, route := range p.source.tlsroutes {
// Filter the TLSRoutes that match the gateway which Contour is configured to watch.
// RouteBindingSelector defines a schema for associating routes with the Gateway.
// If Namespaces and Selector are defined, only routes matching both selectors are associated with the Gateway.
// If all the match criteria for this HTTPRoute match the Gateway, then add
// the route to the set of matchingRoutes.
if selMatches && nsMatches {

// ## RouteBindingSelector ##
//
// Selector specifies a set of route labels used for selecting routes to associate
// with the Gateway. If this Selector is defined, only routes matching the Selector
// are associated with the Gateway. An empty Selector matches all routes.
gatewayAllowMatches := p.gatewayMatches(route.Spec.Gateways, route.Namespace)
if (listener.Routes.Selector != nil || listener.Routes.Namespaces != nil) && !gatewayAllowMatches {

nsMatches, err := p.namespaceMatches(listener.Routes.Namespaces, route.Namespace)
if err != nil {
p.Errorf("error validating namespaces against Listener.Routes.Namespaces: %s", err)
}
// If a label selector or namespace selector matches, but the gateway Allow doesn't
// then set the "Admitted: false" for the route.
routeAccessor, commit := p.dag.StatusCache.ConditionsAccessor(k8s.NamespacedNameOf(route), route.Generation, status.ResourceHTTPRoute, route.Status.Gateways)
routeAccessor.AddCondition(gatewayapi_v1alpha1.ConditionRouteAdmitted, metav1.ConditionFalse, status.ReasonGatewayAllowMismatch, "Gateway RouteSelector matches, but GatewayAllow has mismatch.")
commit()
continue
}

selMatches, err := selectorMatches(listener.Routes.Selector, route.Labels)
if err != nil {
p.Errorf("error validating routes against Listener.Routes.Selector: %s", err)
if gatewayAllowMatches {
// Empty Selector matches all routes.
matchingHTTPRoutes = append(matchingHTTPRoutes, route)
}
}
}
case KindTLSRoute:
for _, route := range p.source.tlsroutes {
// Filter the TLSRoutes that match the gateway which Contour is configured to watch.
// RouteBindingSelector defines a schema for associating routes with the Gateway.
// If Namespaces and Selector are defined, only routes matching both selectors are associated with the Gateway.

// ## RouteBindingSelector ##
//
// Selector specifies a set of route labels used for selecting routes to associate
// with the Gateway. If this Selector is defined, only routes matching the Selector
// are associated with the Gateway. An empty Selector matches all routes.

nsMatches, err := p.namespaceMatches(listener.Routes.Namespaces, route.Namespace)
if err != nil {
p.Errorf("error validating namespaces against Listener.Routes.Namespaces: %s", err)
}

selMatches, err := selectorMatches(listener.Routes.Selector, route.Labels)
if err != nil {
p.Errorf("error validating routes against Listener.Routes.Selector: %s", err)
}

if selMatches && nsMatches {
// Empty Selector matches all routes.
matchingTLSRoutes = append(matchingTLSRoutes, route)
if selMatches && nsMatches {
// Empty Selector matches all routes.
matchingTLSRoutes = append(matchingTLSRoutes, route)
}
}
}

Expand Down
96 changes: 94 additions & 2 deletions internal/dag/status_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2483,7 +2483,7 @@ func TestDAGStatus(t *testing.T) {
})
}

func TestGatewayAPIDAGStatus(t *testing.T) {
func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) {

type testcase struct {
objs []interface{}
Expand All @@ -2510,7 +2510,7 @@ func TestGatewayAPIDAGStatus(t *testing.T) {
Spec: gatewayapi_v1alpha1.GatewaySpec{
Listeners: []gatewayapi_v1alpha1.Listener{{
Port: 80,
Protocol: "HTTP",
Protocol: gatewayapi_v1alpha1.HTTPProtocolType,
Routes: gatewayapi_v1alpha1.RouteBindingSelector{
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
Expand Down Expand Up @@ -3378,6 +3378,98 @@ func TestGatewayAPIDAGStatus(t *testing.T) {
},
}},
})
}

func TestGatewayAPITLSRouteDAGStatus(t *testing.T) {

type testcase struct {
objs []interface{}
want []*status.ConditionsUpdate
}

run := func(t *testing.T, desc string, tc testcase) {
t.Helper()
t.Run(desc, func(t *testing.T) {
t.Helper()
builder := Builder{
Source: KubernetesCache{
RootNamespaces: []string{"roots", "marketing"},
FieldLogger: fixture.NewTestLogger(t),
ConfiguredGateway: types.NamespacedName{
Namespace: "contour",
Name: "projectcontour",
},
gateway: &gatewayapi_v1alpha1.Gateway{
ObjectMeta: metav1.ObjectMeta{
Name: "contour",
Namespace: "projectcontour",
},
Spec: gatewayapi_v1alpha1.GatewaySpec{
Listeners: []gatewayapi_v1alpha1.Listener{{
Port: 80,
Protocol: gatewayapi_v1alpha1.TLSProtocolType,
Routes: gatewayapi_v1alpha1.RouteBindingSelector{
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"app": "contour",
},
},
Kind: KindTLSRoute,
},
}},
},
},
},
Processors: []Processor{
&IngressProcessor{
FieldLogger: fixture.NewTestLogger(t),
},
&HTTPProxyProcessor{},
&GatewayAPIProcessor{
FieldLogger: fixture.NewTestLogger(t),
},
&ListenerProcessor{},
},
}
for _, o := range tc.objs {
builder.Source.Insert(o)
}
dag := builder.Build()
gotUpdates := dag.StatusCache.GetRouteUpdates()

ops := []cmp.Option{
cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime"),
cmpopts.IgnoreFields(status.ConditionsUpdate{}, "ExistingConditions"),
cmpopts.IgnoreFields(status.ConditionsUpdate{}, "GatewayRef"),
cmpopts.IgnoreFields(status.ConditionsUpdate{}, "Generation"),
cmpopts.IgnoreFields(status.ConditionsUpdate{}, "TransitionTime"),
cmpopts.IgnoreFields(status.ConditionsUpdate{}, "Resource"),
cmpopts.SortSlices(func(i, j metav1.Condition) bool {
return i.Message < j.Message
}),
}

if diff := cmp.Diff(tc.want, gotUpdates, ops...); diff != "" {
t.Fatalf("expected: %v, got %v", tc.want, diff)
}

})
}

kuardService := &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "kuard",
Namespace: "default",
},
Spec: v1.ServiceSpec{
Ports: []v1.ServicePort{{
Name: "http",
Protocol: "TCP",
Port: 8080,
TargetPort: intstr.FromInt(8080),
}},
},
}

run(t, "TLSRoute: spec.rules.forwardTo.serviceName not specified", testcase{
objs: []interface{}{
Expand Down

0 comments on commit 2498d76

Please sign in to comment.