From 214dbc4a48c674eac0e96b89f0831af5d5111e70 Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Fri, 4 Sep 2020 15:11:54 +0200 Subject: [PATCH 1/2] Fix director selection Instead of overwriting the director on each request, provide function for director selection. On concurrent requests the previous implementation could have caused situations where the request was performed on a wrong director. --- pkg/proxy/proxy.go | 102 +++++++++++++++++++++++++-------------------- 1 file changed, 56 insertions(+), 46 deletions(-) diff --git a/pkg/proxy/proxy.go b/pkg/proxy/proxy.go index 6c22cb73b1..1c750a305b 100644 --- a/pkg/proxy/proxy.go +++ b/pkg/proxy/proxy.go @@ -35,6 +35,7 @@ func NewMultiHostReverseProxy(opts ...Option) *MultiHostReverseProxy { logger: options.Logger, config: options.Config, } + rp.Director = rp.directorSelectionDirector if options.Config.Policies == nil { rp.logger.Info().Str("source", "runtime").Msg("Policies") @@ -87,6 +88,61 @@ func NewMultiHostReverseProxy(opts ...Option) *MultiHostReverseProxy { return rp } +func (p *MultiHostReverseProxy) directorSelectionDirector(r *http.Request) { + pol, err := p.PolicySelector(r.Context(), r) + if err != nil { + p.logger.Error().Msgf("Error while selecting pol %v", err) + return + } + + if _, ok := p.Directors[pol]; !ok { + p.logger. + Error(). + Msgf("policy %v is not configured", pol) + return + } + + // find matching director + for _, rt := range config.RouteTypes { + var handler func(string, url.URL) bool + switch rt { + case config.QueryRoute: + handler = p.queryRouteMatcher + case config.RegexRoute: + handler = p.regexRouteMatcher + case config.PrefixRoute: + fallthrough + default: + handler = p.prefixRouteMatcher + } + for endpoint := range p.Directors[pol][rt] { + if handler(endpoint, *r.URL) { + p.logger. + Debug(). + Str("policy", pol). + Str("prefix", endpoint). + Str("path", r.URL.Path). + Str("routeType", string(rt)). + Msg("director found") + p.Directors[pol][rt][endpoint](r) + return + } + } + } + + // override default director with root. If any + if p.Directors[pol][config.PrefixRoute]["/"] != nil { + p.Directors[pol][config.PrefixRoute]["/"](r) + return + } + + p.logger. + Warn(). + Str("policy", pol). + Str("path", r.URL.Path). + Msg("no director found") +} + func singleJoiningSlash(a, b string) string { aslash := strings.HasSuffix(a, "/") bslash := strings.HasPrefix(b, "/") @@ -135,18 +191,6 @@ func (p *MultiHostReverseProxy) AddHost(policy string, target *url.URL, rt confi } func (p *MultiHostReverseProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { - var hit bool - pol, err := p.PolicySelector(r.Context(), r) - if err != nil { - p.logger.Error().Msgf("Error while selecting pol %v", err) - } - - if _, ok := p.Directors[pol]; !ok { - p.logger. - Error(). - Msgf("policy %v is not configured", pol) - } - ctx := context.Background() var span *trace.Span @@ -157,40 +201,6 @@ func (p *MultiHostReverseProxy) ServeHTTP(w http.ResponseWriter, r *http.Request p.propagator.SpanContextToRequest(span.SpanContext(), r) } -Loop: - for _, rt := range config.RouteTypes { - var handler func(string, url.URL) bool - switch rt { - case config.QueryRoute: - handler = p.queryRouteMatcher - case config.RegexRoute: - handler = p.regexRouteMatcher - case config.PrefixRoute: - fallthrough - default: - handler = p.prefixRouteMatcher - } - for endpoint := range p.Directors[pol][rt] { - if handler(endpoint, *r.URL) { - p.Director = p.Directors[pol][rt][endpoint] - hit = true - p.logger. - Debug(). - Str("policy", pol). - Str("prefix", endpoint). - Str("path", r.URL.Path). - Str("routeType", string(rt)). - Msg("director found") - break Loop - } - } - } - - // override default director with root. If any - if !hit && p.Directors[pol][config.PrefixRoute]["/"] != nil { - p.Director = p.Directors[pol][config.PrefixRoute]["/"] - } - // Call upstream ServeHTTP p.ReverseProxy.ServeHTTP(w, r.WithContext(ctx)) } From 678db5dcd9e5545225174bea048b5d06360c4741 Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Fri, 4 Sep 2020 15:15:50 +0200 Subject: [PATCH 2/2] Changelog --- changelog/unreleased/fix-director-selection.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/unreleased/fix-director-selection.md diff --git a/changelog/unreleased/fix-director-selection.md b/changelog/unreleased/fix-director-selection.md new file mode 100644 index 0000000000..7dc94742d9 --- /dev/null +++ b/changelog/unreleased/fix-director-selection.md @@ -0,0 +1,5 @@ +Bugfix: Fix director selection + +We fixed a bug where simultaneous requests could be executed on the wrong backend. + +https://github.com/owncloud/ocis-proxy/pull/99