From 4f04de1f9ad706971f12508dacee5332df0a3a11 Mon Sep 17 00:00:00 2001 From: Wayne Warren Date: Sun, 27 Dec 2015 23:07:23 -0800 Subject: [PATCH] Use JJBConfig in YamlParser. This commit sees JJBConfig start to take the form it ought to have, namely using attributes to represent different logical sections of configuration that target specific subsystems of JJB. It also moves ConfigParser data retrieval from jenkins_jobs.modules.helpers.get_value_from_yaml_or_config_file() to jenkins_jobs.config.JJBConfig.get_module_config() TODO: Add JJBConfig tests to validate the behavior of JJBConfig in specific circumstances. Change-Id: I053d165559f5325a2f40b239117a86e6d0f3ef37 --- jenkins_jobs/builder.py | 25 +++++---- jenkins_jobs/cli/entry.py | 31 +++++------ jenkins_jobs/config.py | 82 +++++++++++++++++++++++++--- jenkins_jobs/modules/helpers.py | 16 +----- jenkins_jobs/modules/publishers.py | 4 +- jenkins_jobs/parser.py | 38 ++++--------- tests/base.py | 7 ++- tests/builder/test_builder.py | 11 ++-- tests/cmd/subcommands/test_update.py | 11 +++- tests/cmd/test_config.py | 22 ++++---- tests/localyaml/test_localyaml.py | 10 +++- 11 files changed, 157 insertions(+), 100 deletions(-) diff --git a/jenkins_jobs/builder.py b/jenkins_jobs/builder.py index d7d68bbae..e263c13e4 100644 --- a/jenkins_jobs/builder.py +++ b/jenkins_jobs/builder.py @@ -225,15 +225,16 @@ class Jenkins(object): class Builder(object): - def __init__(self, jenkins_url, jenkins_user, jenkins_password, - config=None, jenkins_timeout=_DEFAULT_TIMEOUT, - ignore_cache=False, flush_cache=False, plugins_list=None): - self.jenkins = Jenkins(jenkins_url, jenkins_user, jenkins_password, - jenkins_timeout) - self.cache = CacheStorage(jenkins_url, flush=flush_cache) - self.global_config = config - self.ignore_cache = ignore_cache - self._plugins_list = plugins_list + def __init__(self, jjb_config): + self.jenkins = Jenkins(jjb_config.jenkins['url'], + jjb_config.jenkins['user'], + jjb_config.jenkins['password'], + jjb_config.jenkins['timeout']) + self.cache = CacheStorage(jjb_config.jenkins['url'], + flush=jjb_config.builder['flush_cache']) + self._plugins_list = jjb_config.builder['plugins_info'] + + self.jjb_config = jjb_config @property def plugins_list(self): @@ -242,7 +243,7 @@ class Builder(object): return self._plugins_list def load_files(self, fn): - self.parser = YamlParser(self.global_config, self.plugins_list) + self.parser = YamlParser(self.jjb_config, self.plugins_list) # handle deprecated behavior, and check that it's not a file like # object as these may implement the '__iter__' attribute. @@ -337,7 +338,9 @@ class Builder(object): @parallelize def changed(self, job): md5 = job.md5() - changed = self.ignore_cache or self.cache.has_changed(job.name, md5) + + changed = (self.jjb_config.builder['ignore_cache'] or + self.cache.has_changed(job.name, md5)) if not changed: logger.debug("'{0}' has not changed".format(job.name)) return changed diff --git a/jenkins_jobs/cli/entry.py b/jenkins_jobs/cli/entry.py index f28ad9e52..c4cba4286 100644 --- a/jenkins_jobs/cli/entry.py +++ b/jenkins_jobs/cli/entry.py @@ -73,12 +73,20 @@ class JenkinsJobs(object): self._parse_additional() self.jjb_config.validate() + def _set_config(self, target, option): + """ + Sets the option in target only if the given option was explicitly set + """ + opt_val = getattr(self.options, option, None) + if opt_val is not None: + target[option] = opt_val + def _parse_additional(self): - for opt in ['ignore_cache', 'user', 'password', - 'allow_empty_variables']: - opt_val = getattr(self.options, opt, None) - if opt_val is not None: - setattr(self.jjb_config, opt, opt_val) + + self._set_config(self.jjb_config.builder, 'ignore_cache') + self._set_config(self.jjb_config.yamlparser, 'allow_empty_variables') + self._set_config(self.jjb_config.jenkins, 'user') + self._set_config(self.jjb_config.jenkins, 'password') if getattr(self.options, 'plugins_info_path', None) is not None: with io.open(self.options.plugins_info_path, 'r', @@ -87,7 +95,7 @@ class JenkinsJobs(object): if not isinstance(plugins_info, list): self.parser.error("{0} must contain a Yaml list!".format( self.options.plugins_info_path)) - self.jjb_config.plugins_info = plugins_info + self.jjb_config.builder['plugins_info'] = plugins_info if getattr(self.options, 'path', None): if hasattr(self.options.path, 'read'): @@ -118,17 +126,8 @@ class JenkinsJobs(object): self.options.path = paths def execute(self): - config = self.jjb_config.config_parser options = self.options - - builder = Builder(config.get('jenkins', 'url'), - self.jjb_config.user, - self.jjb_config.password, - self.jjb_config.config_parser, - jenkins_timeout=self.jjb_config.timeout, - ignore_cache=self.jjb_config.ignore_cache, - flush_cache=options.flush_cache, - plugins_list=self.jjb_config.plugins_info) + builder = Builder(self.jjb_config) if options.command == 'delete': for job in options.name: diff --git a/jenkins_jobs/config.py b/jenkins_jobs/config.py index 60a36b820..2ec2c87db 100644 --- a/jenkins_jobs/config.py +++ b/jenkins_jobs/config.py @@ -15,6 +15,7 @@ # Manage JJB Configuration sources, defaults, and access. +from collections import defaultdict import io import logging import os @@ -118,12 +119,18 @@ class JJBConfig(object): self.config_parser = config_parser self.ignore_cache = False + self.flush_cache = False self.user = None self.password = None self.plugins_info = None self.timeout = builder._DEFAULT_TIMEOUT self.allow_empty_variables = None + self.jenkins = defaultdict(None) + self.builder = defaultdict(None) + self.yamlparser = defaultdict(None) + self.hipchat = defaultdict(None) + self._setup() def _init_defaults(self): @@ -205,27 +212,88 @@ class JJBConfig(object): self.recursive = config.getboolean('job_builder', 'recursive') self.excludes = config.get('job_builder', 'exclude').split(os.pathsep) + # The way we want to do things moving forward: + self.jenkins['url'] = config.get('jenkins', 'url') + self.jenkins['user'] = self.user + self.jenkins['password'] = self.password + self.jenkins['timeout'] = self.timeout + + self.builder['ignore_cache'] = self.ignore_cache + self.builder['flush_cache'] = self.flush_cache + self.builder['plugins_info'] = self.plugins_info + + # keep descriptions ? (used by yamlparser) + keep_desc = False + if (config and config.has_section('job_builder') and + config.has_option('job_builder', 'keep_descriptions')): + keep_desc = config.getboolean('job_builder', + 'keep_descriptions') + self.yamlparser['keep_descriptions'] = keep_desc + + # figure out the include path (used by yamlparser) + path = ["."] + if (config and config.has_section('job_builder') and + config.has_option('job_builder', 'include_path')): + path = config.get('job_builder', + 'include_path').split(':') + self.yamlparser['include_path'] = path + + # allow duplicates? + allow_duplicates = False + if config and config.has_option('job_builder', 'allow_duplicates'): + allow_duplicates = config.getboolean('job_builder', + 'allow_duplicates') + self.yamlparser['allow_duplicates'] = allow_duplicates + + # allow empty variables? + self.yamlparser['allow_empty_variables'] = ( + self.allow_empty_variables or + config and config.has_section('job_builder') and + config.has_option('job_builder', 'allow_empty_variables') and + config.getboolean('job_builder', 'allow_empty_variables')) + def validate(self): config = self.config_parser # Inform the user as to what is likely to happen, as they may specify # a real jenkins instance in test mode to get the plugin info to check # the XML generated. - if self.user is None and self.password is None: + if self.jenkins['user'] is None and self.jenkins['password'] is None: logger.info("Will use anonymous access to Jenkins if needed.") - elif (self.user is not None and self.password is None) or ( - self.user is None and self.password is not None): + elif ((self.jenkins['user'] is not None and + self.jenkins['password'] is None) or + (self.jenkins['user'] is None and + self.jenkins['password'] is not None)): raise JenkinsJobsException( "Cannot authenticate to Jenkins with only one of User and " "Password provided, please check your configuration." ) - if (self.plugins_info is not None and - not isinstance(self.plugins_info, list)): + if (self.builder['plugins_info'] is not None and + not isinstance(self.builder['plugins_info'], list)): raise JenkinsJobsException("plugins_info must contain a list!") # Temporary until yamlparser is refactored to query config object - if self.allow_empty_variables is not None: + if self.yamlparser['allow_empty_variables'] is not None: config.set('job_builder', 'allow_empty_variables', - str(self.allow_empty_variables)) + str(self.yamlparser['allow_empty_variables'])) + + def get_module_config(self, section, key): + """ 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 = '' + try: + result = self.config_parser.get( + section, key + ) + except (configparser.NoSectionError, configparser.NoOptionError, + JenkinsJobsException) as e: + logger.warning("You didn't set a " + key + + " neither in the yaml job definition nor in" + + " the " + section + " section, blank default" + + " value will be applied:\n{0}".format(e)) + return result diff --git a/jenkins_jobs/modules/helpers.py b/jenkins_jobs/modules/helpers.py index 51ad2a109..9b9f597c3 100644 --- a/jenkins_jobs/modules/helpers.py +++ b/jenkins_jobs/modules/helpers.py @@ -12,11 +12,8 @@ # License for the specific language governing permissions and limitations # under the License. -import logging import xml.etree.ElementTree as XML -from six.moves import configparser - from jenkins_jobs.errors import InvalidAttributeError from jenkins_jobs.errors import JenkinsJobsException from jenkins_jobs.errors import MissingAttributeError @@ -254,19 +251,10 @@ def findbugs_settings(xml_parent, data): def get_value_from_yaml_or_config_file(key, section, data, parser): - logger = logging.getLogger(__name__) result = data.get(key, '') + jjb_config = parser.jjb_config if result == '': - try: - result = parser.config.get( - section, key - ) - except (configparser.NoSectionError, configparser.NoOptionError, - JenkinsJobsException) as e: - logger.warning("You didn't set a " + key + - " neither in the yaml job definition nor in" + - " the " + section + " section, blank default" + - " value will be applied:\n{0}".format(e)) + result = jjb_config.get_module_config(section, key) return result diff --git a/jenkins_jobs/modules/publishers.py b/jenkins_jobs/modules/publishers.py index 03594e30c..c3148f759 100644 --- a/jenkins_jobs/modules/publishers.py +++ b/jenkins_jobs/modules/publishers.py @@ -409,8 +409,8 @@ def trigger_parameterized_builds(parser, xml_parent, data): ] try: - if parser.config.getboolean('__future__', - 'param_order_from_yaml'): + if parser.jjb_config.config_parser.getboolean('__future__', + 'param_order_from_yaml'): orig_order = None except six.moves.configparser.NoSectionError: pass diff --git a/jenkins_jobs/parser.py b/jenkins_jobs/parser.py index 4d5cf2464..8a1949d14 100644 --- a/jenkins_jobs/parser.py +++ b/jenkins_jobs/parser.py @@ -70,27 +70,17 @@ def combination_matches(combination, match_combinations): class YamlParser(object): - def __init__(self, config=None, plugins_info=None): + def __init__(self, jjb_config=None, plugins_info=None): self.data = {} self.jobs = [] self.xml_jobs = [] - self.config = config - self.registry = ModuleRegistry(self.config, plugins_info) - self.path = ["."] - if self.config: - if config.has_section('job_builder') and \ - config.has_option('job_builder', 'include_path'): - self.path = config.get('job_builder', - 'include_path').split(':') - self.keep_desc = self.get_keep_desc() - def get_keep_desc(self): - keep_desc = False - if self.config and self.config.has_section('job_builder') and \ - self.config.has_option('job_builder', 'keep_descriptions'): - keep_desc = self.config.getboolean('job_builder', - 'keep_descriptions') - return keep_desc + self.jjb_config = jjb_config + self.keep_desc = jjb_config.yamlparser['keep_descriptions'] + self.path = jjb_config.yamlparser['include_path'] + + self.registry = ModuleRegistry(jjb_config.config_parser, + plugins_info) def parse_fp(self, fp): # wrap provided file streams to ensure correct encoding used @@ -129,8 +119,7 @@ class YamlParser(object): def _handle_dups(self, message): - if not (self.config and self.config.has_section('job_builder') and - self.config.getboolean('job_builder', 'allow_duplicates')): + if not self.jjb_config.yamlparser['allow_duplicates']: logger.error(message) raise JenkinsJobsException(message) else: @@ -311,19 +300,14 @@ class YamlParser(object): logger.debug('Excluding combination %s', str(params)) continue - allow_empty_variables = self.config \ - and self.config.has_section('job_builder') \ - and self.config.has_option( - 'job_builder', 'allow_empty_variables') \ - and self.config.getboolean( - 'job_builder', 'allow_empty_variables') - for key in template.keys(): if key not in params: params[key] = template[key] params['template-name'] = template_name - expanded = deep_format(template, params, allow_empty_variables) + expanded = deep_format( + template, params, + self.jjb_config.yamlparser['allow_empty_variables']) job_name = expanded.get('name') if jobs_glob and not matches(job_name, jobs_glob): diff --git a/tests/base.py b/tests/base.py index 5742a7499..c4737f5f2 100644 --- a/tests/base.py +++ b/tests/base.py @@ -131,14 +131,15 @@ class BaseTestCase(LoggingFixture): def _get_config(self): jjb_config = JJBConfig(self.conf_filename) + jjb_config.validate() - return jjb_config.config_parser + return jjb_config def test_yaml_snippet(self): if not self.in_filename: return - config = self._get_config() + jjb_config = self._get_config() expected_xml = self._read_utf8_content() yaml_content = self._read_yaml_content(self.in_filename) @@ -151,7 +152,7 @@ class BaseTestCase(LoggingFixture): self.addDetail("plugins-info", text_content(str(plugins_info))) - parser = YamlParser(config, plugins_info) + parser = YamlParser(jjb_config, plugins_info) pub = self.klass(parser.registry) diff --git a/tests/builder/test_builder.py b/tests/builder/test_builder.py index 6a0bb94fd..7a2d7a437 100644 --- a/tests/builder/test_builder.py +++ b/tests/builder/test_builder.py @@ -14,9 +14,11 @@ # License for the specific language governing permissions and limitations # under the License. +from jenkins_jobs.config import JJBConfig import jenkins_jobs.builder from tests.base import LoggingFixture from tests.base import mock + from testtools import TestCase @@ -24,11 +26,10 @@ from testtools import TestCase class TestCaseTestBuilder(LoggingFixture, TestCase): def setUp(self): super(TestCaseTestBuilder, self).setUp() - self.builder = jenkins_jobs.builder.Builder( - 'http://jenkins.example.com', - 'doesnot', 'matter', - plugins_list=['plugin1', 'plugin2'], - ) + jjb_config = JJBConfig() + jjb_config.builder['plugins_info'] = ['plugin1', 'plugin2'] + jjb_config.validate() + self.builder = jenkins_jobs.builder.Builder(jjb_config) def test_plugins_list(self): self.assertEqual(self.builder.plugins_list, ['plugin1', 'plugin2']) diff --git a/tests/cmd/subcommands/test_update.py b/tests/cmd/subcommands/test_update.py index 9508ccfb5..fa598d858 100644 --- a/tests/cmd/subcommands/test_update.py +++ b/tests/cmd/subcommands/test_update.py @@ -18,6 +18,7 @@ import os import six from jenkins_jobs import builder +from jenkins_jobs.config import JJBConfig from tests.base import mock from tests.cmd.test_cmd import CmdTestsBase @@ -79,9 +80,13 @@ class UpdateTests(CmdTestsBase): jobs = ['old_job001', 'old_job002', 'unmanaged'] extra_jobs = [{'name': name} for name in jobs] - builder_obj = builder.Builder('http://jenkins.example.com', - 'doesnot', 'matter', - plugins_list={}) + jjb_config = JJBConfig() + jjb_config.jenkins['url'] = 'http://example.com' + jjb_config.jenkins['user'] = 'doesnot' + jjb_config.jenkins['password'] = 'matter' + jjb_config.builder['plugins_info'] = [] + jjb_config.validate() + builder_obj = builder.Builder(jjb_config) # get the instance created by mock and redirect some of the method # mocks to call real methods on a the above test object. diff --git a/tests/cmd/test_config.py b/tests/cmd/test_config.py index ca50ea756..7447703a9 100644 --- a/tests/cmd/test_config.py +++ b/tests/cmd/test_config.py @@ -83,11 +83,12 @@ class TestConfigs(CmdTestsBase): 'settings_from_config.ini') args = ['--conf', config_file, 'test', 'dummy.yaml'] jenkins_jobs = entry.JenkinsJobs(args) - self.assertEqual(jenkins_jobs.jjb_config.ignore_cache, True) - self.assertEqual(jenkins_jobs.jjb_config.user, "jenkins_user") - self.assertEqual(jenkins_jobs.jjb_config.password, "jenkins_password") - self.assertEqual(jenkins_jobs.jjb_config.config_parser.get( - 'job_builder', 'allow_empty_variables'), "True") + jjb_config = jenkins_jobs.jjb_config + self.assertEqual(jjb_config.jenkins['user'], "jenkins_user") + self.assertEqual(jjb_config.jenkins['password'], "jenkins_password") + self.assertEqual(jjb_config.builder['ignore_cache'], True) + self.assertEqual( + jjb_config.yamlparser['allow_empty_variables'], True) def test_config_options_overriden_by_cli(self): """ @@ -98,8 +99,9 @@ class TestConfigs(CmdTestsBase): '--ignore-cache', '--allow-empty-variables', 'test', 'dummy.yaml'] jenkins_jobs = entry.JenkinsJobs(args) - self.assertEqual(jenkins_jobs.jjb_config.ignore_cache, True) - self.assertEqual(jenkins_jobs.jjb_config.user, "myuser") - self.assertEqual(jenkins_jobs.jjb_config.password, "mypassword") - self.assertEqual(jenkins_jobs.jjb_config.config_parser.get( - 'job_builder', 'allow_empty_variables'), "True") + jjb_config = jenkins_jobs.jjb_config + self.assertEqual(jjb_config.jenkins['user'], "myuser") + self.assertEqual(jjb_config.jenkins['password'], "mypassword") + self.assertEqual(jjb_config.builder['ignore_cache'], True) + self.assertEqual( + jjb_config.yamlparser['allow_empty_variables'], True) diff --git a/tests/localyaml/test_localyaml.py b/tests/localyaml/test_localyaml.py index 57c07f3c7..a40002f78 100644 --- a/tests/localyaml/test_localyaml.py +++ b/tests/localyaml/test_localyaml.py @@ -22,6 +22,7 @@ from testtools import TestCase from yaml.composer import ComposerError from jenkins_jobs import builder +from jenkins_jobs.config import JJBConfig from tests.base import get_scenarios from tests.base import JsonTestCase from tests.base import LoggingFixture @@ -76,6 +77,11 @@ class TestCaseLocalYamlIncludeAnchors(LoggingFixture, TestCase): files = ["custom_same_anchor-001-part1.yaml", "custom_same_anchor-001-part2.yaml"] - b = builder.Builder("http://example.com", "jenkins", None, - plugins_list=[]) + jjb_config = JJBConfig() + jjb_config.jenkins['url'] = 'http://example.com' + jjb_config.jenkins['user'] = 'jenkins' + jjb_config.jenkins['password'] = 'password' + jjb_config.builder['plugins_info'] = [] + jjb_config.validate() + b = builder.Builder(jjb_config) b.load_files([os.path.join(self.fixtures_path, f) for f in files])