Skip to content

Commit

Permalink
ingress/gateway-api: sorted virtual hosts
Browse files Browse the repository at this point in the history 
Currently, while translating K8s Ingress or Gateway API resources into
Envoy resources, the virtualhosts aren't sorted. This leads to situations
(especially in combination with Shared Ingress) where the order of the virtual
hosts isn't guaranteed.

Therefore, this commit orders the virtualhosts within a Envoy RouteConfiguration
by their name. This influences the Envoy route matching process
(https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_conn_man/route_matching),
but only by making it constant and not random.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
  • Loading branch information
@mhofstetter @sayboras
mhofstetter authored and sayboras committed Mar 26, 2024
1 parent 16f6afe commit 7992f75
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 69 deletions.
3 changes: 3 additions & 0 deletions operator/pkg/model/translation/cec_translator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
package translation

import (
"cmp"
"fmt"
goslices "slices"
"sort"

envoy_config_cluster_v3 "github.com/cilium/proxy/go/envoy/config/cluster/v3"
Expand Down Expand Up @@ -292,6 +294,7 @@ func (i *cecTranslator) getEnvoyHTTPRouteConfiguration(m *model.Model) []ciliumv
// the route name should match the value in http connection manager
// otherwise the request will be dropped by envoy
routeName := fmt.Sprintf("listener-%s", port)
goslices.SortStableFunc(virtualhosts, func(a, b *envoy_config_route_v3.VirtualHost) int { return cmp.Compare(a.Name, b.Name) })
rc, _ := NewRouteConfiguration(routeName, virtualhosts)
res = append(res, rc)
}
Expand Down
56 changes: 56 additions & 0 deletions operator/pkg/model/translation/cec_translator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,62 @@ func TestSharedIngressTranslator_getClusters(t *testing.T) {
}
}

func TestGetEnvoyHTTPRouteConfiguration_VirtualHostSorted(t *testing.T) {
defT := &cecTranslator{}

routes := []model.HTTPRoute{
{
Backends: []model.Backend{
{
Name: "default-backend",
Namespace: "random-namespace",
Port: &model.BackendPort{
Port: 8080,
},
},
},
},
}

l1 := []model.HTTPListener{
{
Port: 443,
Hostname: "foo.bar",
ForceHTTPtoHTTPSRedirect: true,
Routes: routes,
},
{
Port: 443,
Hostname: "bar.foo",
Routes: routes,
},
}

l2 := []model.HTTPListener{
{
Port: 443,
Hostname: "bar.foo",
Routes: routes,
},
{
Port: 443,
Hostname: "foo.bar",
ForceHTTPtoHTTPSRedirect: true,
Routes: routes,
},
}

res1 := defT.getEnvoyHTTPRouteConfiguration(&model.Model{HTTP: l1})
res2 := defT.getEnvoyHTTPRouteConfiguration(&model.Model{HTTP: l2})

diffOutput := cmp.Diff(res1, res2, protocmp.Transform())
if len(diffOutput) != 0 {
t.Errorf("CiliumEnvoyConfigs did not match:\n%s\n", diffOutput)
}

// assert.Equal(t, res1, res2)
}

func TestSharedIngressTranslator_getEnvoyHTTPRouteConfiguration(t *testing.T) {
type args struct {
m *model.Model
Expand Down
59 changes: 30 additions & 29 deletions operator/pkg/model/translation/fixture_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,22 +315,22 @@ var hostRulesExpectedConfigEnforceHTTPS = []*envoy_config_route_v3.RouteConfigur
Name: "listener-insecure",
VirtualHosts: []*envoy_config_route_v3.VirtualHost{
{
Name: "foo.bar.com",
Domains: domainsHelper("foo.bar.com"),
Name: "*.foo.com",
Domains: domainsHelper("*.foo.com"),
Routes: []*envoy_config_route_v3.Route{
{
Match: envoyRouteMatchRootPath(),
Action: envoyHTTPSRouteRedirect(),
Match: withAuthority(envoyRouteMatchRootPath(), "^[^.]+[.]foo[.]com$"),
Action: envoyRouteAction("random-namespace", "wildcard-foo-com", "8080"),
},
},
},
{
Name: "*.foo.com",
Domains: domainsHelper("*.foo.com"),
Name: "foo.bar.com",
Domains: domainsHelper("foo.bar.com"),
Routes: []*envoy_config_route_v3.Route{
{
Match: withAuthority(envoyRouteMatchRootPath(), "^[^.]+[.]foo[.]com$"),
Action: envoyRouteAction("random-namespace", "wildcard-foo-com", "8080"),
Match: envoyRouteMatchRootPath(),
Action: envoyHTTPSRouteRedirect(),
},
},
},
Expand Down Expand Up @@ -573,7 +573,8 @@ var pathRulesExpectedConfig = []*envoy_config_route_v3.RouteConfiguration{
{
Match: envoyRouteMatchPrefixPath("/aaa"),
Action: envoyRouteAction("random-namespace", "aaa-prefix", "8080"),
}},
},
},
},
{
Name: "trailing-slash-path-rules",
Expand Down Expand Up @@ -1019,26 +1020,26 @@ var complexIngressExpectedConfigEnforceHTTPS = []*envoy_config_route_v3.RouteCon
Name: "listener-insecure",
VirtualHosts: []*envoy_config_route_v3.VirtualHost{
{
Name: "another-very-secure.server.com",
Domains: domainsHelper("another-very-secure.server.com"),
Name: "*",
Domains: domainsHelper("*"),
Routes: []*envoy_config_route_v3.Route{
{
Match: envoyRouteMatchExactPath("/dummy-path"),
Action: envoyHTTPSRouteRedirect(),
Action: envoyRouteAction("dummy-namespace", "dummy-backend", "8080"),
},
{
Match: envoyRouteMatchPrefixPath("/another-dummy-path"),
Action: envoyHTTPSRouteRedirect(),
Action: envoyRouteAction("dummy-namespace", "another-dummy-backend", "8081"),
},
{
Match: envoyRouteMatchRootPath(),
Action: envoyHTTPSRouteRedirect(),
Action: envoyRouteAction("dummy-namespace", "default-backend", "8080"),
},
},
},
{
Name: "very-secure.server.com",
Domains: domainsHelper("very-secure.server.com"),
Name: "another-very-secure.server.com",
Domains: domainsHelper("another-very-secure.server.com"),
Routes: []*envoy_config_route_v3.Route{
{
Match: envoyRouteMatchExactPath("/dummy-path"),
Expand All @@ -1055,20 +1056,20 @@ var complexIngressExpectedConfigEnforceHTTPS = []*envoy_config_route_v3.RouteCon
},
},
{
Name: "*",
Domains: domainsHelper("*"),
Name: "very-secure.server.com",
Domains: domainsHelper("very-secure.server.com"),
Routes: []*envoy_config_route_v3.Route{
{
Match: envoyRouteMatchExactPath("/dummy-path"),
Action: envoyRouteAction("dummy-namespace", "dummy-backend", "8080"),
Action: envoyHTTPSRouteRedirect(),
},
{
Match: envoyRouteMatchPrefixPath("/another-dummy-path"),
Action: envoyRouteAction("dummy-namespace", "another-dummy-backend", "8081"),
Action: envoyHTTPSRouteRedirect(),
},
{
Match: envoyRouteMatchRootPath(),
Action: envoyRouteAction("dummy-namespace", "default-backend", "8080"),
Action: envoyHTTPSRouteRedirect(),
},
},
},
Expand Down Expand Up @@ -1270,8 +1271,8 @@ var multipleRouteHostnamesExpectedConfig = []*envoy_config_route_v3.RouteConfigu
Name: "listener-insecure",
VirtualHosts: []*envoy_config_route_v3.VirtualHost{
{
Name: "foo.example.com",
Domains: domainsHelper("foo.example.com"),
Name: "bar.example.com",
Domains: domainsHelper("bar.example.com"),
Routes: []*envoy_config_route_v3.Route{
{
Match: envoyRouteMatchRootPath(),
Expand All @@ -1280,22 +1281,22 @@ var multipleRouteHostnamesExpectedConfig = []*envoy_config_route_v3.RouteConfigu
},
},
{
Name: "bar.example.com",
Domains: domainsHelper("bar.example.com"),
Name: "baz.example.com",
Domains: domainsHelper("baz.example.com"),
Routes: []*envoy_config_route_v3.Route{
{
Match: envoyRouteMatchRootPath(),
Action: envoyRouteAction("dummy-namespace", "dummy-backend", "8080"),
Action: envoyRouteAction("dummy-namespace", "another-dummy-backend", "8081"),
},
},
},
{
Name: "baz.example.com",
Domains: domainsHelper("baz.example.com"),
Name: "foo.example.com",
Domains: domainsHelper("foo.example.com"),
Routes: []*envoy_config_route_v3.Route{
{
Match: envoyRouteMatchRootPath(),
Action: envoyRouteAction("dummy-namespace", "another-dummy-backend", "8081"),
Action: envoyRouteAction("dummy-namespace", "dummy-backend", "8080"),
},
},
},
Expand Down
52 changes: 26 additions & 26 deletions operator/pkg/model/translation/gateway-api/translator_fixture_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1323,25 +1323,17 @@ var hostnameIntersectionHTTPListenersCiliumEnvoyConfig = &ciliumv2.CiliumEnvoyCo
Name: "listener-insecure",
VirtualHosts: []*envoy_config_route_v3.VirtualHost{
{
Name: "very.specific.com",
Domains: []string{"very.specific.com", "very.specific.com:*"},
Name: "*.anotherwildcard.io",
Domains: []string{"*.anotherwildcard.io", "*.anotherwildcard.io:*"},
Routes: []*envoy_config_route_v3.Route{
{
Match: &envoy_config_route_v3.RouteMatch{
PathSpecifier: &envoy_config_route_v3.RouteMatch_PathSeparatedPrefix{
PathSeparatedPrefix: "/s1",
PathSeparatedPrefix: "/s4",
},
},
Action: routeActionBackendV1,
},
{
Match: &envoy_config_route_v3.RouteMatch{
PathSpecifier: &envoy_config_route_v3.RouteMatch_PathSeparatedPrefix{
PathSeparatedPrefix: "/s3",
},
},
Action: routeActionBackendV3,
},
},
},
{
Expand Down Expand Up @@ -1387,17 +1379,25 @@ var hostnameIntersectionHTTPListenersCiliumEnvoyConfig = &ciliumv2.CiliumEnvoyCo
},
},
{
Name: "*.anotherwildcard.io",
Domains: []string{"*.anotherwildcard.io", "*.anotherwildcard.io:*"},
Name: "very.specific.com",
Domains: []string{"very.specific.com", "very.specific.com:*"},
Routes: []*envoy_config_route_v3.Route{
{
Match: &envoy_config_route_v3.RouteMatch{
PathSpecifier: &envoy_config_route_v3.RouteMatch_PathSeparatedPrefix{
PathSeparatedPrefix: "/s4",
PathSeparatedPrefix: "/s1",
},
},
Action: routeActionBackendV1,
},
{
Match: &envoy_config_route_v3.RouteMatch{
PathSpecifier: &envoy_config_route_v3.RouteMatch_PathSeparatedPrefix{
PathSeparatedPrefix: "/s3",
},
},
Action: routeActionBackendV3,
},
},
},
},
Expand Down Expand Up @@ -1561,58 +1561,58 @@ var listenerHostNameMatchingCiliumEnvoyConfig = &ciliumv2.CiliumEnvoyConfig{
Name: "listener-insecure",
VirtualHosts: []*envoy_config_route_v3.VirtualHost{
{
Name: "bar.com",
Domains: []string{"bar.com", "bar.com:*"},
Name: "*.bar.com",
Domains: []string{"*.bar.com", "*.bar.com:*"},
Routes: []*envoy_config_route_v3.Route{
{
Match: &envoy_config_route_v3.RouteMatch{
PathSpecifier: &envoy_config_route_v3.RouteMatch_Prefix{
Prefix: "/",
},
},
Action: routeActionBackendV1,
Action: routeActionBackendV3,
},
},
},
{
Name: "foo.bar.com",
Domains: []string{"foo.bar.com", "foo.bar.com:*"},
Name: "*.foo.com",
Domains: []string{"*.foo.com", "*.foo.com:*"},
Routes: []*envoy_config_route_v3.Route{
{
Match: &envoy_config_route_v3.RouteMatch{
PathSpecifier: &envoy_config_route_v3.RouteMatch_Prefix{
Prefix: "/",
},
},
Action: routeActionBackendV2,
Action: routeActionBackendV3,
},
},
},
{
Name: "*.bar.com",
Domains: []string{"*.bar.com", "*.bar.com:*"},
Name: "bar.com",
Domains: []string{"bar.com", "bar.com:*"},
Routes: []*envoy_config_route_v3.Route{
{
Match: &envoy_config_route_v3.RouteMatch{
PathSpecifier: &envoy_config_route_v3.RouteMatch_Prefix{
Prefix: "/",
},
},
Action: routeActionBackendV3,
Action: routeActionBackendV1,
},
},
},
{
Name: "*.foo.com",
Domains: []string{"*.foo.com", "*.foo.com:*"},
Name: "foo.bar.com",
Domains: []string{"foo.bar.com", "foo.bar.com:*"},
Routes: []*envoy_config_route_v3.Route{
{
Match: &envoy_config_route_v3.RouteMatch{
PathSpecifier: &envoy_config_route_v3.RouteMatch_Prefix{
Prefix: "/",
},
},
Action: routeActionBackendV3,
Action: routeActionBackendV2,
},
},
},
Expand Down
28 changes: 14 additions & 14 deletions operator/pkg/model/translation/ingress/dedicated_ingress_fixture_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -478,20 +478,6 @@ var hostRulesListenersEnforceHTTPSCiliumEnvoyConfig = &ciliumv2.CiliumEnvoyConfi
Any: toAny(&envoy_config_route_v3.RouteConfiguration{
Name: "listener-insecure",
VirtualHosts: []*envoy_config_route_v3.VirtualHost{
{
Name: "foo.bar.com",
Domains: []string{"foo.bar.com", "foo.bar.com:*"},
Routes: []*envoy_config_route_v3.Route{
{
Match: &envoy_config_route_v3.RouteMatch{
PathSpecifier: &envoy_config_route_v3.RouteMatch_Prefix{
Prefix: "/",
},
},
Action: toHTTPSRedirectAction(),
},
},
},
{
Name: "*.foo.com",
Domains: []string{"*.foo.com", "*.foo.com:*"},
Expand Down Expand Up @@ -521,6 +507,20 @@ var hostRulesListenersEnforceHTTPSCiliumEnvoyConfig = &ciliumv2.CiliumEnvoyConfi
},
},
},
{
Name: "foo.bar.com",
Domains: []string{"foo.bar.com", "foo.bar.com:*"},
Routes: []*envoy_config_route_v3.Route{
{
Match: &envoy_config_route_v3.RouteMatch{
PathSpecifier: &envoy_config_route_v3.RouteMatch_Prefix{
Prefix: "/",
},
},
Action: toHTTPSRedirectAction(),
},
},
},
},
}),
},
Expand Down

0 comments on commit 7992f75

Please sign in to comment.