Opened 4 years ago

Closed 4 years ago

#3062 closed defect (fixed)

Webserver resource ownership broken

Reported by: matze Assignee: matze
Priority: P1 Milestone:
Module: Infrastructure Keywords:
Cc: fhd, fred, sebastian Blocked By:
Blocking: #3016 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29327575
https://codereview.adblockplus.org/29328011

Description

An excerpt from the Vagrant log:

$ vagrant provision filter1 2>&1 | tee /tmp/filter1.log | grep err:
==> filter1: err: /Stage[main]/Notificationserver/Exec[fetch_notifications]/returns: change from notrun to 0 failed: hg clone -
-noupdate https://hg.adblockplus.org/notifications /opt/notifications && chown -R nginx /opt/notifications returned 1 instead of one of [0] at /tmp/vagrant-puppet-2/modules-0/notificationserver/manifests/init.pp:21

The access logs for https://hg.adblockplus.org/notifications show that the requests during the hg clone where served properly. Thus I first assumed this being yet another ordering issue (the nginx user may not have existed yet when the subsequent chown command was issued), but it turns out to be a bit more complicated:

  1. We explicitly operate Nginx as user www-data on all our servers. Thus changing the owner of the repository to user nginx does not seem to make much sense. Furthermore, the only processes I found run by user nginx are the multiplexer.fcgi handlers from module sitescripts.
  1. The package provided by the Nginx PPA (#3016) does not create user nginx any more, but relies on root and www-data only. Thus boxes where the former package was not installed before fail to provision due to a missing system user.

The proper fix is quite trivial: let user www-data operate these modules exclusively. The roll-out, however, again requires special caution in form of proper migration resources or manual intervention.
The alternative would be an explicit resource that ensures user nginx is present.

Note that while the latter is not as clean, I still consider it preferable right now: until #3016 is solved, we should not introduce more complexity here. Thus this ticket while result in two separate patch-sets. The first will implement the alternative or quick fix. The second will follow #3016, migrating the resource ownership.

Change History (6)

comment:1 Changed 4 years ago by matze

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:2 Changed 4 years ago by matze

  • Review URL(s) modified (diff)

The first patch-set has been pushed, another one (regarding /var/cache/nginx) is in review now.

comment:3 Changed 4 years ago by matze

  • Blocking 3083 added

comment:5 Changed 4 years ago by matze

  • Blocking 3083 removed

comment:6 Changed 4 years ago by matze

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.