Adds package builds for Focal #5465

kushaldas · 2020-08-24T13:17:03Z

Status

Ready for review

Description of Changes

This PR contains the changes required for securedrop-app-code and ossec packages
to be built on Ubuntu Focal. This requires #5458 to be merged first, so that I can rebase.

Testing

make build-debs-focal should create all the related packages for Focal
make build-debs builds packages for Xenial.

Deployment

Any special considerations for deployment? Consider both:

Upgrading existing production instances.
New installs.

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 `securedrop-admin`:

Linting and tests (make -C admin test) pass in the admin 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

If you made changes to documentation:

Doc linting (make docs-lint) passed locally

If you added or updated a code dependency:

Choose one of the following:

I have performed a diff review and pasted the contents to the packaging wiki
I would like someone else to do the diff review

conorsch

First pass on visual review, requesting some cleanup between the various build directories. Much of the work here is intentionally copy/pasted from the xenial dirs, and I appreciate the your caution in avoiding breaking xenial-specific logic, @kushaldas. See in-line for opportunities to consolidate, or if not, at least document explicitly what's different and why it's different under focal.

Although I should have suggested it in #5444, the "make build-debs-focal" target is skipping all tests, even though the tests are present (but failing). We should either fix the packaging tests to get them passing, or else update the makefile target to include the "notest" string for clarity.

conorsch · 2020-08-25T00:34:15Z

install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/files/rules-focal

+override_dh_virtualenv:
+	dh_virtualenv \
+		--python=/usr/bin/python3.8 \
+		--extra-pip-arg "--verbose" \


The only changes I can find in the rules file between xenial & focal are:

python3.5 vs. python3.8

xenial contains --setuptools, whereas focal does not

Ideally we'd have this file templated so the rest is the same, but for now, can you add a comment explaining why setuptools is required for xenial but not for focal?

I will add the setuptools, that was a mistake. Also, I will try to make it a template.

I did and then remembered why i don't need it. By default it will install latest setuptools and with --setuptools flag we can now specify a specific version.

By default it will install

Does "it" refer to the dh-virtualenv package? Even if the --setuptools flag is truly not required for the build process—and I agree, it doesn't appear to be—we still have an entire file duplicated except for this one flag, so a comment should be added to explain why it's not necessary.

conorsch · 2020-08-25T00:41:35Z

install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/files/control-focal

+Homepage: https://securedrop.org
+Build-Depends: debhelper (>= 9), dh-python, python3-all, python3-setuptools, dh-systemd, dh-virtualenv
+Standards-Version: 3.9.8
+X-Python3-Version: >= 3.8


Is the X-Python3-Version field actually required? A quick glance at the dh-virtualenv source indicates it's not really used anywhere.

conorsch · 2020-08-25T00:44:43Z

install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/files/control-focal

+Architecture: amd64
+Conflicts: libapache2-mod-wsgi,supervisor
+Replaces: libapache2-mod-wsgi,supervisor
+Depends: ${dist:Depends}, ${misc:Depends}, ${python3:Depends}, apache2, apparmor-utils, coreutils, gnupg2, haveged, libapache2-mod-xsendfile, libpython3.8, paxctld, python3, redis-server, securedrop-config, securedrop-keyring, sqlite3


The "Depends" line differs only in 3.5 -> 3.8 for the python packages. Can we not use the same name "python3" for both? I realize you'd have to touch xenial files to make this work. What do you think would be easiest to maintain for the remainder of the LTS window?

LTS releases are not supposed to upgrade Python version. That is why I wanted to go this (easy) way.

install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/files/usr.sbin.apache2-focal

conorsch · 2020-08-25T00:50:19Z

install_files/securedrop-ossec-agent/DEBIAN/control.j2

+Conflicts: securedrop-ossec-server
+Description: Installs the securedrop pre-configured OSSEC agent
+  This package installs an OSSEC agent pre-configured for the
+  SecureDrop app server.


Solid approach to multi-distro support. 👍 However, elsewhere in the diff I see that install_files/securedrop-ossec-agent/DEBIAN/control → ...s/build-ossec-deb-pkg/files/control-focal. Given thecontrol.j2introduction here, it apepars thatbuild-ossec-deb-pkg/files/control-focal` is no longer necessary, and should be removed.

@conorsch These are two different packages :) ossec-agent and securedrop-ossec-agent.

conorsch · 2020-08-25T00:52:52Z

install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/files/changelog-focal

+
+ -- SecureDrop Team <[email protected]>  Tue, 12 May 2020 18:37:42 +0000
+
+securedrop-app-code (1.2.2+xenial) xenial; urgency=medium


Focal changelog should not include historical xenial versions. Remove those, and start with the current/latest version for the focal history. (We did the same when moving from trusty to xenial.)

rmol · 2020-08-26T20:59:18Z

It would run a higher risk of breaking the existing Xenial packaging, of course, but would it be possible (or worth the effort) to isolate all the release-specific differences (package dependency lists, paths, whatever) into variable files, and have those included at a single point, instead of sprinkling the "if focal" throughout?

emkll

Thanks @kushaldas , I agree with what others have said here, and I am pleased to report the packages are now built successfully 🎉

As mentioned by a previous reviewer in #5465 (review), this will not run the testinfra tests for packages, which can be useful to guard against regressions when doing packaging logic changes. When running the focal builder with tests (build-debs.sh test focal), I am observing 22 failing tests.

We should address as part of this PR, or file a follow-up ticket (and track it in the epic). I would lean towards addressing the tests here to both ease review and prevent future regressions.

install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/files/usr.sbin.apache2-focal

kushaldas · 2020-08-27T12:46:36Z

It would run a higher risk of breaking the existing Xenial packaging, of course, but would it be possible (or worth the effort) to isolate all the release-specific differences (package dependency lists, paths, whatever) into variable files, and have those included at a single point, instead of sprinkling the "if focal" throughout?

I am not sure, also the plan was to keep as minimal change as possible for this PR.

eloquence · 2020-08-27T16:59:23Z

install_files/ansible-base/roles/build-ossec-deb-pkg/defaults/main.yml

@@ -1,5 +1,5 @@
 ---
-build_ossec_deb_pkg_dependencies: ['libevent1-dev','libpcre2-dev']
+build_ossec_deb_pkg_dependencies: ['libevent-dev','libpcre2-dev']


Why is this dependency changed?

+1, if we don't have to change this dependency, let's not. @kushaldas can you check whether the libevent1-dev is sufficient for both platforms? If not, can you provide documentation that libevent-dev is correct for both?

This has to updated based on Focal or Xenial.

install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/files/usr.sbin.apache2

conorsch · 2020-08-27T22:49:54Z

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

 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
+{% endif %}


Much improved! As discussed today in tech meeting, can we further consolidate the dependencies for libpython3.8 -> libpython3 for both platforms? Maybe that's not possible, but if it is, it'd greatly simplify the logic in play.

It seems this can not be done.

# apt install libpython3 Reading package lists... Done Building dependency tree Reading state information... Done E: Unable to locate package libpython3

conorsch · 2020-08-27T22:50:47Z

install_files/securedrop-ossec-server/DEBIAN/control.j2

@@ -6,7 +6,11 @@ Homepage: https://securedrop.org
 Package: securedrop-ossec-server
 Version: 3.6.0+1.6.0~rc1
 Architecture: amd64
+{% if securedrop_build_focal_support %}
+Depends: libevent-2.1.7,libpcre2-8-0,ossec-server,securedrop-keyring,securedrop-config
+{% else %}
 Depends: libevent-1.4-2,libpcre2-8-0,ossec-server,securedrop-keyring,securedrop-config


As above, perhaps we can standardize on a single libevent metapackage that would work well. Can you check whether that's possible and add if so?

# apt install libevent Reading package lists... Done Building dependency tree Reading state information... Done E: Unable to locate package libevent

This is also not possible due to package naming.

conorsch · 2020-08-27T22:53:32Z

Only very small changes remain, mostly cleaning up package names and preferring cross-distribution metapackage names where possible. It may not be possible everywhere, but let's reduce the number of times we're forced to say if/else in the build logic.

As mentioned by a previous reviewer in #5465 (review), this will not run the testinfra tests for packages, which can be useful to guard against regressions when doing packaging logic changes. When running the focal builder with tests (build-debs.sh test focal), I am observing 22 failing tests.

I took the liberty of re-enabling the tests, since recent changes by @kushaldas resulted in all of them passing for me locally. I noticed an apparently-unrelated test flake on the staging (xenial) job in CI, so bounced.

After a few more package renames if possible, I think this is good to merge—then the next big item will be creating the staging env to use these new packages.

conorsch · 2020-09-03T00:55:09Z

I plan to re-review this tomorrow, although it'd be grand to get #5477 in first, so we can be sure there are no regressions with the Xenial build logic.

This is dependent on #5458

With different dependencies for Xenial and Focal

securedrop-app-code control file is now a template. We don't need --setuptools in rules file as by default virtualenv will install the latest setuptools. Also clean changelog for Focal. We are using single usr.sbin.apache2 for both distribution. Updates the tests var as we rebased against develop.

The testinfra suite for packages is fully passing for focal packages, but the test suite was being skipped. Updated the package build targets to be a bit more explicit about platforms.

libevent1-dev and libevent are two different package, libevent1 is being used in Xenial and libevent (which is the version 2) is being used in Focal.

The latest virtualenv in Focal needs `--setuptools version` in the command line, where as the old verison on Xenial will work with just `--setuptools`. This commit also uses the same build system from Xenial in Focal for building the securedrop-app-code package.

The test suite for validating deb packages was duplicated between Xenial & Focal. That's going to run a risk of tests only being updated in a single location, as was done in #5477. We have the ability to customize behavior of the tests via env vars, so let's keep the same test suite for now.

conorsch

Looks great! The separation between distros is well done here. I suspect we may want to use platform-specific vars in the Ansible logic as continue to drive toward full Focal support, but that's best addressed as part of #5468 anyway.

I made one more change, de-duplicating the packaging tests between Xenial & Focal, to include the new tests we added in #5477. Both packaging targets and all the corresponding tests are passing for me locally, will merge once CI passes.

kushaldas requested review from conorsch and emkll as code owners August 24, 2020 13:17

eloquence changed the title ~~Adds pakcage builds for Focal~~ Adds package builds for Focal Aug 24, 2020

kushaldas force-pushed the focal_package_build branch from 1f458c9 to 1b4f03b Compare August 24, 2020 16:06

redshiftzero mentioned this pull request Aug 24, 2020

Support for Ubuntu 20.04 (Focal) #4768

Closed

53 tasks

conorsch suggested changes Aug 25, 2020

View reviewed changes

kushaldas force-pushed the focal_package_build branch from 771ac3c to aaf9dcc Compare August 26, 2020 13:23

emkll reviewed Aug 26, 2020

View reviewed changes

install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/files/usr.sbin.apache2-focal Outdated Show resolved Hide resolved

kushaldas force-pushed the focal_package_build branch from aaf9dcc to 1b28043 Compare August 27, 2020 12:39

eloquence reviewed Aug 27, 2020

View reviewed changes

install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/files/usr.sbin.apache2 Outdated Show resolved Hide resolved

kushaldas force-pushed the focal_package_build branch from 1b28043 to 57fca68 Compare August 27, 2020 17:06

conorsch reviewed Aug 27, 2020

View reviewed changes

kushaldas force-pushed the focal_package_build branch from e018a87 to 8a4d184 Compare September 2, 2020 09:57

This was referenced Sep 2, 2020

making new docker builder image is broken in xenial #5476

Closed

Fixes Xenial docker build #5477

Merged

kushaldas and others added 6 commits September 3, 2020 16:55

Fixes #5449 changes to build securedrop-app-code in focal

77be0b1

This is dependent on #5458

Fixes #5449 builds ossec packages on Focal

ae1a78c

With different dependencies for Xenial and Focal

Updates focal debs target to run tests

bd58aff

The testinfra suite for packages is fully passing for focal packages, but the test suite was being skipped. Updated the package build targets to be a bit more explicit about platforms.

Adds libevent-dev package dep for Focal

fe90068

libevent1-dev and libevent are two different package, libevent1 is being used in Xenial and libevent (which is the version 2) is being used in Focal.

kushaldas force-pushed the focal_package_build branch from 8a4d184 to dd872db Compare September 3, 2020 13:41

eloquence mentioned this pull request Sep 3, 2020

Add preliminary Focal packages to apt-test freedomofpress/securedrop-apt-test#59

Closed

conorsch self-requested a review September 3, 2020 22:51

conorsch approved these changes Sep 3, 2020

View reviewed changes

conorsch merged commit 6013c5e into develop Sep 3, 2020

emkll mentioned this pull request Sep 14, 2020

Run Focal debs tests in CI #5494

Closed

This was referenced Sep 25, 2020

Remove docs, update CI config & tooling #5520

Merged

update_version script is broken #5530

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds package builds for Focal #5465

Adds package builds for Focal #5465

kushaldas commented Aug 24, 2020 •

edited

Loading

conorsch left a comment

conorsch Aug 25, 2020

kushaldas Aug 26, 2020

kushaldas Aug 26, 2020

conorsch Aug 27, 2020

conorsch Aug 25, 2020

kushaldas Aug 26, 2020

conorsch Aug 25, 2020

kushaldas Aug 26, 2020

conorsch Aug 25, 2020

kushaldas Aug 26, 2020

conorsch Aug 25, 2020

rmol commented Aug 26, 2020

emkll left a comment •

edited

Loading

kushaldas commented Aug 27, 2020

eloquence Aug 27, 2020

conorsch Aug 27, 2020

kushaldas Aug 31, 2020

conorsch Aug 27, 2020

kushaldas Aug 31, 2020

conorsch Aug 27, 2020

kushaldas Aug 31, 2020

conorsch commented Aug 27, 2020

conorsch commented Sep 3, 2020

conorsch left a comment


		-- SecureDrop Team <[email protected]> Tue, 12 May 2020 18:37:42 +0000

		securedrop-app-code (1.2.2+xenial) xenial; urgency=medium

		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
		{% endif %}

Adds package builds for Focal #5465

Adds package builds for Focal #5465

Conversation

kushaldas commented Aug 24, 2020 • edited Loading

Status

Description of Changes

Testing

Deployment

Checklist

If you made changes to the server application code:

If you made changes to securedrop-admin:

If you made changes to the system configuration:

If you made non-trivial code changes:

If you made changes to documentation:

If you added or updated a code dependency:

conorsch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmol commented Aug 26, 2020

emkll left a comment • edited Loading

Choose a reason for hiding this comment

kushaldas commented Aug 27, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

conorsch commented Aug 27, 2020

conorsch commented Sep 3, 2020

conorsch left a comment

Choose a reason for hiding this comment

kushaldas commented Aug 24, 2020 •

edited

Loading

If you made changes to `securedrop-admin`:

emkll left a comment •

edited

Loading