Skip to content

Commit

Permalink
Allow empty HTTPRoute hostnames (#650)
Browse files Browse the repository at this point in the history 
* Allow empty HTTPRoute hostnames

If an HTTPRoute didn't supply any hostnames, then we wouldn't attach it to any listeners. This fixes that issue and uses the wildcard hostname if a listener doesn't supply a hostname, otherwise just use the hostnames of the listeners.
  • Loading branch information
@sjberman
sjberman authored May 18, 2023
1 parent 6868bc2 commit b336df1
Show file tree
Hide file tree
Showing 7 changed files with 168 additions and 184 deletions.
7 changes: 3 additions & 4 deletions internal/state/dataplane/configuration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,10 +267,9 @@ func (hpr *hostPathRules) upsertListener(l *graph.Listener) {

for routeNsName, r := range l.Routes {
var hostnames []string

for _, h := range r.Source.Spec.Hostnames {
if _, exist := l.AcceptedHostnames[string(h)]; exist {
hostnames = append(hostnames, string(h))
for _, p := range r.ParentRefs {
if val, exist := p.Attachment.AcceptedHostnames[string(l.Source.Name)]; exist {
hostnames = val
}
}

Expand Down
92 changes: 32 additions & 60 deletions internal/state/dataplane/configuration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,25 @@ func TestBuildConfiguration(t *testing.T) {

createInternalRoute := func(
source *v1beta1.HTTPRoute,
listenerName string,
paths []pathAndType,
) *graph.Route {
hostnames := make([]string, 0, len(source.Spec.Hostnames))
for _, h := range source.Spec.Hostnames {
hostnames = append(hostnames, string(h))
}
r := &graph.Route{
Source: source,
Rules: createRules(source, paths),
ParentRefs: []graph.ParentRef{
{
Attachment: &graph.ParentRefAttachmentStatus{
AcceptedHostnames: map[string][]string{
listenerName: hostnames,
},
},
},
},
}
return r
}
Expand Down Expand Up @@ -162,7 +176,7 @@ func TestBuildConfiguration(t *testing.T) {
*v1beta1.HTTPRoute, []BackendGroup, *graph.Route,
) {
hr := createRoute(name, hostname, listenerName, paths...)
route := createInternalRoute(hr, paths)
route := createInternalRoute(hr, listenerName, paths)
groups := createExpBackendGroupsForRoute(route)
return hr, groups, route
}
Expand Down Expand Up @@ -217,7 +231,7 @@ func TestBuildConfiguration(t *testing.T) {
pathAndType{path: "/valid", pathType: prefix}, pathAndType{path: invalidMatchesPath, pathType: prefix},
)

hr7, hr7Groups, routeHR7 := createTestResources(
hr7, expHR7Groups, routeHR7 := createTestResources(
"hr-7",
"foo.example.com",
"listener-80-1",
Expand Down Expand Up @@ -258,6 +272,8 @@ func TestBuildConfiguration(t *testing.T) {
"listener-443-with-hostname",
pathAndType{path: "/", pathType: prefix},
)
// add extra attachment for this route for duplicate listener test
httpsRouteHR5.ParentRefs[0].Attachment.AcceptedHostnames["listener-443-1"] = []string{"example.com"}

httpsHR6, expHTTPSHR6Groups, httpsRouteHR6 := createTestResources(
"https-hr-6",
Expand Down Expand Up @@ -352,10 +368,9 @@ func TestBuildConfiguration(t *testing.T) {
Source: &v1beta1.Gateway{},
Listeners: map[string]*graph.Listener{
"listener-80-1": {
Source: listener80,
Valid: true,
Routes: map[types.NamespacedName]*graph.Route{},
AcceptedHostnames: map[string]struct{}{},
Source: listener80,
Valid: true,
Routes: map[types.NamespacedName]*graph.Route{},
},
},
},
Expand All @@ -381,18 +396,16 @@ func TestBuildConfiguration(t *testing.T) {
Source: &v1beta1.Gateway{},
Listeners: map[string]*graph.Listener{
"listener-443-1": {
Source: listener443, // nil hostname
Valid: true,
Routes: map[types.NamespacedName]*graph.Route{},
AcceptedHostnames: map[string]struct{}{},
SecretPath: secretPath,
Source: listener443, // nil hostname
Valid: true,
Routes: map[types.NamespacedName]*graph.Route{},
SecretPath: secretPath,
},
"listener-443-with-hostname": {
Source: listener443WithHostname, // non-nil hostname
Valid: true,
Routes: map[types.NamespacedName]*graph.Route{},
AcceptedHostnames: map[string]struct{}{},
SecretPath: secretPath,
Source: listener443WithHostname, // non-nil hostname
Valid: true,
Routes: map[types.NamespacedName]*graph.Route{},
SecretPath: secretPath,
},
},
},
Expand Down Expand Up @@ -458,10 +471,6 @@ func TestBuildConfiguration(t *testing.T) {
{Namespace: "test", Name: "hr-1"}: routeHR1,
{Namespace: "test", Name: "hr-2"}: routeHR2,
},
AcceptedHostnames: map[string]struct{}{
"foo.example.com": {},
"bar.example.com": {},
},
},
},
},
Expand Down Expand Up @@ -533,10 +542,6 @@ func TestBuildConfiguration(t *testing.T) {
{Namespace: "test", Name: "https-hr-1"}: httpsRouteHR1,
{Namespace: "test", Name: "https-hr-2"}: httpsRouteHR2,
},
AcceptedHostnames: map[string]struct{}{
"foo.example.com": {},
"bar.example.com": {},
},
},
"listener-443-with-hostname": {
Source: listener443WithHostname,
Expand All @@ -545,9 +550,6 @@ func TestBuildConfiguration(t *testing.T) {
Routes: map[types.NamespacedName]*graph.Route{
{Namespace: "test", Name: "https-hr-5"}: httpsRouteHR5,
},
AcceptedHostnames: map[string]struct{}{
"example.com": {},
},
},
},
},
Expand Down Expand Up @@ -649,9 +651,6 @@ func TestBuildConfiguration(t *testing.T) {
{Namespace: "test", Name: "hr-3"}: routeHR3,
{Namespace: "test", Name: "hr-4"}: routeHR4,
},
AcceptedHostnames: map[string]struct{}{
"foo.example.com": {},
},
},
"listener-443-1": {
Source: listener443,
Expand All @@ -661,9 +660,6 @@ func TestBuildConfiguration(t *testing.T) {
{Namespace: "test", Name: "https-hr-3"}: httpsRouteHR3,
{Namespace: "test", Name: "https-hr-4"}: httpsRouteHR4,
},
AcceptedHostnames: map[string]struct{}{
"foo.example.com": {},
},
},
},
},
Expand Down Expand Up @@ -815,9 +811,6 @@ func TestBuildConfiguration(t *testing.T) {
Routes: map[types.NamespacedName]*graph.Route{
{Namespace: "test", Name: "hr-1"}: routeHR1,
},
AcceptedHostnames: map[string]struct{}{
"foo.example.com": {},
},
},
},
},
Expand All @@ -840,9 +833,6 @@ func TestBuildConfiguration(t *testing.T) {
Routes: map[types.NamespacedName]*graph.Route{
{Namespace: "test", Name: "hr-1"}: routeHR1,
},
AcceptedHostnames: map[string]struct{}{
"foo.example.com": {},
},
},
},
},
Expand Down Expand Up @@ -880,9 +870,6 @@ func TestBuildConfiguration(t *testing.T) {
Routes: map[types.NamespacedName]*graph.Route{
{Namespace: "test", Name: "hr-5"}: routeHR5,
},
AcceptedHostnames: map[string]struct{}{
"foo.example.com": {},
},
},
},
},
Expand Down Expand Up @@ -952,9 +939,6 @@ func TestBuildConfiguration(t *testing.T) {
Routes: map[types.NamespacedName]*graph.Route{
{Namespace: "test", Name: "hr-6"}: routeHR6,
},
AcceptedHostnames: map[string]struct{}{
"foo.example.com": {},
},
},
"listener-443-1": {
Source: listener443,
Expand All @@ -963,9 +947,6 @@ func TestBuildConfiguration(t *testing.T) {
Routes: map[types.NamespacedName]*graph.Route{
{Namespace: "test", Name: "https-hr-6"}: httpsRouteHR6,
},
AcceptedHostnames: map[string]struct{}{
"foo.example.com": {},
},
},
},
},
Expand Down Expand Up @@ -1049,9 +1030,6 @@ func TestBuildConfiguration(t *testing.T) {
Routes: map[types.NamespacedName]*graph.Route{
{Namespace: "test", Name: "hr-7"}: routeHR7,
},
AcceptedHostnames: map[string]struct{}{
"foo.example.com": {},
},
},
},
},
Expand All @@ -1074,7 +1052,7 @@ func TestBuildConfiguration(t *testing.T) {
{
MatchIdx: 0,
RuleIdx: 1,
BackendGroup: hr7Groups[1],
BackendGroup: expHR7Groups[1],
Source: hr7,
},
},
Expand All @@ -1086,7 +1064,7 @@ func TestBuildConfiguration(t *testing.T) {
{
MatchIdx: 0,
RuleIdx: 0,
BackendGroup: hr7Groups[0],
BackendGroup: expHR7Groups[0],
Source: hr7,
},
},
Expand All @@ -1096,7 +1074,7 @@ func TestBuildConfiguration(t *testing.T) {
},
SSLServers: []VirtualServer{},
Upstreams: []Upstream{fooUpstream},
BackendGroups: []BackendGroup{hr7Groups[0], hr7Groups[1]},
BackendGroups: []BackendGroup{expHR7Groups[0], expHR7Groups[1]},
},
msg: "duplicate paths with different types",
},
Expand All @@ -1116,9 +1094,6 @@ func TestBuildConfiguration(t *testing.T) {
Routes: map[types.NamespacedName]*graph.Route{
{Namespace: "test", Name: "https-hr-5"}: httpsRouteHR5,
},
AcceptedHostnames: map[string]struct{}{
"example.com": {},
},
},
"listener-443-1": {
Source: listener443,
Expand All @@ -1127,9 +1102,6 @@ func TestBuildConfiguration(t *testing.T) {
Routes: map[types.NamespacedName]*graph.Route{
{Namespace: "test", Name: "https-hr-5"}: httpsRouteHR5,
},
AcceptedHostnames: map[string]struct{}{
"example.com": {},
},
},
},
},
Expand Down
10 changes: 3 additions & 7 deletions internal/state/graph/gateway_listener.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@ type Listener struct {
// Routes holds the routes attached to the Listener.
// Only valid routes are attached.
Routes map[types.NamespacedName]*Route
// AcceptedHostnames is an intersection between the hostnames supported by the Listener and the hostnames
// from the attached routes.
AcceptedHostnames map[string]struct{}
// SecretPath is the path to the secret on disk.
SecretPath string
// Conditions holds the conditions of the Listener.
Expand Down Expand Up @@ -147,10 +144,9 @@ func (c *listenerConfigurator) configure(listener v1beta1.Listener) *Listener {
}

l := &Listener{
Source: listener,
Routes: make(map[types.NamespacedName]*Route),
AcceptedHostnames: make(map[string]struct{}),
Valid: true,
Source: listener,
Routes: make(map[types.NamespacedName]*Route),
Valid: true,
}

// resolvers might add different conditions to the listener, so we run them all.
Expand Down
Loading

0 comments on commit b336df1

Please sign in to comment.