Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stops route sorting on HTTPProxy CRD #5772

Closed
wants to merge 17 commits into from
Closed
5 changes: 5 additions & 0 deletions apis/projectcontour/v1alpha1/contourconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,11 @@ type HTTPProxyConfig struct {
// use as fallback when a non-SNI request is received.
// +optional
FallbackCertificate *NamespacedName `json:"fallbackCertificate,omitempty"`

// OmitRouteSorting determines if routes will be sorted by Contour before
// being sent to Envoy or the original order will be determined.
// Defaults to routing being sorted.
OmitRouteSorting bool `json:"omitRouteSorting,omitempty"`
}

// NetworkParameters hold various configurable network values.
Expand Down
3 changes: 3 additions & 0 deletions changelogs/unreleased/5772-davinci26-small.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
## Allow cluster operators to disable route sorting with `HTTPProxy`

This change allows contour administrators to turn on a `flag`, `OmitRouteSorting` that disables route sorting. When this configuration flag is turned on routes are sent to Envoy in the same order as they are described in the `HTTPProxy` CRD. This allows operators to build more complex routing tables but they need to be careful with changes since now order becomes important. Includes are resolved in a depth first fashion.
3 changes: 3 additions & 0 deletions cmd/contour/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,7 @@
globalRateLimitService: contourConfiguration.RateLimitService,
maxRequestsPerConnection: contourConfiguration.Envoy.Cluster.MaxRequestsPerConnection,
perConnectionBufferLimitBytes: contourConfiguration.Envoy.Cluster.PerConnectionBufferLimitBytes,
sortHTTProxyRoutes: !contourConfiguration.HTTPProxy.OmitRouteSorting,

Check warning on line 554 in cmd/contour/serve.go

View check run for this annotation

Codecov / codecov/patch

cmd/contour/serve.go#L554

Added line #L554 was not covered by tests
})

// Build the core Kubernetes event handler.
Expand Down Expand Up @@ -1105,6 +1106,7 @@
maxRequestsPerConnection *uint32
perConnectionBufferLimitBytes *uint32
globalRateLimitService *contour_api_v1alpha1.RateLimitServiceConfig
sortHTTProxyRoutes bool
}

func (s *Server) getDAGBuilder(dbc dagBuilderConfig) *dag.Builder {
Expand Down Expand Up @@ -1197,6 +1199,7 @@
GlobalRateLimitService: dbc.globalRateLimitService,
PerConnectionBufferLimitBytes: dbc.perConnectionBufferLimitBytes,
SetSourceMetadataOnRoutes: true,
ShouldSortRoutes: dbc.sortHTTProxyRoutes,
davinci26 marked this conversation as resolved.
Show resolved Hide resolved
},
}

Expand Down
19 changes: 19 additions & 0 deletions cmd/contour/serve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ func TestGetDAGBuilder(t *testing.T) {

httpProxyProcessor := mustGetHTTPProxyProcessor(t, builder)
assert.True(t, httpProxyProcessor.SetSourceMetadataOnRoutes)
assert.False(t, httpProxyProcessor.ShouldSortRoutes)
}

t.Run("all default options", func(t *testing.T) {
Expand Down Expand Up @@ -189,6 +190,24 @@ func TestGetDAGBuilder(t *testing.T) {
assert.EqualValues(t, ingressClassNames, got.Source.IngressClassNames)
})

t.Run("Sort HTTPRoutes config", func(t *testing.T) {
serve := &Server{
log: logrus.StandardLogger(),
}
got := serve.getDAGBuilder(dagBuilderConfig{rootNamespaces: []string{}, dnsLookupFamily: contour_api_v1alpha1.AutoClusterDNSFamily, sortHTTProxyRoutes: true})

assert.Empty(t, got.Source.ConfiguredSecretRefs)
assert.Len(t, got.Processors, 4)
assert.IsType(t, &dag.ListenerProcessor{}, got.Processors[0])

ingressProcessor := mustGetIngressProcessor(t, got)
assert.True(t, ingressProcessor.SetSourceMetadataOnRoutes)

httpProxyProcessor := mustGetHTTPProxyProcessor(t, got)
assert.True(t, httpProxyProcessor.SetSourceMetadataOnRoutes)
assert.True(t, httpProxyProcessor.ShouldSortRoutes)
})

// TODO(3453): test additional properties of the DAG builder (processor fields, cache fields, Gateway tests (requires a client fake))
}

Expand Down
10 changes: 10 additions & 0 deletions examples/contour/01-crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,11 @@ spec:
- name
- namespace
type: object
omitRouteSorting:
description: OmitRouteSorting determines if routes will be sorted
by Contour before being sent to Envoy or the original order
will be determined. Defaults to routing being sorted.
type: boolean
rootNamespaces:
description: Restrict Contour to searching these namespaces for
root ingress routes.
Expand Down Expand Up @@ -4102,6 +4107,11 @@ spec:
- name
- namespace
type: object
omitRouteSorting:
description: OmitRouteSorting determines if routes will be
sorted by Contour before being sent to Envoy or the original
order will be determined. Defaults to routing being sorted.
type: boolean
rootNamespaces:
description: Restrict Contour to searching these namespaces
for root ingress routes.
Expand Down
10 changes: 10 additions & 0 deletions examples/render/contour-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,11 @@ spec:
- name
- namespace
type: object
omitRouteSorting:
description: OmitRouteSorting determines if routes will be sorted
by Contour before being sent to Envoy or the original order
will be determined. Defaults to routing being sorted.
type: boolean
rootNamespaces:
description: Restrict Contour to searching these namespaces for
root ingress routes.
Expand Down Expand Up @@ -4321,6 +4326,11 @@ spec:
- name
- namespace
type: object
omitRouteSorting:
description: OmitRouteSorting determines if routes will be
sorted by Contour before being sent to Envoy or the original
order will be determined. Defaults to routing being sorted.
type: boolean
rootNamespaces:
description: Restrict Contour to searching these namespaces
for root ingress routes.
Expand Down
10 changes: 10 additions & 0 deletions examples/render/contour-gateway-provisioner.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,11 @@ spec:
- name
- namespace
type: object
omitRouteSorting:
description: OmitRouteSorting determines if routes will be sorted
by Contour before being sent to Envoy or the original order
will be determined. Defaults to routing being sorted.
type: boolean
rootNamespaces:
description: Restrict Contour to searching these namespaces for
root ingress routes.
Expand Down Expand Up @@ -4113,6 +4118,11 @@ spec:
- name
- namespace
type: object
omitRouteSorting:
description: OmitRouteSorting determines if routes will be
sorted by Contour before being sent to Envoy or the original
order will be determined. Defaults to routing being sorted.
type: boolean
rootNamespaces:
description: Restrict Contour to searching these namespaces
for root ingress routes.
Expand Down
10 changes: 10 additions & 0 deletions examples/render/contour-gateway.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -841,6 +841,11 @@ spec:
- name
- namespace
type: object
omitRouteSorting:
description: OmitRouteSorting determines if routes will be sorted
by Contour before being sent to Envoy or the original order
will be determined. Defaults to routing being sorted.
type: boolean
rootNamespaces:
description: Restrict Contour to searching these namespaces for
root ingress routes.
Expand Down Expand Up @@ -4324,6 +4329,11 @@ spec:
- name
- namespace
type: object
omitRouteSorting:
description: OmitRouteSorting determines if routes will be
sorted by Contour before being sent to Envoy or the original
order will be determined. Defaults to routing being sorted.
type: boolean
rootNamespaces:
description: Restrict Contour to searching these namespaces
for root ingress routes.
Expand Down
10 changes: 10 additions & 0 deletions examples/render/contour.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,11 @@ spec:
- name
- namespace
type: object
omitRouteSorting:
description: OmitRouteSorting determines if routes will be sorted
by Contour before being sent to Envoy or the original order
will be determined. Defaults to routing being sorted.
type: boolean
rootNamespaces:
description: Restrict Contour to searching these namespaces for
root ingress routes.
Expand Down Expand Up @@ -4321,6 +4326,11 @@ spec:
- name
- namespace
type: object
omitRouteSorting:
description: OmitRouteSorting determines if routes will be
sorted by Contour before being sent to Envoy or the original
order will be determined. Defaults to routing being sorted.
type: boolean
rootNamespaces:
description: Restrict Contour to searching these namespaces
for root ingress routes.
Expand Down
8 changes: 2 additions & 6 deletions internal/dag/accessors.go
Original file line number Diff line number Diff line change
Expand Up @@ -335,9 +335,7 @@
for _, l := range d.Listeners {
for _, vhost := range l.VirtualHosts {
var routes []*Route
for _, r := range vhost.Routes {
routes = append(routes, r)
}
routes = append(routes, vhost.Routes...)

Check warning on line 338 in internal/dag/accessors.go

View check run for this annotation

Codecov / codecov/patch

internal/dag/accessors.go#L338

Added line #L338 was not covered by tests
if len(routes) > 0 {
res[vhost] = routes
}
Expand All @@ -353,9 +351,7 @@
for _, l := range d.Listeners {
for _, vhost := range l.SecureVirtualHosts {
var routes []*Route
for _, r := range vhost.Routes {
routes = append(routes, r)
}
routes = append(routes, vhost.Routes...)

Check warning on line 354 in internal/dag/accessors.go

View check run for this annotation

Codecov / codecov/patch

internal/dag/accessors.go#L354

Added line #L354 was not covered by tests
if len(routes) > 0 {
res[vhost] = routes
}
Expand Down
8 changes: 4 additions & 4 deletions internal/dag/accessors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,8 @@ func TestGetServiceClusters(t *testing.T) {
"http-1": {
VirtualHosts: []*VirtualHost{
{
Routes: map[string]*Route{
"foo": {
Routes: []*Route{
{
Clusters: []*Cluster{
{Upstream: &Service{ExternalName: "bar.com"}},
{Upstream: &Service{}},
Expand All @@ -274,8 +274,8 @@ func TestGetServiceClusters(t *testing.T) {
"http-1": {
VirtualHosts: []*VirtualHost{
{
Routes: map[string]*Route{
"foo": {
Routes: []*Route{
{
Clusters: []*Cluster{
{Upstream: &Service{}},
},
Expand Down
Loading
Loading