[milo] better ACL system for masters.

milo/buildsource/buildbot/pubsub.go

Index: milo/buildsource/buildbot/pubsub.go
diff --git a/milo/buildsource/buildbot/pubsub.go b/milo/buildsource/buildbot/pubsub.go
index 15814813e7e37d706a503cf05c35f82e5b82eff0..105115999c44cd8f0a9fbedb8c269a481470224c 100644
--- a/milo/buildsource/buildbot/pubsub.go
+++ b/milo/buildsource/buildbot/pubsub.go
@@ -80,6 +80,12 @@ type buildbotMasterEntry struct {
 	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 {
@@ -98,6 +104,16 @@ func putDSMasterJSON(
 		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}
@@ -109,7 +125,7 @@ func putDSMasterJSON(
 	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).