From 7d0d5b668041d771975673e8e7d153e19381edaa Mon Sep 17 00:00:00 2001 From: Lukas Holecek Date: Aug 21 2019 06:10:43 +0000 Subject: Add required flag to RemoteRule Signed-off-by: Lukas Holecek --- diff --git a/docs/package-specific-policies.rst b/docs/package-specific-policies.rst index 0687e09..2314979 100644 --- a/docs/package-specific-policies.rst +++ b/docs/package-specific-policies.rst @@ -70,6 +70,45 @@ The side effect is that all the policies defined in the gating.yaml file will be completely ignored by Greenwave. +.. _missing-gating-yaml: + +Missing gating.yaml file +------------------------ + +Missing gating.yaml file (i.e. not present in dist-git repo of the tested +package in the required revision) is just skipped and not treated as +unsatisfied requirement by default. To change this, ``required`` boolean +attribute of ``RemoteRule`` must be set to ``true``. + +.. code-block:: yaml + + --- !Policy + id: some_policy + product_versions: [fedora-*] + decision_context: bodhi_update_push_testing + subject_type: koji_build + rules: + - !RemoteRule {required: true} + +For such policy, if gating.yaml is missing, could result in the following +decision. + +.. code-block:: json + + { + "applicable_policies": ["some_policy"], + "policies_satisfied": false, + "satisfied_requirements": [] + "summary": "1 of 1 required tests failed", + "unsatisfied_requirements": [{ + "subject_identifier": "nethack-1.2.3-1.f31", + "subject_type": "koji_build", + "testcase": "missing-gating-yaml", + "type": "missing-gating-yaml" + }], + } + + .. _tutorial-configure-remoterule: Tutorial - How to configure the RemoteRule diff --git a/greenwave/policies.py b/greenwave/policies.py index 5baba0c..4b2b31e 100644 --- a/greenwave/policies.py +++ b/greenwave/policies.py @@ -10,6 +10,7 @@ from werkzeug.exceptions import BadRequest from flask import current_app from greenwave.safe_yaml import ( + SafeYAMLBool, SafeYAMLChoice, SafeYAMLList, SafeYAMLObject, @@ -204,6 +205,29 @@ class InvalidGatingYaml(RuleNotSatisfied): return None +class MissingGatingYaml(RuleNotSatisfied): + """ + Remote policy not found in remote repository. + """ + + test_case_name = 'missing-gating-yaml' + + def __init__(self, subject_type, subject_identifier): + self.subject_type = subject_type + self.subject_identifier = subject_identifier + + def to_json(self): + return { + 'type': 'missing-gating-yaml', + 'testcase': self.test_case_name, + 'subject_type': self.subject_type, + 'subject_identifier': self.subject_identifier, + } + + def to_waived(self): + return None + + class TestResultPassed(RuleSatisfied): """ A required test case passed (that is, its outcome in ResultsDB was @@ -363,7 +387,9 @@ class Rule(SafeYAMLObject): class RemoteRule(Rule): yaml_tag = '!RemoteRule' - safe_yaml_attributes = {} + safe_yaml_attributes = { + 'required': SafeYAMLBool(optional=True, default=False), + } def _get_sub_policies(self, policy, subject_identifier): if policy.subject_type not in ['koji_build', 'redhat-module']: @@ -380,7 +406,7 @@ class RemoteRule(Rule): if response is None: # greenwave extension file not found - return [] + return None policies = RemotePolicy.safe_load_all(response) if isinstance(policy, OnDemandPolicy): @@ -408,6 +434,11 @@ class RemoteRule(Rule): policy.subject_type, subject_identifier, 'invalid-gating-yaml', str(e)) ] + if policies is None: + if self.required: + return [MissingGatingYaml(policy.subject_type, subject_identifier)] + return [] + answers = [] for remote_policy in policies: if remote_policy.matches_product_version(product_version): diff --git a/greenwave/safe_yaml.py b/greenwave/safe_yaml.py index ac82b96..d65e2a8 100644 --- a/greenwave/safe_yaml.py +++ b/greenwave/safe_yaml.py @@ -30,6 +30,30 @@ class SafeYAMLAttribute(object): raise NotImplementedError() +class SafeYAMLBool(SafeYAMLAttribute): + """ + YAML object attribute representing a boolean value. + """ + def __init__(self, default=False, **args): + super().__init__(**args) + self.default = default + + def from_yaml(self, loader, node): + value = loader.construct_scalar(node) + value = yaml.safe_load(value) + if isinstance(value, bool): + return value + + raise SafeYAMLError('Expected a boolean value, got: {}'.format(value)) + + def to_json(self, value): + return value + + @property + def default_value(self): + return self.default + + class SafeYAMLString(SafeYAMLAttribute): """ YAML object attribute representing a string value. diff --git a/greenwave/tests/test_api_v1.py b/greenwave/tests/test_api_v1.py index 4368660..77a67d3 100644 --- a/greenwave/tests/test_api_v1.py +++ b/greenwave/tests/test_api_v1.py @@ -139,6 +139,30 @@ def test_make_decision_with_no_tests_required_and_missing_gating_yaml(mock_resul mock_waivers.assert_not_called() +def test_make_decision_with_missing_required_gating_yaml(mock_results, mock_waivers): + mock_results.return_value = [] + mock_waivers.return_value = [] + policies = """ + --- !Policy + id: "test_policy" + product_versions: + - fedora-rawhide + decision_context: test_policies + subject_type: koji_build + rules: + - !RemoteRule {required: true} + """ + with mock.patch('greenwave.resources.retrieve_scm_from_koji') as scm: + scm.return_value = ('rpms', 'nethack', 'c3c47a08a66451cb9686c49f040776ed35a0d1bb') + with mock.patch('greenwave.resources.retrieve_yaml_remote_rule') as f: + f.return_value = None + response = make_decision(policies=policies) + assert 200 == response.status_code + assert not response.json['policies_satisfied'] + assert '1 of 1 required tests failed' == response.json['summary'] + mock_waivers.assert_called_once() + + def test_life_decision(): app = create_app('greenwave.config.TestingConfig') client = app.test_client() diff --git a/greenwave/tests/test_policies.py b/greenwave/tests/test_policies.py index bd0c628..4759d18 100644 --- a/greenwave/tests/test_policies.py +++ b/greenwave/tests/test_policies.py @@ -17,6 +17,7 @@ from greenwave.policies import ( TestResultFailed, TestResultPassed, InvalidGatingYaml, + MissingGatingYaml, OnDemandPolicy ) from greenwave.resources import ResultsRetriever @@ -552,6 +553,33 @@ def test_remote_rule_malformed_yaml_with_waiver(tmpdir): assert len(decision) == 0 +def test_remote_rule_required(): + """ Testing the RemoteRule with required flag set """ + nvr = 'nethack-1.2.3-1.el9000' + app = create_app('greenwave.config.TestingConfig') + with app.app_context(): + with mock.patch('greenwave.resources.retrieve_scm_from_koji') as scm: + scm.return_value = ('rpms', 'nethack', 'c3c47a08a66451cb9686c49f040776ed35a0d1bb') + with mock.patch('greenwave.resources.retrieve_yaml_remote_rule') as f: + f.return_value = None + policies = Policy.safe_load_all(dedent(""" + --- !Policy + id: test + product_versions: [fedora-rawhide] + decision_context: test + subject_type: koji_build + rules: + - !RemoteRule {required: true} + """)) + policy = policies[0] + results = DummyResultsRetriever() + decision = policy.check('fedora-rawhide', nvr, results) + assert len(decision) == 1 + assert isinstance(decision[0], MissingGatingYaml) + assert not decision[0].is_satisfied + assert decision[0].subject_identifier == nvr + + def test_parse_policies_missing_tag(): expected_error = "Missing !Policy tag" with pytest.raises(SafeYAMLError, match=expected_error): diff --git a/greenwave/tests/test_rules.py b/greenwave/tests/test_rules.py index 4284952..cb5c2d8 100644 --- a/greenwave/tests/test_rules.py +++ b/greenwave/tests/test_rules.py @@ -1,9 +1,10 @@ import mock +import pytest from textwrap import dedent from greenwave.app_factory import create_app -from greenwave.policies import Policy +from greenwave.policies import Policy, RemoteRule from greenwave.safe_yaml import SafeYAMLError @@ -75,3 +76,57 @@ def test_match_remote_rule(mock_retrieve_scm_from_koji, mock_retrieve_yaml_remot assert not rule.matches(policy, subject_identifier=nvr) assert not rule.matches(policy, subject_identifier=nvr, testcase='some_test_case') assert not rule.matches(policy, subject_identifier=nvr, testcase='other_test_case') + + +@pytest.mark.parametrize(('required_flag', 'required_value'), ( + ('true', True), + ('True', True), + ('on', True), + ('On', True), + ('ON', True), + ('yes', True), + ('Yes', True), + ('YES', True), + + ('false', False), + ('False', False), + ('off', False), + ('Off', False), + ('OFF', False), + ('no', False), + ('No', False), + ('NO', False), +)) +def test_remote_rule_requiered_flag(required_flag, required_value): + policy_yaml = dedent(""" + --- !Policy + id: test + product_versions: [fedora-rawhide] + decision_context: test + subject_type: koji_build + rules: + - !RemoteRule {required: %s} + """) % required_flag + policies = Policy.safe_load_all(policy_yaml) + assert len(policies) == 1 + assert len(policies[0].rules) == 1 + assert isinstance(policies[0].rules[0], RemoteRule) + assert policies[0].rules[0].required == required_value + + +@pytest.mark.parametrize('required_flag', ( + '', '0', '1', 'nope', 'TRUe', 'oN' +)) +def test_remote_rule_requiered_flag_bad(required_flag): + policy_yaml = dedent(""" + --- !Policy + id: test + product_versions: [fedora-rawhide] + decision_context: test + subject_type: koji_build + rules: + - !RemoteRule {required: %s} + """) % required_flag + error = 'Expected a boolean value, got: {}'.format(required_flag) + with pytest.raises(SafeYAMLError, match=error): + Policy.safe_load_all(policy_yaml)