Milo: Internal buildbot masters support

milo/appengine/buildbot/build.go

 // Copyright 2016 The LUCI Authors. All rights reserved.
 // Use of this source code is governed under the Apache License, Version 2.0
 // that can be found in the LICENSE file.
 
 package buildbot
 
 import (
         "encoding/json"
         "fmt"
         "io/ioutil"
         "net/http"
-        "os"
         "path/filepath"
         "regexp"
         "sort"
         "strconv"
         "strings"
         "time"
 
         "github.com/luci/gae/service/datastore"
-        "github.com/luci/gae/service/memcache"
-        log "github.com/luci/luci-go/common/logging"
+        "github.com/luci/luci-go/common/logging"
         "github.com/luci/luci-go/milo/api/resp"
-        "github.com/luci/luci-go/server/auth"
+        "github.com/luci/luci-go/milo/appengine/settings"
+        "github.com/luci/luci-go/milo/common/miloerror"
         "golang.org/x/net/context"
 )
 
-func getURL(c context.Context, URL string) ([]byte, error) {
-        fmt.Fprintf(os.Stderr, "Fetching %s\n", URL)
-        tr, err := auth.GetRPCTransport(c, auth.NoAuth)
-        if err != nil {
-                return nil, err
-        }
-        client := http.Client{Transport: tr}
-        resp, err := client.Get(URL)
-        if err != nil {
-                return nil, err
-        }
-        if resp.StatusCode != 200 {
-                return nil, fmt.Errorf("Failed to fetch %s, status code %d", URL, resp.StatusCode)
-        }
-        defer resp.Body.Close()
-        return ioutil.ReadAll(resp.Body)
+var errBuildNotFound = miloerror.Error{
+        Message: "Build not found",
+        Code:    http.StatusNotFound,
 }
 
-// getBuild fetches a build from BuildBot.
+// getBuild fetches a buildbot build from the datastore and checks ACLs.
 func getBuild(c context.Context, master, builder, buildNum string) (*buildbotBuild, error) {
         result := &buildbotBuild{
                 Master:      master,
                 Buildername: builder,
         }
         if num, err := strconv.Atoi(buildNum); err != nil {
-                panic(fmt.Errorf("%s does not look like a number", buildNum))
+                return nil, miloerror.Error{
+                        Message: fmt.Sprintf("%s does not look like a number", buildNum),
+                        Code:    http.StatusBadRequest,
+                }
         } else {
                 result.Number = num
         }
-        // Check memcache first.
-        mc := memcache.Get(c)
-        if item, err := mc.Get(result.getID()); err == nil {
-                log.Debugf(c, "Found in Memcache!")
-                json.Unmarshal(item.Value(), result)
-                return result, nil
-        }
-        // Then check local datastore.
         ds := datastore.Get(c)
         err := ds.Get(result)
-        if err == nil {
-                log.Debugf(c, "Found build in local cache!")
-                if result.Times[1] != nil {
-                        bs, ierr := json.Marshal(result)
-                        if ierr == nil {
-                                item := mc.NewItem(result.getID()).SetValue(bs)
-                                mc.Set(item)
-                        }
-                }
-                return result, nil
-        }
-        log.Debugf(c, "Could not find log in local cache: %s", err.Error())
-        // Now check CBE.
-        cbeURL := fmt.Sprintf(
-                "https://chrome-build-extract.appspot.com/p/%s/builders/%s/builds/%s?json=1",
-                master, builder, buildNum)
-        if buf, err := getURL(c, cbeURL); err == nil {
-                return result, json.Unmarshal(buf, result)
-        }
-        // TODO(hinoka): Cache finished builds.
-        url := fmt.Sprintf(
-                "https://build.chromium.org/p/%s/json/builders/%s/builds/%s", master, builder, buildNum)
-        buf, err := getURL(c, url)
-        if err != nil {
+        switch err {

[Vadim Sh., 2016/08/23 22:35:46]

nit: I usually do

switch {
  case err == datastore.ErrNo....:
    ...
  case err != nil:
    return nil, err
}
// success 

it's simpler

[Ryan Tseng, 2016/08/24 21:07:24]

Done.

+        case nil:
+        case datastore.ErrNoSuchEntity:
+                return nil, errBuildNotFound
+        default:
                 return nil, err
         }
+        if result.Internal {
+                allowed, err := settings.IsAllowedInternal(c)
+                if err != nil {
+                        return nil, err
+                }
+                if !allowed {
+                        return nil, errBuildNotFound
+                }
+        }
 
-        return result, json.Unmarshal(buf, result)
+        return result, nil
 }
 
 // result2Status translates a buildbot result integer into a resp.Status.
 func result2Status(s *int) (status resp.Status) {
         if s == nil {
                 return resp.Running
         }
         switch *s {
         case 0:
                 status = resp.Success
@@ skipping 40 matching lines @@
 // getBanner fetches the banner information about the bot that the build
 // ran on.  This is best effort and:
 // * Will return an empty list of banners if the master is not found.
 // * May return incorrect data if the platform of the slave of the same name
 //   was changed recently, and this build was older than before the slave
 //   changed platforms.
 func getBanner(c context.Context, b *buildbotBuild, m *buildbotMaster) *resp.LogoBanner {
         logos := &resp.LogoBanner{}
         // Fetch the master info from datastore if not provided.
         if m == nil {
-                m1, i, _, err := getMasterJSON(c, b.Master)
+                m1, _, _, err := getMasterJSON(c, b.Master)
                 m = m1
-                if i {
-                        // TODO(hinoka): Support internal masters.
-                        return nil
-                }
                 if err != nil {
-                        log.Warningf(c, "Failed to fetch master information for banners on master %s", b.Master)
+                        logging.Warningf(c, "Failed to fetch master information for banners on master %s", b.Master)
                         return nil
                 }
         }
 
         s, ok := m.Slaves[b.Slave]
         if !ok {
-                log.Warningf(c, "Could not find slave %s in master %s", b.Slave, b.Master)
+                logging.Warningf(c, "Could not find slave %s in master %s", b.Slave, b.Master)
                 return nil
         }
         hostInfo := map[string]string{}
         for _, v := range strings.Split(s.Host, "\n") {
                 if info := strings.SplitN(v, ":", 2); len(info) == 2 {
                         hostInfo[info[0]] = strings.TrimSpace(info[1])
                 }
         }
 
         // Extract OS and OS Family
@@ skipping 15 matching lines @@
                         result.LogoBase = resp.Ubuntu
                 default:
                         return nil
                 }
                 result.Subtitle = version
                 return result
         }()
         if osLogo != nil {
                 logos.OS = append(logos.OS, *osLogo)
         } else {
-                log.Warningf(c, "No OS info found.  Host: %s", s.Host)
+                logging.Warningf(c, "No OS info found.  Host: %s", s.Host)
         }
-        log.Infof(c, "OS: %s/%s", os, version)
+        logging.Infof(c, "OS: %s/%s", os, version)
         return logos
 }
 
 // summary Extract the top level summary from a buildbot build as a
 // BuildComponent
 func summary(c context.Context, b *buildbotBuild) resp.BuildComponent {
         // TODO(hinoka): use b.toStatus()
         // Status
         var status resp.Status
         if b.Currentstep != nil {
@@ skipping 65 matching lines @@
                         bc.Status = resp.Running
                 } else {
                         if len(step.Results) > 0 {
                                 status := int(step.Results[0].(float64))
                                 bc.Status = result2Status(&status)
                         } else {
                                 bc.Status = resp.Success
                         }
                 }
 
-                // Now, link to the logs.
-                for _, log := range step.Logs {
+                // Now, link to the loggings.
+                for _, l := range step.Logs {
                         link := &resp.Link{
-                                Label: log[0],
-                                URL:   log[1],
+                                Label: l[0],
+                                URL:   l[1],
                         }
                         if link.Label == "stdio" {
                                 bc.MainLink = link
                         } else {
                                 bc.SubLink = append(bc.SubLink, link)
                         }
                 }
 
                 // Figure out the times
                 bc.Started, bc.Finished, bc.Duration = parseTimes(step.Times)
@@ skipping 134 matching lines @@
         issue := int64(-1)
         // TODO(hinoka): Gerrit URLs.
         for _, prop := range b.Properties {
                 key := prop[0].(string)
                 value := prop[1]
                 switch key {
                 case "rietveld":
                         if v, ok := value.(string); ok {
                                 rietveld = v
                         } else {
-                                log.Warningf(c, "Field rietveld is not a string: %#v", value)
+                                logging.Warningf(c, "Field rietveld is not a string: %#v", value)
                         }
                 case "issue":
                         if v, ok := value.(float64); ok {
                                 issue = int64(v)
                         } else {
-                                log.Warningf(c, "Field issue is not a float: %#v", value)
+                                logging.Warningf(c, "Field issue is not a float: %#v", value)
                         }
 
                 case "got_revision":
                         if v, ok := value.(string); ok {
                                 ss.Revision = v
                         } else {
-                                log.Warningf(c, "Field got_revision is not a string: %#v", value)
+                                logging.Warningf(c, "Field got_revision is not a string: %#v", value)
                         }
 
                 }
         }
         if issue != -1 {
                 if rietveld != "" {
                         rietveld = strings.TrimRight(rietveld, "/")
                         ss.Changelist = &resp.Link{
                                 Label: fmt.Sprintf("Issue %d", issue),
                                 URL:   fmt.Sprintf("%s/%d", rietveld, issue),
                         }
                 } else {
-                        log.Warningf(c, "Found issue but not rietveld property.")
+                        logging.Warningf(c, "Found issue but not rietveld property.")
                 }
         }
         return ss
 }
 
 func getDebugBuild(c context.Context, builder, buildNum string) (*buildbotBuild, error) {
         fname := fmt.Sprintf("%s.%s.json", builder, buildNum)
         // ../buildbot below assumes that
         // - this code is not executed by tests outside of this dir
         // - this dir is a sibling of frontend dir
@@ skipping 21 matching lines @@
 
         // TODO(hinoka): Do all fields concurrently.
         return &resp.MiloBuild{
                 SourceStamp:   sourcestamp(c, b),
                 Summary:       summary(c, b),
                 Components:    components(b),
                 PropertyGroup: properties(b),
                 Blame:         blame(b),
         }, nil
 }