Merge ~utkarsh/ubuntu/+source/isc-dhcp:lp1894172-fix-env-focal into ubuntu/+source/isc-dhcp:ubuntu/focal

[Utkarsh Gupta]

This MR fixes env variables for $INTERFACES (LP: #1894172).

PPA: https://launchpad.net/~utkarsh/+archive/ubuntu/experimental-dump/

Unfortunately there weren't any tests in this package but the change is trivial and I've done a manual test of this.

Should you need any more details, let me know. Requesting you to please review and sponsor the upload.

[Sergio Durigan Junior (sergiodj)]

Thanks for the MP, Utkarsh.

As we've discussed and agreed on the fix below, I don't think I have anything else to say about the technical side; my only comment is about style.

As for the SRU template, good job! If I'm reading correctly, it seems like you have addressed the comments made by Robie during the standup and included a few sentences specifically mentioning how the issue impacts users. I have only one suggestion to make about the template.

In the "Test Case" section, my personal preference is to write an actual "recipe" of how to reproduce the problem, as in "real commands that you can actually copy-and-paste into a terminal". You can see one example of what I usually do here:

https://bugs.launchpad.net/ubuntu/+source/openldap/+bug/1557157

Anyway, as I said, this is just a suggestion. Your "Test Case" section is good as-is, and you don't need to rewrite it if you don't want.

I'm approving this MP, but I'm not sponsoring the upload (yet) because it's too late here and I'd like to take a fresher look in the morning. :-)

review: Approve

[Utkarsh Gupta (utkarsh)]

Awesome, thanks for the pointers, Sergio. Your comment about writing actual commands definitely make sense (and I should've used them in the first place) but definitely a good thing that you mentioned about this.

Whilst I'll definitely adapt to this style (your pointers) from now on but I'll still try to edit this bug description as well.

Once again, thanks for taking a look and your feedback! :D

[Utkarsh Gupta (utkarsh)]

> ... I'll still try to edit this bug description as well.

Done! The bug description has been adapted to your (great) suggestion!

[Christian Ehrhardt  (paelzer)]

Also LGTM now - I think that the state of this is good enough to prep the MPs for Bionc/Groovy as well. Because we usually would want to review&sponsor them as one (if discussion is expected it is wise to start with one, settle on it and then open the others - to avoid reworking too much).
But e.g. we'd not be allowed to SRU this to focal without also fixing groovy (or it would break on upgrade).
Therefore my suggestion would be to open bionic/groovy MPs as well now.

[Christian Ehrhardt  (paelzer)]

I'd have one more statement suggestion for the "where problems occur":

If a user played with INTERFACESv4/INTERFACESv6 without them having an effect and left them set actually wanting the "listen on all interface behavior" then the fix will change behavior on upgrade (by finally following the config that is in place).

[Utkarsh Gupta (utkarsh)]

> Therefore my suggestion would be to open bionic/groovy MPs as
> well now.

Yep, those two shall land today. Before pre- or post-standup, which I am likely to miss.

[Utkarsh Gupta (utkarsh)]

> If a user played with INTERFACESv4/INTERFACESv6 without them having
> an effect and left them set actually wanting the "listen on all
> interface behavior" then the fix will change behavior on upgrade
> (by finally following the config that is in place).

True but listening on all interfaces this way is more of a bug than a feature, I believe. The right way would be to set and use both IPv4 and IPv6. So if users *just* did a workaround because of this bug, then yes, they'd see a slightly changed behavior. Not a major pain-point, TBH.

[Sergio Durigan Junior (sergiodj)]

Thanks, pushed and uploaded.

$ git push pkg upload/4.4.1-2.1ubuntu6
Enumerating objects: 15, done.
Counting objects: 100% (15/15), done.
Delta compression using up to 8 threads
Compressing objects: 100% (10/10), done.
Writing objects: 100% (10/10), 1.39 KiB | 177.00 KiB/s, done.
Total 10 (delta 7), reused 0 (delta 0)
To ssh://git.launchpad.net/ubuntu/+source/isc-dhcp
 * [new tag] upload/4.4.1-2.1ubuntu6 -> upload/4.4.1-2.1ubuntu6

$ dput isc-dhcp_4.4.1-2.1ubuntu6_source.changes
Trying to upload package to ubuntu
Checking signature on .changes
gpg: /home/sergio/work/isc-dhcp/isc-dhcp_4.4.1-2.1ubuntu6_source.changes: Valid signature from 106DA1C8C3CBBF14
Checking signature on .dsc
gpg: /home/sergio/work/isc-dhcp/isc-dhcp_4.4.1-2.1ubuntu6.dsc: Valid signature from 106DA1C8C3CBBF14
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading isc-dhcp_4.4.1-2.1ubuntu6.dsc: done.
  Uploading isc-dhcp_4.4.1-2.1ubuntu6.debian.tar.xz: done.
  Uploading isc-dhcp_4.4.1-2.1ubuntu6_source.buildinfo: done.
  Uploading isc-dhcp_4.4.1-2.1ubuntu6_source.changes: done.
Successfully uploaded packages.

[Sergio Durigan Junior (sergiodj)]

Ops, the upload got rejected because there is already a isc-dhcp_4.4.1-2.1ubuntu6 in the archive. Unfortunately this package is not following the conventions for version numbers in Ubuntu; you can see that the isc-dhcp in Groovy is using the exact same version numbers as the one in Focal, so this conflict was bound to happen :-/.

Anyway, you will have to change the version here. The following page contains good resources on this:

https://wiki.ubuntu.com/SecurityTeam/UpdatePreparation

In a nutshell, the new version will have to be 4.4.1-2.1ubuntu5.20.04.1, because this will be a Focal-specific version.

review: Needs Fixing

[Utkarsh Gupta (utkarsh)]

Aaah. I see. Hopefully I'll get my head around to adapt the versioning scheme. Thanks and fixed!

[Sergio Durigan Junior (sergiodj)]

OK, now uploaded & accepted for real:

$ git push pkg upload/4.4.1-2.1ubuntu5.20.04.1
Enumerating objects: 15, done.
Counting objects: 100% (15/15), done.
Delta compression using up to 8 threads
Compressing objects: 100% (10/10), done.
Writing objects: 100% (10/10), 1.41 KiB | 288.00 KiB/s, done.
Total 10 (delta 7), reused 0 (delta 0)
To ssh://git.launchpad.net/ubuntu/+source/isc-dhcp
 * [new tag] upload/4.4.1-2.1ubuntu5.20.04.1 -> upload/4.4.1-2.1ubuntu5.20.04.1

$ dput isc-dhcp_4.4.1-2.1ubuntu5.20.04.1_source.changes
Trying to upload package to ubuntu
Checking signature on .changes
gpg: /home/sergio/work/isc-dhcp/isc-dhcp_4.4.1-2.1ubuntu5.20.04.1_source.changes: Valid signature from 106DA1C8C3CBBF14
Checking signature on .dsc
gpg: /home/sergio/work/isc-dhcp/isc-dhcp_4.4.1-2.1ubuntu5.20.04.1.dsc: Valid signature from 106DA1C8C3CBBF14
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading isc-dhcp_4.4.1-2.1ubuntu5.20.04.1.dsc: done.
  Uploading isc-dhcp_4.4.1-2.1ubuntu5.20.04.1.debian.tar.xz: done.
  Uploading isc-dhcp_4.4.1-2.1ubuntu5.20.04.1_source.buildinfo: done.
  Uploading isc-dhcp_4.4.1-2.1ubuntu5.20.04.1_source.changes: done.
Successfully uploaded packages.

diff --git a/debian/changelog b/debian/changelog
index 399eeb2..3646525 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
+isc-dhcp (4.4.1-2.1ubuntu5.20.04.1) focal; urgency=medium
+
+  * Fix env variable for INTERFACES (LP: #1894172)
+    - d/isc-dhcp-server.isc-dhcp-server{,6}.service: Replace $INTERFACES
+      variable with $INTERFACEv4 and $INTERFACESv6, respectively, for
+      respective services file.
+
+ -- Utkarsh Gupta <[e-mail]>  Tue, 09 Mar 2021 20:49:45 +0530
+
 isc-dhcp (4.4.1-2.1ubuntu5) focal; urgency=medium
 
   * debian/apparmor/usr.sbin.dhcpd: also allow write on /proc/*/comm and
diff --git a/debian/isc-dhcp-server.isc-dhcp-server.service b/debian/isc-dhcp-server.isc-dhcp-server.service
index b3e2794..4e7fe42 100644
--- a/debian/isc-dhcp-server.isc-dhcp-server.service
+++ b/debian/isc-dhcp-server.isc-dhcp-server.service
@@ -18,7 +18,8 @@ ExecStart=/bin/sh -ec '\
     [ -e /var/lib/dhcp/dhcpd.leases ] || touch /var/lib/dhcp/dhcpd.leases; \
     chown root:dhcpd /var/lib/dhcp /var/lib/dhcp/dhcpd.leases; \
     chmod 775 /var/lib/dhcp ; chmod 664 /var/lib/dhcp/dhcpd.leases; \
-    exec dhcpd -user dhcpd -group dhcpd -f -4 -pf /run/dhcp-server/dhcpd.pid -cf $CONFIG_FILE $INTERFACES'
+    if test -n "$INTERFACES" -a -z "$INTERFACESv4"; then INTERFACESv4="$INTERFACES"; fi; \
+    exec dhcpd -user dhcpd -group dhcpd -f -4 -pf /run/dhcp-server/dhcpd.pid -cf $CONFIG_FILE $INTERFACESv4'
 
 [Install]
 WantedBy=multi-user.target
diff --git a/debian/isc-dhcp-server.isc-dhcp-server6.service b/debian/isc-dhcp-server.isc-dhcp-server6.service
index e3b0828..8593fda 100644
--- a/debian/isc-dhcp-server.isc-dhcp-server6.service
+++ b/debian/isc-dhcp-server.isc-dhcp-server6.service
@@ -18,7 +18,8 @@ ExecStart=/bin/sh -ec '\
     [ -e /var/lib/dhcp/dhcpd6.leases ] || touch /var/lib/dhcp/dhcpd6.leases; \
     chown root:dhcpd /var/lib/dhcp /var/lib/dhcp/dhcpd6.leases; \
     chmod 775 /var/lib/dhcp ; chmod 664 /var/lib/dhcp/dhcpd6.leases; \
-    exec dhcpd -user dhcpd -group dhcpd -f -6 -pf /run/dhcp-server/dhcpd6.pid -cf $CONFIG_FILE $INTERFACES'
+    if test -n "$INTERFACES" -a -z "$INTERFACESv6"; then INTERFACESv6="$INTERFACES"; fi; \
+    exec dhcpd -user dhcpd -group dhcpd -f -6 -pf /run/dhcp-server/dhcpd6.pid -cf $CONFIG_FILE $INTERFACESv6'
 
 [Install]
 WantedBy=multi-user.target