Revert to old dh-virtualenv, use built-in venv module #5484

rmol · 2020-09-04T16:35:46Z

Status

Ready for review

Description of Changes

This takes another run at controlling setuptools: instead of installing a newer version of dh-virtualenv from Debian unstable for its ability to specify the setuptools version via the alternative build system, use it as before but with the Python 3 venv module, which will use the local version of setuptools that's already present, then replace that in the final package with the version pinned in the securedrop-app-code requirements.

This was the second plan of attack listed in #5480. The old version of dh-virtualenv available on Xenial did not provide a way to pass --no-download to virtualenv, but does offer --builtin-venv, which seems to be the way to go anyway, from our work on #5110. This approach lets us use the distribution-provided version of dh-virtualenv, which should be safer and more consistent, at least once the dh-virtualenv team is providing Focal packages.

Fixes #5480.

Testing

cd molecule/builder-xenial && make
export BUILDER_IMAGE="quay.io/freedomofpress/sd-docker-builder-xenial:2020_09_03" # (adjust if your image name is different)
cd ../.. && make build-debs && make staging
Manually verify that the staging site is functioning, or if you're not on Qubes, run the configuration tests.

Deployment

This changes the production packaging. Ignoring the recent vacillation, the difference to 1.5.0 is primarily the switch to using the Python 3 venv module to create the production virtual environment. One change to the AppArmor configuration was needed, for pyvenv.cfg.

Checklist

If you made changes to the server application code:

Linting (make lint) and tests (make test) pass in the development container

If you made changes to the system configuration:

Configuration tests pass

If you made non-trivial code changes:

I have written a test plan and validated it for this PR

This takes another run at controlling setuptools: instead of installing a newer version of dh-virtualenv from Debian unstable for its ability to specify the setuptools version via the alternative build system, use it as before but with the Python 3 venv module, which will use the local version of setuptools that's already present, then replace that in the final package with the version pinned in the securedrop-app-code requirements.

conorsch · 2020-09-04T16:55:11Z

@rmol Did you happen to check whether these changes straighten out the VIRTUAL_ENV path in post/activate? (As discussed in #5472 (comment).) Let's aim to add a test for that, xfail for now if need be.

Also prior to merge we'll need to rebuild and repush the xenial builder image.

rmol · 2020-09-04T17:17:42Z

It does fix the activate scripts, but +1 to the test.

kushaldas

Initial testing on the staging worked with the general QA steps. I am going to setup a prod instance, and then upgrade to this version. And then try again.

Makes sure that the virtualenv activation script in the securedrop-app-code package has the right path to the virtualenv.

kushaldas

In a production vm the upgrade path worked well. Did the standard QA steps after that, and things seems to working as it should be (including backup & restore). Approved from my side.

conorsch · 2020-09-09T16:19:05Z

Also prior to merge we'll need to rebuild and repush the xenial builder image.

For future reference, we can build & push to aid in review next time. We needn't wait for approval, since the build logic is careful to pin explicit hashes of containers, so even if the image exists in the remote image repo, it won't be used in CI on other PRs until this PR is merged.

conorsch · 2020-09-09T23:26:18Z

After some additional discussion, @rmol pointed out that we'll want to update the Focal build logic to match these changes. We're not yet running the make build-debs-focal target in CI, but soon will.

So, after updating the logic for Focal to ensure working packages via the venv build system, we'll also need to build & push the Focal container, same as for Xenial. Therefore I'm dismissing @kushaldas's approving review, just to force another approval after the container changes land.

kushaldas · 2020-09-10T13:08:28Z

NOTE:

The setuptools version on Focal is 44.0.0. While using the same style of using venv in the Focal build is causing pip missing all the dependency packages for securedrop-app-code package and then failing there. The error output is more than anything I can copy-paste even on a separate file.

If I try manually in the container to install setuptools on a virtual environment, I seeing error similar to pypa/setuptools#2029.

So, for the experiment purpose, I decided to remove the old version of setuptools wheel from the container, and then wget setuptools-46.0.0 there. This solves the initial packaging problem, all packages get built properly. This is the diff of the Dockerfile i used

diff --git a/molecule/builder-focal/Dockerfile b/molecule/builder-focal/Dockerfile
index a75cde302..97b82e743 100644
--- a/molecule/builder-focal/Dockerfile
+++ b/molecule/builder-focal/Dockerfile
@@ -27,13 +27,15 @@ RUN apt-get -y update && apt-get upgrade -y && apt-get install -y \
         python3-all \
         python3-pip \
         python3-setuptools \
+        python3-venv \
         rsync \
         ruby \
         sqlite \
         sudo \
         tzdata \
         libevent-dev \
-        unzip
+        unzip \
+       wget
 
 
 # TEMPORARY: install dh-virtualenv from debian unstable, pending focal package:
@@ -44,6 +46,9 @@ RUN apt-get install -y debian-archive-keyring
 RUN ln -s /usr/share/keyrings/debian-archive-keyring.gpg /etc/apt/trusted.gpg.d/
 
 RUN apt-get update && apt-get install -y dh-virtualenv
+RUN wget https://files.pythonhosted.org/packages/70/b8/b23170ddda9f07c3444d49accde49f2b92f97bb2f2ebc312618ef12e4bd6/setuptools-46.0.0-py3-none-any.whl
+RUN rm -f /usr/share/python-wheels/setuptools-*.whl
+RUN mv setuptools-46.0.0*.whl /usr/share/python-wheels/
 
 RUN paxctl -cm /usr/bin/python3.8 && mkdir -p /tmp/build
 RUN apt-get clean \

But, then while trying to install the package on the Focal staging using #5486, the apache2 service is failing to start. Looking at the error I can see the error is same in #5443. The .so file name is not what mod_wsgi looking.

We should not block this PR for the focal changes, I suggest that after we merge this, we can do a separate PR to fix the Focal package build.

If anyone wants to verify my findings so far, download https://kushaldas.in/focal.tar.gz into build/ and untar, it will create the focal directory with all the packages from my system. Then do molecule converge -s libvirt-staging-focal -- --skip-tags grsecurity using the branch mentioned at #5486 , you should be able to reproduce the same apache2 error.

✦ ❯ sha256sum focal.tar.gz 
1a0082fbd689899d627a1a9faf9213cb642119f0bb197be9a9806986b3763564  focal.tar.gz

Also create a virtualenv for translation tasks in the build container, instead of installing all of our requirements system-wide.

kushaldas

I can build the debian packages on both Xenial and on Focal.
make staging works
Adds libvirt staging environment for Focal #5486 draft PR also works

Approved, we need to push new builder image for Xenial and Focal, and then restart the CI.

kushaldas

The service file tests need update:

**Environment=PYTHONPATH="/var/www/securedrop:/opt/venvs/securedrop-app-code/lib/python3.5/site-packages"**

did not happen (no ansible var substitution for securedrop_venv in the tests:

Environment=PYTHONPATH="/var/www/securedrop:{{ securedrop_venv }}/lib/python3.5/site-packages"

Seems like https://github.com/freedomofpress/securedrop/blob/5480-old-dhve-builtin-venv/molecule/testinfra/staging/vars/app-staging.yml#L15-L16 these two lines are causing trouble.

The file paths are (and they are good in my Xenial staging instance):

/lib/systemd/system/securedrop_source_deleter.service
/lib/systemd/system/securedrop_rqworker.service
/lib/systemd/system/securedrop_shredder.service
/lib/systemd/system/securedrop_rqrequeue.service

molecule/testinfra/staging/vars/staging.yml

The Vagrant box used for the libvirt staging env lacks python3-distutils, which breaks mod_wsgi's "module-config" function, as the swallowed exception when trying to import distutils causes the correct shared library path construction logic to be skipped. Also revert the templatization of the testinfra staging vars.

emkll · 2020-09-11T21:33:47Z

install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/templates/control.j2

 {% else %}
-Depends: ${dist:Depends}, ${misc:Depends}, ${python3:Depends}, apache2, apparmor-utils, coreutils, gnupg2, haveged, libapache2-mod-xsendfile, libpython3.5, paxctld, python3 (>= 3.5), python3 (<< 3.6), redis-server, securedrop-config, securedrop-keyring, sqlite3
+Depends: ${dist:Depends}, ${misc:Depends}, ${python3:Depends}, apache2, apparmor-utils, coreutils, gnupg2, haveged, libapache2-mod-xsendfile, libpython3.5, paxctld, python3 (>= 3.5), python3 (<< 3.6), python3-distutils, redis-server, securedrop-config, securedrop-keyring, sqlite3


Looks like in both cases, python-distutils is not in the security channel, we would need to mirror the package and possibly its dependencies. I can't find that exact package in Xenial (maybe it's a metapackage?)

https://packages.ubuntu.com/xenial/python3-distutils-extra

https://packages.ubuntu.com/focal/python3-distutils

In Xenial the package libpython3.5-stdlib provides distutils module.

distutils is provided by libpython3.5-stdlib in Xenial.

kushaldas

Tested with the following steps:

package built on xenial
tested on xenial staging by hand
package built on Focal with one patch fixing ossec dependency
finished staging install using @conorsch's focal branch

emkll · 2020-09-14T15:23:21Z

install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/tasks/main.yml

-    pip3 install --no-deps --no-binary :all: --require-hashes -r {{ securedrop_app_code_prep_dir }}/requirements.txt
-  tags:
-    - pip
+    pip3 download --require-hashes -r "{{ securedrop_app_code_prep_dir }}/requirements.txt" --dest /tmp/securedrop-app-code-requirements-download


Is there a reason for omitting the --no-depsargument here? Looks like download supports it, and it may be prudent to add it here to avoid downloading transitive dependencies (other than the ones that are hash-pinned in requirements.txt). I guess we are checking them at the install phase later on, so it may not be strictly necessary

No, I suppose it could save some time. I'll add it.

kushaldas · 2020-09-14T16:04:44Z

@rmol please cherry-pick 7682467 this one in this PR.

Fixing a typo in the libevent name package names. Hadn't noticed during review of previous PRs because we're only just approaching the point of being able install these packages in a staging environment. (cherry picked from commit 7682467)

conorsch

Re-tested on staging env. Confirmed functional app on Xenial. For Focal, confirmed that debs build, but did not install them.

rmol requested review from conorsch, emkll and kushaldas as code owners September 4, 2020 16:35

rmol force-pushed the 5480-old-dhve-builtin-venv branch from c9a262e to f160fe8 Compare September 4, 2020 16:38

kushaldas reviewed Sep 8, 2020

View reviewed changes

Add test of virtualenv activation script construction

812c788

Makes sure that the virtualenv activation script in the securedrop-app-code package has the right path to the virtualenv.

kushaldas previously approved these changes Sep 9, 2020

View reviewed changes

conorsch requested a review from kushaldas September 9, 2020 23:26

conorsch mentioned this pull request Sep 10, 2020

Creates a symlink to the .so file in postint for mod_wsgi #5443

Closed

12 tasks

Control setuptools version used in build container virtualenvs

84cdcd1

Also create a virtualenv for translation tasks in the build container, instead of installing all of our requirements system-wide.

rmol dismissed kushaldas’s stale review via 84cdcd1 September 11, 2020 00:22

kushaldas previously approved these changes Sep 11, 2020

View reviewed changes

kushaldas mentioned this pull request Sep 11, 2020

Adds libvirt staging environment for Focal #5486

Merged

7 tasks

updated builder image hashes

3ca16a5

zenmonkeykstop dismissed kushaldas’s stale review via 3ca16a5 September 11, 2020 14:35

kushaldas suggested changes Sep 11, 2020

View reviewed changes

molecule/testinfra/staging/vars/staging.yml Outdated Show resolved Hide resolved

rmol mentioned this pull request Sep 11, 2020

Creates a symlink to the .so file in postint for mod_wsgi #5489

Closed

12 tasks

emkll reviewed Sep 11, 2020

View reviewed changes

Removes python3-distutils as dependency in Xenial

c499f16

distutils is provided by libpython3.5-stdlib in Xenial.

kushaldas previously approved these changes Sep 14, 2020

View reviewed changes

emkll reviewed Sep 14, 2020

View reviewed changes

Conor Schaefer and others added 2 commits September 14, 2020 12:14

Fixes libevent deps for ossec under Focal

30ba408

Fixing a typo in the libevent name package names. Hadn't noticed during review of previous PRs because we're only just approaching the point of being able install these packages in a staging environment. (cherry picked from commit 7682467)

Add --no-deps to setuptools download

d50e413

rmol dismissed kushaldas’s stale review via d50e413 September 14, 2020 16:16

conorsch approved these changes Sep 14, 2020

View reviewed changes

conorsch merged commit 06ceac2 into develop Sep 14, 2020

rmol deleted the 5480-old-dhve-builtin-venv branch September 15, 2020 13:07

kushaldas mentioned this pull request Sep 15, 2020

Use system provided pip/setuptools #5110

Closed

11 tasks

emkll mentioned this pull request Sep 16, 2020

The package for securedrop-app-code in Focal is broken #5488

Closed

eloquence added this to the 1.6.0 milestone Sep 17, 2020

This was referenced Sep 28, 2020

Moving to python3-venv to create the virtual environment for SecureDrop #5134

Closed

Release SecureDrop 1.6.0 #5501

Closed

rmol mentioned this pull request Sep 28, 2020

versions of pip, setuptools, wheel used by dh_virtualenv during securedrop-app-code build are not pinned. #5109

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revert to old dh-virtualenv, use built-in venv module #5484

Revert to old dh-virtualenv, use built-in venv module #5484

rmol commented Sep 4, 2020

conorsch commented Sep 4, 2020

rmol commented Sep 4, 2020

kushaldas left a comment

kushaldas left a comment

conorsch commented Sep 9, 2020

conorsch commented Sep 9, 2020

kushaldas commented Sep 10, 2020 •

edited

Loading

kushaldas left a comment •

edited

Loading

kushaldas left a comment •

edited

Loading

emkll Sep 11, 2020

kushaldas Sep 12, 2020

kushaldas left a comment •

edited

Loading

emkll Sep 14, 2020

rmol Sep 14, 2020

kushaldas commented Sep 14, 2020

conorsch left a comment

Revert to old dh-virtualenv, use built-in venv module #5484

Revert to old dh-virtualenv, use built-in venv module #5484

Conversation

rmol commented Sep 4, 2020

Status

Description of Changes

Testing

Deployment

Checklist

If you made changes to the server application code:

If you made changes to the system configuration:

If you made non-trivial code changes:

conorsch commented Sep 4, 2020

rmol commented Sep 4, 2020

kushaldas left a comment

Choose a reason for hiding this comment

kushaldas left a comment

Choose a reason for hiding this comment

conorsch commented Sep 9, 2020

conorsch commented Sep 9, 2020

kushaldas commented Sep 10, 2020 • edited Loading

kushaldas left a comment • edited Loading

Choose a reason for hiding this comment

kushaldas left a comment • edited Loading

Choose a reason for hiding this comment

emkll Sep 11, 2020

Choose a reason for hiding this comment

kushaldas Sep 12, 2020

Choose a reason for hiding this comment

kushaldas left a comment • edited Loading

Choose a reason for hiding this comment

emkll Sep 14, 2020

Choose a reason for hiding this comment

rmol Sep 14, 2020

Choose a reason for hiding this comment

kushaldas commented Sep 14, 2020

conorsch left a comment

Choose a reason for hiding this comment

kushaldas commented Sep 10, 2020 •

edited

Loading

kushaldas left a comment •

edited

Loading

kushaldas left a comment •

edited

Loading

kushaldas left a comment •

edited

Loading