[milo] better ACL system for masters.

milo/buildsource/buildbot/pubsub.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 (
         "bytes"
         "compress/gzip"
         "compress/zlib"
@@ skipping 62 matching lines @@
         // Name of the buildbot master.
         Name string `gae:"$id"`
         // Internal
         Internal bool
         // Data is the json serialzed and gzipped blob of the master data.
         Data []byte `gae:",noindex"`
         // Modified is when this entry was last modified.
         Modified time.Time
 }
 
+// buildbotMasterPublic is a struct that exists for public builtbot masters, and
+// not for internal masters. It's used for ACL checks.
+type buildbotMasterPublic struct {

[dnj, 2017/07/11 18:46:34]

Is there a potential problem with already-ingested builds becoming inaccessible
to the public?

[iannucci, 2017/07/11 21:01:39]

We'll deploy the pubsub module first and make sure all the public masters are
tagged first.

+        Name string `gae:"$id"`
+}
+
 func putDSMasterJSON(
         c context.Context, master *buildbotMaster, internal bool) error {
         for _, builder := range master.Builders {
                 // Trim out extra info in the "Changes" portion of the pending build state,
                 // we don't actually need comments, files, and properties
                 for _, pbs := range builder.PendingBuildStates {
                         for i := range pbs.Source.Changes {
                                 pbs.Source.Changes[i].Comments = ""
                                 pbs.Source.Changes[i].Files = nil
                                 pbs.Source.Changes[i].Properties = nil
                         }
                 }
         }
         entry := buildbotMasterEntry{
                 Name:     master.Name,
                 Internal: internal,
                 Modified: clock.Now(c).UTC(),
         }
+        toPut := []interface{}{&entry}

[Ryan Tseng, 2017/07/11 18:37:16]

why not []*entry{}?

[dnj, 2017/07/11 18:46:34]

b/c this contains both a "buildbotMasterEntry" and potentially a
"buildbotMasterPublic".

[iannucci, 2017/07/11 21:01:39]

yep

+        if internal {
+                // do the deletion immediately so that the 'public' bit is removed from
+                // datastore before any internal details are actually written to datastore.
+                if err := ds.Delete(c, &buildbotMasterPublic{master.Name}); err != nil && err != ds.ErrNoSuchEntity {

[dnj, 2017/07/11 18:46:34]

nit: declare the public variable outside of this conditional so we only have to
specify it once.

[iannucci, 2017/07/11 21:01:39]

Done.

+                        return err
+                }
+        } else {
+                toPut = append(toPut, &buildbotMasterPublic{master.Name})
+        }
         gzbs := bytes.Buffer{}
         gsw := gzip.NewWriter(&gzbs)
         cw := iotools.CountingWriter{Writer: gsw}
         e := json.NewEncoder(&cw)
         if err := e.Encode(master); err != nil {
                 return err
         }
         gsw.Close()
         entry.Data = gzbs.Bytes()
         logging.Debugf(c, "Length of json data: %d", cw.Count)
         logging.Debugf(c, "Length of gzipped data: %d", len(entry.Data))
-        return ds.Put(c, &entry)
+        return ds.Put(c, toPut)
 }
 
 // GetData returns the expanded form of Data (decoded from base64).
 func (m *pubSubSubscription) GetData() ([]byte, error) {
         return base64.StdEncoding.DecodeString(m.Message.Data)
 }
 
 // unmarshal a gzipped byte stream into a list of buildbot builds and masters.
 func unmarshal(
         c context.Context, msg []byte) ([]*buildbotBuild, *buildbotMaster, error) {
@@ skipping 275 matching lines @@
 
         }
         if master != nil {
                 code := doMaster(c, master, internal)
                 if code != 0 {
                         return code
                 }
         }
         return http.StatusOK
 }