cmd/coordinator: use proxy.golang.org instead of private Athens

Fixes golang/go#31770

Change-Id: Ia21b3c835b3692351d83e00b9a8cb73db2103192
Reviewed-on: https://go-review.googlesource.com/c/build/+/175057
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

cmd/buildlet/buildlet.go

@@ -929,18 +929,6 @@ func handleExec(w http.ResponseWriter, r *http.Request) {
 
 	env := append(baseEnv(goarch), postEnv...)
 
-	// Setup an localhost HTTP server to proxy module cache, if requested by environment.
-	if goproxyHandler != nil && getEnv(postEnv, "GO_BUILDER_SET_GOPROXY") == "coordinator" {
-		ln, err := net.Listen("tcp", "localhost:0")
-		if err != nil {
-			http.Error(w, "failed to listen on localhost for GOPROXY=coordinator: "+err.Error(), http.StatusInternalServerError)
-			return
-		}
-		defer ln.Close()
-		srv := &http.Server{Handler: goproxyHandler}
-		go srv.Serve(ln)
-		env = append(env, fmt.Sprintf("GOPROXY=http://localhost:%d", ln.Addr().(*net.TCPAddr).Port))
-	}
 	if v := processTmpDirEnv; v != "" {
 		env = append(env, "TMPDIR="+v)
 	}

cmd/buildlet/reverse.go

@@ -16,7 +16,6 @@ import (
 	"log"
 	"net"
 	"net/http"
-	"net/http/httputil"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -68,34 +67,6 @@ func isDevReverseMode() bool {
 	return !strings.HasPrefix(*coordinator, "farmer.golang.org")
 }
 
-// proxyToCoordinatorHandler is a GOPROXY proxy, proxying to
-// https://farmer.golang.org while adding HTTP basic auth (of the
-// reverse buildlet type & its key) and the
-// X-Proxy-Service:module-cache header.
-type proxyToCoordinatorHandler struct {
-	user, pass string
-	rp         *httputil.ReverseProxy
-}
-
-func (h *proxyToCoordinatorHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
-	outReq := r.WithContext(r.Context())
-	outReq.SetBasicAuth(h.user, h.pass)
-	outReq.Header.Set("X-Proxy-Service", "module-cache")
-	h.rp.ServeHTTP(w, outReq)
-}
-
-// goproxyHandler is non-nil for reverse buildlets.
-var goproxyHandler *proxyToCoordinatorHandler
-
-func newProxyToCoordinatorHandler(user, pass string) *proxyToCoordinatorHandler {
-	u, _ := url.Parse("https://farmer.golang.org")
-	return &proxyToCoordinatorHandler{
-		user: user,
-		pass: pass,
-		rp:   httputil.NewSingleHostReverseProxy(u),
-	}
-}
-
 func dialCoordinator() error {
 	devMode := isDevReverseMode()
 
@@ -114,7 +85,6 @@ func dialCoordinator() error {
 			}
 			keys = append(keys, key)
 		}
-		goproxyHandler = newProxyToCoordinatorHandler(modes[0], keys[0])
 	} else {
 		// New way.
 		key, err := keyForMode(*reverseType)
@@ -122,7 +92,6 @@ func dialCoordinator() error {
 			log.Fatalf("failed to find key for %s: %v", *reverseType, err)
 		}
 		keys = append(keys, key)
-		goproxyHandler = newProxyToCoordinatorHandler(*reverseType, key)
 	}
 
 	addr := *coordinator

cmd/coordinator/athens-prod.yaml

@@ -1,37 +0,0 @@
-apiVersion: extensions/v1beta1
-kind: Deployment
-metadata:
-  name: athens-deployment
-spec:
-  template:
-    metadata:
-      labels:
-        app: athens
-      annotations:
-        container.seccomp.security.alpha.kubernetes.io/athens: docker/default
-        container.apparmor.security.beta.kubernetes.io/athens: runtime/default
-    spec:
-      volumes:
-      - name: cache-volume
-        emptyDir: {}
-      containers:
-      - name: athens
-        image: gomods/athens:v0.2.0
-        imagePullPolicy: Always
-        command: ["/bin/athens-proxy", "-config_file=/config/config.toml"]
-        volumeMounts:
-        - mountPath: "/athens-cache"
-          name: cache-volume
-        env:
-        - name: GO_ENV
-          value: "production"
-        - name: ATHENS_STORAGE_TYPE
-          value: "disk"
-        - name: ATHENS_DISK_STORAGE_ROOT
-          value: "/athens-cache"
-        ports:
-        - containerPort: 3000
-        resources:
-          requests:
-            cpu: "1"
-            memory: "2Gi"

cmd/coordinator/coordinator.go

@@ -221,8 +221,11 @@ func (httpRouter) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		requireBuildletProxyAuth(http.HandlerFunc(proxyBuildletHTTP)).ServeHTTP(w, r)
 		return
 	}
+	// TODO: delete this check in a few weeks from 2019-05-02:
 	if r.Header.Get("X-Proxy-Service") == "module-cache" {
-		proxyModuleCache(w, r)
+		user, _, _ := r.BasicAuth()
+		log.Printf("modproxy: buildlet %q is still hitting old endpoint", user)
+		http.Error(w, "service turned down", 500)
 		return
 	}
 	http.DefaultServeMux.ServeHTTP(w, r)
@@ -341,6 +344,7 @@ func main() {
 			dashboard.Builders = stagingClusterBuilders()
 		}
 
+		go listenAndServeInternalModuleProxy()
 		go findWorkLoop(workc)
 		go findTryWorkLoop()
 		go reportMetrics(context.Background())
@@ -2461,16 +2465,13 @@ func (st *buildStatus) runSubrepoTests() (remoteErr, err error) {
 // moduleProxy returns the GOPROXY environment value to use for module-enabled
 // tests.
 //
-// We go through a GCP-project-internal module proxy ("GOPROXY") to
-// eliminate load on the origin servers. Our builder VMs are ephemeral
-// and only run for the duration of one build. They also often don't
-// have all the VCS tools installed (or even available: there is no
-// git for plan9).
+// We go through an internal (10.0.0.0/8) proxy that then hits
+// https://proxy.golang.org/ so we're still able to firewall
+// non-internal outbound connections on builder nodes.
 //
-// moduleProxy in prod mode (when running on GKE) returns an http
+// This moduleProxy func in prod mode (when running on GKE) returns an http
 // URL to the current GKE pod's IP with a Kubernetes NodePort service
-// port that forwards to the internal Athens module proxy cache
-// service we run on GKE.
+// port that forwards back to the coordinator's 8123. See comment below.
 //
 // In localhost dev mode it just returns the value of GOPROXY.
 func moduleProxy() string {
@@ -2480,15 +2481,15 @@ func moduleProxy() string {
 		return os.Getenv("GOPROXY")
 	}
 	// We run a NodePort service on each GKE node
-	// (cmd/coordinator/module-proxy-service.yaml) on port 30156
-	// that maps to the Athens service. We could round robin over
-	// all the GKE nodes' IPs if we wanted, but the coordinator is
-	// running on GKE so our node by definition is up, so just use it.
-	// It won't be much traffic.
+	// (cmd/coordinator/module-proxy-service.yaml) on port 30157
+	// that maps back the coordinator's port 8123. (We could round
+	// robin over all the GKE nodes' IPs if we wanted, but the
+	// coordinator is running on GKE so our node by definition is
+	// up, so just use it. It won't be much traffic.)
 	// TODO: migrate to a GKE internal load balancer with an internal static IP
 	// once we migrate symbolic-datum-552 off a Legacy VPC network to the modern
 	// scheme that supports internal static IPs.
-	return "http://" + gkeNodeIP + ":30156"
+	return "http://" + gkeNodeIP + ":30157"
 }
 
 // affectedPkgs returns the name of every package affected by this commit.

cmd/coordinator/deployment-prod.yaml

@@ -20,6 +20,7 @@ spec:
         - containerPort: 80
         - containerPort: 443
         - containerPort: 2222 # ssh proxy port
+        - containerPort: 8123 # module proxy port (internal, not accessible directly from public)
         resources:
           requests:
             cpu: "2"

cmd/coordinator/modproxy.go

@@ -11,71 +11,37 @@ import (
 	"net/http"
 	"net/http/httputil"
 	"net/url"
-	"strings"
 )
 
-// proxyModuleCache proxies from https://farmer.golang.org (with a
-// magic header, as handled by coordinator.go's httpRouter type) to
-// Go's private module proxy server running on GKE. The module proxy
-// protocol does not define authentication, so we do it ourselves.
-//
-// The complete path is the buildlet listens on localhost:3000 to run
-// an unauthenticated module proxy server for the cmd/go binary to use
-// via GOPROXY=http://localhost:3000. That localhost:3000 server
-// proxies it to https://farmer.golang.org with auth headers and a
-// sentinel X-Proxy-Service:module-cache header. Then coordinator.go's
-// httpRouter sends it here.
-//
-// This code then does the final reverse proxy, sent without auth.
-//
-// In summary:
-//
-//   cmd/go -> localhost:3000 -> buildlet -> coordinator -> GKE server
-func proxyModuleCache(w http.ResponseWriter, r *http.Request) {
-	if r.TLS == nil {
-		http.Error(w, "https required", http.StatusBadRequest)
-		return
-	}
-	builder, pass, ok := r.BasicAuth()
-	if !ok {
-		http.Error(w, "missing required authentication", http.StatusBadRequest)
-		return
-	}
+var proxyGolangOrg *httputil.ReverseProxy // initialized just below
 
-	// For old buildlets that didn't TrimSpace their gobuildkey
-	// file contents (Issue 30749), remove the space here too.
-	// Once all buildlets are upgraded to version 22 or higher
-	// this can be removed. They should all auto-update, but some
-	// are misconfigured and don't.
-	pass = strings.TrimSpace(pass)
-
-	if !strings.Contains(builder, "-") || builderKey(builder) != pass {
-		log.Printf("modproxy: sending 401 Unauthorized due to invalid key for builder %q", builder)
-		http.Error(w, "bad username or password", http.StatusUnauthorized)
-		return
-	}
-
-	targetURL := moduleProxy()
-	if !strings.HasPrefix(targetURL, "http") {
-		log.Printf("unsupported GOPROXY backend value %q; not proxying", targetURL)
-		http.Error(w, "no GOPROXY backend available", http.StatusInternalServerError)
-		return
-	}
-	backend, err := url.Parse(targetURL)
+func init() {
+	u, err := url.Parse("https://proxy.golang.org")
 	if err != nil {
-		log.Printf("failed to parse GOPROXY value as URL: %v", err)
-		http.Error(w, "module proxy misconfigured", http.StatusInternalServerError)
-		return
+		log.Fatal(err)
 	}
-	// TODO: maybe only create this once early. But probably doesn't matter.
-	rp := httputil.NewSingleHostReverseProxy(backend)
+	rp := httputil.NewSingleHostReverseProxy(u)
 	rp.ModifyResponse = func(res *http.Response) error {
-		if res.StatusCode/100 != 2 {
-			log.Printf("modproxy: proxying HTTP %s response from backend for builder %s, %s %s", res.Status, builder, r.Method, r.RequestURI)
+		r := res.Request
+		if res.StatusCode/100 != 2 && r != nil {
+			log.Printf("modproxy: proxying HTTP %s response from backend for %s, %s %s", res.Status, r.RemoteAddr, r.Method, r.RequestURI)
 		}
 		return nil
 	}
-	r.Header.Del("Authorization")
-	r.Header.Del("X-Proxy-Service")
-	rp.ServeHTTP(w, r)
+	proxyGolangOrg = rp
+}
+
+func listenAndServeInternalModuleProxy() {
+	err := http.ListenAndServe(":8123", http.HandlerFunc(proxyModuleCache))
+	log.Fatalf("error running internal module proxy: %v", err)
+}
+
+// proxyModuleCache proxies requests to https://proxy.golang.org/
+func proxyModuleCache(w http.ResponseWriter, r *http.Request) {
+	// Delete any Host header so it's the one sent to the backend
+	// is proxy.golang.org by the default ReverseProxy.Director
+	// setting the URL.Host. (Host takes priority for Host header,
+	// if present)
+	r.Host = ""
+	proxyGolangOrg.ServeHTTP(w, r)
 }

cmd/coordinator/module-proxy-service.yaml

@@ -1,19 +1,19 @@
 apiVersion: v1
 kind: Service
 metadata:
-  name: go-module-proxy
+  name: proxygolang-proxy
   annotations:
     cloud.google.com/load-balancer-type: "Internal"
 spec:
   type: NodePort
   ports:
-    - name: module-proxy
-      port: 3000
-      targetPort: 3000
-      nodePort: 30156
+    - name: proxygolang-proxy
+      port: 8123
+      targetPort: 8123
+      nodePort: 30157
       protocol: TCP
   selector:
-    app: athens
+    app: coordinator
 
 # TODO(bradfitz): migrate (destroy & recreate) symbolic-datum-552 to get it off legacy networking
 # so we can use an internal LoadBalancer with a static internal IP instead, and then:
@@ -24,7 +24,9 @@ spec:
 #  loadBalancerSourceRanges:
 #  - "10.0.0.0/8"
 #  ports:
-#    - port: 3000
-#      targetPort: 3000
+#    - port: 8123
+#      targetPort: 8123
 #  selector:
-#    app: athens
+#    app: coordinator
+#
+# ... and update the 30157 in coordinator to be 8123.

dashboard/builders.go

@@ -763,7 +763,7 @@ func (c *BuildConfig) Env() []string {
 // ("go", "oauth2", "net", etc).
 func (c *BuildConfig) ModulesEnv(repo string) (env []string) {
 	if c.IsReverse() && repo != "go" {
-		env = append(env, "GO_BUILDER_SET_GOPROXY=coordinator")
+		env = append(env, "GOPROXY=https://proxy.golang.org")
 	}
 	switch repo {
 	case "go":