gddo-server: redirect based on config
shouldRedirectRequest now uses data from the DynamicConfig to decide
whether a request should be redirected.
Change-Id: I69a3d4df275248003b170c0e48ca6959cad93603
Reviewed-on: https://go-review.googlesource.com/c/gddo/+/285933
Trust: Julie Qiu <julie@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
diff --git a/gddo-server/dynconfig/dynconfig.go b/gddo-server/dynconfig/dynconfig.go
index d75f8bc..5207164 100644
--- a/gddo-server/dynconfig/dynconfig.go
+++ b/gddo-server/dynconfig/dynconfig.go
@@ -26,6 +26,10 @@
type DynamicConfig struct {
// Fields can be added at any time, but removing or changing a field
// requires careful coordination with the config file contents.
+ RedirectBadges bool
+ RedirectHomepage bool
+ RedirectSearch bool
+ RedirectPaths []string
}
// Read reads dynamic configuration from the given location.
diff --git a/gddo-server/pkgsite.go b/gddo-server/pkgsite.go
index 5c883f8..162d6e1 100644
--- a/gddo-server/pkgsite.go
+++ b/gddo-server/pkgsite.go
@@ -17,6 +17,8 @@
"strings"
"time"
+ "github.com/golang/gddo/gddo-server/dynconfig"
+ "github.com/golang/gddo/gddo-server/poller"
"golang.org/x/mod/module"
"golang.org/x/net/context/ctxhttp"
)
@@ -185,11 +187,14 @@
// pkgGoDevRedirectHandler redirects requests from godoc.org to pkg.go.dev,
// based on whether a cookie is set for pkggodev-redirect. The cookie
// can be turned on/off using a query param.
-func pkgGoDevRedirectHandler(f func(http.ResponseWriter, *http.Request) error) func(http.ResponseWriter, *http.Request) error {
+func pkgGoDevRedirectHandler(configPoller *poller.Poller, f func(http.ResponseWriter, *http.Request) error) func(http.ResponseWriter, *http.Request) error {
return func(w http.ResponseWriter, r *http.Request) error {
- if shouldRedirectRequest(r) {
- http.Redirect(w, r, pkgGoDevURL(r.URL).String(), http.StatusFound)
- return nil
+ redirectURL := pkgGoDevURL(r.URL).String()
+ if configPoller != nil {
+ if shouldRedirectURL(r, configPoller) {
+ http.Redirect(w, r, redirectURL, http.StatusFound)
+ return nil
+ }
}
if userReturningFromPkgGoDev(r) {
@@ -207,32 +212,50 @@
if !shouldRedirectToPkgGoDev(r) {
return f(w, r)
}
- http.Redirect(w, r, pkgGoDevURL(r.URL).String(), http.StatusFound)
+ http.Redirect(w, r, redirectURL, http.StatusFound)
return nil
}
}
-// alwaysRedirectPaths is a set of paths that are always redirected to
-// pkg.go.dev.
-var alwaysRedirectPaths = map[string]bool{
- "/-/about": true,
+// shouldRedirectURL reports whether a request to the given URL should be
+// redirected to pkg.go.dev.
+func shouldRedirectURL(r *http.Request, poller *poller.Poller) bool {
+ cfg := poller.Current().(*dynconfig.DynamicConfig)
+ return shouldRedirectURLForSnapshot(r, cfg)
}
-func shouldRedirectRequest(r *http.Request) bool {
+func shouldRedirectURLForSnapshot(r *http.Request, cfg *dynconfig.DynamicConfig) bool {
+ if cfg == nil {
+ log.Printf("shouldRedirectURLForSnapshot(%q): cfg is nil", r.URL.Path)
+ return false
+ }
+
// Requests to api.godoc.org and talks.godoc.org are not redirected.
if strings.HasPrefix(r.URL.Host, "api") || strings.HasPrefix(r.URL.Host, "talks") {
return false
}
- // Badge SVGs will be redirected last.
+
_, isSVG := r.URL.Query()["status.svg"]
_, isPNG := r.URL.Query()["status.png"]
if isSVG || isPNG {
+ return cfg.RedirectBadges
+ }
+ for _, p := range cfg.RedirectPaths {
+ if r.URL.Path == p {
+ return true
+ }
+ }
+ q := strings.TrimSpace(r.Form.Get("q"))
+ if r.URL.Path == "/" || r.URL.Path == "" {
+ if q == "" && cfg.RedirectHomepage {
+ return true
+ }
+ if q != "" && cfg.RedirectSearch {
+ return true
+ }
return false
}
- if alwaysRedirectPaths[r.URL.Path] {
- return true
- }
// TODO: redirect based on rollout percentage.
return false
}
diff --git a/gddo-server/pkgsite_test.go b/gddo-server/pkgsite_test.go
index 18ffe08..34ad51b 100644
--- a/gddo-server/pkgsite_test.go
+++ b/gddo-server/pkgsite_test.go
@@ -14,6 +14,7 @@
"strings"
"testing"
+ "github.com/golang/gddo/gddo-server/dynconfig"
"github.com/google/go-cmp/cmp"
)
@@ -66,19 +67,13 @@
cookie: &http.Cookie{Name: "pkggodev-redirect", Value: "on"},
wantStatusCode: http.StatusOK,
},
- {
- name: "always redirect /-/about",
- url: "http://godoc.org/-/about",
- wantLocationHeader: "https://pkg.go.dev/about?utm_source=godoc",
- wantStatusCode: http.StatusFound,
- },
} {
t.Run(test.name, func(t *testing.T) {
req := httptest.NewRequest("GET", test.url, nil)
if test.cookie != nil {
req.AddCookie(test.cookie)
}
- handler := pkgGoDevRedirectHandler(func(w http.ResponseWriter, r *http.Request) error {
+ handler := pkgGoDevRedirectHandler(nil, func(w http.ResponseWriter, r *http.Request) error {
return nil
})
@@ -498,3 +493,74 @@
}
}
}
+
+func TestShouldRedirectURLForSnapshot(t *testing.T) {
+ var testRequests []*http.Request
+ for _, tu := range []string{
+ "https://api.godoc.org/-/about",
+ "https://api.godoc.org/?q=http",
+ "https://api.godoc.org/-/net/http",
+ "https://godoc.org",
+ "https://godoc.org/-/about",
+ "https://godoc.org/-/bot",
+ "https://godoc.org/-/go",
+ "https://godoc.org/-/subrepo",
+ "https://godoc.org/?q=http",
+ "https://godoc.org/net/http",
+ "https://godoc.org/cloud.google.com/go",
+ } {
+ u, err := url.Parse(tu)
+ if err != nil {
+ t.Fatal(err)
+ }
+ req := httptest.NewRequest("GET", u.String(), nil)
+ req.URL = u
+ req.Form = u.Query()
+ testRequests = append(testRequests, req)
+ }
+
+ for _, test := range []struct {
+ name string
+ snapshot *dynconfig.DynamicConfig
+ redirect map[string]bool
+ }{
+ {
+ name: "redirect homepage",
+ snapshot: &dynconfig.DynamicConfig{
+ RedirectHomepage: true,
+ },
+ redirect: map[string]bool{"https://godoc.org": true},
+ },
+ {
+ name: "redirect search page",
+ snapshot: &dynconfig.DynamicConfig{
+ RedirectSearch: true,
+ },
+ redirect: map[string]bool{"https://godoc.org/?q=http": true},
+ },
+ {
+ name: "redirect static page",
+ snapshot: &dynconfig.DynamicConfig{
+ RedirectPaths: []string{"/-/about", "/-/subrepo", "/-/go"},
+ },
+ redirect: map[string]bool{
+ "https://godoc.org/-/about": true,
+ "https://godoc.org/-/go": true,
+ "https://godoc.org/-/subrepo": true,
+ },
+ },
+ } {
+ t.Run(test.name, func(t *testing.T) {
+ for _, req := range testRequests {
+ u := req.URL.String()
+ got := shouldRedirectURLForSnapshot(req, test.snapshot)
+ if test.redirect[u] && !got {
+ t.Errorf("%q should redirect but didn't", u)
+ }
+ if !test.redirect[u] && got {
+ t.Errorf("%q should not redirect and did", u)
+ }
+ }
+ })
+ }
+}
diff --git a/gddo-server/server.go b/gddo-server/server.go
index fdf5ce4..40c1954 100644
--- a/gddo-server/server.go
+++ b/gddo-server/server.go
@@ -557,11 +557,9 @@
if err != nil {
return err
}
-
return s.templates.execute(resp, "home"+templateExt(req), http.StatusOK, nil,
map[string]interface{}{
- "Popular": pkgs,
-
+ "Popular": pkgs,
"showPkgGoDevRedirectToast": userReturningFromPkgGoDev(req),
})
}
@@ -607,8 +605,7 @@
func (s *server) serveAbout(resp http.ResponseWriter, req *http.Request) error {
return s.templates.execute(resp, "about.html", http.StatusOK, nil,
map[string]interface{}{
- "Host": req.Host,
-
+ "Host": req.Host,
"showPkgGoDevRedirectToast": userReturningFromPkgGoDev(req),
})
}
@@ -886,7 +883,7 @@
httpClient: newHTTPClient(v),
importGraphSem: make(chan struct{}, 10),
}
- cfg, err := newConfigPoller(ctx, 1*time.Millisecond)
+ cfg, err := newConfigPoller(ctx, 1*time.Minute)
if err != nil {
log.Printf("newConfigPoller: %q", err)
}
@@ -965,10 +962,10 @@
}
}
- mux.Handle("/-/about", handler(pkgGoDevRedirectHandler(s.serveAbout)))
- mux.Handle("/-/bot", handler(pkgGoDevRedirectHandler(s.serveBot)))
- mux.Handle("/-/go", handler(pkgGoDevRedirectHandler(s.serveGoIndex)))
- mux.Handle("/-/subrepo", handler(pkgGoDevRedirectHandler(s.serveGoSubrepoIndex)))
+ mux.Handle("/-/about", handler(pkgGoDevRedirectHandler(s.configPoller, s.serveAbout)))
+ mux.Handle("/-/bot", handler(pkgGoDevRedirectHandler(s.configPoller, s.serveBot)))
+ mux.Handle("/-/go", handler(pkgGoDevRedirectHandler(s.configPoller, s.serveGoIndex)))
+ mux.Handle("/-/subrepo", handler(pkgGoDevRedirectHandler(s.configPoller, s.serveGoSubrepoIndex)))
mux.Handle("/-/refresh", handler(s.serveRefresh))
mux.Handle("/about", http.RedirectHandler("/-/about", http.StatusMovedPermanently))
mux.Handle("/favicon.ico", staticServer.FileHandler("favicon.ico"))
@@ -978,7 +975,7 @@
mux.Handle("/BingSiteAuth.xml", staticServer.FileHandler("BingSiteAuth.xml"))
mux.Handle("/C", http.RedirectHandler("http://golang.org/doc/articles/c_go_cgo.html", http.StatusMovedPermanently))
mux.Handle("/code.jquery.com/", http.NotFoundHandler())
- mux.Handle("/", handler(pkgGoDevRedirectHandler(s.serveHome)))
+ mux.Handle("/", handler(pkgGoDevRedirectHandler(s.configPoller, s.serveHome)))
ahMux := http.NewServeMux()
ready := new(health.Handler)
@@ -1080,7 +1077,7 @@
}
func (s *server) teeRequestToPkgGoDev(r *http.Request, latency time.Duration, status int) {
- if shouldRedirectRequest(r) {
+ if shouldRedirectURL(r, s.configPoller) {
log.Printf("shouldRedirectToPkgGoDev(%q, %q)= true: not teeing request because it is redirected to pkg.go.dev", r.URL.Host, r.URL.Path)
return
}
