From 1d630b0c728051cd9d71c35f126d4fd36692b736 Mon Sep 17 00:00:00 2001 From: "sergiyb@chromium.org" Date: Wed, 24 Jun 2015 13:41:40 +0000 Subject: [PATCH] Update cq_client and add validate command to commit_queue binary R=akuegel@chromium.org, pgervais@chromium.org BUG=472612, 503068 Review URL: https://codereview.chromium.org/1200863002 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@295818 0039d316-1c4b-4281-b951-d872f2087c98 --- commit_queue.py | 20 ++++ third_party/cq_client/README.md | 17 +++ third_party/cq_client/test/cq_example.cfg | 55 +++++++++ .../cq_client/test/validate_config_test.py | 52 ++++++++ third_party/cq_client/validate_config.py | 112 ++++++++++++++++++ 5 files changed, 256 insertions(+) create mode 100644 third_party/cq_client/README.md create mode 100644 third_party/cq_client/test/cq_example.cfg create mode 100755 third_party/cq_client/test/validate_config_test.py create mode 100644 third_party/cq_client/validate_config.py diff --git a/commit_queue.py b/commit_queue.py index b08d4fec7..25ddb3812 100755 --- a/commit_queue.py +++ b/commit_queue.py @@ -26,6 +26,7 @@ THIRD_PARTY_DIR = os.path.join(os.path.dirname(__file__), 'third_party') sys.path.insert(0, THIRD_PARTY_DIR) from cq_client import cq_pb2 +from cq_client import validate_config from protobuf26 import text_format def usage(more): @@ -143,6 +144,25 @@ def CMDbuilders(parser, args): CMDbuilders.func_usage_more = '' + +def CMDvalidate(parser, args): + """Validates a CQ config. + + Takes a single argument - path to the CQ config to be validated. Returns 0 on + valid config, non-zero on invalid config. Errors and warnings are printed to + screen. + """ + _, args = parser.parse_args(args) + if len(args) != 1: + parser.error('Expected a single path to CQ config. Got: %s' % + ' '.join(args)) + + with open(args[0]) as config_file: + cq_config = config_file.read() + return 0 if validate_config.IsValid(cq_config) else 1 + +CMDvalidate.func_usage_more = '' + ############################################################################### ## Boilerplate code diff --git a/third_party/cq_client/README.md b/third_party/cq_client/README.md new file mode 100644 index 000000000..d37caa6af --- /dev/null +++ b/third_party/cq_client/README.md @@ -0,0 +1,17 @@ +This directory contains CQ client library to be distributed to other repos. If +you need to modify some files in this directory, please make sure that you are +changing the canonical version of the source code and not one of the copies, +which should only be updated as a whole using Glyco (when available, see +http://crbug.com/489420). + +The canonical version is located at `https://chrome-internal.googlesource.com/ +infra/infra_internal/+/master/commit_queue/cq_client`. + +To generate `cq_pb2.py`, please use protoc version 2.6.1: + + cd commit_queue/cq_client + protoc cq.proto --python_out $(pwd) + +Additionally, please make sure to use proto3-compatible syntax, e.g. no default +values, no required fields. Ideally, we should use proto3 generator already, +however alpha version thereof is still unstable. diff --git a/third_party/cq_client/test/cq_example.cfg b/third_party/cq_client/test/cq_example.cfg new file mode 100644 index 000000000..dc69acfcb --- /dev/null +++ b/third_party/cq_client/test/cq_example.cfg @@ -0,0 +1,55 @@ +version: 1 +cq_name: "infra" +cq_status_url: "https://chromium-cq-status.appspot.com" +hide_ref_in_committed_msg: true +commit_burst_delay: 600 +max_commit_burst: 10 +in_production: false +git_repo_url: "http://github.com/infra/infra.git" +target_ref: "refs/pending/heads/master" + +rietveld { + url: "https://codereview.chromium.org" + project_bases: "https://chromium.googlesource.com/infra/infra.git@master" +} + +verifiers { + reviewer_lgtm: { + committer_list: "chromium" + max_wait_secs: 600 + no_lgtm_msg: "LGTM is missing" + } + + tree_status: { + tree_status_url: "https://infra-status.appspot.com" + } + + try_job { + buckets { + name: "tryserver.blink" + builders { name: "android_blink_compile_dbg" } + builders { name: "android_blink_compile_rel" } + builders { + name: "win_blink_rel" + triggered: true + } + } + buckets { + name: "tryserver.chromium.linux" + builders { + name: "android_arm64_dbg_recipe" + } + builders { + name: "linux_chromium_rel_ng" + experiment_percentage: 0.1 + } + } + buckets { + name: "tryserver.chromium.mac" + builders { + name: "ios_dbg_simulator_ninja" + experiment_percentage: 1.0 + } + } + } +} \ No newline at end of file diff --git a/third_party/cq_client/test/validate_config_test.py b/third_party/cq_client/test/validate_config_test.py new file mode 100755 index 000000000..d7c09714a --- /dev/null +++ b/third_party/cq_client/test/validate_config_test.py @@ -0,0 +1,52 @@ +# Copyright 2015 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +"""Unit tests for tools/validate_config.py.""" + +import mock +import os +import unittest + +from cq_client import cq_pb2 +from cq_client import validate_config + + +TEST_DIR = os.path.dirname(os.path.abspath(__file__)) + + +class TestValidateConfig(unittest.TestCase): + def test_is_valid(self): + with open(os.path.join(TEST_DIR, 'cq_example.cfg'), 'r') as test_config: + self.assertTrue(validate_config.IsValid(test_config.read())) + + def test_has_field(self): + config = cq_pb2.Config() + + self.assertFalse(validate_config._HasField(config, 'version')) + config.version = 1 + self.assertTrue(validate_config._HasField(config, 'version')) + + self.assertFalse(validate_config._HasField( + config, 'rietveld.project_bases')) + config.rietveld.project_bases.append('foo://bar') + self.assertTrue(validate_config._HasField( + config, 'rietveld.project_bases')) + + self.assertFalse(validate_config._HasField( + config, 'verifiers.try_job.buckets')) + self.assertFalse(validate_config._HasField( + config, 'verifiers.try_job.buckets.name')) + + bucket = config.verifiers.try_job.buckets.add() + bucket.name = 'tryserver.chromium.linux' + + + self.assertTrue(validate_config._HasField( + config, 'verifiers.try_job.buckets')) + self.assertTrue(validate_config._HasField( + config, 'verifiers.try_job.buckets.name')) + + config.verifiers.try_job.buckets.add() + self.assertFalse(validate_config._HasField( + config, 'verifiers.try_job.buckets.name')) diff --git a/third_party/cq_client/validate_config.py b/third_party/cq_client/validate_config.py new file mode 100644 index 000000000..1e34b868e --- /dev/null +++ b/third_party/cq_client/validate_config.py @@ -0,0 +1,112 @@ +#!/usr/bin/env python +# Copyright 2015 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +"""CQ config validation library.""" + +import argparse +# The 'from google import protobuf' below was replaced to fix an issue where +# some users may have built-in google package installed on their system, which +# is incompatible with cq_pb2 below. This hack can be removed after +# http://crbug.com/503067 is resolved. +import protobuf26 as protobuf +import logging +import re +import sys + +from cq_client import cq_pb2 + + +REQUIRED_FIELDS = [ + 'version', + 'rietveld', + 'rietveld.url', + 'verifiers', + 'cq_name', +] + +LEGACY_FIELDS = [ + 'svn_repo_url', + 'server_hooks_missing', + 'verifiers_with_patch', +] + +EMAIL_REGEXP = '^[^@]+@[^@]+\.[^@]+$' + + +def _HasField(message, field_path): + """Checks that at least one field with given path exist in the proto message. + + This function correctly handles repeated fields and will make sure that each + repeated field will have required sub-path, e.g. if 'abc' is a repeated field + and field_path is 'abc.def', then the function will only return True when each + entry for 'abc' will contain at least one value for 'def'. + + Args: + message (google.protobuf.message.Message): Protocol Buffer message to check. + field_path (string): Path to the target field separated with ".". + + Return: + True if at least one such field is explicitly set in the message. + """ + path_parts = field_path.split('.', 1) + field_name = path_parts[0] + sub_path = path_parts[1] if len(path_parts) == 2 else None + + field_labels = {fd.name: fd.label for fd in message.DESCRIPTOR.fields} + repeated_field = (field_labels[field_name] == + protobuf.descriptor.FieldDescriptor.LABEL_REPEATED) + + if sub_path: + field = getattr(message, field_name) + if repeated_field: + if not field: + return False + return all(_HasField(entry, sub_path) for entry in field) + else: + return _HasField(field, sub_path) + else: + if repeated_field: + return len(getattr(message, field_name)) > 0 + else: + return message.HasField(field_name) + + +def IsValid(cq_config): + """Validates a CQ config and prints errors/warnings to the screen. + + Args: + cq_config (string): Unparsed text format of the CQ config proto. + + Returns: + True if the config is valid. + """ + try: + config = cq_pb2.Config() + protobuf.text_format.Merge(cq_config, config) + except protobuf.text_format.ParseError as e: + logging.error('Failed to parse config as protobuf:\n%s', e) + return False + + for fname in REQUIRED_FIELDS: + if not _HasField(config, fname): + logging.error('%s is a required field', fname) + return False + + for fname in LEGACY_FIELDS: + if _HasField(config, fname): + logging.warn('%s is a legacy field', fname) + + + for base in config.rietveld.project_bases: + try: + re.compile(base) + except re.error: + logging.error('failed to parse "%s" in project_bases as a regexp', base) + return False + + # TODO(sergiyb): For each field, check valid values depending on its + # semantics, e.g. email addresses, regular expressions etc. + + return True