1162928 - Prevent builds-pending.js from having entries with switched revision and buildername

Status: RESOLVED WONTFIX

(Release Engineering :: General, defect)

Description

[Ed Morley [:emorley]]

Treeherder has been seeing instances of switched revision and buildername properties in builds-pending.js, eg:

      "Ubuntu VM 12": [{
        "submitted_at": 1431066225,
        "id": 69163063,
        "buildername": "afb4dfdccfc1"
      },
      {
        "submitted_at": 1431066226,
        "id": 69163064,
        "buildername": "afb4dfdccfc1"
      }],

(Where the revision would be expected as the initial key, not "Ubuntu VM 12")

Needless to say this catches Treeherder out a little, particularly since the "revision" is truncated to the same length as the normal SHA, so "looks" fine. (I guess we'll need to start matching against a regex).

Comment 1

[Ed Morley [:emorley]]

Attachment: builds-pending.js

The bad entries were under the "try" section.

Comment 2

[Ed Morley [:emorley]]

Ah here it is on self-serve:
https://secure.pub.build.mozilla.org/buildapi/self-serve/try/request/69163063

{
  "build_ids": [],
  "complete": 0,
  "buildername": "afb4dfdccfc1",
  "claimed_at": 0,
  "reason": "Self-serve: Requested by cmanchester@<snip>.com",
  "properties": {
    "buildid": "20150507232345",
    "builduid": "261e2e6e816e4e9b9297bbf71eeb1c07"
  },
  "complete_at": null,
  "priority": 0,
  "submittime": 1431066225,
  "branch": "try",
  "request_id": 69163063,
  "revision": "Ubuntu VM 12.04 try opt test xpcshell"
}

Chris, was there anything unusual about this job? Were you using mozci or something?

Comment 3

[Chris Manchester (limited bugmail, email directly)]

Right - I was testing a selfserve based tool last night and this would be a consequence of a bug I found in it. Sorry if this caused any trouble for you guys!

Comment 4

[Ed Morley [:emorley]]

That's ok - Treeherder just tries to fetch revisions it doesn't know about and so it caused a number of requests to the json pushlog, but nothing too serious.

Is there a way we can prevent against this kind of thing happening? Or more importantly, prevent the result of it ending up in builds-pending.js? :-)

Comment 5

[Armen [:armenzg]]

chmanchester: I'm actually curious as how you did it :)
In few places we check for the validity of a buildername.
We should probably add it within _make_request().
If that is how it happened we will move the validation just before a request.

Comment 6

[Chris Manchester (limited bugmail, email directly)]

I fixed the bug and will take reasonable measures to prevent it regressing, but this api is available to anyone with ldap credentials, so a more centralized validation might make sense. 

I caused this by invoking /self-serve/{branch}/builders/{builder_name}/{revision} with builder_name and revision reversed. I wasn't using mozci at the time.

Comment 7

[Ed Morley [:emorley]]

Even just validating the revision against something like [a-z0-9]{12,40} would really help consumers of builds-pending.js :-)