From b3be8705b8f1c3ef17185dbdd393c0e4cfd651ba Mon Sep 17 00:00:00 2001 From: Lukas Holecek Date: Jul 16 2018 12:20:20 +0000 Subject: [PATCH 1/2] Make YAML parsing type-safe Policy attributes are parsed with strict types and policy files can only contain policies as top level objects. --- diff --git a/greenwave/policies.py b/greenwave/policies.py index 9a4366f..cde6454 100644 --- a/greenwave/policies.py +++ b/greenwave/policies.py @@ -1,10 +1,17 @@ # SPDX-License-Identifier: GPL-2.0+ from fnmatch import fnmatch -import yaml import logging import greenwave.resources +from greenwave.safe_yaml import ( + SafeYAMLChoice, + SafeYAMLList, + SafeYAMLObject, + SafeYAMLString, + SafeYAMLError, +) + log = logging.getLogger(__name__) @@ -12,26 +19,6 @@ class DisallowedRuleError(RuntimeError): pass -def validate_policies(policies, disallowed_rules=None): - disallowed_rules = disallowed_rules or [] - for policy in policies: - if not isinstance(policy, Policy): - raise RuntimeError('Policies are not configured properly as policy %s ' - 'is not an instance of Policy' % policy) - for required_attribute in ['decision_context', 'product_versions', 'subject_type']: - if not hasattr(policy, required_attribute): - raise RuntimeError('Policies are not configured properly as policy %s ' - 'is missing attribute %s' % (policy.id, required_attribute)) - for rule in policy.rules: - if not isinstance(rule, Rule): - raise RuntimeError('Policies are not configured properly as rule %s ' - 'is not an instance of Rule' % rule) - for disallowed_rule in disallowed_rules: - if isinstance(rule, disallowed_rule): - raise DisallowedRuleError('Policies are not configured properly as rule %s ' - 'is an instance of %s' % (rule, disallowed_rule)) - - def subject_type_identifier_to_item(subject_type, subject_identifier): """ Greenwave < 0.8 included an "item" key in the "unsatisfied_requirements". @@ -160,18 +147,23 @@ class TestResultFailed(RuleNotSatisfied): class InvalidGatingYaml(RuleNotSatisfied): + """ + Remote policy parsing failed. + """ - def __init__(self, subject_type, subject_identifier, test_case_name): + def __init__(self, subject_type, subject_identifier, test_case_name, details): self.subject_type = subject_type self.subject_identifier = subject_identifier self.test_case_name = test_case_name + self.details = details def to_json(self): return { 'type': 'invalid-gating-yaml', 'testcase': self.test_case_name, 'subject_type': self.subject_type, - 'subject_identifier': self.subject_identifier + 'subject_identifier': self.subject_identifier, + 'details': self.details } @@ -234,27 +226,30 @@ def summarize_answers(answers): Returns: str: Human-readable summary. """ - if len(answers) == 0: + if not answers: return 'no tests are required' - if all(answer.is_satisfied for answer in answers): - return 'all required tests passed' + failure_count = len([answer for answer in answers if isinstance(answer, TestResultFailed)]) missing_count = len([answer for answer in answers if isinstance(answer, TestResultMissing)]) - invalid_gating_yaml = any(answer for answer in answers - if isinstance(answer, InvalidGatingYaml)) + if failure_count and missing_count: return '{} of {} required tests failed, {} result{} missing'.format( failure_count, len(answers), missing_count, 's' if missing_count > 1 else '') - elif failure_count: + + if failure_count > 0: return '{} of {} required tests failed'.format(failure_count, len(answers)) - elif missing_count: + + if missing_count > 0: return '{} of {} required test results missing'.format(missing_count, len(answers)) - elif invalid_gating_yaml: - return 'misconfigured gating.yaml file' + + if all(answer.is_satisfied for answer in answers): + return 'all required tests passed' + + assert False, 'Unexpected unsatisfied result' return 'inexplicable result' -class Rule(yaml.YAMLObject): +class Rule(SafeYAMLObject): """ An individual rule within a policy. A policy consists of multiple rules. When the policy is evaluated, each rule returns an answer @@ -279,26 +274,16 @@ class Rule(yaml.YAMLObject): """ raise NotImplementedError() - def to_json(self): - """ Return a dict representation of this rule. - - Returns: - dict: A representation of this Rule as a dict for an API response. - """ - raise NotImplementedError() - -def handle_misconfigured_gating_yaml(subject_type, subject_identifier, waivers): - if any((d['testcase'] == 'invalid-gating-yaml' and d['subject']['type'] == subject_type and - d['subject']['item'] == subject_identifier) for d in waivers): - return [] - else: - return InvalidGatingYaml(subject_type, subject_identifier, 'invalid-gating-yaml') +def waives_invalid_gating_yaml(waiver, subject_type, subject_identifier): + return (waiver['testcase'] == 'invalid-gating-yaml' and + waiver['subject']['type'] == subject_type and + waiver['subject']['item'] == subject_identifier) class RemoteRule(Rule): yaml_tag = '!RemoteRule' - yaml_loader = yaml.SafeLoader + safe_yaml_attributes = {} def check(self, subject_type, subject_identifier, results, waivers): if subject_type != 'koji_build': @@ -313,26 +298,16 @@ class RemoteRule(Rule): return [] try: - policies = yaml.safe_load_all(response) - # policies is a generator, so listifying it - policies = list(policies) - except yaml.parser.ParserError as e: - # if the yaml file is malformed we skip these policies - log.warning("Error parsing gating.yaml for package %s: %s", pkg_name, e) - return handle_misconfigured_gating_yaml(subject_type, subject_identifier, waivers) - # policies in dist-git are always about a package - for policy in policies: - policy.subject_type = 'koji_build' - # Attribute 'id' in remote policy is optional. - policy_id = getattr(policy, 'id', 'untitled') - # Prefix the id for better error reporting. - policy.id = 'dist-git-gating-policy-{}-{}'.format(policy_id, pkg_name) - try: - validate_policies(policies, [RemoteRule]) - except DisallowedRuleError: - log.warning('Policies are not configured properly as there is a policy ' - 'that is an instance of RemoteRule') - return handle_misconfigured_gating_yaml(subject_type, subject_identifier, waivers) + policies = RemotePolicy.safe_load_all(response) + except SafeYAMLError as e: + if any(waives_invalid_gating_yaml(waiver, subject_type, subject_identifier) + for waiver in waivers): + return [] + return [ + InvalidGatingYaml( + subject_type, subject_identifier, 'invalid-gating-yaml', str(e)) + ] + answers = [] for policy in policies: response = policy.check(subject_identifier, results, waivers) @@ -354,7 +329,11 @@ class PassingTestCaseRule(Rule): a non-passing result with a waiver. """ yaml_tag = '!PassingTestCaseRule' - yaml_loader = yaml.SafeLoader + + safe_yaml_attributes = { + 'test_case_name': SafeYAMLString(), + 'scenario': SafeYAMLString(optional=True), + } def check(self, subject_type, subject_identifier, results, waivers): matching_results = [ @@ -363,7 +342,7 @@ class PassingTestCaseRule(Rule): w for w in waivers if (w['testcase'] == self.test_case_name and w['waived'] is True)] # Rules may optionally specify a scenario to limit applicability. - if self._scenario: + if self.scenario: matching_results = [r for r in matching_results if self.scenario in r['data'].get('scenario', [])] @@ -371,9 +350,9 @@ class PassingTestCaseRule(Rule): if not matching_results: if not matching_waivers: return TestResultMissing(subject_type, subject_identifier, self.test_case_name, - self._scenario) + self.scenario) return TestResultMissingWaived( - subject_type, subject_identifier, self.test_case_name, self._scenario) + subject_type, subject_identifier, self.test_case_name, self.scenario) # If we find multiple matching results, we always use the first one which # will be the latest chronologically, because ResultsDB always returns @@ -389,21 +368,13 @@ class PassingTestCaseRule(Rule): w['waived'] for w in waivers): return TestResultPassed(self.test_case_name, matching_result['id']) return TestResultFailed(subject_type, subject_identifier, self.test_case_name, - self._scenario, matching_result['id']) - - @property - def _scenario(self): - return getattr(self, 'scenario', None) - - def __repr__(self): - return "%s(test_case_name=%r, scenario=%r)" % ( - self.__class__.__name__, self.test_case_name, self._scenario) + self.scenario, matching_result['id']) def to_json(self): return { 'rule': self.__class__.__name__, 'test_case_name': self.test_case_name, - 'scenario': self._scenario, + 'scenario': self.scenario, } @@ -415,9 +386,10 @@ class PackageSpecificRule(Rule): This intermediary class should be considered abstract, and not used directly. """ - def __init__(self, test_case_name, repos): - self.test_case_name = test_case_name - self.repos = repos + safe_yaml_attributes = { + 'test_case_name': SafeYAMLString(), + 'repos': SafeYAMLList(str), + } def check(self, subject_type, subject_identifier, results, waivers): """ Check that the subject passes testcase for the given results, but @@ -445,10 +417,6 @@ class PackageSpecificRule(Rule): rule.test_case_name = self.test_case_name return rule.check(subject_type, subject_identifier, results, waivers) - def __repr__(self): - return "%s(test_case_name=%s, repos=%r)" % ( - self.__class__.__name__, self.test_case_name, self.repos) - def to_json(self): return { 'rule': self.__class__.__name__, @@ -459,18 +427,27 @@ class PackageSpecificRule(Rule): class FedoraAtomicCi(PackageSpecificRule): yaml_tag = '!FedoraAtomicCi' - yaml_loader = yaml.SafeLoader class PackageSpecificBuild(PackageSpecificRule): yaml_tag = '!PackageSpecificBuild' - yaml_loader = yaml.SafeLoader -class Policy(yaml.YAMLObject): - yaml_tag = '!Policy' - yaml_loader = yaml.SafeLoader - blacklist = [] +class Policy(SafeYAMLObject): + root_yaml_tag = '!Policy' + + safe_yaml_attributes = { + 'id': SafeYAMLString(), + 'product_versions': SafeYAMLList(str), + 'decision_context': SafeYAMLString(), + # TODO: Handle brew-build value better. + 'subject_type': SafeYAMLChoice( + 'koji_build', 'bodhi_update', 'compose', 'brew-build'), + 'rules': SafeYAMLList(Rule), + 'blacklist': SafeYAMLList(str, optional=True), + 'relevance_key': SafeYAMLString(optional=True), + 'relevance_value': SafeYAMLString(optional=True), + } def applies_to(self, decision_context, product_version, subject_type): return (decision_context == self.decision_context and @@ -492,22 +469,29 @@ class Policy(yaml.YAMLObject): answers.append(response) return answers - def __repr__(self): - return "%s(id=%r, product_versions=%r, decision_context=%r, subject_type=%r, rules=%r)" % ( - self.__class__.__name__, self.id, self.product_versions, self.decision_context, - self.subject_type, self.rules) - - def to_json(self): - return { - 'id': self.id, - 'product_versions': self.product_versions, - 'decision_context': self.decision_context, - 'subject_type': self.subject_type, - 'rules': [rule.to_json() for rule in self.rules], - 'blacklist': self.blacklist, - 'relevance_key': getattr(self, 'relevance_key', None), - 'relevance_value': getattr(self, 'relevance_value', None), - } - def _applies_to_product_version(self, product_version): return any(fnmatch(product_version, version) for version in self.product_versions) + + @property + def safe_yaml_label(self): + return 'Policy {!r}'.format(self.id or 'untitled') + + +class RemotePolicy(Policy): + root_yaml_tag = '!Policy' + + safe_yaml_attributes = { + 'id': SafeYAMLString(optional=True), + 'product_versions': SafeYAMLList(str), + 'decision_context': SafeYAMLString(), + 'rules': SafeYAMLList(Rule), + 'blacklist': SafeYAMLList(str, optional=True), + } + + subject_type = 'koji_build' + + def validate(self): + for rule in self.rules: + if isinstance(rule, RemoteRule): + raise SafeYAMLError('RemoteRule is not allowed in remote policies') + super().validate() diff --git a/greenwave/safe_yaml.py b/greenwave/safe_yaml.py new file mode 100644 index 0000000..6b407ee --- /dev/null +++ b/greenwave/safe_yaml.py @@ -0,0 +1,214 @@ +# SPDX-License-Identifier: GPL-2.0+ +""" +Provides a way of defining type-safe YAML parsing. +""" +import yaml + + +class SafeYAMLError(RuntimeError): + """ + Exception raised when an unexpected type is found in YAML. + or validation fails (see SafeYAMLObject.validate()). + """ + pass + + +class SafeYAMLAttribute(object): + """ + Base class for SafeYAMLObject attributes (in SafeYAMLObject.safe_yaml_attributes dict). + """ + def __init__(self, optional=False): + self.optional = optional + + def from_yaml(self, loader, node): + raise NotImplementedError() + + def to_json(self, value): + raise NotImplementedError() + + @property + def default_value(self): + raise NotImplementedError() + + +class SafeYAMLString(SafeYAMLAttribute): + """ + YAML object attribute representing a string value. + """ + def from_yaml(self, loader, node): + value = loader.construct_scalar(node) + return str(value) + + def to_json(self, value): + return value + + @property + def default_value(self): + return None + + +class SafeYAMLChoice(SafeYAMLAttribute): + """ + YAML object attribute with only specific values allowed. + """ + def __init__(self, *values, **kwargs): + super().__init__(**kwargs) + self.values = values + + def from_yaml(self, loader, node): + value = loader.construct_scalar(node) + if value not in self.values: + raise SafeYAMLError( + 'Value must be one of: {}'.format(', '.join(self.values))) + return str(value) + + def to_json(self, value): + return value + + @property + def default_value(self): + return self.values[0] + + +class SafeYAMLList(SafeYAMLAttribute): + """ + YAML object attribute represeting a list of values. + """ + def __init__(self, item_type, **kwargs): + super().__init__(**kwargs) + self.item_type = item_type + + def from_yaml(self, loader, node): + values = loader.construct_sequence(node) + for value in values: + if not isinstance(value, self.item_type): + raise SafeYAMLError( + 'Expected list of {} objects'.format(self.item_type.__name__)) + return values + + @property + def default_value(self): + return [] + + def to_json(self, values): + return [self._item_to_json(value) for value in values] + + def _item_to_json(self, value): + if isinstance(value, SafeYAMLObject): + return value.to_json() + return value + + +class SafeYAMLObjectMetaclass(yaml.YAMLObjectMetaclass): + """ + The metaclass for SafeYAMLObject. + + Enabled YAML loader to accept only root objects of specific type. + """ + def __init__(cls, name, bases, kwds): + super().__init__(name, bases, kwds) + + tag = getattr(cls, 'root_yaml_tag', None) + if tag: + class Loader(cls.yaml_loader): + def get_node(self): + node = super().get_node() + + if node.tag != tag: + raise SafeYAMLError('Missing {} tag'.format(tag)) + + if not isinstance(node, yaml.MappingNode): + raise SafeYAMLError('Expected mapping for {} tagged object'.format(tag)) + + return node + + Loader.add_constructor(tag, cls.from_yaml) + cls.yaml_loader = Loader + + +class SafeYAMLObject(yaml.YAMLObject, metaclass=SafeYAMLObjectMetaclass): + """ + Base class for safer YAML map objects. + + Allows to specify attribute types and whether these are optional. + + Optionally, set class attribute root_yaml_tag to YAML tag name. This will + be used to verify the root object has this tag. + + Define class attribute safe_yaml_attributes which is dict mapping attribute + name to a SafeYAMLAttribute object. + """ + yaml_loader = yaml.SafeLoader + + @classmethod + def __new__(cls, *args, **kwargs): + result = super().__new__(*args, **kwargs) + + for attribute_name, yaml_attribute in cls.safe_yaml_attributes.items(): + value = yaml_attribute.default_value + setattr(result, attribute_name, value) + + return result + + @classmethod + def from_yaml(cls, loader, node): + nodes = { + name_node.value: value_node + for name_node, value_node in node.value + } + result = cls() + + for attribute_name, yaml_attribute in cls.safe_yaml_attributes.items(): + child_node = nodes.get(attribute_name) + if child_node is None: + if not yaml_attribute.optional: + msg = '{}: Attribute {!r} is required'.format( + result.safe_yaml_label, attribute_name) + raise SafeYAMLError(msg) + value = yaml_attribute.default_value + else: + try: + value = yaml_attribute.from_yaml(loader, child_node) + except (SafeYAMLError, yaml.YAMLError) as e: + msg = '{}: Attribute {!r}: {}'.format( + result.safe_yaml_label, attribute_name, str(e)) + raise SafeYAMLError(msg) + setattr(result, attribute_name, value) + + try: + result.validate() + except SafeYAMLError as e: + msg = '{}: {}'.format(result.safe_yaml_label, str(e)) + raise SafeYAMLError(msg) + + return result + + @classmethod + def safe_load_all(cls, file_or_content): + """ + Load objects from file or a data. + + :raises: SafeYAMLError: if root object tag doesn't match yaml_tag, + attributes don't match their types or parsing fails. + """ + try: + values = yaml.load_all(file_or_content, Loader=cls.yaml_loader) + values = list(values) + except yaml.YAMLError as e: + raise SafeYAMLError('YAML Parser Error: {}'.format(e)) + + return values + + @property + def safe_yaml_label(self): + return 'YAML object {}'.format(self.yaml_tag) + + def validate(self): + pass + + def to_json(self): + return { + attribute_name: yaml_attribute.to_json( + getattr(self, attribute_name, None)) + for attribute_name, yaml_attribute in self.safe_yaml_attributes.items() + } diff --git a/greenwave/tests/test_policies.py b/greenwave/tests/test_policies.py index 25d9d20..e0c93eb 100644 --- a/greenwave/tests/test_policies.py +++ b/greenwave/tests/test_policies.py @@ -4,15 +4,20 @@ import pytest import mock +from textwrap import dedent + from greenwave.app_factory import create_app from greenwave.policies import ( summarize_answers, + Policy, + RemotePolicy, RuleSatisfied, TestResultMissing, TestResultFailed, InvalidGatingYaml ) from greenwave.utils import load_policies +from greenwave.safe_yaml import SafeYAMLError def test_summarize_answers(): @@ -180,9 +185,9 @@ subject_type: bodhi_update rules: - !PassingTestCaseRule {test_case_name: dist.abicheck} """) - with pytest.raises(RuntimeError) as excinfo: + expected_error = "Missing !Policy tag" + with pytest.raises(SafeYAMLError, match=expected_error): load_policies(tmpdir.strpath) - assert 'Policies are not configured properly' in str(excinfo.value) def test_misconfigured_policy_rules(tmpdir): @@ -197,9 +202,13 @@ subject_type: bodhi_update rules: - {test_case_name: dist.abicheck} """) - with pytest.raises(RuntimeError) as excinfo: + expected_error = ( + "Policy 'taskotron_release_critical_tasks': " + "Attribute 'rules': " + "Expected list of Rule objects" + ) + with pytest.raises(SafeYAMLError, match=expected_error): load_policies(tmpdir.strpath) - assert 'Policies are not configured properly' in str(excinfo.value) def test_passing_testcasename_with_scenario(tmpdir): @@ -338,12 +347,12 @@ rules: policy = policies[0] results, waivers = [], [] - expected_error = ( - 'policy dist-git-gating-policy-untitled-nethack' - ' is missing attribute product_versions' - ) - with pytest.raises(RuntimeError, match=expected_error): - policy.check(nvr, results, waivers) + expected_details = "Policy 'untitled': Attribute 'product_versions' is required" + decision = policy.check(nvr, results, waivers) + assert len(decision) == 1 + assert isinstance(decision[0], InvalidGatingYaml) + assert decision[0].is_satisfied is False + assert decision[0].details == expected_details def test_remote_rule_malformed_yaml(tmpdir): @@ -459,3 +468,223 @@ rules: }] decision = policy.check(nvr, results, waivers) assert len(decision) == 0 + + +def test_parse_policies_missing_tag(): + expected_error = "Missing !Policy tag" + with pytest.raises(SafeYAMLError, match=expected_error): + Policy.safe_load_all("""---""") + + +def test_parse_policies_unexpected_type(): + policies = dedent(""" + --- !Policy + 42 + """) + expected_error = "Expected mapping for !Policy tagged object" + with pytest.raises(SafeYAMLError, match=expected_error): + RemotePolicy.safe_load_all(policies) + + +def test_parse_policies_missing_id(): + expected_error = "Policy 'untitled': Attribute 'id' is required" + with pytest.raises(SafeYAMLError, match=expected_error): + Policy.safe_load_all(dedent(""" + --- !Policy + product_versions: [fedora-rawhide] + decision_context: test + subject_type: compose + blacklist: [] + rules: + - !PassingTestCaseRule {test_case_name: compose.cloud.all} + """)) + + +def test_parse_policies_missing_product_versions(): + expected_error = "Policy 'test': Attribute 'product_versions' is required" + with pytest.raises(SafeYAMLError, match=expected_error): + Policy.safe_load_all(dedent(""" + --- !Policy + id: test + decision_context: test + subject_type: compose + blacklist: [] + rules: + - !PassingTestCaseRule {test_case_name: compose.cloud.all} + """)) + + +def test_parse_policies_missing_decision_context(): + expected_error = "Policy 'test': Attribute 'decision_context' is required" + with pytest.raises(SafeYAMLError, match=expected_error): + Policy.safe_load_all(dedent(""" + --- !Policy + id: test + product_versions: [fedora-rawhide] + subject_type: compose + blacklist: [] + rules: + - !PassingTestCaseRule {test_case_name: compose.cloud.all} + """)) + + +def test_parse_policies_invalid_subject_type(): + expected_error = ( + r"Policy 'test': Attribute 'subject_type': " + "Value must be one of:.*" + ) + with pytest.raises(SafeYAMLError, match=expected_error): + Policy.safe_load_all(dedent(""" + --- !Policy + id: test + product_versions: [fedora-rawhide] + decision_context: test + subject_type: bad_subject + blacklist: [] + rules: + - !PassingTestCaseRule {test_case_name: compose.cloud.all} + - 0 + """)) + + +def test_parse_policies_invalid_rule(): + expected_error = "Policy 'test': Attribute 'rules': Expected list of Rule objects" + with pytest.raises(SafeYAMLError, match=expected_error): + Policy.safe_load_all(dedent(""" + --- !Policy + id: test + product_versions: [fedora-rawhide] + decision_context: test + subject_type: compose + blacklist: [] + rules: + - !PassingTestCaseRule {test_case_name: compose.cloud.all} + - bad_rule + """)) + + +def test_parse_policies_remote_missing_tag(): + expected_error = "Missing !Policy tag" + with pytest.raises(SafeYAMLError, match=expected_error): + RemotePolicy.safe_load_all("""---""") + + +def test_parse_policies_remote_missing_id_is_ok(): + policies = RemotePolicy.safe_load_all(dedent(""" + --- !Policy + product_versions: [fedora-rawhide] + decision_context: test + subject_type: koji_build + rules: + - !PassingTestCaseRule {test_case_name: test.case.name} + """)) + assert len(policies) == 1 + assert policies[0].id is None + + +def test_parse_policies_remote_missing_subject_type_is_ok(): + policies = RemotePolicy.safe_load_all(dedent(""" + --- !Policy + product_versions: [fedora-rawhide] + decision_context: test + rules: + - !PassingTestCaseRule {test_case_name: test.case.name} + """)) + assert len(policies) == 1 + assert policies[0].subject_type == 'koji_build' + + +def test_parse_policies_remote_recursive(): + expected_error = "Policy 'test': RemoteRule is not allowed in remote policies" + with pytest.raises(SafeYAMLError, match=expected_error): + RemotePolicy.safe_load_all(dedent(""" + --- !Policy + id: test + product_versions: [fedora-rawhide] + decision_context: bodhi_update_push_stable_with_remoterule + subject_type: koji_build + rules: + - !RemoteRule {} + """)) + + +def test_parse_policies_remote_multiple(): + policies = RemotePolicy.safe_load_all(dedent(""" + --- !Policy + id: test1 + product_versions: [fedora-rawhide] + decision_context: test + rules: + - !PassingTestCaseRule {test_case_name: test.case.name} + + --- !Policy + id: test2 + product_versions: [fedora-rawhide] + decision_context: test + rules: + - !PassingTestCaseRule {test_case_name: test.case.name} + """)) + assert len(policies) == 2 + assert policies[0].id == 'test1' + assert policies[1].id == 'test2' + + +def test_parse_policies_remote_multiple_missing_tag(): + expected_error = "Missing !Policy tag" + with pytest.raises(SafeYAMLError, match=expected_error): + RemotePolicy.safe_load_all(dedent(""" + --- !Policy + id: test1 + product_versions: [fedora-rawhide] + decision_context: test + rules: + - !PassingTestCaseRule {test_case_name: test.case.name} + + --- + id: test2 + product_versions: [fedora-rawhide] + decision_context: test + rules: + - !PassingTestCaseRule {test_case_name: test.case.name} + """)) + + +def test_parse_policies_remote_missing_rule_attribute(): + expected_error = ( + "Policy 'test': " + "Attribute 'rules': " + "YAML object !PassingTestCaseRule: " + "Attribute 'test_case_name' is required" + ) + with pytest.raises(SafeYAMLError, match=expected_error): + RemotePolicy.safe_load_all(dedent(""" + --- !Policy + id: test + product_versions: [fedora-rawhide] + decision_context: test + rules: + - !PassingTestCaseRule {test_case: test.case.name} + """)) + + +def test_policies_to_json(): + policies = Policy.safe_load_all(dedent(""" + --- !Policy + id: test + product_versions: [fedora-rawhide] + decision_context: test + subject_type: compose + blacklist: [] + rules: [] + """)) + assert len(policies) == 1 + assert policies[0].to_json() == { + 'id': 'test', + 'product_versions': ['fedora-rawhide'], + 'decision_context': 'test', + 'subject_type': 'compose', + 'blacklist': [], + 'rules': [], + 'relevance_key': None, + 'relevance_value': None, + } diff --git a/greenwave/utils.py b/greenwave/utils.py index 66b4417..e32f44a 100644 --- a/greenwave/utils.py +++ b/greenwave/utils.py @@ -7,7 +7,6 @@ import os import time import hashlib -import yaml from flask import jsonify, current_app, request from flask.config import Config from requests import ConnectionError, Timeout @@ -119,8 +118,8 @@ def load_policies(policies_dir): policy_pathnames = glob.glob(os.path.join(policies_dir, '*.yaml')) policies = [] for policy_pathname in policy_pathnames: - policies.extend(yaml.safe_load_all(open(policy_pathname, 'r'))) - greenwave.policies.validate_policies(policies) + with open(policy_pathname, 'r') as f: + policies.extend(greenwave.policies.Policy.safe_load_all(f)) log.debug("Loaded %i policies from %s", len(policies), policies_dir) return policies From 99a252c610e2dd02420ddbd69c28ab8306bf1fd7 Mon Sep 17 00:00:00 2001 From: Lukas Holecek Date: Jul 16 2018 12:38:24 +0000 Subject: [PATCH 2/2] Provide API for validating gating.yaml Example command: http localhost:5005/api/v1.0/validate-gating-yaml \ < ../greenwave-policies/fedora.yaml Related to #217. --- diff --git a/functional-tests/test_api_v1.py b/functional-tests/test_api_v1.py index 3abcf17..4bc47d1 100644 --- a/functional-tests/test_api_v1.py +++ b/functional-tests/test_api_v1.py @@ -2,6 +2,8 @@ import json +from textwrap import dedent + from greenwave import __version__ @@ -826,3 +828,41 @@ def test_make_a_decision_about_brew_build(requests_session, greenwave_server, te assert res_data['policies_satisfied'] is True assert res_data['applicable_policies'] == ['osci_compose'] assert res_data['summary'] == 'all required tests passed' + + +def test_validate_gating_yaml_valid(requests_session, greenwave_server): + gating_yaml = dedent(""" + --- !Policy + id: "test" + product_versions: + - fedora-26 + decision_context: test + rules: + - !PassingTestCaseRule {test_case_name: test} + """) + result = requests_session.post( + greenwave_server + 'api/v1.0/validate-gating-yaml', data=gating_yaml) + assert result.json().get('message') == 'All OK' + assert result.status_code == 200 + + +def test_validate_gating_yaml_empty(requests_session, greenwave_server): + result = requests_session.post(greenwave_server + 'api/v1.0/validate-gating-yaml') + assert result.json().get('message') == 'No policies defined' + assert result.status_code == 400 + + +def test_validate_gating_yaml_missing_tag(requests_session, greenwave_server): + gating_yaml = dedent(""" + --- + id: "test" + product_versions: + - fedora-26 + decision_context: test + rules: + - !PassingTestCaseRule {test_case_name: test} + """) + result = requests_session.post( + greenwave_server + 'api/v1.0/validate-gating-yaml', data=gating_yaml) + assert result.json().get('message') == "Missing !Policy tag" + assert result.status_code == 400 diff --git a/greenwave/api_v1.py b/greenwave/api_v1.py index 41a9687..9481185 100644 --- a/greenwave/api_v1.py +++ b/greenwave/api_v1.py @@ -3,8 +3,9 @@ from flask import Blueprint, request, current_app, jsonify, url_for, redirect from werkzeug.exceptions import BadRequest, NotFound, UnsupportedMediaType, InternalServerError from greenwave import __version__ -from greenwave.policies import summarize_answers, RemoteRule +from greenwave.policies import summarize_answers, RemotePolicy, RemoteRule from greenwave.resources import retrieve_results, retrieve_waivers, retrieve_builds_in_update +from greenwave.safe_yaml import SafeYAMLError from greenwave.utils import insert_headers, jsonp api = (Blueprint('api_v1', __name__)) @@ -372,3 +373,54 @@ def make_decision(): resp = insert_headers(resp) resp.status_code = 200 return resp + + +@api.route('/validate-gating-yaml', methods=['GET', 'POST']) +@jsonp +def validate_gating_yaml_post(): + """ + Validates contents of "gating.yaml" file. + + POST data is the file content. + + The response is JSON object containing lists of "errors", "successes" and + "messages". + + **Sample response for failed validation**: + + .. sourcecode:: none + + HTTP/1.0 200 OK + Content-Length: 52 + Content-Type: application/json + Date: Fri, 22 Jun 2018 11:19:35 GMT + Server: Werkzeug/0.12.2 Python/3.6.5 + + { + "message": "Missing !Policy tag" + } + + **Sample response for successful validation**: + + .. sourcecode:: none + + HTTP/1.0 200 OK + Content-Length: 38 + Content-Type: application/json + Date: Fri, 22 Jun 2018 11:23:16 GMT + Server: Werkzeug/0.12.2 Python/3.6.5 + + { + "message": "All OK" + } + """ + content = request.get_data().decode('utf-8') + try: + policies = RemotePolicy.safe_load_all(content) + except SafeYAMLError as e: + raise BadRequest(str(e)) + + if not policies: + raise BadRequest('No policies defined') + + return jsonify({'message': 'All OK'})