Merge lp:~dooferlad/linaro-android-frontend/proxy_lava-job-info into lp:linaro-android-frontend

Proposed by James Tunnicliffe
Status: Merged
Approved by: Paul Sokolovsky
Approved revision: 246
Merged at revision: 241
Proposed branch: lp:~dooferlad/linaro-android-frontend/proxy_lava-job-info
Merge into: lp:linaro-android-frontend
Diff against target: 124 lines (+74/-13)
3 files modified
android_build/frontend/api.py (+29/-0)
android_build/urls.py (+1/-0)
static/buildDetails.js (+44/-13)
To merge this branch: bzr merge lp:~dooferlad/linaro-android-frontend/proxy_lava-job-info
Reviewer Review Type Date Requested Status
Paul Sokolovsky Approve
Review via email: mp+87971@code.launchpad.net

Description of the change

Adds a proxy for lava-job-info files for the front end so results files will be loaded and parsed without being blocked by the license request web page.

django: Added proxy to the API. Checks to see if file is called lava-job-info so it can't just be used to proxy anything. Possibly should tie down a list of acceptable server names as well at some point.

JavaScript: Added a new URL to try to download lava-job-info from and push all requests through above proxy.

To post a comment you must log in.
Revision history for this message
James Tunnicliffe (dooferlad) wrote :

Updated: Rev 245 locks down the proxy so only snapshots.linaro.org will be proxied, and only lava-job-info files on it. Front end understands this and will only use the proxy to access snapshots.linaro.org.

245. By James Tunnicliffe

Locked down proxy so only snapshot.linaro.org is proxied. Front end will only use proxy for URLs with snapshots.linaro.org within them.

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

I don't understand all the new_style_url/old_style_url gyrations. In particular, it sure looks as if both could go via the proxy, in which case both will succeed at the http level (/api/get-lava-job-info always returns http 200) and then its a race as to which response gets displayed.

The other bits look fine.

246. By James Tunnicliffe

Removed unncessarey mangling of old_style_url.
Protect against having two calls to receivedLavaJobInfo that contain parsable data.

Revision history for this message
James Tunnicliffe (dooferlad) wrote :

On 9 January 2012 22:24, Michael Hudson-Doyle
<email address hidden> wrote:
> I don't understand all the new_style_url/old_style_url gyrations.  In
> particular, it sure looks as if both could go via the proxy, in which
> case both will succeed at the http level (/api/get-lava-job-info always
> returns http 200) and then its a race as to which response gets displayed.

I did have both go via the proxy at one point, but it does leave the
proxy open to anyone wanting to proxy a file that they can call
lava-job-info. It seemed like restricting the domain to be proxied to
snapshots.linaro.org was reasonable.

I have got rid of the code that makes old_style_url always absolute,
since it has gone back to a non-proxied Y.io call.

The reason for there being two URLs and there being no protection
about which gets loaded is that old_style_url will only match jobs
that have jenkins native archiving enabled and my understanding is
that those jobs will be going away shortly and don't account for any
jobs with artefacts published to snapshots.linaro.org.

It is simple enough to stop receivedLavaJobInfo from modifying the
page twice if you think it is worth protecting against. I have checked
in this change to do so:

    function receivedLavaJobInfo (e, response) {
        if (response.response.match(/^Server error: 404$/))
        {
            return; // Nothing to parse
        }

        if (typeof receivedLavaJobInfo.got_info != "undefined")
        {
            alert("Two resutls files found. Please report this error
by submitting a bug report against" +
                  "
https://code.launchpad.net/~linaro-infrastructure/linaro-android-frontend/trunk")
            return;
        }

        receivedLavaJobInfo.got_info = true;

I don't think there is a right location to get lava-job-info from if
it is in more than one location, so this should be sufficient. I added
an alert because it would be nice to know if getting two files ever
happens.

--
James Tunnicliffe

Revision history for this message
Paul Sokolovsky (pfalcon) wrote :

Ok, looks good for v1.0. I'm sure we'd need more tweaks to it, but let's proceed with integration so we know we do the right tweaks.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'android_build/frontend/api.py'
--- android_build/frontend/api.py 2011-11-17 22:53:32 +0000
+++ android_build/frontend/api.py 2012-01-10 11:42:24 +0000
@@ -254,3 +254,32 @@
254 if resp:254 if resp:
255 return resp255 return resp
256 return HttpResponse('foo')256 return HttpResponse('foo')
257
258def get_lava_job_info(request):
259 """Give Web UI a proxy to fetch lava-job-info for it to parse.
260
261 This is required because the android files are now protected by the need
262 for the user to have accepted a license. If a URL isn't for a file called
263 lava-job-info, we return a 404. We could lock down to just
264 snapshots.linaro.org, but that would break old builds unless we complicate
265 the UI javaScript.
266 """
267
268 url = ""
269 for thing in request.GET.getlist("url"):
270 url = thing
271
272 if not (re.search("/lava-job-info$", url) and
273 re.search("^https?://snapshots.linaro.org/", url)):
274 raise Http404
275
276 try:
277 response = urllib2.urlopen(url)
278 except urllib2.HTTPError, e:
279 message = "Server error: {}".format(e.code)
280 return HttpResponse(message)
281 except urllib2.URLError, e:
282 message = "We failed to reach a server. Reason{}".format(e.reason)
283 return HttpResponse(message)
284 else:
285 return HttpResponse(response.read())
257286
=== modified file 'android_build/urls.py'
--- android_build/urls.py 2011-03-21 02:19:57 +0000
+++ android_build/urls.py 2012-01-10 11:42:24 +0000
@@ -27,6 +27,7 @@
27 (r'^api/is-daily$', 'android_build.frontend.api.is_daily'),27 (r'^api/is-daily$', 'android_build.frontend.api.is_daily'),
28 (r'^api/new$', 'android_build.frontend.api.new'),28 (r'^api/new$', 'android_build.frontend.api.new'),
29 (r'^api/set-daily$', 'android_build.frontend.api.set_daily'),29 (r'^api/set-daily$', 'android_build.frontend.api.set_daily'),
30 (r'^api/get-lava-job-info$', 'android_build.frontend.api.get_lava_job_info'),
3031
31 (r'^openid/', include('django_openid_auth.urls')),32 (r'^openid/', include('django_openid_auth.urls')),
32 (r'^logout$', 'django.contrib.auth.views.logout'),33 (r'^logout$', 'django.contrib.auth.views.logout'),
3334
=== modified file 'static/buildDetails.js'
--- static/buildDetails.js 2011-12-16 16:33:24 +0000
+++ static/buildDetails.js 2012-01-10 11:42:24 +0000
@@ -95,6 +95,20 @@
95 }95 }
9696
97 function receivedLavaJobInfo (e, response) {97 function receivedLavaJobInfo (e, response) {
98 if (response.response.match(/^Server error: 404$/))
99 {
100 return; // Nothing to parse
101 }
102
103 if (typeof receivedLavaJobInfo.got_info != "undefined")
104 {
105 alert("Two resutls files found. Please report this error by submitting a bug report against" +
106 " https://code.launchpad.net/~linaro-infrastructure/linaro-android-frontend/trunk")
107 return;
108 }
109
110 receivedLavaJobInfo.got_info = true;
111
98 var job_info = Y.JSON.parse(response.responseText);112 var job_info = Y.JSON.parse(response.responseText);
99 var job_info_json_url = job_info.lava_url + 'scheduler/job/' + job_info.job_id;113 var job_info_json_url = job_info.lava_url + 'scheduler/job/' + job_info.job_id;
100 Y.jsonp(114 Y.jsonp(
@@ -144,20 +158,37 @@
144 + results.buildUrl + '/console">Tail</a> - <a href="' + results.buildUrl + '/consoleText">Raw</a>';158 + results.buildUrl + '/console">Tail</a> - <a href="' + results.buildUrl + '/consoleText">Raw</a>';
145 },159 },
146 testresults: function (results) {160 testresults: function (results) {
147 if (results.artifacts.length) {161
148 for (var i = 0; i < results.artifacts.length; i++) {162 /* The build result can exist in one of two locations depending
149 var artifact = results.artifacts[i];163 on how old the test result is. We create both URLs and fire
150 if (artifact.displayPath == 'lava-job-info') {164 off fetches to both locations. The one that works will be
151 Y.io(165 used to display the results.
152 results.buildUrl + '/artifact/' + artifact.relativePath,166 */
153 {167 var old_style_url = results.buildUrl + '/artifact/build/out/lava-job-info';
154 on: { success: receivedLavaJobInfo }168
155 });169 /* Check URL. If it points to snapshot.linaro.org, proxy it through
156 return 'Loading...';170 the master, which doesn't get stopped to ask to accept a license. */
157 }171 if (old_style_url.match(/https:\/\/snapshots.linaro.org\//))
158 }172 {
173 old_style_url = '/api/get-lava-job-info?url=' + old_style_url
159 }174 }
160 return 'none found';175
176 var new_style_url = '/api/get-lava-job-info?url=http://snapshots.linaro.org/android/'
177 + rawBuildName + '/' + results.number.toString() + '/lava-job-info';
178
179 // Try both locations. File should exist only in one location...
180 Y.io(
181 old_style_url,
182 {
183 on: { success: receivedLavaJobInfo }
184 });
185 Y.io(
186 new_style_url,
187 {
188 on: { success: receivedLavaJobInfo }
189 });
190
191 return 'Loading...';
161 },192 },
162 output: function (results) {193 output: function (results) {
163 var downloadAreaUrl = 'http://snapshots.linaro.org/android/' + rawBuildName + '/' + results.number.toString() + '/';194 var downloadAreaUrl = 'http://snapshots.linaro.org/android/' + rawBuildName + '/' + results.number.toString() + '/';

Subscribers

People subscribed via source and target branches