Milo: Move instance configuration to luci-config

Milo: Move instance configuration to luci-config

This moves:
* Buildbot Pubsub configs
* Buildbucket host pointer
* Swarming host pointer and whitelist

To be configured in luci-config

And some minor renaming for consistency.

BUG=700055
Review-Url: https://codereview.chromium.org/2765383002
Committed: https://github.com/luci/luci-go/commit/be60414ac83c2d9586377814cb594dff08e1ec11

[Ryan Tseng, 2017-03-22 19:06:19]

The CQ bit was checked by hinoka@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-22 19:06:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hinoka, 2017-03-22 19:07:45]

Description was changed from

==========
Milo: Move instance configuration to luci-cfg

This moves:
* Buildbot Pubsub configs
* Buildbucket host pointer
* Swarming host pointer and whitelist

To be configured in luci-config

BUG=700055
==========

to

==========
Milo: Move instance configuration to luci-cfg

This moves:
* Buildbot Pubsub configs
* Buildbucket host pointer
* Swarming host pointer and whitelist

To be configured in luci-config

And some minor renaming for consistency.

BUG=700055
==========

[hinoka, 2017-03-22 19:07:45]

hinoka@chromium.org changed reviewers:
+ nodir@chromium.org

[hinoka, 2017-03-22 19:07:51]

hinoka@chromium.org changed reviewers:
+ hinoka@chromium.org

[hinoka, 2017-03-22 19:07:51]

ptal

[commit-bot: I haz the power, 2017-03-22 19:12:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-22 19:12:39]

Dry run: This issue passed the CQ dry run.

[nodir, 2017-03-24 08:38:04]

Description was changed from

==========
Milo: Move instance configuration to luci-cfg

This moves:
* Buildbot Pubsub configs
* Buildbucket host pointer
* Swarming host pointer and whitelist

To be configured in luci-config

And some minor renaming for consistency.

BUG=700055
==========

to

==========
Milo: Move instance configuration to luci-config

This moves:
* Buildbot Pubsub configs
* Buildbucket host pointer
* Swarming host pointer and whitelist

To be configured in luci-config

And some minor renaming for consistency.

BUG=700055
==========

[nodir, 2017-03-24 08:41:15]

https://codereview.chromium.org/2765383002/diff/1/milo/appengine/buildbot/bui...
File milo/appengine/buildbot/build.go (right):

https://codereview.chromium.org/2765383002/diff/1/milo/appengine/buildbot/bui...
milo/appengine/buildbot/build.go:116: // summary Extract the top level summary
from a buildbot build as a
extracts

https://codereview.chromium.org/2765383002/diff/1/milo/appengine/buildbot/pub...
File milo/appengine/buildbot/pubsub.go (right):

https://codereview.chromium.org/2765383002/diff/1/milo/appengine/buildbot/pub...
milo/appengine/buildbot/pubsub.go:313: return 500
please use http.Status* contants
In unchanged code too

https://codereview.chromium.org/2765383002/diff/1/milo/common/config/settings...
File milo/common/config/settings.proto (right):

https://codereview.chromium.org/2765383002/diff/1/milo/common/config/settings...
milo/common/config/settings.proto:40: // allowed_hosts is a list of whitelist
hostnames of swarming instances
is a whitelist of hostnames

https://codereview.chromium.org/2765383002/diff/1/milo/common/config/settings...
milo/common/config/settings.proto:42: // reasons, because Milo will hand out its
oauth2 token to a swarming host.
We also do that for buildbucket. Should we have a whitelist of buildbucket
hostnames?

[Ryan Tseng, 2017-03-28 17:47:17]

The CQ bit was checked by hinoka@google.com to run a CQ dry run

[hinoka, 2017-03-28 17:47:21]

https://codereview.chromium.org/2765383002/diff/1/milo/appengine/buildbot/bui...
File milo/appengine/buildbot/build.go (right):

https://codereview.chromium.org/2765383002/diff/1/milo/appengine/buildbot/bui...
milo/appengine/buildbot/build.go:116: // summary Extract the top level summary
from a buildbot build as a
On 2017/03/24 08:41:15, nodir wrote:
> extracts

Done.

https://codereview.chromium.org/2765383002/diff/1/milo/appengine/buildbot/pub...
File milo/appengine/buildbot/pubsub.go (right):

https://codereview.chromium.org/2765383002/diff/1/milo/appengine/buildbot/pub...
milo/appengine/buildbot/pubsub.go:313: return 500
On 2017/03/24 08:41:15, nodir wrote:
> please use http.Status* contants
> In unchanged code too

Done.

https://codereview.chromium.org/2765383002/diff/1/milo/common/config/settings...
File milo/common/config/settings.proto (right):

https://codereview.chromium.org/2765383002/diff/1/milo/common/config/settings...
milo/common/config/settings.proto:40: // allowed_hosts is a list of whitelist
hostnames of swarming instances
On 2017/03/24 08:41:15, nodir wrote:
> is a whitelist of hostnames 

lol oops

https://codereview.chromium.org/2765383002/diff/1/milo/common/config/settings...
milo/common/config/settings.proto:42: // reasons, because Milo will hand out its
oauth2 token to a swarming host.
On 2017/03/24 08:41:15, nodir wrote:
> We also do that for buildbucket. Should we have a whitelist of buildbucket
> hostnames?

For buildbucket I settled on a 1:1 mapping instead of a whitelist.  wdyt?

[commit-bot: I haz the power, 2017-03-28 17:47:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-28 17:53:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-28 17:53:16]

Dry run: This issue passed the CQ dry run.

[nodir, 2017-03-28 18:25:49]

https://codereview.chromium.org/2765383002/diff/1/milo/common/config/settings...
File milo/common/config/settings.proto (right):

https://codereview.chromium.org/2765383002/diff/1/milo/common/config/settings...
milo/common/config/settings.proto:42: // reasons, because Milo will hand out its
oauth2 token to a swarming host.
On 2017/03/28 17:47:21, hinoka wrote:
> On 2017/03/24 08:41:15, nodir wrote:
> > We also do that for buildbucket. Should we have a whitelist of buildbucket
> > hostnames?
> 
> For buildbucket I settled on a 1:1 mapping instead of a whitelist.  wdyt?

It sounds good, but in the latest patchset the buildbucket builder handler still
accepts "server" query string parameter. What you say here sounds like we should
not allow users to specify that because there can be only one buildbucket
hostname

https://codereview.chromium.org/2765383002/diff/20001/milo/appengine/buildbuc...
File milo/appengine/buildbucket/html.go (right):

https://codereview.chromium.org/2765383002/diff/20001/milo/appengine/buildbuc...
milo/appengine/buildbucket/html.go:28: logging.WithError(err).Errorf(c, "failed
to get settings")
if err != nil, we didn't fail to get settings. This may be misleading

[Ryan Tseng, 2017-03-28 19:19:22]

The CQ bit was checked by hinoka@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-28 19:19:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hinoka, 2017-03-28 19:21:02]

https://codereview.chromium.org/2765383002/diff/1/milo/common/config/settings...
File milo/common/config/settings.proto (right):

https://codereview.chromium.org/2765383002/diff/1/milo/common/config/settings...
milo/common/config/settings.proto:42: // reasons, because Milo will hand out its
oauth2 token to a swarming host.
On 2017/03/28 18:25:49, nodir wrote:
> On 2017/03/28 17:47:21, hinoka wrote:
> > On 2017/03/24 08:41:15, nodir wrote:
> > > We also do that for buildbucket. Should we have a whitelist of buildbucket
> > > hostnames?
> > 
> > For buildbucket I settled on a 1:1 mapping instead of a whitelist.  wdyt?
> 
> It sounds good, but in the latest patchset the buildbucket builder handler
still
> accepts "server" query string parameter. What you say here sounds like we
should
> not allow users to specify that because there can be only one buildbucket
> hostname

You're right, that was a mistake.  It's been fixed.

https://codereview.chromium.org/2765383002/diff/20001/milo/appengine/buildbuc...
File milo/appengine/buildbucket/html.go (right):

https://codereview.chromium.org/2765383002/diff/20001/milo/appengine/buildbuc...
milo/appengine/buildbucket/html.go:28: logging.WithError(err).Errorf(c, "failed
to get settings")
On 2017/03/28 18:25:49, nodir wrote:
> if err != nil, we didn't fail to get settings. This may be misleading

We did tho

But anyways, this is split into two separate errors now.

[commit-bot: I haz the power, 2017-03-28 19:27:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-28 19:27:02]

Dry run: This issue passed the CQ dry run.

[hinoka, 2017-03-28 22:14:42]

ptal, want to land this today.

[nodir, 2017-03-28 22:22:33]

lgtm

[hinoka, 2017-03-28 22:23:08]

The CQ bit was checked by hinoka@chromium.org

[commit-bot: I haz the power, 2017-03-28 22:23:16]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-28 22:29:29]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490739788603930,
"parent_rev": "f842e4483134ce898e50d3387babae715ff87b24", "commit_rev":
"be60414ac83c2d9586377814cb594dff08e1ec11"}

[commit-bot: I haz the power, 2017-03-28 22:29:33]

Description was changed from

==========
Milo: Move instance configuration to luci-config

This moves:
* Buildbot Pubsub configs
* Buildbucket host pointer
* Swarming host pointer and whitelist

To be configured in luci-config

And some minor renaming for consistency.

BUG=700055
==========

to

==========
Milo: Move instance configuration to luci-config

This moves:
* Buildbot Pubsub configs
* Buildbucket host pointer
* Swarming host pointer and whitelist

To be configured in luci-config

And some minor renaming for consistency.

BUG=700055

Review-Url: https://codereview.chromium.org/2765383002
Committed:
https://github.com/luci/luci-go/commit/be60414ac83c2d9586377814cb594dff08e1ec11
==========

[commit-bot: I haz the power, 2017-03-28 22:29:34]

Committed patchset #3 (id:40001) as
https://github.com/luci/luci-go/commit/be60414ac83c2d9586377814cb594dff08e1ec11