On 10/24/2014 03:05 PM, David Kupka wrote:
On 10/24/2014 01:06 PM, David Kupka wrote:
On 10/24/2014 10:43 AM, Martin Basti wrote:
On 24/10/14 09:51, David Kupka wrote:
https://fedorahosted.org/freeipa/ticket/4585
NACK
1)
Why is there line with 'DS System User?' The comment should depend on
service.
+ args = [
+ paths.USERADD,
+ '-g', group,
+ '-c', 'DS System User',
+ '-d', homedir,
+ '-s', shell,
+ '-M', '-r', name,
+ ]
This was part of the original code and I didn't notice it. Nice catch,
thanks.
2)
code create_system_user is duplicated between base and redhat tasks with
platform dependent changes.
IMO it would be better to have one method to create user, with keyword
arguments. And then platform dependent method which will call method to
create user with appropriate arguments (or with default arguments)
You're right it was ugly.
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
I shouldn't break SOLID principles.
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Using super is probably better that explicit naming of parent class.
Let user (developer) override UID/GID and hope that he knows why ...
--
David Kupka
From da0e375ac81d297a78649de7be98e8610aa83dcc Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Wed, 22 Oct 2014 09:07:44 -0400
Subject: [PATCH] Respect UID and GID soft static allocation.
https://fedoraproject.org/wiki/Packaging:UsersAndGroups?rd=Packaging/UsersAndGroups#Soft_static_allocation
https://fedorahosted.org/freeipa/ticket/4585
---
ipaplatform/base/tasks.py | 48 +++++++++++++++++++++++++++++++++++++++
ipaplatform/redhat/tasks.py | 21 +++++++++++++++++
ipaserver/install/cainstance.py | 2 +-
ipaserver/install/dsinstance.py | 2 +-
ipaserver/install/installutils.py | 42 ----------------------------------
5 files changed, 71 insertions(+), 44 deletions(-)
diff --git a/ipaplatform/base/tasks.py b/ipaplatform/base/tasks.py
index 408447e43cd36d0cdf11a1877b3bc9880c4785de..f2ba81f44bb991b218232aad84d7810cdae839ef 100644
--- a/ipaplatform/base/tasks.py
+++ b/ipaplatform/base/tasks.py
@@ -22,7 +22,13 @@
This module contains default platform-specific implementations of system tasks.
'''
+import pwd
+import grp
from ipaplatform.paths import paths
+from ipapython.ipa_log_manager import log_mgr
+from ipapython import ipautil
+
+log = log_mgr.get_logger(__name__)
class BaseTaskNamespace(object):
@@ -150,5 +156,47 @@ class BaseTaskNamespace(object):
return
+ def create_system_user(self, name, group, homedir, shell, uid = None, gid = None, comment = None):
+ """Create a system user with a corresponding group"""
+ try:
+ grp.getgrnam(group)
+ except KeyError:
+ log.debug('Adding group %s', group)
+ args = [paths.GROUPADD, '-r', group]
+ if gid:
+ args += ['-g', str(gid)]
+ try:
+ ipautil.run(args)
+ log.debug('Done adding group')
+ except ipautil.CalledProcessError as e:
+ log.critical('Failed to add group: %s', e)
+ raise
+ else:
+ log.debug('group %s exists', group)
+
+ try:
+ pwd.getpwnam(name)
+ except KeyError:
+ log.debug('Adding user %s', name)
+ args = [
+ paths.USERADD,
+ '-g', group,
+ '-d', homedir,
+ '-s', shell,
+ '-M', '-r', name,
+ ]
+ if uid:
+ args += ['-u', str(uid)]
+ if comment:
+ args += ['-c', comment]
+ try:
+ ipautil.run(args)
+ log.debug('Done adding user')
+ except ipautil.CalledProcessError as e:
+ log.critical('Failed to add user: %s', e)
+ raise
+ else:
+ log.debug('user %s exists', name)
+
task_namespace = BaseTaskNamespace()
diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py
index 555516d90a6d1a7d3d9aced5de82a5c1efe6b8c2..7c03fe7cebaa7a5c5ac8715fe23991be7570ea75 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -393,5 +393,26 @@ class RedHatTaskNamespace(BaseTaskNamespace):
return True
+ def create_system_user(self, name, group, homedir, shell, uid = None, gid = None, comment = None):
+ """
+ Create a system user with a corresponding group
+
+ According to https://fedoraproject.org/wiki/Packaging:UsersAndGroups?rd=Packaging/UsersAndGroups#Soft_static_allocation
+ some system users should have fixed UID, GID and other parameters set.
+ This values should be constant and may be hardcoded.
+ Add other values for other users when needed.
+ """
+ if name == 'pkiuser':
+ if uid is None:
+ uid = 17
+ if gid is None:
+ gid = 17
+ comment = 'CA System User'
+ if name == 'dirsrv':
+ comment = 'DS System User'
+
+ super(RedHatTaskNamespace, self).create_system_user(self, name, group,
+ homedir, shell, uid, gid, comment)
+
tasks = RedHatTaskNamespace()
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 0c31d21648689ad5c577e9112fefdf47857b4915..6ccbe415eca08b8d3b3dc7b04c04cffb5fdf7459 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -261,7 +261,7 @@ def is_step_one_done():
def create_ca_user():
"""Create PKI user/group if it doesn't exist yet."""
- installutils.create_system_user(
+ tasks.create_system_user(
name=PKI_USER,
group=PKI_USER,
homedir=paths.VAR_LIB,
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index da535347117166e6cb445b0ebf14ad71787f72ba..4293c5c90e9b1371a417e1158d63013a6c3bcee3 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -152,7 +152,7 @@ def is_ds_running(server_id=''):
def create_ds_user():
"""Create DS user/group if it doesn't exist yet."""
- installutils.create_system_user(
+ tasks.create_system_user(
name=DS_USER,
group=DS_USER,
homedir=paths.VAR_LIB_DIRSRV,
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 1e010edc6bf569a879c4f68efd2b273f57b01770..9cda26f16f09ddc04e708a94858fb039ee3a8360 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -29,8 +29,6 @@ from ConfigParser import SafeConfigParser, NoOptionError
import traceback
import textwrap
from contextlib import contextmanager
-import pwd
-import grp
from dns import resolver, rdatatype
from dns.exception import DNSException
@@ -83,8 +81,6 @@ class ReplicaConfig:
subject_base = ipautil.dn_attribute_property('_subject_base')
-log = log_mgr.get_logger(__name__)
-
def get_fqdn():
fqdn = ""
try:
@@ -974,41 +970,3 @@ def load_external_cert(files, subject_base):
ca_file.flush()
return cert_file, ca_file
-
-
-def create_system_user(name, group, homedir, shell):
- """Create a system user with a corresponding group"""
- try:
- grp.getgrnam(group)
- except KeyError:
- log.debug('Adding group %s', group)
- args = [paths.GROUPADD, '-r', group]
- try:
- ipautil.run(args)
- log.debug('Done adding group')
- except ipautil.CalledProcessError as e:
- log.critical('Failed to add group: %s', e)
- raise
- else:
- log.debug('group %s exists', group)
-
- try:
- pwd.getpwnam(name)
- except KeyError:
- log.debug('Adding user %s', name)
- args = [
- paths.USERADD,
- '-g', group,
- '-c', 'DS System User',
- '-d', homedir,
- '-s', shell,
- '-M', '-r', name,
- ]
- try:
- ipautil.run(args)
- log.debug('Done adding user')
- except ipautil.CalledProcessError as e:
- log.critical('Failed to add user: %s', e)
- raise
- else:
- log.debug('user %s exists', name)
--
2.1.0
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
