Milo: ACLs for internal swarm jobs

Milo: ACLs for internal swarm jobs

This uses the luci_project:<project name> tag for ACLing swarming builds.

BUG=697640
Review-Url: https://codereview.chromium.org/2818563002
Committed: https://github.com/luci/luci-go/commit/dd5690205f17fb9b744d1466368093e9c07efef6

[Ryan Tseng, 2017-04-12 20:39:14]

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

[commit-bot: I haz the power, 2017-04-12 20:39:21]

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

[hinoka, 2017-04-12 20:40:16]

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

[hinoka, 2017-04-12 20:40:17]

ptal, this was a request by touch-swarming (new client???)

All of the logic is in swarming/build.go, everything else is test related.

[nodir, 2017-04-12 20:45:03]

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

https://codereview.chromium.org/2818563002/diff/1/milo/appengine/swarming/bui...
milo/appengine/swarming/build.go:670: // 1. allow_milo:1 is present.  If so,
it's a public job.
why do we need this if we check access? all public projects have "access: all"

https://codereview.chromium.org/2818563002/diff/1/milo/appengine/swarming/bui...
milo/appengine/swarming/build.go:679: if strings.HasPrefix(t, "luci_project") {
"luci_project:"

https://codereview.chromium.org/2818563002/diff/1/milo/appengine/swarming/bui...
milo/appengine/swarming/build.go:685: // This is the project name.
project id

https://codereview.chromium.org/2818563002/diff/1/milo/appengine/swarming/bui...
File milo/appengine/swarming/buildinfo.go (right):

https://codereview.chromium.org/2818563002/diff/1/milo/appengine/swarming/bui...
milo/appengine/swarming/buildinfo.go:68: logging.Warningf(c, "User requested
nonexistant task or does not have permissions.")
nonexistent

[nodir, 2017-04-12 20:48:32]

how will we decide whether a user is allowed to cancel/retry a build?

[commit-bot: I haz the power, 2017-04-12 20:51:11]

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

[commit-bot: I haz the power, 2017-04-12 20:51:12]

Dry run: This issue passed the CQ dry run.

[Ryan Tseng, 2017-04-12 22:33:22]

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

[commit-bot: I haz the power, 2017-04-12 22:33:27]

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

[hinoka, 2017-04-12 22:34:24]

Users being allowed to retry/cancel a build should be done on the buildbucket
level.  IE the action should be either a direct POST to buildbucket, or via
delegation tokens to buildbucket.

Correct me if I'm wrong but there should be no use for native swarming retries
in the luci work.

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

https://codereview.chromium.org/2818563002/diff/1/milo/appengine/swarming/bui...
milo/appengine/swarming/build.go:670: // 1. allow_milo:1 is present.  If so,
it's a public job.
On 2017/04/12 20:45:03, nodir wrote:
> why do we need this if we check access? all public projects have "access: all"

I guess we don't really, I just elected to keep this for now "just incase, for
compatibility".  Could remove it either in this CL or another CL.

https://codereview.chromium.org/2818563002/diff/1/milo/appengine/swarming/bui...
milo/appengine/swarming/build.go:679: if strings.HasPrefix(t, "luci_project") {
On 2017/04/12 20:45:03, nodir wrote:
> "luci_project:"

Done.

https://codereview.chromium.org/2818563002/diff/1/milo/appengine/swarming/bui...
milo/appengine/swarming/build.go:685: // This is the project name.
On 2017/04/12 20:45:03, nodir wrote:
> project id

Done.

https://codereview.chromium.org/2818563002/diff/1/milo/appengine/swarming/bui...
File milo/appengine/swarming/buildinfo.go (right):

https://codereview.chromium.org/2818563002/diff/1/milo/appengine/swarming/bui...
milo/appengine/swarming/buildinfo.go:68: logging.Warningf(c, "User requested
nonexistant task or does not have permissions.")
On 2017/04/12 20:45:03, nodir wrote:
> nonexistent

Done.

[nodir, 2017-04-12 22:38:29]

OK, let's use existing buildbucket ACLs for retries and cancellations. And yes,
we will do retries and cancellations using buildbucket api, not swarming api

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

https://codereview.chromium.org/2818563002/diff/1/milo/appengine/swarming/bui...
milo/appengine/swarming/build.go:670: // 1. allow_milo:1 is present.  If so,
it's a public job.
On 2017/04/12 22:34:24, hinoka wrote:
> On 2017/04/12 20:45:03, nodir wrote:
> > why do we need this if we check access? all public projects have "access:
all"
> 
> I guess we don't really, I just elected to keep this for now "just incase, for
> compatibility".  Could remove it either in this CL or another CL.

ok, sg

[nodir, 2017-04-12 22:38:37]

lgtm

[commit-bot: I haz the power, 2017-04-12 22:39:28]

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

[commit-bot: I haz the power, 2017-04-12 22:39:29]

Dry run: This issue passed the CQ dry run.

[hinoka, 2017-04-14 00:08:14]

The CQ bit was checked by hinoka@chromium.org

[commit-bot: I haz the power, 2017-04-14 00:08:25]

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-04-14 00:14:17]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1492128494973310,
"parent_rev": "0a0dba06054fdac1c08b08676414f6cf1188c8a8", "commit_rev":
"dd5690205f17fb9b744d1466368093e9c07efef6"}

[commit-bot: I haz the power, 2017-04-14 00:14:24]

Description was changed from

==========
Milo: ACLs for internal swarm jobs

This uses the luci_project:<project name> tag for ACLing swarming builds.

BUG=697640
==========

to

==========
Milo: ACLs for internal swarm jobs

This uses the luci_project:<project name> tag for ACLing swarming builds.

BUG=697640

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

[commit-bot: I haz the power, 2017-04-14 00:14:25]

Committed patchset #2 (id:20001) as
https://github.com/luci/luci-go/commit/dd5690205f17fb9b744d1466368093e9c07efef6