sunset google.golang.org/appengine module

The appengine packages that were designed for the go1 runtime, were left
functional in go111, but Google is requesting that users preemptively
migrate away due to their rigid structure and incompatibility with other
environments.

This change affects the reviewdog module dependencies, but code changes
are limited to doghouse. The appengine runtime has been increased to
go112 for extended support and to enforce that appengine packages break
if used.

appengine/urlfetch.Client has been migrated to net/http.Client.
appengine/datastore has been migrated to cloud.google.com/go/datastore.
This took more work than the Client changes, since the two packages are
incompatible. There may be work to further refactor the abstractions,
but that is out of scope for this change.

appengine.IsDevServer has no clear successor, thus its usage and
conditional logic has been purged. If there is intereset to have a
comprehensive local development version of doghouse, this can be added
back, but you will lose all the other mocks that dev_appserver.py
offers: user auth, datastore access, etc.

appengine/log contains leveled print functions, Errorf/Infof/etc., that
have no clear parallel in the stdlib log package. As a stopgap,
[SEVERITY] prefixes have been added to all the new log.Printf calls.
This is not indended as a comprehensive solution, and it may be better
to move to another package for logging, say logrus or log15, but it is
out of scope at the moment.

Finally, a note: with the change from using appengine.Context to using
http.Request.Context, there are places where an http.Request and
context.Context are passed. This could be simplified, but is out of
scope.

Fixes #226

doghouse/appengine/app.yaml

@@ -1,7 +1,7 @@
 # reviewdog app config: https://github.com/settings/apps/reviewdog
 
 # https://cloud.google.com/appengine/docs/standard/go/config/appref
-runtime: go111
+runtime: go112
 
 # https://cloud.google.com/appengine/docs/standard/go/warmup-requests/configuring
 inbound_services:

doghouse/appengine/checker.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"log"
 	"net/http"
 	"net/url"
 	"strings"
@@ -12,9 +13,6 @@ import (
 	"github.com/reviewdog/reviewdog/doghouse/server"
 	"github.com/reviewdog/reviewdog/doghouse/server/ciutil"
 	"github.com/reviewdog/reviewdog/doghouse/server/storage"
-	"google.golang.org/appengine"
-	"google.golang.org/appengine/log"
-	"google.golang.org/appengine/urlfetch"
 )
 
 type githubChecker struct {
@@ -29,7 +27,7 @@ func (gc *githubChecker) handleCheck(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusMethodNotAllowed)
 		return
 	}
-	ctx := appengine.NewContext(r)
+	ctx := r.Context()
 
 	var req doghouse.CheckRequest
 	if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
@@ -47,21 +45,21 @@ func (gc *githubChecker) handleCheck(w http.ResponseWriter, r *http.Request) {
 		PrivateKey:        gc.privateKey,
 		IntegrationID:     gc.integrationID,
 		RepoOwner:         req.Owner,
-		Client:            urlfetch.Client(ctx),
+		Client:            &http.Client{},
 		InstallationStore: gc.ghInstStore,
 	}
 
 	gh, err := server.NewGitHubClient(ctx, opt)
 	if err != nil {
-		log.Errorf(ctx, "failed to create GitHub client: %v", err)
+		log.Printf("[ERROR] failed to create GitHub client: %v\n", err)
 		w.WriteHeader(http.StatusBadRequest)
 		fmt.Fprintln(w, err)
 		return
 	}
 
 	res, err := server.NewChecker(&req, gh).Check(ctx)
 	if err != nil {
-		log.Errorf(ctx, "failed to run checker: %v", err)
+		log.Printf("[ERROR] failed to run checker: %v\n", err)
 		w.WriteHeader(http.StatusBadRequest)
 		fmt.Fprintln(w, err)
 		return
@@ -77,11 +75,11 @@ func (gc *githubChecker) validateCheckRequest(ctx context.Context, w http.Respon
 	if extractBearerToken(r) == "" {
 		// Update Travis IP Addresss before checking IP to reduce the # of
 		// flaky errors when token is not present.
-		if err := ciutil.UpdateTravisCIIPAddrs(urlfetch.Client(ctx)); err != nil {
-			log.Errorf(ctx, "failed to update travis CI IP addresses: %v", err)
+		if err := ciutil.UpdateTravisCIIPAddrs(&http.Client{}); err != nil {
+			log.Printf("[ERROR] failed to update travis CI IP addresses: %v\n", err)
 		}
 	}
-	log.Infof(ctx, "Remote Addr: %s", r.RemoteAddr)
+	log.Printf("[INFO] Remote Addr: %s\n", r.RemoteAddr)
 	if ciutil.IsFromCI(r) {
 		// Skip token validation if it's from trusted CI providers.
 		return true
@@ -99,7 +97,7 @@ func (gc *githubChecker) validateCheckToken(ctx context.Context, w http.Response
 	}
 	_, wantToken, err := gc.ghRepoTokenStore.Get(ctx, owner, repo)
 	if err != nil {
-		log.Errorf(ctx, "failed to get repository (%s/%s) token: %v", owner, repo, err)
+		log.Printf("[ERROR] failed to get repository (%s/%s) token: %v\n", owner, repo, err)
 	}
 	if wantToken == nil {
 		w.WriteHeader(http.StatusNotFound)
@@ -127,13 +125,10 @@ func doghouseBaseURL(ctx context.Context, r *http.Request) *url.URL {
 	}
 	if scheme == "" {
 		scheme = "https"
-		if appengine.IsDevAppServer() {
-			scheme = "http"
-		}
 	}
 	u, err := url.Parse(scheme + "://" + r.Host)
 	if err != nil {
-		log.Errorf(ctx, "%v", err)
+		log.Printf("[ERROR] %v\n", err)
 	}
 	return u
 }

doghouse/appengine/github.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"log"
 	"net/http"
 	"net/url"
 	"strings"
@@ -19,9 +20,6 @@ import (
 	"github.com/reviewdog/reviewdog/doghouse/server/cookieman"
 	"github.com/reviewdog/reviewdog/doghouse/server/storage"
 	"golang.org/x/oauth2"
-	"google.golang.org/appengine"
-	"google.golang.org/appengine/log"
-	"google.golang.org/appengine/urlfetch"
 )
 
 type GitHubHandler struct {
@@ -102,7 +100,7 @@ func (g *GitHubHandler) buildGithubAuthURL(r *http.Request, state string) string
 }
 
 func (g *GitHubHandler) HandleAuthCallback(w http.ResponseWriter, r *http.Request) {
-	ctx := appengine.NewContext(r)
+	ctx := r.Context()
 	code, state := r.FormValue("code"), r.FormValue("state")
 	if code == "" || state == "" {
 		w.WriteHeader(http.StatusBadRequest)
@@ -122,7 +120,7 @@ func (g *GitHubHandler) HandleAuthCallback(w http.ResponseWriter, r *http.Reques
 	// Request and save access token.
 	token, err := g.requestAccessToken(ctx, code, state)
 	if err != nil {
-		log.Errorf(ctx, "failed to get access token: %v", err)
+		log.Printf("[ERROR] failed to get access token: %v\n", err)
 		w.WriteHeader(http.StatusBadRequest)
 		fmt.Fprintln(w, "failed to get GitHub access token")
 		return
@@ -145,13 +143,12 @@ func (g *GitHubHandler) HandleLogout(w http.ResponseWriter, r *http.Request) {
 
 func (g *GitHubHandler) LogInHandler(h http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		ctx := appengine.NewContext(r)
 		if g.isLoggedIn(r) {
 			h.ServeHTTP(w, r)
 			return
 		}
 		// Not logged in yet.
-		log.Debugf(ctx, "Not logged in yet.")
+		log.Println("[DEBUG] Not logged in yet.")
 		state := securerandom(16)
 		g.redirURLStore.Set(w, []byte(r.URL.RequestURI()))
 		g.authStateStore.Set(w, []byte(state))
@@ -174,7 +171,7 @@ func securerandom(n int) string {
 // POST https://github.com/login/oauth/access_token
 func (g *GitHubHandler) requestAccessToken(ctx context.Context, code, state string) (string, error) {
 	const u = "https://github.com/login/oauth/access_token"
-	cli := urlfetch.Client(ctx)
+	cli := &http.Client{}
 	data := url.Values{}
 	data.Set("client_id", g.clientID)
 	data.Set("client_secret", g.clientSecret)
@@ -206,7 +203,7 @@ func (g *GitHubHandler) requestAccessToken(ctx context.Context, code, state stri
 	}
 
 	if token.AccessToken == "" {
-		log.Errorf(ctx, "response doesn't contain token (resopnse: %s)", b)
+		log.Printf("[ERROR] response doesn't contain token (response: %s)\n", b)
 		return "", errors.New("response doesn't contain GitHub access token")
 	}
 
@@ -222,7 +219,7 @@ func (g *GitHubHandler) token(r *http.Request) (bool, string) {
 }
 
 func (g *GitHubHandler) HandleGitHubTop(w http.ResponseWriter, r *http.Request) {
-	ctx := appengine.NewContext(r)
+	ctx := r.Context()
 
 	ok, token := g.token(r)
 	if !ok {
@@ -233,7 +230,7 @@ func (g *GitHubHandler) HandleGitHubTop(w http.ResponseWriter, r *http.Request)
 	ts := oauth2.StaticTokenSource(
 		&oauth2.Token{AccessToken: token},
 	)
-	ghcli := github.NewClient(NewAuthClient(ctx, urlfetch.Client(ctx).Transport, ts))
+	ghcli := github.NewClient(NewAuthClient(ctx, http.DefaultTransport, ts))
 
 	// /gh/{owner}/{repo}
 	paths := strings.Split(strings.Trim(r.URL.Path, "/"), "/")
@@ -280,7 +277,7 @@ func (g *GitHubHandler) handleTop(ctx context.Context, ghcli *github.Client, w h
 	}
 
 	ghAppCli, err := server.NewGitHubClient(ctx, &server.NewGitHubClientOption{
-		Client:        urlfetch.Client(ctx),
+		Client:        &http.Client{},
 		IntegrationID: g.integrationID,
 		PrivateKey:    g.privateKey,
 	})

doghouse/appengine/github_webhook.go

@@ -4,12 +4,10 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"io/ioutil"
 	"net/http"
 
 	"github.com/google/go-github/v28/github"
 	"github.com/reviewdog/reviewdog/doghouse/server/storage"
-	"google.golang.org/appengine"
 )
 
 type githubWebhookHandler struct {
@@ -22,7 +20,7 @@ func (g *githubWebhookHandler) handleWebhook(w http.ResponseWriter, r *http.Requ
 		w.WriteHeader(http.StatusMethodNotAllowed)
 		return
 	}
-	ctx := appengine.NewContext(r)
+	ctx := r.Context()
 	payload, err := g.validatePayload(r)
 	if err != nil {
 		w.WriteHeader(http.StatusUnauthorized)
@@ -50,9 +48,6 @@ func (g *githubWebhookHandler) handleWebhook(w http.ResponseWriter, r *http.Requ
 }
 
 func (g *githubWebhookHandler) validatePayload(r *http.Request) (payload []byte, err error) {
-	if appengine.IsDevAppServer() {
-		return ioutil.ReadAll(r.Body)
-	}
 	return github.ValidatePayload(r, g.secret)
 }
 

doghouse/appengine/main.go

@@ -11,7 +11,6 @@ import (
 	"github.com/justinas/nosurf"
 	"github.com/reviewdog/reviewdog/doghouse/server/cookieman"
 	"github.com/reviewdog/reviewdog/doghouse/server/storage"
-	"google.golang.org/appengine"
 )
 
 func mustCookieMan() *cookieman.CookieMan {
@@ -24,7 +23,7 @@ func mustCookieMan() *cookieman.CookieMan {
 	c := cookieman.CookieOption{
 		Cookie: http.Cookie{
 			HttpOnly: true,
-			Secure:   !appengine.IsDevAppServer(),
+			Secure:   true,
 			Path:     "/",
 		},
 	}
@@ -102,7 +101,7 @@ func main() {
 	mu.Handle("/gh/", nosurf.New(ghHandler.LogInHandler(http.HandlerFunc(ghHandler.HandleGitHubTop))))
 
 	http.Handle("/", mu)
-	appengine.Main()
+	log.Fatal(http.ListenAndServe(":"+os.Getenv("PORT"), nil))
 }
 
 func handleTop(w http.ResponseWriter, r *http.Request) {

doghouse/appengine/warmup.go

@@ -1,21 +1,18 @@
 package main
 
 import (
+	"log"
 	"net/http"
 
 	"github.com/reviewdog/reviewdog/doghouse/server/ciutil"
-	"google.golang.org/appengine"
-	"google.golang.org/appengine/log"
-	"google.golang.org/appengine/urlfetch"
 )
 
 func warmupHandler(w http.ResponseWriter, r *http.Request) {
-	ctx := appengine.NewContext(r)
-	log.Infof(ctx, "warming up server...")
+	log.Printf("[INFO] warming up server...\n")
 
-	if err := ciutil.UpdateTravisCIIPAddrs(urlfetch.Client(ctx)); err != nil {
-		log.Errorf(ctx, "failed to update travis CI IP addresses: %v", err)
+	if err := ciutil.UpdateTravisCIIPAddrs(&http.Client{}); err != nil {
+		log.Printf("[ERROR] failed to update travis CI IP addresses: %v\n", err)
 	}
 
-	log.Infof(ctx, "warmup done")
+	log.Println("[INFO] warmup done")
 }

doghouse/server/storage/datastore.go

@@ -0,0 +1,18 @@
+package storage
+
+import (
+	"context"
+
+	"cloud.google.com/go/compute/metadata"
+	"cloud.google.com/go/datastore"
+)
+
+// datastoreClient constructs a *datastore.Client using runtime introspection
+// to target the current project's datastore.
+func datastoreClient(ctx context.Context) (*datastore.Client, error) {
+	id, err := metadata.ProjectID()
+	if err != nil {
+		return nil, err
+	}
+	return datastore.NewClient(ctx, id)
+}

doghouse/server/storage/installation.go

@@ -3,7 +3,7 @@ package storage
 import (
 	"context"
 
-	"google.golang.org/appengine/datastore"
+	"cloud.google.com/go/datastore"
 )
 
 // GitHubInstallation represents GitHub Apps Installation data.
@@ -32,34 +32,43 @@ type GitHubInstallationDatastore struct{}
 
 func (g *GitHubInstallationDatastore) newKey(ctx context.Context, accountName string) *datastore.Key {
 	const kind = "GitHubInstallation"
-	return datastore.NewKey(ctx, kind, accountName, 0, nil)
+	return datastore.NameKey(kind, accountName, nil)
 }
 
 // Put save GitHubInstallation. It reduces datastore write call as much as possible.
 func (g *GitHubInstallationDatastore) Put(ctx context.Context, inst *GitHubInstallation) error {
-	return datastore.RunInTransaction(ctx, func(ctx context.Context) error {
-		ok, foundInst, err := g.Get(ctx, inst.AccountName)
+	d, err := datastoreClient(ctx)
+	if err != nil {
+		return err
+	}
+	_, err = d.RunInTransaction(ctx, func(t *datastore.Transaction) error {
+		var foundInst GitHubInstallation
+		var ok bool
+		err := t.Get(g.newKey(ctx, inst.AccountName), &foundInst)
+		if err != datastore.ErrNoSuchEntity {
+			ok = true
+		}
 		if err != nil {
 			return err
 		}
 		// Insert if not found or installation ID is different.
 		if !ok || foundInst.InstallationID != inst.InstallationID {
-			return g.put(ctx, inst)
+			_, err = t.Put(g.newKey(ctx, inst.AccountName), inst)
+			return err
 		}
 		return nil // Do nothing.
 	}, nil)
-}
-
-func (g *GitHubInstallationDatastore) put(ctx context.Context, inst *GitHubInstallation) error {
-	key := g.newKey(ctx, inst.AccountName)
-	_, err := datastore.Put(ctx, key, inst)
 	return err
 }
 
 func (g *GitHubInstallationDatastore) Get(ctx context.Context, accountName string) (ok bool, inst *GitHubInstallation, err error) {
 	key := g.newKey(ctx, accountName)
 	inst = new(GitHubInstallation)
-	if err := datastore.Get(ctx, key, inst); err != nil {
+	d, err := datastoreClient(ctx)
+	if err != nil {
+		return false, nil, err
+	}
+	if err := d.Get(ctx, key, inst); err != nil {
 		if err == datastore.ErrNoSuchEntity {
 			return false, nil, nil
 		}