[SRU] Configuration should be purged only in nginx-common

Bug #1206878 reported by Klaus Ethgen
32
This bug affects 4 people
Affects Status Importance Assigned to Milestone
nginx (Debian)
Fix Released
Unknown
nginx (Ubuntu)
Fix Released
Critical
Unassigned
Precise
Fix Released
Critical
Unassigned

Bug Description

[Impact]

 * When purging nginx-light, nginx-full, or nginx-naxsi with apt-get remove --purge (or similar), the /etc/nginx/ folder and all configurations are removed.

 * nginx-common should be the package that, when purged, will remove the configuration data as well as the program.

 * Removal of a user's configuration when the -common package is not removed is a severe issue, and when Thomas Ward consulted with Clint Byrum on bug importance, this was determined to be a "Critical" bug as user configuration removals should not be happening in this case.

[Test Cases]

Make sure to run all three test cases. The third one, in particular, was not fixed with the others at the same time in Debian, and relies on a separate commit. --rbasak

[Test Case 1]

 * Produced by purging nginx-full, nginx-light, or nginx-naxsi.

 * By running a purge of nginx-full or nginx-light or nginx-naxsi without removing nginx-common, the system should not be removing a user's configuration files for nginx. Only if nginx-common is removed should the system

[Test Case 2 (added by rbasak)]
1. apt-get install nginx
2. dpkg -P nginx nginx-full
3. apt-get install nginx

Expected results: nginx-full installed again and functional, with the daemon running.
Actual results: nginx-full postinst failure due to missing /etc/nginx directory causing daemon start failure.

[Test Case 3 (added by rbasak)]
1. apt-get install nginx-extras
2. dpkg -P nginx-extras
3. apt-get install nginx-extras

Expected results: nginx-extras installed again and functional, with the daemon running.
Actual results: nginx-extras postinst failure due to missing /etc/nginx directory causing daemon start failure.

[Regression Potential]

 * Minimal - The patch from Debian has been in the NGINX packages since 1.2.1 and is also included in Quantal and later with no regressions.

Added by rbasak:

Consider the upgrade path carefully. For example: if the nginx-common binary package is held back, and other binaries are upgraded with this SRU, then it's possible that purging functionality will disappear and the old nginx-common binary package could then be purged and /etc/nginx would not be cleaned up. I [rbasak] don't think this case is worth considering in an SRU though, since at worse it'll leave /etc/nginx behind in a very unusual case, which shouldn't have any other repercussions anyway.

Only postinsts are changed here. The nginx-{full,light,naxsi} postrms are removed, since they didn't have any other functionality. The purge functionality is wholesale added to nginx-common's postrm. So I expect that any regressions will affect maintainer scripts only, not in functionality of the package once configured and installed. If something does go wrong, it will be with users' configurations being affected, but should only happen when they purge a package, which is unlikely in production.

Finally, make sure to check nginx-extras separately, as this was missed in Debian the first time round.

[Other Info]

 * Patch to fix this issue (to be attached here shortly) originated from Debian.

 * Debdiffs to be attached after build-tests are completed.

------

Original Description:

nginx in ubuntu has a very violently bug when purging one of the packages
nginx-full, nginx-light or nginx-naxsi while moving from light to full or other
ways. The packages -light, -full and -naxsi will remove /etc/nginx,
/var/log/nginx and /var/lib/nginx completely leaving the system broken. The
directories are not owned by this respective packages at all then owned by
nginx-common. So after such a transition the whole nginx installation is broken
and one has to also first to purge nginx-common and reinstalling it.

Unneeded to say that all own site configurations has been deleted too.

This is a massive violation of debian policy. However, debian had never has
this bug in one of there stable distributions, so this bug only affects ubuntu
(12.04 LTS was checked by me)

Revision history for this message
Klaus Ethgen (klaus+ubuntu) wrote :

As there is no field to note the version that is affected, here it is:
1.1.19-1ubuntu0.2

Ah, and there is an old debian bug report #678060 that was handling this issue.

description: updated
Revision history for this message
Thomas Ward (teward) wrote :

Thanks for the report.

I am aware of this bug having cropped up in Debian. I'll take a look at this after my vacation is over since I can't bug fix at the beach.

And for future bug reporting, we do have an Ubuntu bug reporting tool, called 'ubuntu-bug' which can be called from the terminal on Ubuntu machines, though, which helps to make bug reporting a little easier.

Revision history for this message
Klaus Ethgen (klaus+ubuntu) wrote :

Just a note about "ubuntu-bug": I tryed to get it on a precise (12.04) system but failed. There is no ubuntu-bug installed and apt-file do not find it at all.

Revision history for this message
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in nginx (Ubuntu):
status: New → Confirmed
Changed in nginx (Debian):
status: Unknown → Fix Released
Revision history for this message
Thomas Ward (teward) wrote :

Klaus, before I go poking around and working on fixing these, do you know if any other versions of nginx in any of the other Ubuntu releases suffer from this same violation of Debian policy?

Changed in nginx (Ubuntu Precise):
status: New → Confirmed
Changed in nginx (Ubuntu):
importance: Undecided → Critical
Changed in nginx (Ubuntu Precise):
importance: Undecided → Critical
Revision history for this message
Thomas Ward (teward) wrote :

Marking fixed released for any other versions other than Precise, because 1.2.1-2 (and later) was imported from Debian to the Ubuntu repositories, and therefore those versions have the fix for this, courtesy of Debian.

Changed in nginx (Ubuntu):
status: Confirmed → Fix Released
Changed in nginx (Ubuntu Precise):
assignee: nobody → Thomas Ward (teward)
status: Confirmed → In Progress
Revision history for this message
Thomas Ward (teward) wrote :

Added SRU template in preparation for SRU.

description: updated
Revision history for this message
Thomas Ward (teward) wrote :

Attached patch is on the Debian bug, and will fix this issue. Patch and related code already included in Quantal and later.

Revision history for this message
Thomas Ward (teward) wrote :

I have build-tested this as is with the patch attached in comment #8 within a PPA, adding ~ppa0 to the end only for build testing. https://launchpad.net/~teward/+archive/nginx-ubuntu-updates/+packages is where the packages were built.

The packages have built successfully, so I will upload a debdiff to this bug shortly.

Revision history for this message
Thomas Ward (teward) wrote :

Attached debdiff includes the code applied by the nginx.patch file, and contains a changelog entry for this SRU.

Changed in nginx (Ubuntu Precise):
assignee: Thomas Ward (teward) → nobody
status: In Progress → Triaged
Thomas Ward (teward)
summary: - Configuration should be purged only in nginx-common
+ [SRU] Configuration should be purged only in nginx-common
Robie Basak (racb)
description: updated
Revision history for this message
Robie Basak (racb) wrote :

@Thomas

Thanks, this looks good.

One problem: nginx-extras.postrm suffers from the same problem and was fixed in Debian bug http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=681758 (http://anonscm.debian.org/gitweb/?p=collab-maint/nginx.git;a=commit;h=dc1813f6207cf54ff1565ace6304102195b2e4a1). It looks like you missed this, so I've amended my upload to also remove debian/nginx-extras.postrm.

I'm not sure I agree with Clint's Critical priority designation, but given that Thomas has been kind enough to quickly prepare an SRU, I don't think it's worth debating. Let's just get the fix in. Uploaded.

Revision history for this message
Thomas Ward (teward) wrote :

Robie: Ahhhh, thanks for catching my mistake. I forgot all about -extras. Thanks for amending the upload to have a fix for -extras as well.

Revision history for this message
Brian Murray (brian-murray) wrote : Please test proposed package

Hello Klaus, or anyone else affected,

Accepted nginx into precise-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/nginx/1.1.19-1ubuntu0.3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in nginx (Ubuntu Precise):
status: Triaged → Fix Committed
tags: added: verification-needed
Revision history for this message
Thomas Ward (teward) wrote :

After testing what's in proposed, this patch actually fails. Upon doing testing:

---

WITH PRECISE 1.1.19-1ubuntu0.2:
sudo apt-get install nginx-full nginx-common; sudo apt-get purge nginx-full; sudo apt-get install nginx-full - FAILS (this bug)

---

WITH PRECISE 1.1.19-1ubuntu0.3:
sudo apt-get install nginx-full/precise-proposed nginx-common/precise-proposed; sudo apt-get purge nginx-full; sudo apt-get install nginx-full/precise-proposed - FAILS:

Setting up nginx-full (1.1.19-1ubuntu0.3) ...
ln: failed to create symbolic link `/etc/nginx/sites-enabled/default': File exists

The nginx-full (and also the nginx binary installers) are trying to write the configuration files and create symlinks even though they already exist. This should be done in nginx-common, not nginx-full. This introduces a new bug, and therefore this is verification-failed.

tags: added: verification-failed
removed: verification-needed
Thomas Ward (teward)
Changed in nginx (Ubuntu Precise):
status: Fix Committed → Triaged
Revision history for this message
Thomas Ward (teward) wrote :

Here's a replacement debdiff, based off of what was in proposed that failed (1.1.19-1ubuntu0.3), which bumps the version numbering to 1.1.19-1ubuntu0.4 and resolves the issue of the nginx-light, nginx-full, nginx-extras, and nginx-naxsi packages attempting to create a symlink for the default site configuration file and failing because it already exists.

That bug was introduced by the verification-failed debdiff that was attached here (1.1.19-1ubuntu0.3) because removing the "purge" of configs from the nginx-light, nginx-full, nginx-extras, and nginx-naxsi packages would prevent the symlinks and such from being deleted.

This debdiff removes that functionality from the specified packages' postinst files, and adds the functionality to the nginx-common package, which is the only package that should do that.

When running the test-built version of this package through the three specified test-cases here on this SRU bug, this version of the package with the attached debdiff applied will NOT trigger the new bug, and will repair the original bug that's the focus of this SRU.

Thomas Ward (teward)
tags: removed: verification-failed
Revision history for this message
Iain Lane (laney) wrote :

Uploaded with some tweaks, thanks.

Changed in nginx (Ubuntu Precise):
status: Triaged → In Progress
assignee: nobody → Iain Lane (laney)
assignee: Iain Lane (laney) → nobody
Revision history for this message
Brian Murray (brian-murray) wrote :

Hello Klaus, or anyone else affected,

Accepted nginx into precise-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/nginx/1.1.19-1ubuntu0.4 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in nginx (Ubuntu Precise):
status: In Progress → Fix Committed
tags: added: verification-needed
Revision history for this message
Thomas Ward (teward) wrote :

I've tested 1.1.19-1ubuntu0.4 in precise-proposed in a newly-installed Precise VM, and this bug is indeed resolved by 1.1.19-1ubuntu0.4.

tags: added: verification-done
removed: verification-needed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package nginx - 1.1.19-1ubuntu0.4

---------------
nginx (1.1.19-1ubuntu0.4) precise; urgency=low

  [ Thomas Ward ]
  * Move postinst symlinking of default nginx config to nginx-common only.
    (closes LP: #1206878)

  [ Iain Lane ]
  * Take additional change from Debian patch to check sites-enabled and
    sites-available are directories before symlinking .../default.
 -- Thomas Ward <email address hidden> Thu, 10 Oct 2013 10:48:16 +0100

Changed in nginx (Ubuntu Precise):
status: Fix Committed → Fix Released
Revision history for this message
Brian Murray (brian-murray) wrote : Update Released

The verification of this Stable Release Update has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regresssions.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.