Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make buildbot log files available via CORS #354

Open
jdm opened this issue May 3, 2016 · 3 comments
Open

Make buildbot log files available via CORS #354

jdm opened this issue May 3, 2016 · 3 comments

Comments

@jdm
Copy link
Member

jdm commented May 3, 2016

I'm trying to add XHR support to http://hoppipolla.co.uk/410/reftest-analyser-structured.xhtml so we could make investigating reftest failures really easy. Unfortunately attempts to grab URLs like http://build.servo.org/builders/mac-rel-css/builds/711/steps/test/logs/test-css.log/text via XHR end up with CORS errors like:

Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at http://build.servo.org/builders/mac-rel-css/builds/711/steps/test/logs/test-css.log/text. (Reason: CORS header 'Access-Control-Allow-Origin' missing).
@jdm
Copy link
Member Author

jdm commented May 3, 2016

Using crossorigin.me doesn't work, presumably because it has a size limit that is smaller than our 150mb logs :)

@metajack
Copy link
Contributor

metajack commented May 3, 2016

So it should be really easy to add the Allow-Origin headers on the builder. If not in buildbot, then at least nginx should be able to inject them during its proxy handling.

cc @edunham

@aneeshusa
Copy link
Contributor

I don't think Buildbot 8 has this feature (but it seems 9 does), so +1 for doing this through Nginx.

bors-servo pushed a commit that referenced this issue Sep 29, 2016
Use Nginx to make Buildbot logs available via CORS

Buildbot itself (neither the current version eight nor the upcoming
verison nine) does not seem to have built-in CORS support, so use Nginx
to supply CORS headers while reverse proxying.

This is only needed to download log files, so start with a very
conservative set of CORS headers. In particular:
  - Only support basic CORS requests (those that don't require preflight
    requests via OPTIONS). This means supporting only GET requests.
  - Do not allow sending cookies or exposing any headers.
In return, whitelist all domains for CORS, and send the Allow-Origin
header unconditionally, as the actual value of the Origin header does
not matter.

Because we are using proxy_pass to reverse proxy Buildbot, we need to
tell Nginx to always add the CORS headers, instead of only adding them
for successful responses. (I.e. add CORS headers even for 404s and other
erroneous status codes). This features is not available in the Nginx
packaged for Ubuntu Trusty, so replace it with a newer Nginx from
upstream Nginx's respositories.

Because Debian/Ubuntu modify the default configuration for Nginx, use
Salt to ensure that the installed nginx.conf properly looks into the
sites-enabled directory (which is a Debian-ism) and that the default
handler in /etc/nginx/conf.d/ is purged.

In order to only support CORS on GET requests, use the 'if' directive.
However, because Nginx's if is evil,
(https://www.nginx.com/resources/wiki/start/topics/depth/ifisevil/)
add a bunch of tests to ensure that the configuration is working.

Also, update the Nginx states to follow Salt best practices.

Fixes #354. Blocked on #374 because I encountered various failures
while upgrading from the Ubuntu Nginx to upstream Nginx, and I'd
like to make sure I caught them all.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/375)
<!-- Reviewable:end -->
@edunham edunham modified the milestone: Buildbot Enhancements Jul 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants