Merge ~ahasenack/ubuntu/+source/squid3:xenial-squid-reload-1738412 into ubuntu/+source/squid3:ubuntu/xenial-devel

Proposed by Andreas Hasenack on 2018-10-30
Status: Merged
Approved by: Andreas Hasenack on 2018-10-31
Approved revision: 952103bbf339cabc5fbe17214eb6909d2f23652e
Merged at revision: 952103bbf339cabc5fbe17214eb6909d2f23652e
Proposed branch: ~ahasenack/ubuntu/+source/squid3:xenial-squid-reload-1738412
Merge into: ubuntu/+source/squid3:ubuntu/xenial-devel
Diff against target: 110 lines (+25/-14)
5 files modified
debian/changelog (+12/-0)
debian/squid.rc (+3/-3)
debian/tests/control (+1/-1)
debian/tests/squid (+5/-0)
debian/tests/test-squid.py (+4/-10)
Reviewer Review Type Date Requested Status
Christian Ehrhardt  2018-10-30 Approve on 2018-10-31
Canonical Server Team 2018-10-31 Pending
Canonical Server packageset reviewers 2018-10-30 Pending
Review via email: mp+358050@code.launchpad.net

Description of the change

Fixes the regexp used to detect problems in the squid configuration file.

The (SRU) bug has test details, and regression considerations.

I also took the oportunity to fix the DEP8 tests for xenial. I have a bileto run, but can't get to the test logs there, so I uploaded the results here: http://people.ubuntu.com/~ahasenack/dep8-squid3-xenial/

Bileto ticket: https://bileto.ubuntu.com/#/ticket/3502

I debated adding a python-minimal dependency to d/t/control. I can still do that if preferred, it just seems unecessary.

To post a comment you must log in.
Christian Ehrhardt  (paelzer) wrote :

The changes LGTM +1 for those.

I read and thought about the regression potential twice, but I like the "explicitly broken" much more than "silently broken". I think it is good that you outlined it in the bug, but I'd want the change and not see it as a bad thing.

review: Approve
Andreas Hasenack (ahasenack) wrote :

Thanks, tagged and uploaded.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index b72010c..35a4a77 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,15 @@
6+squid3 (3.5.12-1ubuntu7.6) xenial; urgency=medium
7+
8+ * d/squid.rc: fix regexp for catching FATAL errors (LP: #1738412)
9+ * d/t/test-squid.py: in xenial, initscript, apparmor profile, pidfile and
10+ process are named squid, not squid3. Get rid of the multiple distro
11+ logic since these tests will be only run on xenial.
12+ * d/t/control: drop uneeded dependency on python-unit.
13+ * d/t/squid: use a shorter shutdown timeout for the tests, so they
14+ run faster
15+
16+ -- Andreas Hasenack <andreas@canonical.com> Wed, 31 Oct 2018 09:22:14 -0300
17+
18 squid3 (3.5.12-1ubuntu7.5) xenial-security; urgency=medium
19
20 * SECURITY UPDATE: various denial of service issues
21diff --git a/debian/squid.rc b/debian/squid.rc
22index 204b676..67e6844 100644
23--- a/debian/squid.rc
24+++ b/debian/squid.rc
25@@ -140,7 +140,7 @@ fi
26
27 case "$1" in
28 start)
29- res=`$DAEMON -k parse -f $CONFIG 2>&1 | grep -o "FATAL .*"`
30+ res=`$DAEMON -k parse -f $CONFIG 2>&1 | grep -o "FATAL: .*"`
31 if test -n "$res";
32 then
33 log_failure_msg "$res"
34@@ -163,7 +163,7 @@ case "$1" in
35 fi
36 ;;
37 reload|force-reload)
38- res=`$DAEMON -k parse -f $CONFIG 2>&1 | grep -o "FATAL .*"`
39+ res=`$DAEMON -k parse -f $CONFIG 2>&1 | grep -o "FATAL: .*"`
40 if test -n "$res";
41 then
42 log_failure_msg "$res"
43@@ -176,7 +176,7 @@ case "$1" in
44 fi
45 ;;
46 restart)
47- res=`$DAEMON -k parse -f $CONFIG 2>&1 | grep -o "FATAL .*"`
48+ res=`$DAEMON -k parse -f $CONFIG 2>&1 | grep -o "FATAL: .*"`
49 if test -n "$res";
50 then
51 log_failure_msg "$res"
52diff --git a/debian/tests/control b/debian/tests/control
53index 52efdc4..a97a14d 100644
54--- a/debian/tests/control
55+++ b/debian/tests/control
56@@ -1,3 +1,3 @@
57 Tests: squid
58-Depends: squid, squidclient, python-unit, elinks, netcat, pygopherd, apparmor-utils, vsftpd
59+Depends: squid, squidclient, elinks, netcat, pygopherd, apparmor-utils, vsftpd
60 Restrictions: needs-root
61diff --git a/debian/tests/squid b/debian/tests/squid
62index f17feef..4a76b4b 100755
63--- a/debian/tests/squid
64+++ b/debian/tests/squid
65@@ -8,4 +8,9 @@ set -e
66 sed -i "s/anonymous_enable[[:blank:]]*=[[:blank:]]*.*/anonymous_enable=YES/g" /etc/vsftpd.conf
67 echo "seccomp_sandbox=NO" >> /etc/vsftpd.conf
68 service vsftpd restart 2>&1 > /dev/null
69+
70+# use a shorter timeout so tests run faster
71+if ! grep -qE "^shutdown_lifetime" /etc/squid/squid.conf; then
72+ echo "shutdown_lifetime 2 seconds" >> /etc/squid/squid.conf
73+fi
74 python `dirname $0`/test-squid.py 2>&1
75diff --git a/debian/tests/test-squid.py b/debian/tests/test-squid.py
76index b6dbc45..f5487fb 100644
77--- a/debian/tests/test-squid.py
78+++ b/debian/tests/test-squid.py
79@@ -94,15 +94,13 @@ class BasicTest(testlib_httpd.HttpdCommon, PrivateSquidTest):
80 if self.lsb_release['Release'] == 10.10 and not os.path.exists("/etc/init.d/squid"):
81 os.symlink("/lib/init/upstart-job", "/etc/init.d/squid")
82
83- self._set_initscript("/etc/init.d/squid")
84- if self.lsb_release['Release'] >= 12.04:
85- self._set_initscript("squid3")
86+ self._set_initscript("squid")
87
88 testlib_httpd.HttpdCommon._setUp(self)
89
90 self.gophermap = "/var/gopher/gophermap"
91
92- self.aa_profile = "usr.sbin.squid3"
93+ self.aa_profile = "usr.sbin.squid"
94 self.aa_abs_profile = "/etc/apparmor.d/%s" % self.aa_profile
95 self.version_with_apparmor = 12.10
96 # This hack is only used until we have tests run both confined and
97@@ -116,12 +114,8 @@ class BasicTest(testlib_httpd.HttpdCommon, PrivateSquidTest):
98
99 def test_daemons(self):
100 '''Test daemon'''
101- pidfile = "/run/squid3.pid"
102- exe = "squid3"
103-
104- if self.lsb_release['Release'] < 12.04:
105- pidfile = "/var/run/squid.pid"
106- exe = "squid"
107+ pidfile = "/run/squid.pid"
108+ exe = "squid"
109
110 self.assertTrue(testlib.check_pidfile(exe, pidfile))
111

Subscribers

People subscribed via source and target branches