Skip to content

Commit

Permalink
proxy: Avoid sorting endpoints for every single request
Browse files Browse the repository at this point in the history
The endpoints are no longer hashed by path name in the directors map since
that made iterating over the endpoints unstable. They are now stored in a
slice in the order in which the are defined in the configuration.

Closes: owncloud#4497
  • Loading branch information
rhafer committed Sep 7, 2022
1 parent eae169e commit 26cdbf8
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 21 deletions.
3 changes: 3 additions & 0 deletions changelog/unreleased/refactor-proxy.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,7 @@ Enhancement: Refactor the proxy service
The routes of the proxy service now have a "unprotected" flag. This is used by the authentication middleware to determine if the request needs to be blocked when missing authentication or not.

https://github.com/owncloud/ocis/issues/4401
https://github.com/owncloud/ocis/issues/4497
https://github.com/owncloud/ocis/pull/4461
https://github.com/owncloud/ocis/pull/4498
https://github.com/owncloud/ocis/pull/4514
36 changes: 15 additions & 21 deletions services/proxy/pkg/router/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"net/http"
"net/url"
"regexp"
"sort"
"strings"

"github.com/owncloud/ocis/v2/ocis-pkg/log"
Expand Down Expand Up @@ -58,7 +57,7 @@ func New(policySelector *config.PolicySelector, policies []config.Policy, logger

r := Router{
logger: logger,
directors: make(map[string]map[config.RouteType]map[string]map[string]RoutingInfo),
directors: make(map[string]map[config.RouteType]map[string][]RoutingInfo),
policySelector: selector,
}
for _, pol := range policies {
Expand Down Expand Up @@ -87,6 +86,7 @@ func New(policySelector *config.PolicySelector, policies []config.Policy, logger
// RoutingInfo contains the proxy director and some information about the route.
type RoutingInfo struct {
director func(*http.Request)
endpoint string
unprotected bool
}

Expand All @@ -103,30 +103,31 @@ func (r RoutingInfo) IsRouteUnprotected() bool {
// Router handles the routing of HTTP requests according to the given policies.
type Router struct {
logger log.Logger
directors map[string]map[config.RouteType]map[string]map[string]RoutingInfo
directors map[string]map[config.RouteType]map[string][]RoutingInfo
policySelector policy.Selector
}

func (rt Router) addHost(policy string, target *url.URL, route config.Route) {
targetQuery := target.RawQuery
if rt.directors[policy] == nil {
rt.directors[policy] = make(map[config.RouteType]map[string]map[string]RoutingInfo)
rt.directors[policy] = make(map[config.RouteType]map[string][]RoutingInfo)
}
routeType := config.DefaultRouteType
if route.Type != "" {
routeType = route.Type
}
if rt.directors[policy][routeType] == nil {
rt.directors[policy][routeType] = make(map[string]map[string]RoutingInfo)
rt.directors[policy][routeType] = make(map[string][]RoutingInfo)
}
if rt.directors[policy][routeType][route.Method] == nil {
rt.directors[policy][routeType][route.Method] = make(map[string]RoutingInfo)
rt.directors[policy][routeType][route.Method] = make([]RoutingInfo, 0)
}

reg := registry.GetRegistry()
sel := selector.NewSelector(selector.Registry(reg))

rt.directors[policy][routeType][route.Method][route.Endpoint] = RoutingInfo{
rt.directors[policy][routeType][route.Method] = append(rt.directors[policy][routeType][route.Method], RoutingInfo{
endpoint: route.Endpoint,
unprotected: route.Unprotected,
director: func(req *http.Request) {
if route.Service != "" {
Expand Down Expand Up @@ -170,7 +171,7 @@ func (rt Router) addHost(policy string, target *url.URL, route config.Route) {
req.Header.Set("User-Agent", "")
}
},
}
})
}

// Route is evaluating the policies on the request and returns the RoutingInfo if successful.
Expand Down Expand Up @@ -208,32 +209,25 @@ func (rt Router) Route(r *http.Request) (RoutingInfo, bool) {
method = r.Method
}

endpoints := make([]string, 0, len(rt.directors[pol][rtype][method]))
for endpoint := range rt.directors[pol][rtype][method] {
endpoints = append(endpoints, endpoint)
}
sort.Slice(endpoints, func(i, j int) bool {
return len(endpoints[j]) < len(endpoints[i])
})
for _, endpoint := range endpoints {
if handler(endpoint, *r.URL) {
for _, ri := range rt.directors[pol][rtype][method] {
if handler(ri.endpoint, *r.URL) {
rt.logger.Debug().
Str("policy", pol).
Str("method", r.Method).
Str("prefix", endpoint).
Str("prefix", ri.endpoint).
Str("path", r.URL.Path).
Str("routeType", string(rtype)).
Msg("director found")

return rt.directors[pol][rtype][method][endpoint], true
return ri, true
}
}
}

// override default director with root. If any
if ri, ok := rt.directors[pol][config.PrefixRoute][method]["/"]; ok { // try specific method
if ri := rt.directors[pol][config.PrefixRoute][method][0]; ri.endpoint == "/" { // try specific method
return ri, true
} else if ri, ok := rt.directors[pol][config.PrefixRoute][""]["/"]; ok { // fallback to unspecific method
} else if ri := rt.directors[pol][config.PrefixRoute][""][0]; ri.endpoint == "/" { // fallback to unspecific method
return ri, true
}

Expand Down

0 comments on commit 26cdbf8

Please sign in to comment.