Skip to content

Commit

Permalink
Use Nginx to make Buildbot logs available via CORS
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
aneeshusa committed May 16, 2016
1 parent f10a2ca commit 7442488
Show file tree
Hide file tree
Showing 9 changed files with 253 additions and 24 deletions.
14 changes: 0 additions & 14 deletions nginx/default

This file was deleted.

21 changes: 21 additions & 0 deletions nginx/files/default
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
server {
listen 80 default_server;
server_name build.servo.org;


location / {
proxy_pass http://localhost:8010/;

# Enable CORS for basic (GET-only without OPTIONS preflight) requests
if ($request_method = 'GET') {
add_header 'Access-Control-Allow-Origin' '*' always;
# Do not set Access-Control-Allow-Credentials
# Do not set Access-Control-Expose-Headers
}
}

location /homu/ {
proxy_pass http://localhost:54856/;
}
}

56 changes: 56 additions & 0 deletions nginx/files/nginx.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
user {{ nginx.user }};
worker_processes 4;
pid /run/nginx.pid;

events {
worker_connections 768;
# multi_accept on;
}

http {

##
# Basic Settings
##

sendfile on;
tcp_nopush on;
tcp_nodelay on;
keepalive_timeout 65;
types_hash_max_size 2048;
# server_tokens off;

# server_names_hash_bucket_size 64;
# server_name_in_redirect off;

include /etc/nginx/mime.types;
default_type application/octet-stream;

##
# Logging Settings
##

access_log /var/log/nginx/access.log;
error_log /var/log/nginx/error.log;

##
# Gzip Settings
##

gzip on;
gzip_disable "msie6";

# gzip_vary on;
# gzip_proxied any;
# gzip_comp_level 6;
# gzip_buffers 16 8k;
# gzip_http_version 1.1;
# gzip_types text/plain text/css application/json application/x-javascript text/xml application/xml application/xml+rss text/javascript;

##
# Virtual Host Configs
##

include /etc/nginx/conf.d/*.conf;
include /etc/nginx/sites-enabled/*;
}
28 changes: 28 additions & 0 deletions nginx/files/nginx_signing.key
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
-----BEGIN PGP PUBLIC KEY BLOCK-----
Version: GnuPG v1.4.11 (FreeBSD)

mQENBE5OMmIBCAD+FPYKGriGGf7NqwKfWC83cBV01gabgVWQmZbMcFzeW+hMsgxH
W6iimD0RsfZ9oEbfJCPG0CRSZ7ppq5pKamYs2+EJ8Q2ysOFHHwpGrA2C8zyNAs4I
QxnZZIbETgcSwFtDun0XiqPwPZgyuXVm9PAbLZRbfBzm8wR/3SWygqZBBLdQk5TE
fDR+Eny/M1RVR4xClECONF9UBB2ejFdI1LD45APbP2hsN/piFByU1t7yK2gpFyRt
97WzGHn9MV5/TL7AmRPM4pcr3JacmtCnxXeCZ8nLqedoSuHFuhwyDnlAbu8I16O5
XRrfzhrHRJFM1JnIiGmzZi6zBvH0ItfyX6ttABEBAAG0KW5naW54IHNpZ25pbmcg
a2V5IDxzaWduaW5nLWtleUBuZ2lueC5jb20+iQE+BBMBAgAoBQJOTjJiAhsDBQkJ
ZgGABgsJCAcDAgYVCAIJCgsEFgIDAQIeAQIXgAAKCRCr9b2Ce9m/YpvjB/98uV4t
94d0oEh5XlqEZzVMrcTgPQ3BZt05N5xVuYaglv7OQtdlErMXmRWaFZEqDaMHdniC
sF63jWMd29vC4xpzIfmsLK3ce9oYo4t9o4WWqBUdf0Ff1LMz1dfLG2HDtKPfYg3C
8NESud09zuP5NohaE8Qzj/4p6rWDiRpuZ++4fnL3Dt3N6jXILwr/TM/Ma7jvaXGP
DO3kzm4dNKp5b5bn2nT2QWLPnEKxvOg5Zoej8l9+KFsUnXoWoYCkMQ2QTpZQFNwF
xwJGoAz8K3PwVPUrIL6b1lsiNovDgcgP0eDgzvwLynWKBPkRRjtgmWLoeaS9FAZV
ccXJMmANXJFuCf26iQEcBBABAgAGBQJOTkelAAoJEKZP1bF62zmo79oH/1XDb29S
YtWp+MTJTPFEwlWRiyRuDXy3wBd/BpwBRIWfWzMs1gnCjNjk0EVBVGa2grvy9Jtx
JKMd6l/PWXVucSt+U/+GO8rBkw14SdhqxaS2l14v6gyMeUrSbY3XfToGfwHC4sa/
Thn8X4jFaQ2XN5dAIzJGU1s5JA0tjEzUwCnmrKmyMlXZaoQVrmORGjCuH0I0aAFk
RS0UtnB9HPpxhGVbs24xXZQnZDNbUQeulFxS4uP3OLDBAeCHl+v4t/uotIad8v6J
SO93vc1evIje6lguE81HHmJn9noxPItvOvSMb2yPsE8mH4cJHRTFNSEhPW6ghmlf
Wa9ZwiVX5igxcvaIRgQQEQIABgUCTk5b0gAKCRDs8OkLLBcgg1G+AKCnacLb/+W6
cflirUIExgZdUJqoogCeNPVwXiHEIVqithAM1pdY/gcaQZmIRgQQEQIABgUCTk5f
YQAKCRCpN2E5pSTFPnNWAJ9gUozyiS+9jf2rJvqmJSeWuCgVRwCcCUFhXRCpQO2Y
Va3l3WuB+rgKjsQ=
=A015
-----END PGP PUBLIC KEY BLOCK-----
60 changes: 56 additions & 4 deletions nginx/init.sls
Original file line number Diff line number Diff line change
@@ -1,20 +1,72 @@
{% from tpldir ~ '/map.jinja' import nginx %}
# Fix conflicts if Nginx from Ubuntu is previously installed
nginx-distro-pkgs:
pkg.purged:
- pkgs:
- nginx-common
- nginx-core
nginx:
pkg.installed: []
pkgrepo.managed:
- name: 'deb http://nginx.org/packages/ubuntu/ trusty nginx'
- file: /etc/apt/sources.list.d/nginx.list
# Available online but not via HTTPS, so serve locally instead
- key_url: salt://{{ tpldir }}/files/nginx_signing.key
pkg.installed:
- name: {{ nginx.pkg }}
- version: {{ nginx.version }}
- require:
- pkg: nginx-distro-pkgs
- pkgrepo: nginx
service.running:
- enable: True
- watch:
- pkg: nginx
- file: /etc/nginx/sites-enabled/default
- file: /etc/nginx/conf.d
- file: /etc/nginx/nginx.conf
/etc/apt/sources.list.d/nginx.list:
file.exists:
- require:
- pkgrepo: nginx
- require_in:
- file: /etc/apt/sources.list.d
/etc/nginx/sites-available/default:
file.managed:
- source: salt://nginx/default
- source: salt://{{ tpldir }}/files/default
- user: root
- group: root
- mode: 644
- watch_in:
- service: nginx
- require:
- pkg: nginx
/etc/nginx/sites-enabled/default:
file.symlink:
- target: /etc/nginx/sites-available/default
- require:
- file: /etc/nginx/sites-available/default
- pkg: nginx
/etc/nginx/conf.d:
file.directory:
- user: root
- group: root
- clean: True
- file_mode: 644
- dir_mode: 755
- require:
- pkg: nginx
/etc/nginx/nginx.conf:
file.managed:
- user: root
- group: root
- source: salt://{{ tpldir }}/files/nginx.conf
- template: jinja
- context:
nginx: {{ nginx }}
- require:
- pkg: nginx
13 changes: 13 additions & 0 deletions nginx/map.jinja
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{%
set nginx = salt.grains.filter_by({
'Ubuntu': {
'pkg': 'nginx',
'version': '1.10.0-1~trusty',
'user': 'www-data',
},
},
base='defaults',
merge=salt.pillar.get('nginx', {}),
grain='os'
)
%}
9 changes: 3 additions & 6 deletions test.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,10 @@ def run_tests(tests):
message = 'Test \'{}\' raised an exception:'.format(test)
result = Failure(message, str(e))

if result.is_success():
print('[ {} ] {}'.format(color(GREEN, 'PASS'), result.message))
else:
if result.is_failure():
any_failures = True
print('[ {} ] {}'.format(color(RED, 'FAIL'), result.message))
for line in result.output.splitlines():
print(' {}'.format(line))

print(result)

return 1 if any_failures else 0

Expand Down
61 changes: 61 additions & 0 deletions tests/sls/nginx/buildbot_cors.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import urllib.request
import urllib.error

from tests.util import Failure, Success, format_nested_failure



def format_header(response, header):
"""
Precondition: header exists in response.headers
"""
return "{}: Access-Control-{}".format(header, response.headers[header])


def make_cors_request(url, **kwargs):
try:
cors_request = urllib.request.Request(url, headers={
'Origin': 'http://example.com',
}, **kwargs)
return urllib.request.urlopen(cors_request)
except urllib.error.URLError as e:
# Can read e.headers() if there was a response but the HTTP status code
# indicated error; the method is unavailable if a connection could not
# be made. Nginx is reverse proxying Homu and Buildbot, which will be
# dead on Travis because the test pillar credentials are not valid.
return e

# No need to catch HTTPError or ContentTooShortError specially here


def run():
url = 'http://localhost/'
get = make_cors_request(url)
post = make_cors_request(url, method='POST')

if 'Access-Control-Allow-Origin' not in get.headers:
return Failure("Nginx is not serving CORS headers:", str(get.headers))

failures = []

if 'Access-Control-Allow-Origin' in post.headers:
failures += [Failure("CORS headers are served on non-GET requests",
"")]

if get.headers['Access-Control-Allow-Origin'] != '*':
failures += [Failure("All origins are not whitelisted:",
format_header(get, 'Allow-Origin'))]

if 'Access-Control-Allow-Credentials' in get.headers:
failures += [Failure("Cookies are allowed for CORS requests:",
format_header(get, 'Allow-Credentials'))]

if 'Access-Control-Expose-Headers' in get.headers:
failures += [Failure("Headers are exposed on CORS requests:",
format_header(get, 'Expose-Headers'))]

if len(failures) > 0:
return Failure("Nginx is serving incorrect CORS headers:",
'\n'.join([format_nested_failure(f) for f in failures]))

return Success("Nginx is serving CORS headers properly")
15 changes: 15 additions & 0 deletions tests/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ def is_success(self):
def is_failure(self):
return False

def __str__(self):
return '[ {} ] {}'.format(color(GREEN, 'PASS'), self.message)


class Failure(TestResult):
def __init__(self, message, output):
Expand All @@ -63,3 +66,15 @@ def is_success(self):

def is_failure(self):
return True

def __str__(self):
output = ['[ {} ] {}'.format(color(RED, 'FAIL'), self.message)]
for line in self.output.splitlines():
output.append(' {}'.format(line))
return '\n'.join(output)


def format_nested_failure(failure):
output = ['- {}'.format(failure.message)]
output += [' {}'.format(line) for line in failure.output.splitlines()]
return '\n'.join(output)

0 comments on commit 7442488

Please sign in to comment.