From 4dc8b8fcd51baa486669add9a3f6cd71191f9cfe Mon Sep 17 00:00:00 2001 From: Daniel Thomas Date: Tue, 23 Nov 2021 16:42:24 +0100 Subject: [PATCH] * fixed missing stripping of path-segment * added logging of error messages --- Dockerfile | 1 + proxy/cache.go | 8 ++++++-- proxy/jobs.go | 13 ++++++++++--- proxy/proxy.go | 16 ++++++++++------ 4 files changed, 27 insertions(+), 11 deletions(-) diff --git a/Dockerfile b/Dockerfile index 56b0509..440812b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -25,6 +25,7 @@ RUN upx /src/bin/contentfulproxy ############################## ###### STAGE: PACKAGE ###### ############################## +# TODO: use non-root FROM alpine:latest ENV CONTENTFULPROXY_SERVER_ADDR=0.0.0.0:80 diff --git a/proxy/cache.go b/proxy/cache.go index 7ed3512..0708f40 100644 --- a/proxy/cache.go +++ b/proxy/cache.go @@ -85,8 +85,8 @@ func NewCache(l *zap.Logger, webHooks func() []string) *Cache { return c } -func getCacheIDForRequest(r *http.Request) cacheID { - id := r.URL.RequestURI() +func getCacheIDForRequest(r *http.Request, pathPrefix func() string) cacheID { + id := stripPrefixFromUrl(r.URL.RequestURI(), pathPrefix) keys := make([]string, len(r.Header)) i := 0 for k := range r.Header { @@ -107,3 +107,7 @@ func getCacheIDForRequest(r *http.Request) cacheID { id = hex.EncodeToString(hash.Sum(nil)) return cacheID(id) } + +func stripPrefixFromUrl(url string, pathPrefix func() string) string { + return strings.Replace(url, pathPrefix(), "", 1) +} diff --git a/proxy/jobs.go b/proxy/jobs.go index d659650..a3e1c1c 100644 --- a/proxy/jobs.go +++ b/proxy/jobs.go @@ -1,6 +1,10 @@ package proxy -import "net/http" +import ( + "github.com/foomo/contentfulproxy/packages/go/log" + "go.uber.org/zap" + "net/http" +) type requestJobDone struct { cachedResponse *cachedResponse @@ -15,9 +19,12 @@ type requestJob struct { type jobRunner func(job requestJob, id cacheID) -func getJobRunner(c *Cache, backendURL func() string, chanJobDone chan requestJobDone) jobRunner { +func getJobRunner(l *zap.Logger, c *Cache, backendURL func() string, pathPrefix func() string, chanJobDone chan requestJobDone) jobRunner { return func(job requestJob, id cacheID) { - req, err := http.NewRequest("GET", backendURL()+job.request.URL.RequestURI(), nil) + // backend url is the contentful api domain like https://cdn.contenful.com + calledURL := backendURL() + stripPrefixFromUrl(job.request.URL.RequestURI(), pathPrefix) + l.Info("URL called by job-runner", log.FURL(calledURL)) + req, err := http.NewRequest("GET", calledURL, nil) if err != nil { chanJobDone <- requestJobDone{ id: id, diff --git a/proxy/proxy.go b/proxy/proxy.go index 4d3ee40..e7698bd 100644 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -10,6 +10,7 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/foomo/contentfulproxy/packages/go/log" + keellog "github.com/foomo/keel/log" "go.uber.org/zap" ) @@ -56,7 +57,7 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { case http.MethodGet: p.l.Info("serve get request", zap.String("url", r.RequestURI)) p.metrics.NumProxyRequest.Inc() - cacheID := getCacheIDForRequest(r) + cacheID := getCacheIDForRequest(r, p.pathPrefix) cachedResponse, ok := p.cache.get(cacheID) if !ok { chanDone := make(chan requestJobDone) @@ -66,15 +67,17 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { } jobDone := <-chanDone if jobDone.err != nil { - p.l.Error("Cache / job error", zap.String("url", r.RequestURI)) + keellog.WithError(p.l, jobDone.err).Error("Cache / job error") http.Error(w, "Cache / job error", http.StatusInternalServerError) return } cachedResponse = jobDone.cachedResponse p.l.Info("serve response after cache creation", log.FURL(r.RequestURI), log.FCacheID(string(cacheID))) + p.l.Info("length of response", keellog.FValue(len(cachedResponse.response))) p.metrics.NumAPIRequest.Inc() } else { p.l.Info("serve response from cache", log.FURL(r.RequestURI), log.FCacheID(string(cacheID))) + p.l.Info("length of response", keellog.FValue(len(cachedResponse.response))) } for key, values := range cachedResponse.header { for _, value := range values { @@ -83,7 +86,7 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { } _, err := w.Write(cachedResponse.response) if err != nil { - p.l.Info("writing cached response failed", log.FURL(r.RequestURI), log.FCacheID(string(cacheID))) + keellog.WithError(p.l, err).Error("writing cached response failed", log.FCacheID(string(cacheID))) } default: http.Error(w, "method not allowed", http.StatusMethodNotAllowed) @@ -94,7 +97,7 @@ func NewProxy(ctx context.Context, l *zap.Logger, backendURL func() string, path chanRequest := make(chan requestJob) chanFlush := make(chan requestFlush) c := NewCache(l, webHooks) - go getLoop(ctx, l, backendURL, c, chanRequest, chanFlush) + go getLoop(ctx, l, backendURL, pathPrefix, c, chanRequest, chanFlush) return &Proxy{ l: l, cache: c, @@ -110,13 +113,14 @@ func getLoop( ctx context.Context, l *zap.Logger, backendURL func() string, + pathPrefix func() string, c *Cache, chanRequestJob chan requestJob, chanFlush chan requestFlush, ) { pendingRequests := map[cacheID][]chan requestJobDone{} chanJobDone := make(chan requestJobDone) - jobRunner := getJobRunner(c, backendURL, chanJobDone) + jobRunner := getJobRunner(l, c, backendURL, pathPrefix, chanJobDone) for { select { case <-chanFlush: @@ -124,7 +128,7 @@ func getLoop( c.update() c.callWebHooks() case nextJob := <-chanRequestJob: - cacheID := getCacheIDForRequest(nextJob.request) + cacheID := getCacheIDForRequest(nextJob.request, pathPrefix) pendingRequests[cacheID] = append(pendingRequests[cacheID], nextJob.chanDone) requests := pendingRequests[cacheID] if len(requests) == 1 {