Make FakeConfiguration a representative configuration object.

mig/shared/configuration.py

@@ -1,3 +1,3 @@
 #!/usr/bin/python
 # -*- coding: utf-8 -*-
 #
@@ -20,7 +20,7 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 #
 # -- END_HEADER ---
 #
@@ -31,12 +31,13 @@
 from __future__ import print_function
 from future import standard_library
 from builtins import range
 from builtins import object
 
 import base64
 import copy
 import datetime
 import functools
+import inspect
 import os
 import pwd
 import re
@@ -68,12 +69,23 @@
         valid_filter_methods, default_twofactor_auth_apps, \
         mig_conf_section_dirname
     from mig.shared.logger import Logger, SYSLOG_GDP
     from mig.shared.htmlgen import menu_items, vgrid_items
     from mig.shared.fileio import read_file, load_json, write_file
 except ImportError as ioe:
     print("could not import migrid modules")
 
 
+_CONFIGURATION_NOFORWARD_KEYS = set([
+    'self',
+    'config_file',
+    'mig_server_id',
+    'disable_auth_log',
+    'skip_log',
+    'verbose',
+    'logger',
+])
+
+
 def include_section_contents(logger, config, section, load_path, verbose=False,
                              reject_overrides=[]):
     """Include additional section contents from load_path in config."""
@@ -130,7 +142,7 @@
         return val
     # logger.debug("expand any ENV and FILE content in %r" % val)
     env_pattern = "[a-zA-Z][a-zA-Z0-9_]+"
     cache_pattern = file_pattern = "[^ ]+"
     expanded = val
     expand_pairs = [(keyword_env, env_pattern), (keyword_file, file_pattern)]
     for (key, pattern) in expand_pairs:
@@ -158,7 +170,7 @@
                     #    "reading conf content from file in %r" % cache)
                     content = read_file(path, logger)
                     if cache and content:
                         # logger.debug("caching conf content salt in %r" % cache)
                         write_file(content, cache, logger)
                 if not content:
                     logger.warning("salt file not found or empty: %s" % path)
@@ -232,7 +244,7 @@
         'server_home': '~/state/server_home/',
         'webserver_home': '~/state/webserver_home/',
         'sessid_to_mrsl_link_home': '~/state/sessid_to_mrsl_link_home/',
         'sessid_to_jupyter_mount_link_home': '~/state/sessid_to_jupyter_mount_link_home/',
         'mig_system_files': '~/state/mig_system_files/',
         'mig_system_storage': '~/state/mig_system_storage',
         'mig_system_run': '~/state/mig_system_run/',
@@ -371,7 +383,7 @@
         'architectures': 'X86 AMD64 IA64 SPARC SPARC64 ITANIUM SUN4U SPARC-T1',
         'scriptlanguages': 'sh python java',
         'jobtypes': 'batch interactive bulk all',
         'lrmstypes': 'Native Native-execution-leader Batch Batch-execution-leader',
     }
     scheduler_section = {'algorithm': 'FairFit',
                          'expire_after': '99999999999',
@@ -410,7 +422,7 @@
         'QUOTA': quota_section,
     }
     for section in defaults:
         if not section in config.sections():
             config.add_section(section)
 
     modified = False
@@ -424,7 +436,7 @@
                 modified = True
     if modified:
         backup_path = '%s.%d' % (config_file, time.time())
         print('Backing up existing configuration to %s as update removes all comments'
               % backup_path)
         fd = open(config_file, 'r')
         backup_fd = open(backup_path, 'w')
@@ -436,6 +448,11 @@
         fd.close()
 
 
+def _configuraton_dict_without_noforward_keys(configuration_like):
+    return { k: v for k, v in configuration_like.__dict__.items()
+             if k not in _CONFIGURATION_NOFORWARD_KEYS }
+
+
 class NativeConfigParser(ConfigParser):
     """Wraps configparser.ConfigParser to force get method to return native
     string instead of always returning unicode.
@@ -446,7 +463,11 @@
         return force_native_str(ConfigParser.get(self, *args, **kwargs))
 
 
-_CONFIGURATION_DEFAULTS = {
+# TODO: this static definition is incomplete with many properties being
+#       dynamically assigned to Configuration objects at the point they are
+#       loaded - expand these as code that makes use of those properties comes
+#       under test with the ultimate goal that this becomes exhaustive
+_CONFIGURATION_PROPERTIES = {
     # Optional conf options with default values
     'state_path': os.path.expanduser('~/state'),
     'mig_path': os.path.expanduser('~/mig'),
@@ -470,6 +491,7 @@
     'ca_smtp': '',
     'ca_user': 'mig-ca',
     'resource_home': '',
+    'short_title': 'MiG',
     'vgrid_home': '',
     'vgrid_public_base': '',
     'vgrid_private_base': '',
@@ -512,6 +534,7 @@
     'workflows_vgrid_patterns_home': '',
     'workflows_vgrid_recipes_home': '',
     'workflows_vgrid_history_home': '',
+    'site_user_id_format': DEFAULT_USER_ID_FORMAT,
     'site_prefer_python3': False,
     'site_autolaunch_page': '',
     'site_landing_page': '',
@@ -539,6 +562,12 @@
     'site_password_policy': POLICY_MEDIUM,
     'site_password_legacy_policy': False,
     'site_password_cracklib': False,
+
+    # Salt values which are undonditionally populated on configuration load
+    'site_crypto_salt': '',
+    'site_password_salt': '',
+    'site_digest_salt': '',
+
     'site_extra_userpage_scripts': "",
     'site_extra_userpage_styles': "",
     'hg_path': '',
@@ -713,7 +742,7 @@
                        'VERIFYFILES', 'VGRID', 'SANDBOX'],
     'job_cond_yellow': ['DISK', 'MEMORY', 'CPUTIME'],
     'job_cond_orange': ['CPUCOUNT', 'NODECOUNT'],
     'job_cond_red': ['EXECUTABLES', 'INPUTFILES', 'REGISTERED', 'SEEN_WITHIN_X'],
     'enable_suggest': False,
     'suggest_threshold': 'GREEN',
 
@@ -726,6 +755,7 @@
     'expire_peer': 600,
     'language': ['English'],
     'user_interface': ['V2', 'V3'],
+    'new_user_default_ui': keyword_auto,
     'submitui': ['fields', 'textarea', 'files'],
     # Init user default page with no selection to use site landing page
     'default_page': [''],
@@ -742,6 +772,8 @@
     'auto_add_user_with_peer': [('distinguished_name', '.*')],
     'auto_add_filter_method': '',
     'auto_add_filter_fields': [],
+
+    'cloud_services': [],
 }
 
 
@@ -758,9 +790,9 @@
         self.auth_logger_obj = None
         self.gdp_logger_obj = None
 
-        configuration_options = copy.deepcopy(_CONFIGURATION_DEFAULTS)
+        configuration_properties = copy.deepcopy(_CONFIGURATION_PROPERTIES)
 
-        for k, v in configuration_options.items():
+        for k, v in configuration_properties.items():
             setattr(self, k, v)
 
         if config_file is not None:
@@ -787,7 +819,7 @@
         try:
             if self.logger:
                 self.logger.info('reloading configuration and reopening log')
         except:
             pass
 
         try:
@@ -1013,19 +1045,24 @@
             self.site_title = "Minimum intrusion Grid"
         if config.has_option('SITE', 'short_title'):
             self.short_title = config.get('SITE', 'short_title')
-        else:
-            self.short_title = "MiG"
+
         if config.has_option('SITE', 'user_interface'):
             self.user_interface = config.get(
                 'SITE', 'user_interface').split()
         else:
             self.user_interface = ['V2']
+
         # Allow gradual transition to new user interface - only new sign ups
         if config.has_option('SITE', 'new_user_default_ui'):
             self.new_user_default_ui = config.get(
                 'SITE', 'new_user_default_ui').strip()
-        else:
+        elif self.new_user_default_ui == keyword_auto and self.user_interface:
+            # an explicit default ui value for new users was not specified so
+            # use the first entry in supported user interfaces as a fallback
             self.new_user_default_ui = self.user_interface[0]
+        else:
+            print("No usable value supplied for default new user ui version.")
+            raise IOError
 
         if config.has_option('GLOBAL', 'state_path'):
             self.state_path = config.get('GLOBAL', 'state_path')
@@ -2035,8 +2072,6 @@
                 logger.warning("invalid user_id_format %r - using default" %
                                self.site_user_id_format)
                 self.site_user_id_format = DEFAULT_USER_ID_FORMAT
-        else:
-            self.site_user_id_format = DEFAULT_USER_ID_FORMAT
         if config.has_option('SITE', 'autolaunch_page'):
             self.site_autolaunch_page = config.get('SITE', 'autolaunch_page')
         else:
@@ -2865,6 +2900,21 @@
                     peerfile)
         return peers_dict
 
+    @staticmethod
+    def is_configuration_like(obj):
+        """Does the given object quack like a MiG Configuration."""
+        return inspect.ismethod(getattr(obj, 'reload_config', None))
+
+    @staticmethod
+    def to_dict(obj):
+        """Return a plain dictionary of the loaded configuration values only
+        given a Configuration-like object as input."""
+        assert Configuration.is_configuration_like(obj)
+        return _configuraton_dict_without_noforward_keys(obj)
+
+
+_CONFIGURATION_ARGUMENTS = set(_CONFIGURATION_PROPERTIES.keys()) - _CONFIGURATION_NOFORWARD_KEYS
+
 
 if '__main__' == __name__:
     conf = Configuration(os.path.expanduser('~/mig/server/MiGserver.conf'),

tests/fixture/mig_shared_configuration--new.json

@@ -28,6 +28,7 @@
   "ca_smtp": "",
   "ca_user": "mig-ca",
   "certs_path": "/some/place/certs",
+  "cloud_services": [],
   "config_file": null,
   "cputime_for_empty_jobs": 0,
   "default_page": [
@@ -129,6 +130,7 @@
   "min_seconds_between_live_update_requests": 0,
   "mrsl_files_dir": "",
   "myfiles_py_location": "",
+  "new_user_default_ui": "AUTO",
   "notify_home": "",
   "openid_store": "",
   "passphrase_file": "",
@@ -152,6 +154,7 @@
   "sessid_to_jupyter_mount_link_home": "",
   "sessid_to_mrsl_link_home": "",
   "sharelink_home": "",
+  "short_title": "MiG",
   "site_advanced_vgrid_links": [],
   "site_autolaunch_page": "",
   "site_cloud_access": [
@@ -172,6 +175,9 @@
   "site_password_cracklib": false,
   "site_password_legacy_policy": false,
   "site_password_policy": "MEDIUM",
+  "site_crypto_salt": "",
+  "site_password_salt": "",
+  "site_digest_salt": "",
   "site_peers_notice": "",
   "site_peers_permit": [
     [
@@ -197,6 +203,7 @@
     ]
   ],
   "site_skin": "",
+  "site_user_id_format": "X509",
   "site_vgrid_creators": [
     [
       "distinguished_name",

tests/support/__init__.py

@@ -1,3 +1,3 @@
 #!/usr/bin/python
 # -*- coding: utf-8 -*-
 #
@@ -219,9 +219,9 @@
     # testcase defaults
 
     @staticmethod
-    def _make_configuration_instance(configuration_to_make):
+    def _make_configuration_instance(testcase, configuration_to_make):
         if configuration_to_make == 'fakeconfig':
-            return FakeConfiguration()
+            return FakeConfiguration(logger=testcase.logger)
         elif configuration_to_make == 'testconfig':
             from mig.shared.conf import get_configuration_object
             return get_configuration_object(skip_log=True, disable_auth_log=True)
@@ -230,7 +230,7 @@
                 "MigTestCase: unknown configuration %r" % (configuration_to_make,))
 
     def _provide_configuration(self):
-        return 'fakeconfig'
+        return 'unspecified'
 
     @property
     def configuration(self):
@@ -240,8 +240,13 @@
             return self._configuration
 
         configuration_to_make = self._provide_configuration()
+
+        if configuration_to_make == 'unspecified':
+            raise AssertionError(
+                "configuration access but testcase did not request it")
+
         configuration_instance = self._make_configuration_instance(
-            configuration_to_make)
+            self, configuration_to_make)
 
         if configuration_to_make == 'testconfig':
             # use the paths defined by the loaded configuration to create

tests/support/configsupp.py

@@ -1,3 +1,3 @@
 #!/usr/bin/python
 # -*- coding: utf-8 -*-
 #
@@ -29,19 +29,61 @@
 
 from tests.support.loggersupp import FakeLogger
 
+from mig.shared.compat import SimpleNamespace
+from mig.shared.configuration import \
+    _CONFIGURATION_ARGUMENTS, _CONFIGURATION_PROPERTIES
+
+
+def _ensure_only_configuration_keys(thedict):
+    """Check a dictionary contains only keys valid as Configuration properties.
+    """
+
+    unknown_keys = set(thedict.keys()) - set(_CONFIGURATION_ARGUMENTS)
+    assert len(unknown_keys) == 0, \
+        "non-Configuration keys: %s" % (', '.join(unknown_keys),)
+
+
+def _generate_namespace_kwargs():
+    """Create plain dictionary with supported properties and keys that map to
+    their default values suitable for use in fabricating a namespace.
+    """
+
+    properties_and_defaults = dict(_CONFIGURATION_PROPERTIES)
+    properties_and_defaults['logger'] = None
+    return properties_and_defaults
+
+
+class FakeConfiguration(SimpleNamespace):
+    """An object that can act as a representative Configuration which can be
+    programmed with particular values required to exercise code under test.
+
+    This object will track standard values as would be present on a genuine
+    Configuration instance such that code under test expecting such can be
+    handed something. The defaults are overlaid by any explicit keyword args.
 
-class FakeConfiguration:
-    """A simple helper to pretend we have a real Configuration object with any
-    required attributes explicitly passed.
     Automatically attaches a FakeLogger instance if no logger is provided in
     kwargs.
     """
 
-    def __init__(self, **kwargs):
-        """Initialise instance attributes to be any named args provided and a
-        FakeLogger instance attached if not provided.
+    def __init__(self, logger=None, **kwargs):
+        """Initialise instance attributes based on the defaults plus any
+        supplied additional options.
         """
-        self.__dict__.update(kwargs)
-        if not 'logger' in self.__dict__:
-            dummy_logger = FakeLogger()
-            self.__dict__.update({'logger': dummy_logger})
+
+        SimpleNamespace.__init__(self, **_generate_namespace_kwargs())
+
+        if logger is None:
+            # TODO: remove this conditional once all callers that require a
+            #       FakeConfiguration request it via _provide_configuration()
+            logger = FakeLogger()
+        self.logger = logger
+
+        if kwargs:
+            _ensure_only_configuration_keys(kwargs)
+            for k, v in kwargs.items():
+                setattr(self, k, v)
+
+    def reload_config(self, *args, **kwargs):
+        """Stub defined to quack like Configuration."""
+
+        pass

tests/test_mig_shared_configuration.py

@@ -1,3 +1,3 @@
 # -*- coding: utf-8 -*-
 #
 # --- BEGIN_HEADER ---
@@ -34,20 +34,37 @@
 from tests.support import MigTestCase, TEST_DATA_DIR, PY2, testmain
 from tests.support.fixturesupp import FixtureAssertMixin
 
-from mig.shared.configuration import Configuration
-
-
-def _is_method(value):
-    return type(value).__name__ == 'method'
+from mig.shared.configuration import Configuration, \
+    _CONFIGURATION_ARGUMENTS, _CONFIGURATION_PROPERTIES
 
 
 def _to_dict(obj):
     return {k: v for k, v in inspect.getmembers(obj)
-            if not (k.startswith('__') or _is_method(v))}
+            if not (k.startswith('__') or inspect.ismethod(v) or inspect.isfunction(v))}
+
+
+class MigSharedConfiguration__static_definitions(MigTestCase):
+    """Coverage of the static definitions underlying Configuration objects."""
+
+    def test_consistent_parameters(self):
+        configuration_defaults_keys = set(_CONFIGURATION_PROPERTIES.keys())
+        mismatched = _CONFIGURATION_ARGUMENTS - configuration_defaults_keys
+
+        self.assertEqual(len(mismatched), 0,
+                         "configuration defaults do not match arguments")
+
+
+class MigSharedConfiguration__loaded_configurations(MigTestCase):
+    """Coverage of loaded Configuration instances."""
 
+    def test_argument_new_user_default_ui_is_replaced(self):
+        test_conf_file = os.path.join(
+            TEST_DATA_DIR, 'MiGserver--customised.conf')
+
+        configuration = Configuration(
+            test_conf_file, skip_log=True, disable_auth_log=True)
 
-class MigSharedConfiguration(MigTestCase, FixtureAssertMixin):
-    """Wrap unit tests for the corresponding module"""
+        self.assertEqual(configuration.new_user_default_ui, 'V3')
 
     def test_argument_storage_protocols(self):
         test_conf_file = os.path.join(
@@ -315,6 +332,10 @@
         # TODO: rename file to valid section name we can check and enable next?
         # self.assertEqual(configuration.multi, 'blabla')
 
+
+class MigSharedConfiguration__new_instance(MigTestCase, FixtureAssertMixin):
+    """Coverage of programatically created Configuration instances."""
+
     @unittest.skipIf(PY2, "Python 3 only")
     def test_default_object(self):
         prepared_fixture = self.prepareFixtureAssert(