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

RESOLVED WONTFIX

Status

--
major
RESOLVED WONTFIX
4 years ago
5 months ago

People

(Reporter: emorley, Unassigned)

Tracking

({treeherder})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

225.82 KB, application/x-javascript
Details
(Reporter)

Description

4 years ago
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).
(Reporter)

Comment 1

4 years ago
Created attachment 8603312 [details]
builds-pending.js

The bad entries were under the "try" section.
(Reporter)

Comment 2

4 years ago
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?
Flags: needinfo?(cmanchester)
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!
Flags: needinfo?(cmanchester)
(Reporter)

Comment 4

4 years ago
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? :-)
Flags: needinfo?(cmanchester)
Flags: needinfo?(armenzg)

Comment 5

4 years ago
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.
Flags: needinfo?(armenzg)
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.
Flags: needinfo?(cmanchester)
(Reporter)

Comment 7

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

Updated

3 years ago
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → WONTFIX
Summary: builds-pending.js has entries with switched revision and buildername → Prevent builds-pending.js from having entries with switched revision and buildername
(Assignee)

Updated

5 months ago
Component: General Automation → General
Product: Release Engineering → Release Engineering
You need to log in before you can comment on or make changes to this bug.