[Freeipa-devel] [PATCH] 0025/0026 ipa-client-install --hostname not setting HOSTNAME if it is missing from the configuration file

Alexander Bokovoy abokovoy at redhat.com
Wed Oct 12 13:51:55 UTC 2011


On Wed, 12 Oct 2011, Rob Crittenden wrote:
> >Also, all keys totally missing from the config will be added. Values
> >from replacevars and appendvars are merged before doing it so there is
> >only single key=value pair afterwards. Obviously, it is the caller
> >responsibility to not allow replacevars/appendvars have conflicting
> >values.
> >
> >Patch 0025 introduces the common code and transforms
> >ipapython.services.backup_and_replace_hostname()
> >
> >Patch 0026 uses this new code to change
> >ipaserver/install/krb5instance.py to use new code.
> 
> Why should the caller be responsible for calling restore_context()?
Because ipapython.ipautil cannot depend on ipapython.services as 
underlying platform-specific code uses ipapython.ipautil -- we 
discussed this with Simo few weeks ago when the first version of this 
patch was done for systemd support.

We have very few places where this is needed and wrapper functions 
(like hostname handler) should take care of the context restoration 
as they will also handle state store as well (in addition to file 
store).

> I think:
> 
> if old_values['HOSTNAME']: ...
> 
> should be
> 
> if 'HOSTNAME' in old_values: ...
Updated patch attached.

-- 
/ Alexander Bokovoy
-------------- next part --------------
>From 6c01a86f232d10176372a9fbf7c8a4d40f8e928a Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy at redhat.com>
Date: Wed, 12 Oct 2011 16:42:09 +0300
Subject: [PATCH] Refactor backup_and_replace_hostname() into a flexible
 config modification tool

backup_and_replace_hostname() was doing three things:
    1. Given config file in 'key=value' style, replace value for a specified key (HOSTNAME)
    2. Backup original file and install a replacement
    3. Restore original security context after editing

We have several more places where parts of the functionality are needed,
thus making two tools in ipapython.ipautil:

    1. config_replace_variables(filepath, replacevars=dict(), appendvars=dict())
       Replaces or appends values to specified keys, adding new key=value pairs if key was absent

    2. backup_config_and_replace_variables(fstore, filepath, replacevars=dict(), appendvars=dict())
       Backups config file and calls config_replace_variables()

A caller must handle security context after using these two tools.

In addition, as before, there is ipapython.services.backup_and_replace_hostname() that uses
these common tools and restores security context after editing.

The code will be used extensively for systemd integration for Fedora 16.

Fixes:
    https://fedorahosted.org/freeipa/ticket/1871
---
 ipapython/ipautil.py         |   90 ++++++++++++++++++++++++++++++++++++++++++
 ipapython/platform/redhat.py |   47 +++------------------
 2 files changed, 97 insertions(+), 40 deletions(-)

diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 6e037926ce678557d9273be6ff1e9a6c65d0f80e..49fc64cea7c48d7aaf7e44de3c26d395fddbaa28 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -1185,3 +1185,93 @@ def get_ipa_basedn(conn):
 
     return None
 
+def config_replace_variables(filepath, replacevars=dict(), appendvars=dict()):
+    """
+    Take a key=value based configuration file, and write new version 
+    with certain values replaced or appended 
+
+    All (key,value) pairs from replacevars and appendvars that were not found 
+    in the configuration file, will be added there.
+
+    It is responsibility of a caller to ensure that replacevars and
+    appendvars do not overlap.
+
+    It is responsibility of a caller to back up file.
+
+    returns dictionary of affected keys and their previous values
+
+    One have to run restore_context(filepath) afterwards or
+    security context of the file will not be correct after modification
+    """
+    pattern = re.compile('''
+(^
+                        \s*
+        (?P<option>     [^\#;]+?)
+                        (\s*=\s*)
+        (?P<value>      .+?)?
+                        (\s*((\#|;).*)?)?
+$)''', re.VERBOSE)
+    orig_stat = os.stat(filepath)
+    old_values = dict()
+    temp_filename = None
+    with tempfile.NamedTemporaryFile(delete=False) as new_config:
+        temp_filename = new_config.name
+        with open(filepath, 'r') as f:
+            for line in f:
+                new_line = line
+                m = pattern.match(line)
+                if m:
+                    option, value = m.group('option', 'value')
+                    if option is not None:
+                        if replacevars and option in replacevars:
+                            # replace value completely
+                            new_line = u"%s=%s\n" % (option, replacevars[option])
+                            old_values[option] = value
+                        if appendvars and option in appendvars:
+                            # append new value unless it is already existing in the original one
+                            if value.find(appendvars[option]) == -1:
+                                new_line = u"%s=%s %s\n" % (option, value, appendvars[option])
+                            old_values[option] = value
+                new_config.write(new_line)
+        # Now add all options from replacevars and appendvars that were not found in the file
+        new_vars = replacevars.copy()
+        new_vars.update(appendvars)
+        newvars_view = new_vars.viewkeys() - old_values.viewkeys()
+        append_view = (appendvars.viewkeys() - replacevars.viewkeys()) - old_values.viewkeys()
+        for item in newvars_view:
+            new_config.write("%s=%s\n" % (item,new_vars[item]))
+        for item in append_view:
+            new_config.write("%s=%s\n" % (item,appendvars[item]))
+        new_config.flush()
+        # Make sure the resulting file is readable by others before installing it
+        os.fchmod(new_config.fileno(), orig_stat.st_mode)
+        os.fchown(new_config.fileno(), orig_stat.st_uid, orig_stat.st_gid)
+
+    # At this point new_config is closed but not removed due to 'delete=False' above
+    # Now, install the temporary file as configuration and ensure old version is available as .orig
+    # While .orig file is not used during uninstall, it is left there for administrator.
+    install_file(temp_filename, filepath)
+
+    return old_values
+
+def backup_config_and_replace_variables(fstore, filepath, replacevars=dict(), appendvars=dict()):
+    """
+    Take a key=value based configuration file, back up it, and
+    write new version with certain values replaced or appended 
+
+    All (key,value) pairs from replacevars and appendvars that
+    were not found in the configuration file, will be added there.
+
+    It is responsibility of a caller to ensure that replacevars and
+    appendvars do not overlap.
+
+    returns dictionary of affected keys and their previous values
+
+    One have to run restore_context(filepath) afterwards or
+    security context of the file will not be correct after modification
+    """
+    # Backup original filepath
+    fstore.backup_file(filepath)
+    old_values = config_replace_variables(filepath, replacevars, appendvars)
+
+    return old_values
diff --git a/ipapython/platform/redhat.py b/ipapython/platform/redhat.py
index 6bf8bf348a4c30c79ad29d8b2b30a088bd28372d..0c2ba63628fc512b7e7d00326f7698d9d156bedb 100644
--- a/ipapython/platform/redhat.py
+++ b/ipapython/platform/redhat.py
@@ -133,48 +133,15 @@ def restore_context(filepath):
     ipautil.run(["/sbin/restorecon", filepath], raiseonerr=False)
 
 def backup_and_replace_hostname(fstore, statestore, hostname):
-    network_filename = "/etc/sysconfig/network"
-    # Backup original /etc/sysconfig/network
-    fstore.backup_file(network_filename)
-    hostname_pattern = re.compile('''
-(^
-                        \s*
-        (?P<option>     [^\#;]+?)
-                        (\s*=\s*)
-        (?P<value>      .+?)?
-                        (\s*((\#|;).*)?)?
-$)''', re.VERBOSE)
-    temp_filename = None
-    with tempfile.NamedTemporaryFile(delete=False) as new_config:
-        temp_filename = new_config.name
-        with open(network_filename, 'r') as f:
-            for line in f:
-                new_line = line
-                m = hostname_pattern.match(line)
-                if m:
-                    option, value = m.group('option', 'value')
-                    if option is not None and option == 'HOSTNAME':
-                        if value is not None and hostname != value:
-                            new_line = u"HOSTNAME=%s\n" % (hostname)
-                            statestore.backup_state('network', 'hostname', value)
-                new_config.write(new_line)
-        new_config.flush()
-        # Make sure the resulting file is readable by others before installing it
-        os.fchmod(new_config.fileno(), stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH)
-        os.fchown(new_config.fileno(), 0, 0)
-
-    # At this point new_config is closed but not removed due to 'delete=False' above
-    # Now, install the temporary file as configuration and ensure old version is available as .orig
-    # While .orig file is not used during uninstall, it is left there for administrator.
-    ipautil.install_file(temp_filename, network_filename)
     try:
         ipautil.run(['/bin/hostname', hostname])
     except ipautil.CalledProcessError, e:
         print >>sys.stderr, "Failed to set this machine hostname to %s (%s)." % (hostname, str(e))
-
-    # For SE Linux environments it is important to reset SE labels to the expected ones
-    try:
-        restore_context(network_filename)
-    except ipautil.CalledProcessError, e:
-        print >>sys.stderr, "Failed to set permissions for %s (%s)." % (network_filename, str(e))
+    replacevars = {'HOSTNAME':hostname}
+    old_values = ipautil.backup_config_and_replace_variables(fstore,
+                                                          "/etc/sysconfig/network", 
+                                                          replacevars=replacevars)
+    restore_context("/etc/sysconfig/network")
+    if 'HOSTNAME' in old_values:
+        statestore.backup_state('network', 'hostname', old_values['HOSTNAME'])
 
-- 
1.7.6.4



More information about the Freeipa-devel mailing list