Switch param order from yaml conf to plugin specific

Migrate from a global config __future__ to control behaviour to plugin
specific options to decide on whether to take the param order from yaml
when using the trigger-parameterized-builds plugin or when plugins are
making use of it as well.

Adjust the config retrieval to support a default value to simplify
logic around ensuring the behaviour matches 'true' by default.

Remove some redundant logic in helper module handling a default value
lookup and add some additional conf files to continue having some tests
using the old behaviour.

Remove old conf files setting the __futures__.param_order_from_yaml to
true as this is now the default under the correct plugin setting.

Change-Id: Ibd5f549b6d626bacaaa4221015a70aaf03626b00

jenkins_jobs/config.py

@@ -325,33 +325,36 @@ class JJBConfig(object):
                        'allow_empty_variables',
                        str(self.yamlparser['allow_empty_variables']))
 
-    def get_module_config(self, section, key):
+    def get_module_config(self, section, key, default=None):
         """ Given a section name and a key value, return the value assigned to
         the key in the JJB .ini file if it exists, otherwise emit a warning
         indicating that the value is not set. Default value returned if no
         value is set in the file will be a blank string.
         """
-        result = ''
+        result = default
         try:
             result = self.config_parser.get(
                 section, key
             )
         except (configparser.NoSectionError, configparser.NoOptionError,
                 JenkinsJobsException) as e:
-            logger.warning("You didn't set a " + key +
+            # use of default ignores missing sections/options
-                           " neither in the yaml job definition nor in" +
+            if result is None:
-                           " the " + section + " section, blank default" +
+                logger.warning(
-                           " value will be applied:\n{0}".format(e))
+                    "You didn't set a %s neither in the yaml job definition "
+                    "nor in the %s section, blank default value will be "
+                    "applied:\n%s", key, section, e)
         return result
 
-    def get_plugin_config(self, plugin, key):
+    def get_plugin_config(self, plugin, key, default=None):
-        value = self.get_module_config('plugin "{}"'.format(plugin), key)
+        value = self.get_module_config('plugin "{}"'.format(plugin), key,
+                                       default)
 
         # Backwards compatibility for users who have not switched to the new
         # plugin configuration format in their config. This code should be
         # removed in future versions of JJB after 2.0.
-        if not value:
+        if value is default:
-            value = self.get_module_config(plugin, key)
+            value = self.get_module_config(plugin, key, default)
             logger.warning(
                 "Defining plugin configuration using [" + plugin + "] is"
                 " deprecated. The recommended way to define plugins now is by"

jenkins_jobs/modules/helpers.py

@@ -14,7 +14,6 @@
 
 import logging
 
-import six
 import xml.etree.ElementTree as XML
 
 from jenkins_jobs.errors import InvalidAttributeError
@@ -251,10 +250,7 @@ def findbugs_settings(xml_parent, data):
 
 
 def get_value_from_yaml_or_config_file(key, section, data, jjb_config):
-    result = data.get(key, '')
+    return jjb_config.get_plugin_config(section, key, data.get(key, ''))
-    if result == '':
-        result = jjb_config.get_plugin_config(section, key)
-    return result
 
 
 def cloudformation_region_dict():
@@ -469,10 +465,18 @@ def test_fairy_common(xml_element, data):
     convert_mapping_to_xml(xml_element, data, mappings, fail_required=True)
 
 
-def trigger_get_parameter_order(registry):
+def trigger_get_parameter_order(registry, plugin):
     logger = logging.getLogger("%s:trigger_get_parameter_order" % __name__)
-    # original order
+
-    param_order = [
+    if str(registry.jjb_config.get_plugin_config(
+            plugin, 'param_order_from_yaml', True)).lower() == 'false':
+        logger.warning(
+            "Using deprecated order for parameter sets in %s. It is "
+            "recommended that you update your job definition instead of "
+            "enabling use of the old hardcoded order", plugin)
+
+        # deprecated order
+        return [
             'predefined-parameters',
             'git-revision',
             'property-file',
@@ -485,23 +489,7 @@ def trigger_get_parameter_order(registry):
             'boolean-parameters',
         ]
 
-    try:
+    return None
-        if registry.jjb_config.config_parser.getboolean(
-                '__future__', 'param_order_from_yaml'):
-            param_order = None
-    except six.moves.configparser.NoSectionError:
-        pass
-
-    if param_order:
-        logger.warning(
-            "Using deprecated order for parameter sets in "
-            "triggered-parameterized-builds. This will be changed in a future "
-            "release to inherit the order from the user defined yaml. To "
-            "enable this behaviour immediately, set the config option "
-            "'__future__.param_order_from_yaml' to 'true' and change the "
-            "input job configuration to use the desired order")
-
-    return param_order
 
 
 def trigger_project(tconfigs, project_def, param_order=None):

jenkins_jobs/modules/publishers.py

@@ -532,7 +532,8 @@ def trigger_parameterized_builds(registry, xml_parent, data):
     tbuilder = XML.SubElement(xml_parent, pt_prefix + 'BuildTrigger')
     configs = XML.SubElement(tbuilder, 'configs')
 
-    param_order = helpers.trigger_get_parameter_order(registry)
+    param_order = helpers.trigger_get_parameter_order(
+        registry, 'trigger-parameterized-builds')
 
     for project_def in data:
         tconfig = XML.SubElement(configs, pt_prefix + 'BuildTriggerConfig')
@@ -1839,7 +1840,7 @@ def pipeline(registry, xml_parent, data):
     See 'samples/pipeline.yaml' for an example pipeline implementation.
     """
     logger = logging.getLogger("%s:pipeline" % __name__)
-    param_order = helpers.trigger_get_parameter_order(registry)
+    param_order = helpers.trigger_get_parameter_order(registry, 'pipeline')
 
     if 'project' in data:
         logger.warning(

tests/publishers/fixtures/pipeline002.conf

@@ -0,0 +1,2 @@
+[plugin "pipeline"]
+param_order_from_yaml = False

tests/publishers/fixtures/pipeline003.conf

@@ -0,0 +1,2 @@
+[plugin "pipeline"]
+param_order_from_yaml = False

tests/publishers/fixtures/trigger_parameterized_builds/parameter-override-ordering.conf

@@ -1,2 +0,0 @@
-[__future__]
-param_order_from_yaml = True

tests/publishers/fixtures/trigger_parameterized_builds001.conf

@@ -0,0 +1,2 @@
+[plugin "trigger-parameterized-builds"]
+param_order_from_yaml = False

tests/publishers/fixtures/trigger_parameterized_builds003.conf

@@ -0,0 +1,2 @@
+[plugin "trigger-parameterized-builds"]
+param_order_from_yaml = False

tests/yamlparser/fixtures/trigger_parameterized_builds/parameter-override-ordering-001.conf

@@ -1,2 +0,0 @@
-[__future__]
-param_order_from_yaml = True

tests/yamlparser/fixtures/trigger_parameterized_builds/parameter-override-ordering-002.conf

@@ -1,2 +0,0 @@
-[__future__]
-param_order_from_yaml = True