Merge ~ahasenack/ubuntu/+source/volatildap:focal-volatildap-apparmor-dep8 into ~ahasenack/ubuntu/+source/volatildap:master

[Andreas Hasenack]

volatildap is one of the packages blocking the openldap migation due to DEP8 failures.

This package isn't imported into git-ubuntu, so I did a "gbp import-dsc", and am proposing my changes on top of that.

The DEP8 tests are failing in Ubuntu because of the slapd (openldap daemon) apparmor profile, which doesn't allow the daemon to have its config file and other things in /tmp.

I don't know beforehand which temporary directory will be used in /tmp, since it's random. I could have patched the python code instead and created the apparmor local profile override at that time with a more specific path, but it didn't feel worth it.

I checked debian and it's passing there, as they aren't using apparmor in openldap yet (it's part of our openldap delta), so probably not worth sending to them either.

Here is a run under lxd:
autopkgtest [18:18:36]: test run-tests: [-----------------------
running test
WARNING: Testing via this command is deprecated and will be removed in a future version. Users looking for a generic test entry point independent of test runner are encouraged to use tox.
running egg_info
writing volatildap.egg-info/PKG-INFO
writing dependency_links to volatildap.egg-info/dependency_links.txt
writing top-level names to volatildap.egg-info/top_level.txt
reading manifest file 'volatildap.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
warning: no previously-included files matching '*.py[cod]' found anywhere in distribution
warning: no previously-included files matching '__pycache__' found anywhere in distribution
warning: no previously-included files matching '*.swp' found anywhere in distribution
writing manifest file 'volatildap.egg-info/SOURCES.txt'
running build_ext
test_stop (tests.test_usage.AutoCleanupTests)
Deleting the LdapServer object causes its cleanup. ... config file testing succeeded
ok
test_clear_data (tests.test_usage.ReadWriteTests) ... config file testing succeeded
ok
test_get_missing_entry (tests.test_usage.ReadWriteTests) ... config file testing succeeded
ok
test_initial_data (tests.test_usage.ReadWriteTests) ... config file testing succeeded
ok
test_post_start_add (tests.test_usage.ReadWriteTests) ... config file testing succeeded
ok
test_cleanup (tests.test_usage.ResetTests) ... config file testing succeeded
ok
test_connection (tests.test_usage.TLSTests) ... config file testing succeeded
ok

----------------------------------------------------------------------
Ran 7 tests in 4.249s

[Lucas Kanashiro (lucaskanashiro)]

Why do we need breaks-testbed restriction in this test Andreas? The test works fine without it, that's why I am asking. Other than that it LGTM.

review: Needs Information

[Andreas Hasenack (ahasenack)]

Because we are writing to /etc and changing the environment outside of packaging. From the spec at https://people.debian.org/~mpitt/autopkgtest/README.package-tests.html:
"""
    The test, when run, is liable to break the testbed system. This includes causing data loss, causing services that the machine is running to malfunction, or permanently disabling services; it does not include causing services on the machine to temporarily fail.

    When this restriction is present the test will usually be skipped unless the testbed's virtualisation arrangements are sufficiently powerful, or alternatively if the user explicitly requests.
"""

An alternative would be a shell trap in the test to remove that file when it exits or fails. Would you rather I go that route?

[Lucas Kanashiro (lucaskanashiro)]

IMO remove the apparmor config file in the shell script via trap would be better than have a break environment.

[Andreas Hasenack (ahasenack)]

Ok, will do

[Andreas Hasenack (ahasenack)]

Ok, this got interesting, and hopefully not too complicated. I force-pushed because I had many iterations that would look ugly, but basically the changes are:
- use apparmor_parser -W -T flags, just like dh_apparmor does when updating a package that ships a profile (do a "grep apparmor_parser /var/lib/dpkg/info/*.postinst" to see). This avoids the apparmor cache
- use a trap, as discussed
- backup the original override. We shouldn't have a package shipping an apparmor override file with actual content, but let's play nice
- use set -x

Please take another look, thanks!

[Lucas Kanashiro (lucaskanashiro)]

It looks good Andreas, thanks for the improvements :)

review: Approve

[Andreas Hasenack (ahasenack)]

Thanks!

Uploading 7141d193d5f5f8440464ce5caa9b323f60c484be (not tagging because it's not in git ubuntu)

$ dput ubuntu ../volatildap_1.3.0-2ubuntu1_source.changes
Checking signature on .changes
gpg: ../volatildap_1.3.0-2ubuntu1_source.changes: Valid signature from AC983EB5BF6BCBA9
Checking signature on .dsc
gpg: ../volatildap_1.3.0-2ubuntu1.dsc: Valid signature from AC983EB5BF6BCBA9
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading volatildap_1.3.0-2ubuntu1.dsc: done.
  Uploading volatildap_1.3.0-2ubuntu1.debian.tar.xz: done.
  Uploading volatildap_1.3.0-2ubuntu1_source.buildinfo: done.
  Uploading volatildap_1.3.0-2ubuntu1_source.changes: done.
Successfully uploaded packages.

[Andreas Hasenack (ahasenack)]

This is done.

diff --git a/debian/changelog b/debian/changelog
index cbee615..5c73caf 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+volatildap (1.3.0-2ubuntu1) focal; urgency=medium
+
+  * d/t/control, d/t/run-tests: amend the slapd apparmor profile
+    to allow the tests to use /tmp/** for the slapd daemon
+    (LP: #1858800)
+
+ -- Andreas Hasenack <[e-mail]>  Wed, 08 Jan 2020 18:11:17 -0300
+
 volatildap (1.3.0-2) unstable; urgency=medium
 
   * Source-only release.
diff --git a/debian/control b/debian/control
index 4c70d06..2699a0b 100644
--- a/debian/control
+++ b/debian/control
@@ -1,5 +1,6 @@
 Source: volatildap
-Maintainer: Debian Python Modules Team <[e-mail]>
+Maintainer: Ubuntu Developers <[e-mail]>
+XSBC-Original-Maintainer: Debian Python Modules Team <[e-mail]>
 Uploaders: Pierre-Elliott Bécue <[e-mail]>
 Homepage: https://github.com/rbarrois/volatildap/
 Section: python
diff --git a/debian/tests/control b/debian/tests/control
index 547b5e1..27f7f6a 100644
--- a/debian/tests/control
+++ b/debian/tests/control
@@ -1,7 +1,7 @@
-Test-Command: python3 setup.py test
+Tests: run-tests
 Depends: @,
          python3-setuptools,
          python3-psutil,
          ldapscripts,
          slapd
-Restrictions: allow-stderr
+Restrictions: allow-stderr, needs-root
diff --git a/debian/tests/run-tests b/debian/tests/run-tests
 new file mode 100755
index 0000000..ec89fee
--- /dev/null
+++ b/debian/tests/run-tests
@@ -0,0 +1,27 @@
+#!/bin/sh
+
+set -e
+set -x
+
+bkp=$(mktemp)
+slapd_apparmor_override="/etc/apparmor.d/local/usr.sbin.slapd"
+slapd_apparmor="/etc/apparmor.d/usr.sbin.slapd"
+
+cleanup() {
+    cp -af "$bkp" "$slapd_apparmor_override"
+    rm -f "$bkp"
+    apparmor_parser -r -T -W "$slapd_apparmor"
+}
+
+# backup existing override
+cp -af "$slapd_apparmor_override" "$bkp"
+
+trap "cleanup" 0 INT QUIT ABRT PIPE TERM
+
+# the test suite brings up a test slapd server running
+# off /tmp/<tmpdir>. We don't know the name of that directory
+# beforehand, so let it use anything under /tmp
+echo "/tmp/** rwk," > "$slapd_apparmor_override"
+apparmor_parser -r -T -W "$slapd_apparmor"
+
+python3 setup.py test