diff --git a/pkg/router/controller/hostindex/hostindex.go b/pkg/router/controller/hostindex/hostindex.go index 5e1a7bd5201f..1e57202913a4 100644 --- a/pkg/router/controller/hostindex/hostindex.go +++ b/pkg/router/controller/hostindex/hostindex.go @@ -100,7 +100,10 @@ func (hi *hostIndex) add(route *routeapi.Route, changes *routeChanges) bool { hi.remove(existing, false, changes) // uid changed, which means this is case existing.Spec.Path != route.Spec.Path: - // path changed, must check to see if we displace / are displaced by another route + // path changed, must check to see if we displace / are displaced by + // another route. Remove the existing state to avoid maintaining a + // duplicate claim in the index. + hi.remove(existing, false, changes) default: // if no changes have been made, we don't need to note a change if existing.ResourceVersion == route.ResourceVersion { diff --git a/pkg/router/controller/hostindex/hostindex_test.go b/pkg/router/controller/hostindex/hostindex_test.go index afac756fe23b..cb1e2062bb91 100644 --- a/pkg/router/controller/hostindex/hostindex_test.go +++ b/pkg/router/controller/hostindex/hostindex_test.go @@ -232,6 +232,64 @@ func Test_hostIndex(t *testing.T) { }, active: map[string][]string{"test.com": {"001"}}, }, + { + name: "multiple changes to same path-based route", + activateFn: SameNamespace, + steps: []step{ + {route: newRoute("test", "1", 1, 1, routev1.RouteSpec{Host: "test.com", Path: "/foo"})}, + {route: newRoute("test", "1", 1, 2, routev1.RouteSpec{Host: "test.com", Path: "/bar"})}, + {route: newRoute("test", "1", 1, 3, routev1.RouteSpec{Host: "test.com", Path: "/foo"})}, + {route: newRoute("test", "1", 1, 4, routev1.RouteSpec{Host: "test.com", Path: "/bar"})}, + {route: newRoute("test", "1", 1, 5, routev1.RouteSpec{Host: "test.com", Path: "/foo"})}, + }, + active: map[string][]string{"test.com": {"001"}}, + activates: map[string]struct{}{"001": {}}, + }, + { + name: "remove unchanged path-based route", + activateFn: SameNamespace, + steps: []step{ + {route: newRoute("test", "1", 1, 1, routev1.RouteSpec{Host: "test.com", Path: "/foo"})}, + {remove: true, route: newRoute("test", "1", 0, 1, routev1.RouteSpec{Host: "test.com", Path: "/foo"})}, + }, + }, + { + name: "remove updated path-based route", + activateFn: SameNamespace, + steps: []step{ + {route: newRoute("test", "1", 1, 1, routev1.RouteSpec{Host: "test.com", Path: "/foo"})}, + {route: newRoute("test", "1", 1, 2, routev1.RouteSpec{Host: "test.com", Path: "/bar"})}, + {route: newRoute("test", "1", 1, 3, routev1.RouteSpec{Host: "test.com", Path: "/foo"})}, + {route: newRoute("test", "1", 1, 4, routev1.RouteSpec{Host: "test.com", Path: "/bar"})}, + {remove: true, route: newRoute("test", "1", 1, 4, routev1.RouteSpec{Host: "test.com", Path: "/bar"})}, + }, + }, + { + name: "missed delete of path-based route", + activateFn: SameNamespace, + steps: []step{ + {route: newRoute("test", "1", 2, 1, routev1.RouteSpec{Host: "test.com", Path: "/foo"})}, + {route: newRoute("test", "2", 1, 1, routev1.RouteSpec{Host: "test.com", Path: "/foo"})}, + }, + newRoute: true, + active: map[string][]string{"test.com": {"001"}}, + activates: map[string]struct{}{"001": {}}, + displaces: map[string]struct{}{"002": {}}, + inactive: map[string][]string{"test.com": {"002"}}, + }, + { + name: "path-based route rejection", + activateFn: SameNamespace, + steps: []step{ + {route: newRoute("test", "1", 1, 1, routev1.RouteSpec{Host: "test.com", Path: "/x/y/z"})}, + {route: newRoute("test", "2", 2, 1, routev1.RouteSpec{Host: "test.com", Path: "/foo"})}, + {route: newRoute("test", "2", 2, 2, routev1.RouteSpec{Host: "test.com", Path: "/bar"})}, + {route: newRoute("test", "2", 2, 3, routev1.RouteSpec{Host: "test.com", Path: "/x/y/z"})}, + }, + active: map[string][]string{"test.com": {"001"}}, + displaces: map[string]struct{}{"002": {}}, + inactive: map[string][]string{"test.com": {"002"}}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {