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
diff --git a/debian/changelog b/debian/changelog
index b72010c..35a4a77 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,15 @@
1squid3 (3.5.12-1ubuntu7.6) xenial; urgency=medium
2
3 * d/squid.rc: fix regexp for catching FATAL errors (LP: #1738412)
4 * d/t/test-squid.py: in xenial, initscript, apparmor profile, pidfile and
5 process are named squid, not squid3. Get rid of the multiple distro
6 logic since these tests will be only run on xenial.
7 * d/t/control: drop uneeded dependency on python-unit.
8 * d/t/squid: use a shorter shutdown timeout for the tests, so they
9 run faster
10
11 -- Andreas Hasenack <andreas@canonical.com> Wed, 31 Oct 2018 09:22:14 -0300
12
1squid3 (3.5.12-1ubuntu7.5) xenial-security; urgency=medium13squid3 (3.5.12-1ubuntu7.5) xenial-security; urgency=medium
214
3 * SECURITY UPDATE: various denial of service issues15 * SECURITY UPDATE: various denial of service issues
diff --git a/debian/squid.rc b/debian/squid.rc
index 204b676..67e6844 100644
--- a/debian/squid.rc
+++ b/debian/squid.rc
@@ -140,7 +140,7 @@ fi
140140
141case "$1" in141case "$1" in
142 start)142 start)
143 res=`$DAEMON -k parse -f $CONFIG 2>&1 | grep -o "FATAL .*"`143 res=`$DAEMON -k parse -f $CONFIG 2>&1 | grep -o "FATAL: .*"`
144 if test -n "$res";144 if test -n "$res";
145 then145 then
146 log_failure_msg "$res"146 log_failure_msg "$res"
@@ -163,7 +163,7 @@ case "$1" in
163 fi163 fi
164 ;;164 ;;
165 reload|force-reload)165 reload|force-reload)
166 res=`$DAEMON -k parse -f $CONFIG 2>&1 | grep -o "FATAL .*"`166 res=`$DAEMON -k parse -f $CONFIG 2>&1 | grep -o "FATAL: .*"`
167 if test -n "$res";167 if test -n "$res";
168 then168 then
169 log_failure_msg "$res"169 log_failure_msg "$res"
@@ -176,7 +176,7 @@ case "$1" in
176 fi176 fi
177 ;;177 ;;
178 restart)178 restart)
179 res=`$DAEMON -k parse -f $CONFIG 2>&1 | grep -o "FATAL .*"`179 res=`$DAEMON -k parse -f $CONFIG 2>&1 | grep -o "FATAL: .*"`
180 if test -n "$res";180 if test -n "$res";
181 then181 then
182 log_failure_msg "$res"182 log_failure_msg "$res"
diff --git a/debian/tests/control b/debian/tests/control
index 52efdc4..a97a14d 100644
--- a/debian/tests/control
+++ b/debian/tests/control
@@ -1,3 +1,3 @@
1Tests: squid1Tests: squid
2Depends: squid, squidclient, python-unit, elinks, netcat, pygopherd, apparmor-utils, vsftpd2Depends: squid, squidclient, elinks, netcat, pygopherd, apparmor-utils, vsftpd
3Restrictions: needs-root3Restrictions: needs-root
diff --git a/debian/tests/squid b/debian/tests/squid
index f17feef..4a76b4b 100755
--- a/debian/tests/squid
+++ b/debian/tests/squid
@@ -8,4 +8,9 @@ set -e
8sed -i "s/anonymous_enable[[:blank:]]*=[[:blank:]]*.*/anonymous_enable=YES/g" /etc/vsftpd.conf8sed -i "s/anonymous_enable[[:blank:]]*=[[:blank:]]*.*/anonymous_enable=YES/g" /etc/vsftpd.conf
9echo "seccomp_sandbox=NO" >> /etc/vsftpd.conf9echo "seccomp_sandbox=NO" >> /etc/vsftpd.conf
10service vsftpd restart 2>&1 > /dev/null10service vsftpd restart 2>&1 > /dev/null
11
12# use a shorter timeout so tests run faster
13if ! grep -qE "^shutdown_lifetime" /etc/squid/squid.conf; then
14 echo "shutdown_lifetime 2 seconds" >> /etc/squid/squid.conf
15fi
11python `dirname $0`/test-squid.py 2>&116python `dirname $0`/test-squid.py 2>&1
diff --git a/debian/tests/test-squid.py b/debian/tests/test-squid.py
index b6dbc45..f5487fb 100644
--- a/debian/tests/test-squid.py
+++ b/debian/tests/test-squid.py
@@ -94,15 +94,13 @@ class BasicTest(testlib_httpd.HttpdCommon, PrivateSquidTest):
94 if self.lsb_release['Release'] == 10.10 and not os.path.exists("/etc/init.d/squid"):94 if self.lsb_release['Release'] == 10.10 and not os.path.exists("/etc/init.d/squid"):
95 os.symlink("/lib/init/upstart-job", "/etc/init.d/squid")95 os.symlink("/lib/init/upstart-job", "/etc/init.d/squid")
9696
97 self._set_initscript("/etc/init.d/squid")97 self._set_initscript("squid")
98 if self.lsb_release['Release'] >= 12.04:
99 self._set_initscript("squid3")
10098
101 testlib_httpd.HttpdCommon._setUp(self)99 testlib_httpd.HttpdCommon._setUp(self)
102100
103 self.gophermap = "/var/gopher/gophermap"101 self.gophermap = "/var/gopher/gophermap"
104102
105 self.aa_profile = "usr.sbin.squid3"103 self.aa_profile = "usr.sbin.squid"
106 self.aa_abs_profile = "/etc/apparmor.d/%s" % self.aa_profile104 self.aa_abs_profile = "/etc/apparmor.d/%s" % self.aa_profile
107 self.version_with_apparmor = 12.10105 self.version_with_apparmor = 12.10
108 # This hack is only used until we have tests run both confined and106 # This hack is only used until we have tests run both confined and
@@ -116,12 +114,8 @@ class BasicTest(testlib_httpd.HttpdCommon, PrivateSquidTest):
116114
117 def test_daemons(self):115 def test_daemons(self):
118 '''Test daemon'''116 '''Test daemon'''
119 pidfile = "/run/squid3.pid"117 pidfile = "/run/squid.pid"
120 exe = "squid3"118 exe = "squid"
121
122 if self.lsb_release['Release'] < 12.04:
123 pidfile = "/var/run/squid.pid"
124 exe = "squid"
125119
126 self.assertTrue(testlib.check_pidfile(exe, pidfile))120 self.assertTrue(testlib.check_pidfile(exe, pidfile))
127121

Subscribers

People subscribed via source and target branches