Skip to content

Commit

Permalink
Merge pull request #312 from reviewdog/arnottcr-go112
Browse files Browse the repository at this point in the history
doghouse: sunset google.golang.org/appengine module based on #307
  • Loading branch information
haya14busa committed Sep 21, 2019
2 parents b61b258 + e0ab5a3 commit 1d6a3c8
Show file tree
Hide file tree
Showing 14 changed files with 252 additions and 100 deletions.
2 changes: 1 addition & 1 deletion doghouse/appengine/app.yaml
Original file line number Diff line number Diff line change
@@ -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:
Expand Down
32 changes: 13 additions & 19 deletions doghouse/appengine/checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"fmt"
"log"
"net/http"
"net/url"
"strings"
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -44,24 +42,23 @@ func (gc *githubChecker) handleCheck(w http.ResponseWriter, r *http.Request) {
}

opt := &server.NewGitHubClientOption{
PrivateKey: gc.privateKey,
IntegrationID: gc.integrationID,
RepoOwner: req.Owner,
Client: urlfetch.Client(ctx),
InstallationStore: gc.ghInstStore,
PrivateKey: gc.privateKey,
IntegrationID: gc.integrationID,
RepoOwner: req.Owner,
Client: &http.Client{},
}

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
Expand All @@ -77,11 +74,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
Expand All @@ -99,7 +96,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)
Expand Down Expand Up @@ -127,13 +124,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
}
Expand Down
21 changes: 9 additions & 12 deletions doghouse/appengine/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"fmt"
"io"
"io/ioutil"
"log"
"net/http"
"net/url"
"strings"
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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))
Expand All @@ -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)
Expand Down Expand Up @@ -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")
}

Expand All @@ -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 {
Expand All @@ -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, "/"), "/")
Expand Down Expand Up @@ -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,
})
Expand Down
7 changes: 1 addition & 6 deletions doghouse/appengine/github_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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)
}

Expand Down
1 change: 0 additions & 1 deletion doghouse/appengine/handlers.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,4 @@ handlers:

- url: /_ah/warmup
script: auto
login: admin

5 changes: 2 additions & 3 deletions doghouse/appengine/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -24,7 +23,7 @@ func mustCookieMan() *cookieman.CookieMan {
c := cookieman.CookieOption{
Cookie: http.Cookie{
HttpOnly: true,
Secure: !appengine.IsDevAppServer(),
Secure: true,
Path: "/",
},
}
Expand Down Expand Up @@ -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) {
Expand Down
13 changes: 5 additions & 8 deletions doghouse/appengine/warmup.go
Original file line number Diff line number Diff line change
@@ -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")
}
10 changes: 10 additions & 0 deletions doghouse/server/ciutil/ciutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"net"
"net/http"
"strings"
"sync"
)

Expand Down Expand Up @@ -103,6 +104,15 @@ func IsFromAppveyor(r *http.Request) bool {
}

func ipFromReq(r *http.Request) string {
if f := r.Header.Get("Forwarded"); f != "" {
for _, kv := range strings.Split(f, ";") {
if kvPair := strings.SplitN(kv, "=", 2); len(kvPair) == 2 &&
strings.ToLower(strings.TrimSpace(kvPair[0])) == "for" {
return strings.Trim(kvPair[1], ` "`)
}
}
}

ip, _, err := net.SplitHostPort(r.RemoteAddr)
if err != nil {
return r.RemoteAddr
Expand Down
43 changes: 23 additions & 20 deletions doghouse/server/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,11 @@ package server

import (
"context"
"errors"
"fmt"
"net/http"

"github.com/bradleyfalzon/ghinstallation"
"github.com/google/go-github/v28/github"
"github.com/reviewdog/reviewdog/doghouse/server/storage"
)

type NewGitHubClientOption struct {
Expand All @@ -17,9 +15,8 @@ type NewGitHubClientOption struct {
// Required
IntegrationID int

// RepoOwner AND InstallationStore is required for installation API.
RepoOwner string
InstallationStore storage.GitHubInstallationStore
// RepoOwner is required for installation API.
RepoOwner string

// Optional
Client *http.Client
Expand All @@ -42,29 +39,35 @@ func NewGitHubClient(ctx context.Context, opt *NewGitHubClientOption) (*github.C

func githubAppTransport(ctx context.Context, client *http.Client, opt *NewGitHubClientOption) (http.RoundTripper, error) {
if opt.RepoOwner == "" {
return ghinstallation.NewAppsTransport(client.Transport, opt.IntegrationID, opt.PrivateKey)
return ghinstallation.NewAppsTransport(getTransport(client), opt.IntegrationID, opt.PrivateKey)
}

installationID, err := installationIDFromOpt(ctx, opt)
installationID, err := findInstallationID(ctx, client, opt)
if err != nil {
return nil, err
}
return ghinstallation.New(client.Transport, opt.IntegrationID, int(installationID), opt.PrivateKey)
return ghinstallation.New(getTransport(client), opt.IntegrationID, int(installationID), opt.PrivateKey)
}

func installationIDFromOpt(ctx context.Context, opt *NewGitHubClientOption) (int64, error) {
if opt.RepoOwner == "" {
return 0, errors.New("repo owner is required")
}
if opt.InstallationStore == nil {
return 0, errors.New("instllation store is not provided")
func getTransport(client *http.Client) http.RoundTripper {
if client.Transport != nil {
return client.Transport
}
ok, inst, err := opt.InstallationStore.Get(ctx, opt.RepoOwner)
return http.DefaultTransport
}

func findInstallationID(ctx context.Context, client *http.Client, opt *NewGitHubClientOption) (int64, error) {
appCli, err := NewGitHubClient(ctx, &NewGitHubClientOption{
PrivateKey: opt.PrivateKey,
IntegrationID: opt.IntegrationID,
Client: client,
// Do no set RepoOwner.
})
if err != nil {
return 0, fmt.Errorf("failed to retrieve installation ID: %v", err)
return 0, err
}
if !ok {
return 0, fmt.Errorf("installation ID not found for %s", opt.RepoOwner)
inst, _, err := appCli.Apps.FindUserInstallation(ctx, opt.RepoOwner)
if err != nil {
return 0, err
}
return inst.InstallationID, nil
return inst.GetID(), nil
}
Loading

0 comments on commit 1d6a3c8

Please sign in to comment.