On 07/30/2014 04:26 PM, Petr Viktorin wrote:
On 07/29/2014 06:03 PM, Petr Viktorin wrote:
On 07/29/2014 05:02 PM, Petr Viktorin wrote:
Hello,

The first patch here consolidates our system user creation code a bit.

The second patch fixes an oversight in the restore script.

The third changes the backup script to not include /etc/{passwd,group},
and the restore script to create the PKI user if a CA is being restored.
Note that the DS user is already created early in the restore process.
(In the future we may want a nice generic framework for restoring users,
but I'd like to extrapolate from more than one data point when designing
it.)

Another note: tar uses owner user/group names by default, so no
additional chowning is required even if the IDs change between backup &
restore.


The fourth patch adds a log entry I find very useful in testing
backup/restore.


Rebased onto current master.

Rebased again.


--
PetrĀ³
From 3f5bed20eb014df99cbc5f101f1edac4a00debe7 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Tue, 15 Jul 2014 13:31:01 +0200
Subject: [PATCH] ipaserver.install: Consolidate system user creation

Sytem users and their groups are always created together.
Also, users & groups should never be removed once they exist
on the system (see comit a5a55ce).

Use a single function for generic user creation, and specific
funtions in dsinstance and cainstance.
Remove code left over from when we used to delete the DS user.

Preparation for: https://fedorahosted.org/freeipa/ticket/3866
---
 install/tools/ipa-replica-install |  5 ++--
 install/tools/ipa-server-install  |  7 +++---
 ipaserver/install/cainstance.py   | 28 +++++++++-------------
 ipaserver/install/dsinstance.py   | 50 ++++++---------------------------------
 ipaserver/install/installutils.py | 44 +++++++++++++++++++++++++++++++++-
 ipaserver/install/ipa_restore.py  |  3 +--
 6 files changed, 68 insertions(+), 69 deletions(-)

diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index 7c9e27e2b9d248653681c85236d3541ec9ab0ec7..0445e59c23982b0d0e438f47c4e111fdb5dec977 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -573,9 +573,8 @@ def main():
     api.bootstrap(in_server=True, context='installer')
     api.finalize()
 
-    # Create DS group if it doesn't exist yet
-    group_exists = dsinstance.create_ds_group()
-    sstore.backup_state("install", "group_exists", group_exists)
+    # Create DS user/group if it doesn't exist yet
+    dsinstance.create_ds_user()
 
     #Automatically disable pkinit w/ dogtag until that is supported
     options.setup_pkinit = False
diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 6e77b434a018faec36a2808626c99a54bd493908..73055840af783213f7f2ce0230bb619b5801385f 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -560,7 +560,8 @@ def uninstall():
 
     ipaclient.ntpconf.restore_forced_ntpd(sstore)
 
-    group_exists = sstore.restore_state("install", "group_exists")
+    # Clean up group_exists (unused since IPA 2.2, not being set since 4.1)
+    sstore.restore_state("install", "group_exists")
 
     services.knownservices.ipa.disable()
 
@@ -1060,8 +1061,8 @@ def main():
         # configure /etc/sysconfig/network to contain the custom hostname
         tasks.backup_and_replace_hostname(fstore, sstore, host_name)
 
-    # Create DS group if it doesn't exist yet
-    dsinstance.create_ds_group()
+    # Create DS user/group if it doesn't exist yet
+    dsinstance.create_ds_user()
 
     # Create a directory server instance
     if external != 2:
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index e8bb7d701fb030e6cd11ad8637e51ca3b648e202..0998ee39101a86c396ef166bc67d19397f017346 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -249,6 +249,16 @@ def is_ca_installed_locally():
     return os.path.exists(path)
 
 
+def create_ca_user():
+    """Create PKI user/group if it doesn't exist yet."""
+    installutils.create_system_user(
+        name=PKI_USER,
+        group=PKI_USER,
+        homedir=paths.VAR_LIB,
+        shell=paths.NOLOGIN,
+    )
+
+
 class CADSInstance(service.Service):
     """Certificate Authority DS instance
 
@@ -396,7 +406,7 @@ def configure_instance(self, host_name, domain, dm_password,
             self.cert_chain_file = cert_chain_file
             self.external = 2
 
-        self.step("creating certificate server user", self.__create_ca_user)
+        self.step("creating certificate server user", create_ca_user)
         if self.dogtag_constants.DOGTAG_VERSION >= 10:
             self.step("configuring certificate server instance", self.__spawn_instance)
         else:
@@ -599,22 +609,6 @@ def create_instance(self):
         self.backup_state('installed', True)
         ipautil.run(args, env={'PKI_HOSTNAME':self.fqdn})
 
-    def __create_ca_user(self):
-        try:
-            pwd.getpwnam(PKI_USER)
-            self.log.debug("ca user %s exists", PKI_USER)
-        except KeyError:
-            self.log.debug("adding ca user %s", PKI_USER)
-            args = [paths.USERADD, "-c", "CA System User",
-                                         "-d", paths.VAR_LIB,
-                                         "-s", paths.NOLOGIN,
-                                         "-M", "-r", PKI_USER]
-            try:
-                ipautil.run(args)
-                self.log.debug("done adding user")
-            except ipautil.CalledProcessError, e:
-                self.log.critical("failed to add user %s", e)
-
     def __configure_instance(self):
         # Only used for Dogtag 9
         preop_pin = get_preop_pin(
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index 1719df46ddba740d94281505a45acc24c67c6cac..c9241b9ea1d487da12408b13685a6a3f70977cb9 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -26,7 +26,6 @@
 import time
 import tempfile
 import stat
-import grp
 
 from ipapython.ipa_log_manager import *
 from ipapython import ipautil, sysrestore, ipaldap
@@ -152,50 +151,15 @@ def is_ds_running(server_id=''):
 
 
 def create_ds_user():
-    """
-    Create DS user if it doesn't exist yet.
-    """
-    try:
-        pwd.getpwnam(DS_USER)
-        root_logger.debug('DS user %s exists', DS_USER)
-    except KeyError:
-        root_logger.debug('Adding DS user %s', DS_USER)
-        args = [
-            paths.USERADD,
-            '-g', DS_GROUP,
-            '-c', 'DS System User',
-            '-d', paths.VAR_LIB_DIRSRV,
-            '-s', paths.NOLOGIN,
-            '-M', '-r', DS_USER
-        ]
-        try:
-            ipautil.run(args)
-            root_logger.debug('Done adding DS user')
-        except ipautil.CalledProcessError, e:
-            root_logger.critical('Failed to add DS user: %s', e)
+    """Create DS user/group if it doesn't exist yet."""
+    installutils.create_system_user(
+        name=DS_USER,
+        group=DS_USER,
+        homedir=paths.VAR_LIB_DIRSRV,
+        shell=paths.NOLOGIN,
+    )
 
 
-def create_ds_group():
-    """
-    Create DS group if it doesn't exist yet.
-    Returns True if the group already exists.
-    """
-    try:
-        grp.getgrnam(DS_GROUP)
-        root_logger.debug('DS group %s exists', DS_GROUP)
-        group_exists = True
-    except KeyError:
-        group_exists = False
-        root_logger.debug('Adding DS group %s', DS_GROUP)
-        args = [paths.GROUPADD, '-r', DS_GROUP]
-        try:
-            ipautil.run(args)
-            root_logger.debug('Done adding DS group')
-        except ipautil.CalledProcessError, e:
-            root_logger.critical('Failed to add DS group: %s', e)
-
-    return group_exists
-
 INF_TEMPLATE = """
 [General]
 FullMachineName=   $FQDN
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index dc98d7a51aa743c87b2d7667246b6f029b8a648b..dc8fab418b1adc6b3dfe711d0ad8eab8fdd8ade4 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -29,6 +29,8 @@
 import traceback
 import textwrap
 from contextlib import contextmanager
+import pwd
+import grp
 
 from dns import resolver, rdatatype
 from dns.exception import DNSException
@@ -37,7 +39,7 @@
 
 from ipapython import ipautil, sysrestore, admintool, dogtag, version
 from ipapython.admintool import ScriptError
-from ipapython.ipa_log_manager import root_logger
+from ipapython.ipa_log_manager import root_logger, log_mgr
 from ipalib.util import validate_hostname
 from ipapython import config
 from ipalib import errors, x509
@@ -82,6 +84,8 @@ def __init__(self, top_dir=None):
 
     subject_base = ipautil.dn_attribute_property('_subject_base')
 
+log = log_mgr.get_logger(__name__)
+
 def get_fqdn():
     fqdn = ""
     try:
@@ -978,3 +982,41 @@ def validate_external_cert(cert_file, ca_file, subject_base):
             raise ValueError(
                 "The external CA chain is incomplete (%s is missing from the "
                 "chain)." % certsubject)
+
+
+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)
diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index 948d0be324f4e97250b711f52698e497998821f9..7929503f1b1637e8c6a547d199fc59c221f13722 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -30,7 +30,7 @@
 from ipapython.ipautil import run, user_input
 from ipapython import admintool
 from ipapython.dn import DN
-from ipaserver.install.dsinstance import (realm_to_serverid, create_ds_group,
+from ipaserver.install.dsinstance import (realm_to_serverid,
                                           create_ds_user, DS_USER)
 from ipaserver.install.cainstance import PKI_USER
 from ipaserver.install.replication import (wait_for_task, ReplicationManager,
@@ -188,7 +188,6 @@ def run(self):
         if options.data_only and not instances:
             raise admintool.ScriptError('No instances to restore to')
 
-        create_ds_group()
         create_ds_user()
         pent = pwd.getpwnam(DS_USER)
 
-- 
1.9.3

From 8f75bed27832153715284119dd1ee95e9c4b0595 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Tue, 29 Jul 2014 16:29:42 +0200
Subject: [PATCH] ipa_restore: Split the services list

Make a proper list from the comma-separated string found in
the config.

The only current use of backup_services is in run:
    if 'CA' in self.backup_services:
Without this change, this picked up the 'CA' from 'MEMCACHE'.
---
 ipaserver/install/ipa_restore.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index 7929503f1b1637e8c6a547d199fc59c221f13722..439f68304ae779453bf11636f50be3b7e16d6f3b 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -538,7 +538,7 @@ def read_header(self):
         self.backup_host = config.get('ipa', 'host')
         self.backup_ipa_version = config.get('ipa', 'ipa_version')
         self.backup_version = config.get('ipa', 'version')
-        self.backup_services = config.get('ipa', 'services')
+        self.backup_services = config.get('ipa', 'services').split(',')
 
 
     def extract_backup(self, keyring=None):
-- 
1.9.3

From 76757014dfefb261b0c8bea0d629cdf670154dab Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Tue, 29 Jul 2014 12:41:25 +0200
Subject: [PATCH] backup,restore: Don't overwrite /etc/{passwd,group}

The /etc/passwd and /etc/group files are not saved and restored.
The DS user is always created on restore, and the PKI user is created
if a CA is being restored.

https://fedorahosted.org/freeipa/ticket/3866
---
 ipaserver/install/ipa_backup.py  | 2 --
 ipaserver/install/ipa_restore.py | 4 +++-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ipaserver/install/ipa_backup.py b/ipaserver/install/ipa_backup.py
index 0830eb0c5dc6cf16febb305fc6e143ed862170d3..74936851940c9fe2e0748d668a071a2f13170075 100644
--- a/ipaserver/install/ipa_backup.py
+++ b/ipaserver/install/ipa_backup.py
@@ -149,8 +149,6 @@ class Backup(admintool.AdminTool):
         paths.SSHD_CONFIG,
         paths.SSH_CONFIG,
         paths.KRB5_CONF,
-        paths.GROUP,
-        paths.PASSWD,
         CACERT,
         paths.IPA_DEFAULT_CONF,
         paths.DS_KEYTAB,
diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index 439f68304ae779453bf11636f50be3b7e16d6f3b..e230f0aa3d7ef9b3ae5585e0628b15ffc8246866 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -32,7 +32,7 @@
 from ipapython.dn import DN
 from ipaserver.install.dsinstance import (realm_to_serverid,
                                           create_ds_user, DS_USER)
-from ipaserver.install.cainstance import PKI_USER
+from ipaserver.install.cainstance import PKI_USER, create_ca_user
 from ipaserver.install.replication import (wait_for_task, ReplicationManager,
                                            get_cs_replication_manager)
 from ipaserver.install import installutils
@@ -265,6 +265,8 @@ def run(self):
 
             # We do either a full file restore or we restore data.
             if self.backup_type == 'FULL' and not options.data_only:
+                if 'CA' in self.backup_services:
+                    create_ca_user()
                 if options.online:
                     raise admintool.ScriptError('File restoration cannot be done online.')
                 self.file_restore(options.no_logs)
-- 
1.9.3

From 6d4b9fb92b174f07955798a52d5aae3184e551b9 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Tue, 29 Jul 2014 13:30:39 +0200
Subject: [PATCH] ipa_backup: Log where the backup is be stored

This makes managing multiple backups & logs easier.
---
 ipaserver/install/ipa_backup.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ipaserver/install/ipa_backup.py b/ipaserver/install/ipa_backup.py
index 74936851940c9fe2e0748d668a071a2f13170075..2baa5dde035fa8fb93817d5f94b1fc257483798b 100644
--- a/ipaserver/install/ipa_backup.py
+++ b/ipaserver/install/ipa_backup.py
@@ -553,6 +553,8 @@ def finalize_backup(self, data_only=False, encrypt=False, keyring=None):
 
         shutil.move(self.header, backup_dir)
 
+        self.log.info('Backed up to %s', backup_dir)
+
     def __find_scripts_dir(self, instance):
         """
         IPA stores its 389-ds scripts in a different directory than dogtag
-- 
1.9.3

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to