From 4795838dd2a9ddb0c8495c6b085cca99a83f19d6 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Thu, 10 Jan 2013 17:26:02 -0800 Subject: [PATCH] Add layout file validation. Based on voluptuous library. Basic validation should catch typos, missing or extra attributes. Can be expanded to do more serious validation (ie, specifying a comment in a trigger should require the event be comment-added). Adds a command line option to validate a named layout file and exit. (Also add dist/ to .gitignore.) Change-Id: Ia864ebde1765141d4d1a52bc77033689b6210e81 Reviewed-on: https://review.openstack.org/19443 Reviewed-by: Clark Boylan Reviewed-by: Jeremy Stanley Approved: James E. Blair Tested-by: Jenkins --- .gitignore | 1 + tests/fixtures/layouts/bad_pipelines | 1 + tests/fixtures/layouts/bad_pipelines1.yaml | 4 + tests/fixtures/layouts/bad_pipelines10.yaml | 7 ++ tests/fixtures/layouts/bad_pipelines2.yaml | 6 ++ tests/fixtures/layouts/bad_pipelines3.yaml | 6 ++ tests/fixtures/layouts/bad_pipelines4.yaml | 8 ++ tests/fixtures/layouts/bad_pipelines5.yaml | 9 ++ tests/fixtures/layouts/bad_pipelines6.yaml | 9 ++ tests/fixtures/layouts/bad_pipelines7.yaml | 5 + tests/fixtures/layouts/bad_pipelines8.yaml | 5 + tests/fixtures/layouts/bad_pipelines9.yaml | 8 ++ tests/fixtures/layouts/bad_projects1.yaml | 9 ++ tests/fixtures/layouts/bad_projects2.yaml | 9 ++ tests/fixtures/layouts/good_layout.yaml | 58 ++++++++++ tests/test_layoutvalidator.py | 63 +++++++++++ tools/pip-requires | 1 + zuul/cmd/server.py | 23 ++++ zuul/layoutvalidator.py | 114 ++++++++++++++++++++ zuul/scheduler.py | 20 +++- 20 files changed, 362 insertions(+), 4 deletions(-) create mode 100644 tests/fixtures/layouts/bad_pipelines create mode 100644 tests/fixtures/layouts/bad_pipelines1.yaml create mode 100644 tests/fixtures/layouts/bad_pipelines10.yaml create mode 100644 tests/fixtures/layouts/bad_pipelines2.yaml create mode 100644 tests/fixtures/layouts/bad_pipelines3.yaml create mode 100644 tests/fixtures/layouts/bad_pipelines4.yaml create mode 100644 tests/fixtures/layouts/bad_pipelines5.yaml create mode 100644 tests/fixtures/layouts/bad_pipelines6.yaml create mode 100644 tests/fixtures/layouts/bad_pipelines7.yaml create mode 100644 tests/fixtures/layouts/bad_pipelines8.yaml create mode 100644 tests/fixtures/layouts/bad_pipelines9.yaml create mode 100644 tests/fixtures/layouts/bad_projects1.yaml create mode 100644 tests/fixtures/layouts/bad_projects2.yaml create mode 100644 tests/fixtures/layouts/good_layout.yaml create mode 100644 tests/test_layoutvalidator.py create mode 100644 zuul/layoutvalidator.py diff --git a/.gitignore b/.gitignore index e6dc1478ae..6cb81bf497 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,4 @@ ChangeLog config doc/build/* zuul/versioninfo +dist/ diff --git a/tests/fixtures/layouts/bad_pipelines b/tests/fixtures/layouts/bad_pipelines new file mode 100644 index 0000000000..f627208692 --- /dev/null +++ b/tests/fixtures/layouts/bad_pipelines @@ -0,0 +1 @@ +pipelines: diff --git a/tests/fixtures/layouts/bad_pipelines1.yaml b/tests/fixtures/layouts/bad_pipelines1.yaml new file mode 100644 index 0000000000..4207a2c2aa --- /dev/null +++ b/tests/fixtures/layouts/bad_pipelines1.yaml @@ -0,0 +1,4 @@ +pipelines: + +projects: + - name: foo diff --git a/tests/fixtures/layouts/bad_pipelines10.yaml b/tests/fixtures/layouts/bad_pipelines10.yaml new file mode 100644 index 0000000000..5248c1772e --- /dev/null +++ b/tests/fixtures/layouts/bad_pipelines10.yaml @@ -0,0 +1,7 @@ +pipelines: + - name: check + manager: IndependentPipelineManager + +projects: + - name: foo + merge-mode: foo \ No newline at end of file diff --git a/tests/fixtures/layouts/bad_pipelines2.yaml b/tests/fixtures/layouts/bad_pipelines2.yaml new file mode 100644 index 0000000000..e75a5618f4 --- /dev/null +++ b/tests/fixtures/layouts/bad_pipelines2.yaml @@ -0,0 +1,6 @@ +pipelines: + - noname: check + manager: IndependentPipelineManager + +projects: + - name: foo diff --git a/tests/fixtures/layouts/bad_pipelines3.yaml b/tests/fixtures/layouts/bad_pipelines3.yaml new file mode 100644 index 0000000000..0c11a855c1 --- /dev/null +++ b/tests/fixtures/layouts/bad_pipelines3.yaml @@ -0,0 +1,6 @@ +pipelines: + - name: check + manager: NonexistentPipelineManager + +projects: + - name: foo diff --git a/tests/fixtures/layouts/bad_pipelines4.yaml b/tests/fixtures/layouts/bad_pipelines4.yaml new file mode 100644 index 0000000000..a99b9e29a8 --- /dev/null +++ b/tests/fixtures/layouts/bad_pipelines4.yaml @@ -0,0 +1,8 @@ +pipelines: + - name: check + manager: IndependentPipelineManager + trigger: + - event: non-event + +projects: + - name: foo diff --git a/tests/fixtures/layouts/bad_pipelines5.yaml b/tests/fixtures/layouts/bad_pipelines5.yaml new file mode 100644 index 0000000000..7db7bd1ef4 --- /dev/null +++ b/tests/fixtures/layouts/bad_pipelines5.yaml @@ -0,0 +1,9 @@ +pipelines: + - name: check + manager: IndependentPipelineManager + trigger: + - approval: + - approved: 1 + +projects: + - name: foo diff --git a/tests/fixtures/layouts/bad_pipelines6.yaml b/tests/fixtures/layouts/bad_pipelines6.yaml new file mode 100644 index 0000000000..8d313bc655 --- /dev/null +++ b/tests/fixtures/layouts/bad_pipelines6.yaml @@ -0,0 +1,9 @@ +pipelines: + - name: check + manager: IndependentPipelineManager + trigger: + - event: comment-added + approved: 1 + +projects: + - name: foo diff --git a/tests/fixtures/layouts/bad_pipelines7.yaml b/tests/fixtures/layouts/bad_pipelines7.yaml new file mode 100644 index 0000000000..7517b9ac83 --- /dev/null +++ b/tests/fixtures/layouts/bad_pipelines7.yaml @@ -0,0 +1,5 @@ +pipelines: + - manager: IndependentPipelineManager + +projects: + - name: foo diff --git a/tests/fixtures/layouts/bad_pipelines8.yaml b/tests/fixtures/layouts/bad_pipelines8.yaml new file mode 100644 index 0000000000..eeab0380b1 --- /dev/null +++ b/tests/fixtures/layouts/bad_pipelines8.yaml @@ -0,0 +1,5 @@ +pipelines: + - name: check + +projects: + - name: foo diff --git a/tests/fixtures/layouts/bad_pipelines9.yaml b/tests/fixtures/layouts/bad_pipelines9.yaml new file mode 100644 index 0000000000..ebb2e1fff7 --- /dev/null +++ b/tests/fixtures/layouts/bad_pipelines9.yaml @@ -0,0 +1,8 @@ +pipelines: + - name: check + manager: IndependentPipelineManager + - name: check + manager: IndependentPipelineManager + +projects: + - name: foo diff --git a/tests/fixtures/layouts/bad_projects1.yaml b/tests/fixtures/layouts/bad_projects1.yaml new file mode 100644 index 0000000000..c210c43e04 --- /dev/null +++ b/tests/fixtures/layouts/bad_projects1.yaml @@ -0,0 +1,9 @@ +pipelines: + - name: check + manager: IndependentPipelineManager + +projects: + - name: foo + gate: + - test + diff --git a/tests/fixtures/layouts/bad_projects2.yaml b/tests/fixtures/layouts/bad_projects2.yaml new file mode 100644 index 0000000000..b91ed9dfeb --- /dev/null +++ b/tests/fixtures/layouts/bad_projects2.yaml @@ -0,0 +1,9 @@ +pipelines: + - name: check + manager: IndependentPipelineManager + +projects: + - name: foo + check: + - test + - foo diff --git a/tests/fixtures/layouts/good_layout.yaml b/tests/fixtures/layouts/good_layout.yaml new file mode 100644 index 0000000000..ca024ec129 --- /dev/null +++ b/tests/fixtures/layouts/good_layout.yaml @@ -0,0 +1,58 @@ +includes: + - python-file: openstack_functions.py + +pipelines: + - name: check + manager: IndependentPipelineManager + trigger: + - event: patchset-created + success: + verified: 1 + failure: + verified: -1 + + - name: post + manager: IndependentPipelineManager + trigger: + - event: ref-updated + ref: ^(?!refs/).*$ + + - name: gate + manager: DependentPipelineManager + trigger: + - event: comment-added + approval: + - approved: 1 + success: + verified: 2 + code-review: 1 + submit: true + failure: + verified: -2 + workinprogress: true + start: + verified: 0 + +jobs: + - name: ^.*-merge$ + failure-message: Unable to merge change + hold-following-changes: true + - name: test-merge + parameter-function: devstack_params + - name: test-test + - name: test-merge2 + success-pattern: http://logs.example.com/{change.number}/{change.patchset}/{pipeline.name}/{job.name}/{build.number}/success + failure-pattern: http://logs.example.com/{change.number}/{change.patchset}/{pipeline.name}/{job.name}/{build.number}/fail + +projects: + - name: test-org/test + merge-mode: cherry-pick + check: + - test-merge2: + - test-thing1: + - test-thing2 + - test-thing3 + gate: + - test-thing + post: + - test-post diff --git a/tests/test_layoutvalidator.py b/tests/test_layoutvalidator.py new file mode 100644 index 0000000000..343dc47c4d --- /dev/null +++ b/tests/test_layoutvalidator.py @@ -0,0 +1,63 @@ +#!/usr/bin/env python + +# Copyright 2013 OpenStack Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import unittest +import os +import re +import yaml +import voluptuous + +import zuul.layoutvalidator + +FIXTURE_DIR = os.path.join(os.path.dirname(__file__), + 'fixtures') +LAYOUT_RE = re.compile(r'^(good|bad)_.*\.yaml$') + + +class testScheduler(unittest.TestCase): + def test_layouts(self): + """Test layout file validation""" + print + errors = [] + for fn in os.listdir(os.path.join(FIXTURE_DIR, 'layouts')): + m = LAYOUT_RE.match(fn) + if not m: + continue + print fn + layout = os.path.join(FIXTURE_DIR, 'layouts', fn) + data = yaml.load(open(layout)) + validator = zuul.layoutvalidator.LayoutValidator() + if m.group(1) == 'good': + try: + validator.validate(data) + except voluptuous.Invalid, e: + raise Exception( + 'Unexpected YAML syntax error in %s:\n %s' % + (fn, str(e))) + else: + try: + validator.validate(data) + raise Exception("Expected a YAML syntax error in %s." % + fn) + except voluptuous.Invalid, e: + error = str(e) + print ' ', error + if error in errors: + raise Exception("Error has already beed tested: %s" % + error) + else: + errors.append(error) + pass diff --git a/tools/pip-requires b/tools/pip-requires index 265f663a8b..b0b9054369 100644 --- a/tools/pip-requires +++ b/tools/pip-requires @@ -8,3 +8,4 @@ lockfile python-daemon extras statsd==1.0.0 +voluptuous>=0.5 diff --git a/zuul/cmd/server.py b/zuul/cmd/server.py index 314a57c0ca..2cea2f5211 100755 --- a/zuul/cmd/server.py +++ b/zuul/cmd/server.py @@ -1,4 +1,5 @@ # Copyright 2012 Hewlett-Packard Development Company, L.P. +# Copyright 2013 OpenStack Foundation # # Licensed under the Apache License, Version 2.0 (the "License"); you may # not use this file except in compliance with the License. You may obtain @@ -23,6 +24,7 @@ pid_file_module = extras.try_imports(['daemon.pidlockfile', 'daemon.pidfile']) import logging.config import os +import sys import signal # No zuul imports here because they pull in paramiko which must not be @@ -39,8 +41,12 @@ class Server(object): parser = argparse.ArgumentParser(description='Project gating system.') parser.add_argument('-c', dest='config', help='specify the config file') + parser.add_argument('-l', dest='layout', + help='specify the layout file') parser.add_argument('-d', dest='nodaemon', action='store_true', help='do not run as a daemon') + parser.add_argument('-t', dest='validate', action='store_true', + help='validate layout file syntax') self.args = parser.parse_args() def read_config(self): @@ -77,6 +83,16 @@ class Server(object): signal.signal(signal.SIGUSR1, signal.SIG_IGN) self.sched.exit() + def test_config(self): + # See comment at top of file about zuul imports + import zuul.scheduler + import zuul.launcher.jenkins + import zuul.trigger.gerrit + + logging.basicConfig(level=logging.DEBUG) + self.sched = zuul.scheduler.Scheduler() + self.sched.testConfig(self.config.get('zuul', 'layout_config')) + def main(self): # See comment at top of file about zuul imports import zuul.scheduler @@ -109,6 +125,13 @@ def main(): server.parse_arguments() server.read_config() + if server.args.layout: + server.config.set('zuul', 'layout_config', server.args.layout) + + if server.args.validate: + server.test_config() + sys.exit(0) + if server.config.has_option('zuul', 'state_dir'): state_dir = os.path.expanduser(server.config.get('zuul', 'state_dir')) else: diff --git a/zuul/layoutvalidator.py b/zuul/layoutvalidator.py new file mode 100644 index 0000000000..eac394e1de --- /dev/null +++ b/zuul/layoutvalidator.py @@ -0,0 +1,114 @@ +# Copyright 2013 OpenStack Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import voluptuous as v + + +# Several forms accept either a single item or a list, this makes +# specifying that in the schema easy (and explicit). +def toList(x): + return v.any([x], x) + + +class LayoutSchema(object): + include = {'python-file': str} + includes = [include] + + manager = v.any('IndependentPipelineManager', + 'DependentPipelineManager') + variable_dict = v.Schema({}, extra=True) + + trigger = {v.required('event'): toList(v.any('patchset-created', + 'change-abandoned', + 'change-restored', + 'change-merged', + 'comment-added', + 'ref-updated')), + 'comment_filter': toList(str), + 'email_filter': toList(str), + 'branch': toList(str), + 'ref': toList(str), + 'approval': toList(variable_dict), + } + + pipeline = {v.required('name'): str, + v.required('manager'): manager, + 'description': str, + 'trigger': toList(trigger), + 'success': variable_dict, + 'failure': variable_dict, + 'start': variable_dict, + } + pipelines = [pipeline] + + job = {v.required('name'): str, + 'failure-message': str, + 'success-message': str, + 'failure-pattern': str, + 'success-pattern': str, + 'hold-following-changes': bool, + 'voting': bool, + 'parameter-function': str, + 'branch': toList(str), + } + jobs = [job] + + job_name = v.Schema(v.match("^\S+$")) + + def validateJob(self, value, path=[]): + if isinstance(value, list): + for (i, v) in enumerate(value): + self.validateJob(v, path + [i]) + elif isinstance(value, dict): + for k, v in value.items(): + self.validateJob(v, path + [k]) + else: + self.job_name.validate(path, self.job_name.schema, value) + + def getSchema(self, data): + pipelines = data.get('pipelines') + if not pipelines: + pipelines = [] + pipelines = [p['name'] for p in pipelines if 'name' in p] + project = {'name': str, + 'merge-mode': v.any('cherry-pick'), + } + for p in pipelines: + project[p] = self.validateJob + projects = [project] + + schema = v.Schema({'includes': self.includes, + v.required('pipelines'): self.pipelines, + 'jobs': self.jobs, + v.required('projects'): projects, + }) + return schema + + +class LayoutValidator(object): + def checkDuplicateNames(self, data, path): + items = [] + for i, item in enumerate(data): + if item['name'] in items: + raise v.Invalid("Duplicate name: %s" % item['name'], + path + [i]) + items.append(item['name']) + + def validate(self, data): + schema = LayoutSchema().getSchema(data) + schema(data) + self.checkDuplicateNames(data['pipelines'], ['pipelines']) + if 'jobs' in data: + self.checkDuplicateNames(data['jobs'], ['jobs']) + self.checkDuplicateNames(data['projects'], ['projects']) diff --git a/zuul/scheduler.py b/zuul/scheduler.py index e18035591e..86e78a8982 100644 --- a/zuul/scheduler.py +++ b/zuul/scheduler.py @@ -1,4 +1,5 @@ # Copyright 2012 Hewlett-Packard Development Company, L.P. +# Copyright 2013 OpenStack Foundation # # Licensed under the Apache License, Version 2.0 (the "License"); you may # not use this file except in compliance with the License. You may obtain @@ -23,6 +24,7 @@ import threading import time import yaml +import layoutvalidator import model from model import Pipeline, Job, Project, ChangeQueue, EventFilter import merger @@ -58,6 +60,10 @@ class Scheduler(threading.Thread): self._stopped = True self.wake_event.set() + def testConfig(self, config_path): + self._init() + self._parseConfig(config_path) + def _parseConfig(self, config_path): def toList(item): if not item: @@ -74,6 +80,9 @@ class Scheduler(threading.Thread): config_file = open(config_path) data = yaml.load(config_file) + validator = layoutvalidator.LayoutValidator() + validator.validate(data) + self._config_env = {} for include in data.get('includes', []): if 'python-file' in include: @@ -109,7 +118,7 @@ class Scheduler(threading.Thread): toList(trigger.get('email_filter'))) manager.event_filters.append(f) - for config_job in data['jobs']: + for config_job in data.get('jobs', []): job = self.getJob(config_job['name']) # Be careful to only set attributes explicitly present on # this job, to avoid squashing attributes set by a meta-job. @@ -154,7 +163,7 @@ class Scheduler(threading.Thread): if isinstance(job, str): job_tree.addJob(self.getJob(job)) - for config_project in data['projects']: + for config_project in data.get('projects', []): project = Project(config_project['name']) self.projects[config_project['name']] = project mode = config_project.get('merge-mode') @@ -170,23 +179,25 @@ class Scheduler(threading.Thread): # metajobs so that getJob isn't doing anything weird. self.metajobs = {} - # TODO(jeblair): check that we don't end up with jobs like - # "foo - bar" because a ':' is missing in the yaml for a dependent job for pipeline in self.pipelines.values(): pipeline.manager._postConfig() + def _setupMerger(self): if self.config.has_option('zuul', 'git_dir'): merge_root = self.config.get('zuul', 'git_dir') else: merge_root = '/var/lib/zuul/git' + if self.config.has_option('zuul', 'push_change_refs'): push_refs = self.config.getboolean('zuul', 'push_change_refs') else: push_refs = False + if self.config.has_option('gerrit', 'sshkey'): sshkey = self.config.get('gerrit', 'sshkey') else: sshkey = None + self.merger = merger.Merger(self.trigger, merge_root, push_refs, sshkey) for project in self.projects.values(): @@ -323,6 +334,7 @@ class Scheduler(threading.Thread): self.log.debug("Performing reconfiguration") self._init() self._parseConfig(self.config.get('zuul', 'layout_config')) + self._setupMerger() self._pause = False self._reconfigure = False self.reconfigure_complete_event.set()