Merge branch 'release/0.3.0'

praekeltfoundation · Sep 1, 2017 · 1cc580d · 1cc580d
2 parents 918022c + eb261a3
commit 1cc580d
Show file tree

Hide file tree

Showing 23 changed files with 373 additions and 163 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -1,31 +1,52 @@
 sudo: false
 dist: trusty
 
+# Only build develop, master, and version tags (which are considered branches
+# by Travis). PRs still get built.
+branches:
+  only:
+    - develop
+    - master
+    - /\d+\.\d+(\.\d+)?/
+
 language: python
 matrix:
   include:
     - python: '2.7'
+      env: TOXENV=py27
+    - python: '3.4'
+      env: TOXENV=py34
     - python: '3.5'
+      env: TOXENV=py35
+    - python: '3.6'
+      env: TOXENV=py36
+      addons: { apt_packages: [pandoc] }
+    - python: pypy2.7-5.8.0
+      env: TOXENV=pypy-nocoverage
+    - python: pypy3.5-5.8.0
+      env: TOXENV=pypy3-nocoverage
+    - python: '2.7'
+      env: TOXENV=pep8
     - python: '3.6'
-    - env: PYENV_VERSION=pypy2.7-5.8.0 PYENV_VERSION_STRING='PyPy 5.8.0' PYENV_ROOT="$HOME/.pyenv-pypy" NO_COVERAGE=1
-    - env: PYENV_VERSION=pypy3.5-5.8.0 PYENV_VERSION_STRING='PyPy 5.8.0-beta0' PYENV_ROOT="$HOME/.pyenv-pypy" NO_COVERAGE=1
+      env: TOXENV=py3pep8
     # Docker dev image
     - sudo: required
-      services: [docker]
+      services: docker
+      python: '3.6'
       env:
         - IMAGE_NAME=praekeltfoundation/marathon-acme
         - REGISTRY_USER=praekeltorgdeploy
         - secure: "JSuiWKoxXNr77SYOuYC1RBXl4vXthOBmMxdKJ5ZjPNCkcAC1WeiFlEAYSA1rw+iuNIi0SWhU8ATTOVj+iKWH3aYx3RolCngMf18juFKGktUdQ17EimK2KzGeieyaBIuwRg+sYelgydC4J+d7pqDGAwhaqvU+xTJeKQUhavUIiymNrAZee008f/ncrporBHPTFHQIWbexYdd2ta/zO9Y3VOQqxJP5qp9E6uRAvvl9ZkNIFGcuRYylPbFIiR5QF6guEXks8bnhlnfflxSJUoX9z6NYeG/kQXyk84p4a2fRm7wuFn4XSbjY+G0Rik4tPq1+rBI7QdKM1YOctiej/Mmb0WD8sEkBGlQDwBb5oozdPY3MtqmtFcSXgf7o0yJWaoLt+l1LBKY1NQg+n59Ls7jnxX+pmR5d42W9lDLHPVVadyun+n4aMUhkbTYxlr1y4vAaAoogB6V9K4QRKekJ4i9mnTiVg29XLuqkZpKQjH6BNMMpy1kPML9tGx9cV+5dUGn2qC097YqmepvUCEIcRs0GJt1I7fNgb0VvyFQu3S/gFqVKVL8qZLWJFCs+TrII6x6ePp+iq5SQvST5YvWQ7JvZY9bnUnqsCBpa0Q1te2jyRk4AVJvd8oPBmt4z2OiJFahWnUvAbXduTW9fH9KhwbSw1nxi1lTSbpw/l+PLbocORKc="
 
       # Clear unused steps
-      addons: {}
       before_install: []
       install: []
       after_success: []
 
       before_script:
         - docker pull $IMAGE_NAME:develop || true
       script:
+        - python setup.py bdist_wheel
         - docker build --pull --cache-from $IMAGE_NAME:develop -t $IMAGE_NAME .
 
       before_deploy:
@@ -37,32 +58,18 @@ matrix:
         on:
           branch: develop
 
-cache:
-  - pip
-  - directories:
-    - ~/.pyenv_cache
-addons:
-  apt_packages:
-    - pandoc
+cache: pip
 
 before_install:
-  - |
-    if [[ -n "$PYENV_VERSION" ]]; then
-      wget https://github.com/praekeltfoundation/travis-pyenv/releases/download/0.3.0/setup-pyenv.sh
-      source setup-pyenv.sh
-    fi
   - pip install --upgrade pip
-
 install:
-  - pip install -r requirements-dev.txt
-  - if [[ -z "$NO_COVERAGE" ]]; then pip install codecov; fi
+  - pip install tox codecov
+
 script:
-  - if [[ -z "$NO_COVERAGE" ]]; then COVERAGE_OPT="--cov"; else COVERAGE_OPT=""; fi
-  - py.test marathon_acme $COVERAGE_OPT
-  - flake8 .
+  - tox
 
 after_success:
-  - if [[ -z "$NO_COVERAGE" ]]; then codecov; fi
+  - case $TOXENV in py27|py34|py35|py36|pypy|pypy3) codecov;; esac
 
 before_deploy:
   - pandoc --from=markdown --to=rst --output=README.rst README.md
@@ -74,3 +81,4 @@ deploy:
   distributions: sdist bdist_wheel
   on:
     tags: true
+    python: '3.6'
diff --git a/Dockerfile b/Dockerfile
@@ -2,10 +2,10 @@
 # marathon-acme
 FROM praekeltfoundation/python-base:3.6-alpine
 
-# Copy in the source and install
-COPY marathon_acme /marathon-acme/marathon_acme
-COPY setup.py LICENSE README.md /marathon-acme/
-RUN pip install -e /marathon-acme/.
+# NOTE: This requires that a wheel has been built using the command
+# `python setup.py bdist_wheel`.
+COPY dist/marathon_acme-*.whl .
+RUN pip install marathon_acme-*.whl
 
 # Set up the entrypoint script
 COPY docker-entrypoint.sh /scripts/marathon-acme-entrypoint.sh

diff --git a/MANIFEST.in b/MANIFEST.in
@@ -1 +1 @@
-include README.md LICENSE .coveragerc pytest.ini setup.cfg
+include README.md LICENSE .coveragerc setup.cfg
diff --git a/README.md b/README.md
@@ -25,7 +25,8 @@ The ACME provider that most people are likely to use is [Let's Encrypt](https://
 ```
 > $ docker run --rm praekeltfoundation/marathon-acme --help
 usage: marathon-acme [-h] [-a ACME] [-e EMAIL] [-m MARATHON[,MARATHON,...]]
-                     [-l LB[,LB,...]] [-g GROUP] [--listen LISTEN]
+                     [-l LB[,LB,...]] [-g GROUP] [--allow-multiple-certs]
+                     [--listen LISTEN]
                      [--log-level {debug,info,warn,error,critical}]
                      storage-dir
 
@@ -50,6 +51,10 @@ optional arguments:
   -g GROUP, --group GROUP
                         The marathon-lb group to issue certificates for
                         (default: external)
+  --allow-multiple-certs
+                        Allow multiple certificates for a single app port.
+                        This allows multiple domains for an app, but is not
+                        recommended.
   --listen LISTEN       The address for the port to listen on (default: :8000)
   --log-level {debug,info,warn,error,critical}
                         The minimum severity level to log messages at
@@ -72,11 +77,10 @@ optional arguments:
   ],
   "labels": {
     "HAPROXY_GROUP": "external",
-    "HAPROXY_0_VHOST": "example.com",
+    "HAPROXY_0_VHOST": "marathon-acme.example.com",
     "HAPROXY_0_BACKEND_WEIGHT": "1",
     "HAPROXY_0_PATH": "/.well-known/acme-challenge/",
-    "HAPROXY_0_HTTP_FRONTEND_ACL_WITH_PATH": "  acl path_{backend} path_beg {path}\n  use_backend {backend} if path_{backend}\n",
-    "HAPROXY_0_HTTPS_FRONTEND_ACL_WITH_PATH": "  use_backend {backend} if path_{backend}\n"
+    "HAPROXY_0_HTTP_FRONTEND_ACL_WITH_PATH": "  acl host_{cleanedUpHostname} hdr(host) -i {hostname}\n  acl path_{backend} path_beg {path}\n  redirect prefix http://{hostname} code 302 if !host_{cleanedUpHostname} path_{backend}\n  use_backend {backend} if host_{cleanedUpHostname} path_{backend}\n"
   },
   "container": {
     "type": "DOCKER",
@@ -103,25 +107,81 @@ optional arguments:
 The above should mostly be standard across different deployments. The volume parameters will depend on your particular networked storage solution.
 
 #### `HAPROXY` labels
+```json
+"labels": {
+  "HAPROXY_GROUP": "external",
+  "HAPROXY_0_VHOST": "marathon-acme.example.com",
+  "HAPROXY_0_BACKEND_WEIGHT": "1",
+  "HAPROXY_0_PATH": "/.well-known/acme-challenge/",
+  "HAPROXY_0_HTTP_FRONTEND_ACL_WITH_PATH": "  acl host_{cleanedUpHostname} hdr(host) -i {hostname}\n  acl path_{backend} path_beg {path}\n  redirect prefix http://{hostname} code 302 if !host_{cleanedUpHostname} path_{backend}\n  use_backend {backend} if host_{cleanedUpHostname} path_{backend}\n"
+}
+```
 Several special `marathon-lb` labels are needed in order to forward all HTTP requests whose path begins with `/.well-known/acme-challenge/` to `marathon-acme`, in order to serve ACME [HTTP challenge](https://ietf-wg-acme.github.io/acme/#rfc.section.7.2) responses.
 
 ##### `HAPROXY_GROUP`
+```
+external
+```
 `marathon-lb` instances are assigned a group. Only Marathon apps with a `HAPROXY_GROUP` label that matches their group are routed with that instance. "external" is the common name for publicly-facing load balancers.
 
 ##### `HAPROXY_0_VHOST`
-`marathon-lb` is designed with the assumption that things have domains. `marathon-acme` doesn't technically need one, but if we don’t specify this label, the app is not exposed to the outside world. Any value will do here, since we change the templates to never include this value.
+```
+marathon-acme.example.com
+```
+`marathon-acme` needs its own domain to respond to ACME challenge requests on. This domain must resolve to your `marathon-lb` instance(s).
 
 ##### `HAPROXY_0_BACKEND_WEIGHT`
+```
+1
+```
 We want this rule in HAProxy's config file to come before any others so that requests are routed to `marathon-acme` before we do the (usually) domain-based routing for the other Marathon apps. The default weight is `0`, so we set to `1` so that the rule comes first.
 
 ##### `HAPROXY_0_PATH`
+```
+/.well-known/acme-challenge/
+```
 This is the beginning of the HTTP path to ACME validation challenges.
 
 ##### `HAPROXY_0_HTTP_FRONTEND_ACL_WITH_PATH`
-This is where it gets complicated... It’s possible to edit the templates used for generating the HAProxy on a per-app basis using labels. This is necessary because by default `marathon-lb` will route based on domain first, but we don’t want to do that. You can see the standard template [here](https://github.com/mesosphere/marathon-lb/blob/master/Longhelp.md#haproxy_http_frontend_acl_with_path). We simply remove the first line containing the hostname ACL.
+```
+  acl host_{cleanedUpHostname} hdr(host) -i {hostname}
+  acl path_{backend} path_beg {path}
+  redirect prefix http://{hostname} code 302 if !host_{cleanedUpHostname} path_{backend}
+  use_backend {backend} if host_{cleanedUpHostname} path_{backend}
+```
+This is where it gets complicated... It’s possible to edit the templates used for generating the HAProxy on a per-app basis using labels. This is necessary because by default `marathon-lb` will route based on domain first, but we don’t want to do that. You can see the standard template [here](https://github.com/mesosphere/marathon-lb/blob/master/Longhelp.md#haproxy_http_frontend_acl_with_path).
+
+Here, we add an extra `redirect` rule. This redirects all requests matching the ACME challenge path to `marathon-acme`, except those requests already headed for `marathon-acme`. The Let's Encrypt server will follow redirects.
+
+#### `HAPROXY` HTTPS labels
+It is possible to have `marathon-acme` serve ACME challenge requests over HTTPS, although this is usually not necessary. In this case, a few more labels need to be added:
+```json
+"labels": {
+  ...,
+  "HAPROXY_0_HTTPS_FRONTEND_ACL_WITH_PATH": "  redirect prefix https://{hostname} code 302 if !{{ ssl_fc_sni {hostname} }} path_{backend}\n  use_backend {backend} if {{ ssl_fc_sni {hostname} }} path_{backend}\n",
+  "MARATHON_ACME_0_DOMAIN": "marathon-acme.example.com",
+  "HAPROXY_0_REDIRECT_TO_HTTPS": "true"
+}
+```
 
 ##### `HAPROXY_0_HTTPS_FRONTEND_ACL_WITH_PATH`
-`marathon-lb` exposes apps via port 443/HTTPS by default and there doesn’t seem to be a way to switch it off. We change the ACL template here so that HAProxy doesn’t try to do an SNI match on the hostname. The ACME Simple HTTP spec allows for challenges to occur over HTTPS if the client requests as such and will ignore the certificate presented on our side.
+```
+  redirect prefix https://{hostname} code 302 if !{{ ssl_fc_sni {hostname} }} path_{backend}
+  use_backend {backend} if {{ ssl_fc_sni {hostname} }} path_{backend}
+```
+This is a lot like the `HAPROXY_0_HTTP_FRONTEND_ACL_WITH_PATH` template—we just add a redirect to `marathon-acme`.
+
+##### `MARATHON_ACME_0_DOMAIN`
+```
+marathon-acme.example.com
+```
+Here we set up `marathon-acme` to fetch a certificate for itself.
+
+##### `HAPROXY_0_REDIRECT_TO_HTTPS`
+```
+true
+```
+We redirect the HTTP challenge requests to HTTPS. **Note** that this can only be switched on after the first certificate has been issued for `marathon-acme`'s domain.
 
 #### Docker images
 Docker images are available from [Docker Hub](https://hub.docker.com/r/praekeltfoundation/marathon-acme/). There are two different streams of Docker images available:
@@ -156,17 +216,32 @@ The only real configuration needed for `marathon-lb` is to add the path to `mara
 ```
 
 ### App configuration
-`marathon-acme` uses a single `marathon-lb`-like label to assign domains to app ports: `MARATHON_ACME_{n}_DOMAIN`, where `{n}` is the port index. The value of the label is a set of comma-separated domain names, although currently only the first domain name will be considered.
+`marathon-acme` uses a single `marathon-lb`-like label to assign domains to app ports: `MARATHON_ACME_{n}_DOMAIN`, where `{n}` is the port index. The value of the label is a set of comma- and/or whitespace-separated domain names, although **by default only the first domain name will be considered**.
+
+Currently, `marathon-acme` can only issue certificates with a single domain. This means multiple certificates need to be issued for apps with multiple configured domains.
+
+A limitation was added that limits apps to a single domain. This limit can be removed by passing the `--allow-multiple-certs` command-line option, although this is not recommended as it makes it possible for a large number of certificates to be issued for a single app, potentially exhausting the Let's Encrypt rate limit.
 
 The app or its port must must be in the same `HAPROXY_GROUP` as `marathon-acme` was configured with at start-up.
 
 We decided not to reuse the `HAPROXY_{n}_VHOST` label so as to limit the number of domains that certificates are issued for.
 
 ## Limitations
-The current biggest limitation with `marathon-acme` is that it will only issue one certificate for one domain per app port. This is to limit the number of certificates issued so as to prevent hitting Let's Encrypt rate limits.
-
 The library used for ACME certificate management, `txacme`, is currently quite limited in its functionality. The two biggest limitations are:
 * There is no [Subject Alternative Name](https://en.wikipedia.org/wiki/Subject_Alternative_Name) (SAN) support yet ([#37](https://github.com/mithrandi/txacme/issues/37)). Each certificate will correspond to exactly one domain name. This limitation makes it easier to hit Let's Encrypt's rate limits.
 * There is no support for *removing* certificates from `txacme`'s certificate store ([#77](https://github.com/mithrandi/txacme/issues/77)). Once `marathon-acme` issues a certificate for an app it will try to renew that certificate *forever* unless it is manually deleted from the certificate store.
 
 For a more complete list of issues, see the issues page for this repo.
+
+## Troubleshooting
+### Challenge ping endpoint
+One common problem is that `marathon-lb` is misconfigured and ACME challenge requests are unable to reach `marathon-acme`. You can test challenge request routing to `marathon-acme` using the challenge ping endpoint.
+
+It should be possible to reach the  `/.well-known/acme-challenge/ping` path from all domains served by `marathon-lb`:
+```
+> $ curl cake-service.example.com/.well-known/acme-challenge/ping
+{"message": "pong"}
+
+> $ curl soda-service.example.com/.well-known/acme-challenge/ping
+{"message": "pong"}
+```
diff --git a/codecov.yml b/codecov.yml
@@ -1,17 +1,2 @@
-coverage:
-  precision: 2
-  round: down
-  range: "70...100"
-
-  status:
-    project:
-      target: auto
-      if_no_uploads: error
-
-    patch:
-      target: "80%"
-      if_no_uploads: error
-
 comment:
-  layout: "header, diff, changes, suggestions"
-  behavior: default
+  layout: "diff, files"
diff --git a/dev-requirements.txt b/dev-requirements.txt
@@ -0,0 +1,3 @@
+coverage
+tox
+-e .[test,pep8test]
diff --git a/marathon_acme/cli.py b/marathon_acme/cli.py
@@ -5,8 +5,8 @@
 from twisted.internet.endpoints import quoteStringArgument
 from twisted.internet.task import react
 from twisted.logger import (
-    FilteringLogObserver, globalLogPublisher, Logger, LogLevel,
-    LogLevelFilterPredicate, textFileLogObserver)
+    FilteringLogObserver, LogLevel, LogLevelFilterPredicate, Logger,
+    globalLogPublisher, textFileLogObserver)
 from twisted.python.compat import unicode
 from twisted.python.filepath import FilePath
 from twisted.python.url import URL
@@ -42,6 +42,11 @@
                     help='The marathon-lb group to issue certificates for '
                          '(default: %(default)s)',
                     default='external')
+parser.add_argument('--allow-multiple-certs',
+                    help=('Allow multiple certificates for a single app port. '
+                          'This allows multiple domains for an app, but is '
+                          'not recommended.'),
+                    action='store_true')
 parser.add_argument('--listen',
                     help='The address for the port to listen on (default: '
                          '%(default)s)',
@@ -70,7 +75,7 @@ def main(reactor, raw_args=sys.argv[1:]):
     mlb_addrs = args.lb.split(',')
 
     marathon_acme = create_marathon_acme(
-        args.storage_dir, args.acme, args.email,
+        args.storage_dir, args.acme, args.email, args.allow_multiple_certs,
         marathon_addrs, mlb_addrs, args.group,
         reactor)
 
@@ -135,9 +140,10 @@ def _create_tx_endpoints_string(args, kwargs):
     return ':'.join(args + _kwargs)
 
 
-def create_marathon_acme(storage_dir, acme_directory, acme_email,
-                         marathon_addrs, mlb_addrs, group,
-                         reactor):
+def create_marathon_acme(
+    storage_dir, acme_directory, acme_email, allow_multiple_certs,
+    marathon_addrs, mlb_addrs, group,
+        reactor):
     """
     Create a marathon-acme instance.
 
@@ -146,6 +152,8 @@ def create_marathon_acme(storage_dir, acme_directory, acme_email,
     :param acme_directory: Address for the ACME directory to use.
     :param acme_email:
         Email address to use when registering with the ACME service.
+    :param allow_multiple_certs:
+        Whether to allow multiple certificates per app port.
     :param marathon_addr:
         Address for the Marathon instance to find app domains that require
         certificates.
@@ -168,7 +176,8 @@ def create_marathon_acme(storage_dir, acme_directory, acme_email,
         MarathonLbClient(mlb_addrs, reactor=reactor),
         create_txacme_client_creator(reactor, acme_url, key),
         reactor,
-        acme_email)
+        acme_email,
+        allow_multiple_certs)
 
 
 def init_storage_dir(storage_dir):

diff --git a/marathon_acme/clients.py b/marathon_acme/clients.py
@@ -5,7 +5,7 @@
 from treq.client import HTTPClient as treq_HTTPClient
 from treq.content import json_content
 from twisted.internet.defer import DeferredList
-from twisted.logger import Logger, LogLevel
+from twisted.logger import LogLevel, Logger
 from twisted.web.http import OK
 from uritools import uricompose, uridecode, urisplit
 

diff --git a/marathon_acme/server.py b/marathon_acme/server.py
@@ -3,7 +3,7 @@
 from klein import Klein
 from twisted.internet.endpoints import serverFromString
 from twisted.logger import Logger
-from twisted.web.http import OK, NOT_IMPLEMENTED, SERVICE_UNAVAILABLE
+from twisted.web.http import NOT_IMPLEMENTED, OK, SERVICE_UNAVAILABLE
 from twisted.web.server import Site
 
 
@@ -48,6 +48,15 @@ def acme_challenge(self, request):
         """
         return self.responder_resource
 
+    @app.route('/.well-known/acme-challenge/ping', methods=['GET'])
+    def acme_challenge_ping(self, request):
+        """
+        Respond to requests on ``/.well-known/acme-challenge/ping`` to debug
+        path routing issues.
+        """
+        request.setResponseCode(OK)
+        write_request_json(request, {'message': 'pong'})
+
     def set_health_handler(self, health_handler):
         """
         Set the handler for the health endpoint.