Re: [Freeipa-devel] [PATCH] 0025 Respect UID and GID soft static allocation.
On 11/03/2014 02:04 PM, Martin Basti wrote: On 03/11/14 10:28, David Kupka wrote: On 10/30/2014 10:42 AM, Martin Basti wrote: On 29/10/14 17:23, David Kupka wrote: On 10/29/2014 02:34 PM, David Kupka wrote: 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 ... ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel In your former patch you had pki homedir path VAR_LIB_PKI_DIR : +if name == 'pkiuser': +uid = 17 +gid = 17 +homedir = paths.VAR_LIB_PKI_DIR +shell = paths.NOLOGIN +comment = 'CA System User' in last patch you change it back to: homedir=paths.VAR_LIB, so what is the correct path? The setup package (soft static allocation) claims that pkiuser should use '/usr/share/pki' as home directory. Since pkiuser has /sbin/nologin set as a login shell it's unable to login and does't need home directory at all. We could use '--system' option of useradd utility to skip home directory creation or change to proposed value or just leave the old value and all will result in no change in behavior. I'm not sure if the '--system' option is available universally. IIRC it used to be Red Hat-like-systems specific extension. ACK Pushed to: master: 364d466fd7def3589ddb9e4a9f8d73fc2df80439 ipa-4-1: 71c24b187a8d4b8990c0899d2c907d600b7bcc21 Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0025 Respect UID and GID soft static allocation.
On 03/11/14 10:28, David Kupka wrote: On 10/30/2014 10:42 AM, Martin Basti wrote: On 29/10/14 17:23, David Kupka wrote: On 10/29/2014 02:34 PM, David Kupka wrote: 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 ... ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel In your former patch you had pki homedir path VAR_LIB_PKI_DIR : +if name == 'pkiuser': +uid = 17 +gid = 17 +homedir = paths.VAR_LIB_PKI_DIR +shell = paths.NOLOGIN +comment = 'CA System User' in last patch you change it back to: homedir=paths.VAR_LIB, so what is the correct path? The setup package (soft static allocation) claims that pkiuser should use '/usr/share/pki' as home directory. Since pkiuser has /sbin/nologin set as a login shell it's unable to login and does't need home directory at all. We could use '--system' option of useradd utility to skip home directory creation or change to proposed value or just leave the old value and all will result in no change in behavior. I'm not sure if the '--system' option is available universally. IIRC it used to be Red Hat-like-systems specific extension. If there is no reason to change homedir, don't do it. I will continue with reviewing then. Martin^2 -- Martin Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0025 Respect UID and GID soft static allocation.
On 03/11/14 10:28, David Kupka wrote: On 10/30/2014 10:42 AM, Martin Basti wrote: On 29/10/14 17:23, David Kupka wrote: On 10/29/2014 02:34 PM, David Kupka wrote: 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 ... ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel In your former patch you had pki homedir path VAR_LIB_PKI_DIR : +if name == 'pkiuser': +uid = 17 +gid = 17 +homedir = paths.VAR_LIB_PKI_DIR +shell = paths.NOLOGIN +comment = 'CA System User' in last patch you change it back to: homedir=paths.VAR_LIB, so what is the correct path? The setup package (soft static allocation) claims that pkiuser should use '/usr/share/pki' as home directory. Since pkiuser has /sbin/nologin set as a login shell it's unable to login and does't need home directory at all. We could use '--system' option of useradd utility to skip home directory creation or change to proposed value or just leave the old value and all will result in no change in behavior. I'm not sure if the '--system' option is available universally. IIRC it used to be Red Hat-like-systems specific extension. ACK -- Martin Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0025 Respect UID and GID soft static allocation.
On 29/10/14 17:23, David Kupka wrote: On 10/29/2014 02:34 PM, David Kupka wrote: 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 ... ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel In your former patch you had pki homedir path VAR_LIB_PKI_DIR : +if name == 'pkiuser': +uid = 17 +gid = 17 +homedir = paths.VAR_LIB_PKI_DIR +shell = paths.NOLOGIN +comment = 'CA System User' in last patch you change it back to: homedir=paths.VAR_LIB, so what is the correct path? -- Martin Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0025 Respect UID and GID soft static allocation.
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 16d90a6d1a7d3d9aced5de82a5c1efe6b8c2..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
Re: [Freeipa-devel] [PATCH] 0025 Respect UID and GID soft static allocation.
On 10/29/2014 02:34 PM, David Kupka wrote:
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 ...
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
--
David Kupka
From ec277c8d8d9fc66f31236f306e038d39aa7417fb 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 | 23 +++
ipaserver/install/cainstance.py | 2 +-
ipaserver/install/dsinstance.py | 2 +-
ipaserver/install/installutils.py | 42 --
5 files changed, 73 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 16d90a6d1a7d3d9aced5de82a5c1efe6b8c2..3f5fc90b4b988d44cf0dcaa5a8d569a3c5f453ee 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -393,5 +393,28 @@ 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
Re: [Freeipa-devel] [PATCH] 0025 Respect UID and GID soft static allocation.
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, +] 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) -- Martin Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0025 Respect UID and GID soft static allocation.
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.
--
David Kupka
From 104a196f619d549e3c53fa50df4199535d86fe32 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 | 22 ++
ipaserver/install/cainstance.py | 2 +-
ipaserver/install/dsinstance.py | 2 +-
ipaserver/install/installutils.py | 42 --
5 files changed, 72 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 16d90a6d1a7d3d9aced5de82a5c1efe6b8c2..c37f6e56853382186092d645538e635d49306f87 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -393,5 +393,27 @@ class RedHatTaskNamespace(BaseTaskNamespace):
return True
+def create_system_user(self, name, group, homedir, shell):
+
+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.
+
+uid = gid = comment = None
+if name == 'pkiuser':
+uid = 17
+gid = 17
+homedir = paths.VAR_LIB_PKI_DIR
+shell = paths.NOLOGIN
+comment = 'CA System User'
+if name == 'dirsrv':
+comment = 'DS System User'
+
+BaseTaskNamespace.create_system_user(self, name, group, homedir,
+shell, uid, gid, comment)
+
tasks = RedHatTaskNamespace()
diff --git
Re: [Freeipa-devel] [PATCH] 0025 Respect UID and GID soft static allocation.
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.
--
David Kupka
From d91e9fedd61780793981f347f529280b86fcca97 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 16d90a6d1a7d3d9aced5de82a5c1efe6b8c2..2c7cafae358f74dc45424f90fc702eef844ac0df 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':
+uid = 17
+gid = 17
+homedir = paths.VAR_LIB_PKI_DIR
+shell = paths.NOLOGIN
+comment = 'CA System User'
+
