sendmail_path in LXC, and better op_del #85
No reviewers
Labels
No labels
Bug
Doc
Feature
Forge
Mode
Cluster
Mode
MultiPHP
Script
Server
Web
wontfix
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: evolix/evoadmin-web#85
Loading…
Add table
Reference in a new issue
No description provided.
Delete branch "unstable"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Tested on test-www01
unstableto ITK 4 all, sendmail_path in LXC, and better op_delThis was intentional.
Curently, this (via web-add.sh enable-user-itk & disable-user-itk) will only toggle the
AssignUserID
value in the Apache vhost file of the account (adding or removing thewww-
prefixWhile in mono-PHP setup this is enough, in multi-PHP setup it's only missleading because the user of execution of the PHP code will the be the one defined in the PHP-FPM pool config. Toggling it in that case will only create confusion.
So
2fd65724f7
(+c385c102c5
) should be reverted as long as web-add.sh does not do the change to FPM Pool configSounds good, looking forward to test this :)
@ -109,4 +109,2 @@
if(is_multiphp()) {
printf('<a href="/webadmin/%s/php/">PHP</a> - ', $vhost_info['owner']);
} else {
printf('<a href="/webadmin/%s/itk/">ITK</a> - ', $vhost_info['owner']);
From @lpoujol :
While I do agree with @lpoujol that it would be better not to fail if the web-account was already (partially) removed, the implementation doesn't seems to be correct.
@ -807,3 +810,3 @@
if [ "$WEB_SERVER" == "apache" ]; then
if id www-"$login" &> /dev/null; then
userdel -f www-"$login"
userdel -f www-"$login" || true
Considering the
if id www-"$login"
means thatwww-"$login"
do exist for the system root, failure on this userdel seems to be a real problem (it is already bypassed if the user doesn't exists).@ -812,3 +815,3 @@
for php_version in "${PHP_VERSIONS[@]}"; do
if lxc-attach -n php"${php_version}" -- id www-"$login" &> /dev/null; then
lxc-attach -n php"${php_version}" -- userdel -f www-"$login"
lxc-attach -n php"${php_version}" -- userdel -f www-"$login" || true
Wouldn't it be better to check for the user existence in the container instead of silencing all errors ?
I would do something like this :
or
@ -815,2 +817,3 @@
lxc-attach -n php"${php_version}" -- userdel -f www-"$login" || true
fi
lxc-attach -n php"${php_version}" -- userdel -f "$login"
lxc-attach -n php"${php_version}" -- userdel -f "$login" || true
Wouldn't it be better to check for the user existence in the container instead of silencing all errors ?
I would do something like this :
or
@ -818,3 +821,3 @@
fi
userdel -f "$login"
userdel -f "$login" || true
Wouldn't it be better to check for the user existence in the container instead of silencing all errors ?
I would do something like this :
(or use an if block)
On
sendmail_path
andopen_basedir
, it looks good.@ -17,2 +21,4 @@
### Fixed
* Fix sendmail_path hostname (missing domain / FQDN)
Since
sendmail_path
wasn't defined before this PR, should we really have this in the changelog ?Perhaps, but the change seems to only affect what was added in this PR. At least according to the diffs.
Your right, it was a commit that fixed my previous commit, but this should'nt be in the changlog.
@ -457,2 +458,4 @@
php_admin_value[error_log] = /home/${in_login}/log/php.log
php_admin_value[sendmail_path] = "/usr/sbin/sendmail -t -i -f www-${in_login}@${HOST}"
php_admin_value[open_basedir] = "/usr/share/php:/home/${in_login}:/tmp"
Seems good.
Oops, should have just left a comment, see above
@lpoujol @mtrossevin : Thanks for your remarks.
I'll implement ITK for multiPHP in another PR (shouldn't be too complicated, with sed in the containers). I revert the present change.
@mtrossevin : For
getent passwd <LOGIN>
, I'd rather usegetent passwd <LOGIN> > /dev/null
, don't you think ?@mtrossevin : For
sendmail_path
, it is already defined in the vhost template for php mod, but was missing for FPM in the containers.Done !
(and account removal tested on test-www01)
All good for me.
@mtrossevin ^
lgtm
ITK 4 all, sendmail_path in LXC, and better op_delto sendmail_path in LXC, and better op_del