Force xmlrpc tests to run with in_tree=True so config files in /etc/ipa/ don't get read; cleaned up config.Env automagic with regard to running in-tree vs. installed

ipalib/__init__.py

@@ -923,5 +923,5 @@ def create_api(mode='dummy'):
 api = create_api(mode=None)
 
 if os.environ.get('IPA_UNIT_TEST_MODE', None) == 'cli_test':
-    api.bootstrap(context='cli', in_server=False)
+    api.bootstrap(context='cli', in_server=False, in_tree=True)
     api.finalize()

ipalib/config.py

@@ -387,6 +387,20 @@ class Env(object):
                 i += 1
         return (i, len(items))
 
+    def _join(self, key, *parts):
+        """
+        Append path components in ``parts`` to base path ``self[key]``.
+
+        For example:
+
+        >>> env = Env()
+        >>> env.home = '/people/joe'
+        >>> env._join('home', 'Music', 'favourites')
+        '/people/joe/Music/favourites'
+        """
+        if key in self and self[key] is not None:
+            return path.join(self[key], *parts)
+
     def __doing(self, name):
         if name in self.__done:
             raise StandardError(
@@ -426,37 +440,49 @@ class Env(object):
         """
         self.__doing('_bootstrap')
 
-        # Set run-time variables:
+        # Set run-time variables (cannot be overridden):
         self.host = gethostname()
         self.ipalib = path.dirname(path.abspath(__file__))
         self.site_packages = path.dirname(self.ipalib)
         self.script = path.abspath(sys.argv[0])
         self.bin = path.dirname(self.script)
         self.home = os.environ.get('HOME', None)
-        self.dot_ipa = self._join('home', '.ipa')
+
+        # Merge in overrides:
         self._merge(**overrides)
+
+        # Determine if running in source tree:
         if 'in_tree' not in self:
-            if self.bin == self.site_packages and \
+            if (
-                    path.isfile(path.join(self.bin, 'setup.py')):
+                self.bin == self.site_packages
+                and path.isfile(path.join(self.bin, 'setup.py'))
+            ):
                 self.in_tree = True
             else:
                 self.in_tree = False
+
+        # Set dot_ipa:
+        if 'dot_ipa' not in self:
+            self.dot_ipa = self._join('home', '.ipa')
+
+        # Set context
         if 'context' not in self:
             self.context = 'default'
-        if self.in_tree:
-            base = self.dot_ipa
-        else:
-            base = path.join('/', 'etc', 'ipa')
-        if 'conf' not in self:
-            self.conf = path.join(base, '%s.conf' % self.context)
-        if 'conf_default' not in self:
-            self.conf_default = path.join(base, 'default.conf')
-        if 'conf_dir' not in self:
-            self.conf_dir = base
 
-    def _join(self, key, *parts):
+        # Set confdir:
-        if key in self and self[key] is not None:
+        if 'confdir' not in self:
-            return path.join(self[key], *parts)
+            if self.in_tree:
+                self.confdir = self.dot_ipa
+            else:
+                self.confdir = path.join('/', 'etc', 'ipa')
+
+        # Set conf (config file for this context):
+        if 'conf' not in self:
+            self.conf = self._join('confdir', '%s.conf' % self.context)
+
+        # Set conf_default (default base config used in all contexts):
+        if 'conf_default' not in self:
+            self.conf_default = self._join('confdir', 'default.conf')
 
     def _finalize_core(self, **defaults):
         """
@@ -473,8 +499,8 @@ class Env(object):
                ``self.conf_default`` (if it exists) by calling
                `Env._merge_from_file()`.
 
-            4. Intelligently fill-in the *in_server* and *log* variables
+            4. Intelligently fill-in the *in_server* , *logdir*, and *log*
-               if they haven't already been set.
+               variables if they haven't already been set.
 
             5. Merge-in the variables in ``defaults`` by calling `Env._merge()`.
                In normal circumstances ``defaults`` will simply be those
@@ -494,17 +520,28 @@ class Env(object):
         """
         self.__doing('_finalize_core')
         self.__do_if_not_done('_bootstrap')
+
+        # Merge in context config file and then default config file:
         if self.__d.get('mode', None) != 'dummy':
             self._merge_from_file(self.conf)
             self._merge_from_file(self.conf_default)
+
+        # Determine if in_server:
         if 'in_server' not in self:
             self.in_server = (self.context == 'server')
-        if 'log' not in self:
+
-            name = '%s.log' % self.context
+        # Set logdir:
-            if self.in_tree or self.context == 'cli':
+        if 'logdir' not in self:
-                self.log = path.join(self.dot_ipa, 'log', name)
+            if self.in_tree or not self.in_server:
+                self.logdir = self._join('dot_ipa', 'log')
             else:
-                self.log = path.join('/', 'var', 'log', 'ipa', name)
+                self.logdir = path.join('/', 'var', 'log', 'ipa')
+
+        # Set log file:
+        if 'log' not in self:
+            self.log = self._join('logdir', '%s.log' % self.context)
+
+        # FIXME: move into ca plugin
         if 'ca_host' not in self:
             self.ca_host = self.host
         self._merge(**defaults)

ipalib/constants.py

@@ -124,36 +124,36 @@ DEFAULT_CONFIG = (
     #  The remaining keys are never set from the values here!
     # ********************************************************
     #
-    # Env.__init__() or Env._bootstrap() or Env._finalize_core()
+    # Env._bootstrap() or Env._finalize_core() will have filled in all the keys
-    # will have filled in all the keys below by the time DEFAULT_CONFIG
+    # below by the time DEFAULT_CONFIG is merged in, so the values below are
-    # is merged in, so the values below are never actually used. They are
+    # never actually used.  They are listed both to provide a big picture and
-    # listed both to provide a big picture and also so DEFAULT_CONFIG contains
+    # also so DEFAULT_CONFIG contains at least all the keys that should be
-    # at least all the keys that should be present after Env._finalize_core()
+    # present after Env._finalize_core() is called.
-    # is called.
     #
     # Each environment variable below is sent to ``object``, which just happens
     # to be an invalid value for an environment variable, so if for some reason
     # any of these keys were set from the values here, an exception will be
     # raised.
 
-    # Set in Env.__init__():
+    # Non-overridable vars set in Env._bootstrap():
     ('host', object),
-    ('ipalib', object), # The directory containing ipalib/__init__.py
+    ('ipalib', object),  # The directory containing ipalib/__init__.py
-    ('site_packages', object), # The directory contaning ipalib
+    ('site_packages', object),  # The directory contaning ipalib
-    ('script', object), # sys.argv[0]
+    ('script', object),  # sys.argv[0]
-    ('bin', object), # The directory containing script
+    ('bin', object),  # The directory containing the script
-    ('home', object), # The home directory of user underwhich process is running
+    ('home', object),  # $HOME
-    ('dot_ipa', object), # ~/.ipa directory
 
-    # Set in Env._bootstrap():
+    # Vars set in Env._bootstrap():
-    ('in_tree', object), # Whether or not running in-tree (bool)
+    ('in_tree', object),  # Whether or not running in-tree (bool)
-    ('context', object), # Name of context, default is 'default'
+    ('dot_ipa', object),  # ~/.ipa directory
-    ('conf', object), # Path to config file
+    ('context', object),  # Name of context, default is 'default'
-    ('conf_default', object), # Path to common default config file
+    ('confdir', object),  # Directory containing config files
-    ('conf_dir', object), # Directory containing config files
+    ('conf', object),  # File containing context specific config
+    ('conf_default', object),  # File containing context independent config
 
     # Set in Env._finalize_core():
-    ('in_server', object), # Whether or not running in-server (bool)
+    ('in_server', object),  # Whether or not running in-server (bool)
-    ('log', object), # Path to log file
+    ('logdir', object),  # Directory containing log files
+    ('log', object),  # Path to context specific log file
 
 )

tests/test_ipalib/test_config.py

@@ -25,6 +25,7 @@ Test the `ipalib.config` module.
 import os
 from os import path
 import sys
+import socket
 from tests.util import raises, setitem, delitem, ClassChecker
 from tests.util import getitem, setitem, delitem
 from tests.util import TempDir, TempHome
@@ -407,7 +408,7 @@ class test_Env(ClassChecker):
         assert o.number == 42
         assert o.floating == 3.14
 
-    def new(self):
+    def new(self, in_tree=False):
         """
         Set os.environ['HOME'] to a tempdir.
 
@@ -415,7 +416,10 @@ class test_Env(ClassChecker):
         helper method is used in testing the bootstrap related methods below.
         """
         home = TempHome()
-        return (self.cls(), home)
+        o = self.cls()
+        if in_tree:
+            o.in_tree = True
+        return (o, home)
 
     def bootstrap(self, **overrides):
         """
@@ -437,6 +441,7 @@ class test_Env(ClassChecker):
         (o, home) = self.new()
         o._bootstrap()
         ipalib = path.dirname(path.abspath(config.__file__))
+        assert o.host == socket.gethostname()
         assert o.ipalib == ipalib
         assert o.site_packages == path.dirname(ipalib)
         assert o.script == path.abspath(sys.argv[0])
@@ -445,6 +450,7 @@ class test_Env(ClassChecker):
         assert o.dot_ipa == home.join('.ipa')
         assert o.in_tree is False
         assert o.context == 'default'
+        assert o.confdir == '/etc/ipa'
         assert o.conf == '/etc/ipa/default.conf'
         assert o.conf_default == o.conf
 
@@ -482,68 +488,61 @@ class test_Env(ClassChecker):
             assert getattr(o, key) == value
             assert o[key] == value
 
-    def finalize_core(self, **defaults):
+    def finalize_core(self, ctx, **defaults):
         """
         Helper method used in testing `Env._finalize_core`.
         """
-        (o, home) = self.new()
+        # We must force in_tree=True so we don't load possible config files in
+        # /etc/ipa/, whose contents could break this test:
+        (o, home) = self.new(in_tree=True)
+        if ctx:
+            o.context = ctx
+
+        # Check that calls cascade down the chain:
+        set_here = ('in_server', 'logdir', 'log')
+        assert o._isdone('_bootstrap') is False
         assert o._isdone('_finalize_core') is False
+        assert o._isdone('_finalize') is False
+        for key in set_here:
+            assert key not in o
         o._finalize_core(**defaults)
+        assert o._isdone('_bootstrap') is True
         assert o._isdone('_finalize_core') is True
+        assert o._isdone('_finalize') is False  # Should not cascade
+        for key in set_here:
+            assert key in o
+
+        # Check that it can't be called twice:
         e = raises(StandardError, o._finalize_core)
         assert str(e) == 'Env._finalize_core() already called'
+
         return (o, home)
 
     def test_finalize_core(self):
         """
         Test the `ipalib.config.Env._finalize_core` method.
         """
-        # Check that calls cascade up the chain:
+        # Test that correct defaults are generated:
-        (o, home) = self.new()
+        (o, home) = self.finalize_core(None)
-        assert o._isdone('_bootstrap') is False
-        assert o._isdone('_finalize_core') is False
-        assert o._isdone('_finalize') is False
-        o._finalize_core()
-        assert o._isdone('_bootstrap') is True
-        assert o._isdone('_finalize_core') is True
-        assert o._isdone('_finalize') is False
-
-        # Check that it can't be called twice:
-        e = raises(StandardError, o._finalize_core)
-        assert str(e) == 'Env._finalize_core() already called'
-
-        # Check that _bootstrap() did its job:
-        (o, home) = self.bootstrap()
-        assert 'in_tree' in o
-        assert 'conf' in o
-        assert 'context' in o
-
-        # Check that keys _finalize_core() will set are not set yet:
-        assert 'log' not in o
-        assert 'in_server' not in o
-
-        # Check that _finalize_core() did its job:
-        o._finalize_core()
-        assert 'in_server' in o
-        assert 'log' in o
-        assert o.in_tree is False
-        assert o.context == 'default'
         assert o.in_server is False
-        assert o.log == '/var/log/ipa/default.log'
+        assert o.logdir == home.join('.ipa', 'log')
+        assert o.log == home.join('.ipa', 'log', 'default.log')
 
-        # Check log is in ~/.ipa/log when context='cli'
+        # Test with context='server'
-        (o, home) = self.bootstrap(context='cli')
+        (o, home) = self.finalize_core('server')
-        o._finalize_core()
-        assert o.in_tree is False
-        assert o.log == home.join('.ipa', 'log', 'cli.log')
-
-        # Check **defaults can't set in_server nor log:
-        (o, home) = self.bootstrap(in_server='True')
-        o._finalize_core(in_server=False)
         assert o.in_server is True
-        (o, home) = self.bootstrap(log='/some/silly/log')
+        assert o.logdir == home.join('.ipa', 'log')
-        o._finalize_core(log='/a/different/log')
+        assert o.log == home.join('.ipa', 'log', 'server.log')
-        assert o.log == '/some/silly/log'
+
+        # Test that **defaults can't set in_server, logdir, nor log:
+        (o, home) = self.finalize_core(None,
+            in_server='IN_SERVER',
+            logdir='LOGDIR',
+            log='LOG',
+        )
+        assert o.in_server is False
+        assert o.logdir == home.join('.ipa', 'log')
+        assert o.log == home.join('.ipa', 'log', 'default.log')
 
         # Test loading config file, plus test some in-tree stuff
         (o, home) = self.bootstrap(in_tree=True, context='server')
@@ -555,6 +554,7 @@ class test_Env(ClassChecker):
         assert o.in_tree is True
         assert o.context == 'server'
         assert o.in_server is True
+        assert o.logdir == home.join('.ipa', 'log')
         assert o.log == home.join('.ipa', 'log', 'server.log')
         assert o.yes is True
         assert o.no is False
@@ -563,19 +563,19 @@ class test_Env(ClassChecker):
 
         # Test using DEFAULT_CONFIG:
         defaults = dict(constants.DEFAULT_CONFIG)
-        (o, home) = self.finalize_core(**defaults)
+        (o, home) = self.finalize_core(None, **defaults)
         assert list(o) == sorted(defaults)
         for (key, value) in defaults.items():
             if value is object:
                 continue
-            assert o[key] is value, value
+            assert o[key] == value, '%r is %r; should be %r' % (key, o[key], value)
 
     def test_finalize(self):
         """
         Test the `ipalib.config.Env._finalize` method.
         """
         # Check that calls cascade up the chain:
-        (o, home) = self.new()
+        (o, home) = self.new(in_tree=True)
         assert o._isdone('_bootstrap') is False
         assert o._isdone('_finalize_core') is False
         assert o._isdone('_finalize') is False
@@ -589,7 +589,7 @@ class test_Env(ClassChecker):
         assert str(e) == 'Env._finalize() already called'
 
         # Check that _finalize() calls __lock__()
-        (o, home) = self.new()
+        (o, home) = self.new(in_tree=True)
         assert o.__islocked__() is False
         o._finalize()
         assert o.__islocked__() is True
@@ -597,7 +597,7 @@ class test_Env(ClassChecker):
         assert str(e) == 'Env.__lock__() already called'
 
         # Check that **lastchance works
-        (o, home) = self.finalize_core()
+        (o, home) = self.finalize_core(None)
         key = 'just_one_more_key'
         value = 'with one more value'
         lastchance = {key: value}

tests/test_xmlrpc/xmlrpc_test.py

@@ -31,14 +31,6 @@ from ipalib import errors
 # individually instead of at the top-level. If API.bootstrap()
 # has already been called we continue gracefully. Other errors will be
 # raised.
-try:
-    api.bootstrap(context='cli')
-    api.finalize()
-except StandardError, e:
-    if str(e) == "API.bootstrap() already called":
-        pass
-    else:
-        raise e
 
 class XMLRPC_test(object):
     """