freedomofpress / securethenews

An automated scanner and web dashboard for tracking TLS deployment across news organizations
https://securethe.news
GNU Affero General Public License v3.0
100 stars 25 forks source link

Update setuptools before installing SSLyze to prevent an error #119

Closed garrettr closed 7 years ago

garrettr commented 7 years ago

Fixes #118, based on the recommendation from https://github.com/nabla-c0d3/sslyze/issues/209.

This PR currently only adds the fix to the Ansible playbook responsible for provisioning the development environment. @msheiny What needs to be done to achieve the same fix in the production environment?

msheiny commented 7 years ago

@garrettr --- so we can either add this globally to the system or do some tweaks to the deployment playbooks and add this first to the virtualenv before the rest of the requirements. The first is easier with the existing playbooks but i'm weary of upgrading the system's global setuptools. Usually prefer to add this to the virtualenv. So yeah -- probably best to make some tweaks first.

FYI -- @garrettr I wanted to bring in some local dev docker changes I made on an adjacent project. I wanted to run that by you this week and hopefully we can also bring those changes into the STN repo.

garrettr commented 7 years ago

The first is easier with the existing playbooks but i'm weary of upgrading the system's global setuptools

@msheiny Are you weary, or wary?

If I understand your meaning, you are wary of upgrading the system's global setuptools to a version other than that which is provided by the distribution package.

According to the setuptools changelog, support for range operators in environment markers landed in 17.1. So if you want to avoid diverging from the distribution's version of setuptools, we just need to use a distribution that includes a sufficiently new version of setuptools. Ubuntu Trusty is insufficient, because it includes setuptools 3.3 (dang, that's old!!), but I think Ubuntu Xenial, which includes setuptools 20.7, would avoid this issue.

msheiny commented 7 years ago

Are you weary, or wary?

giphy

Well I mean since this only changes local ansible role changes I wont block on merge -- I'll open a ticket for fixing in prod where we can further discuss.

msheiny commented 7 years ago

Opened #121 to track prod deployment concerns.