From c269f8c5bb76ee399da70196511e4554147b0b76 Mon Sep 17 00:00:00 2001 From: mprahl Date: Jan 17 2019 18:57:10 +0000 Subject: [PATCH 1/2] Add .vscode/ to .gitignore --- diff --git a/.gitignore b/.gitignore index e46eaee..b9ed5dd 100644 --- a/.gitignore +++ b/.gitignore @@ -26,4 +26,7 @@ coverage.xml htmlcov greenwave-test-cache.dbm* .pytest_cache + +# Other .vagrant/ +.vscode/ From 7ef8e5348dd82451db1c98da664efc884d057675 Mon Sep 17 00:00:00 2001 From: mprahl Date: Jan 22 2019 15:03:10 +0000 Subject: [PATCH 2/2] Replace PackageSpecificBuild with a packages whitelist on the policy --- diff --git a/conf/policies/redhat.yaml b/conf/policies/redhat.yaml index fb8bdd2..e51ad65 100644 --- a/conf/policies/redhat.yaml +++ b/conf/policies/redhat.yaml @@ -8,19 +8,16 @@ subject_type: koji_build blacklist: [] excluded_packages: - module-build* +packages: +- avahi +- cockpit +- checkpolicy +- libsemanage +- libselinux +- libsepol +- policycoreutils rules: -- !PackageSpecificBuild { - test_case_name: osci.brew-build.tier0.functional, - repos: [ - "avahi", - "cockpit", - "checkpolicy", - "libsemanage", - "libselinux", - "libsepol", - "policycoreutils", - ] - } +- !PassingTestCaseRule {test_case_name: osci.brew-build.tier0.functional} # A policy for RHEL 8 Modularity --- !Policy id: "osci_compose_modules" diff --git a/docs/policies.rst b/docs/policies.rst index 1602899..4196b0a 100644 --- a/docs/policies.rst +++ b/docs/policies.rst @@ -101,6 +101,13 @@ The document is a map (dictionary) with the following keys: Currently there are a few rule types, ``PassingTestCaseRule`` being one of them. See the :ref:`rule-types` section below for a full list. +``packages`` (optional) + A list of binary RPM package names this policy applies to. + + ``packages`` only takes effect when Greenwave is making a decision about + subjects with ``"item": "koji_build"``. ``blacklist`` and + ``excluded_packages`` both have a higher priority than ``packages``. + ``blacklist`` (**deprecated**) (optional) A list of binary RPM package names which are exempted from this policy. @@ -160,22 +167,6 @@ PassingTestCaseRule given ``test_case_name`` with an outcome of ``PASS``, *or* there must be a corresponding waiver in WaiverDB for the given test case. - -PackageSpecificBuild --------------------- - - Just like the ``PassingTestCaseRule``, the ``PackageSpecificBuild`` rule - requires that a given ``test_case_name`` is passing, but only for certain - source package names (listed in the ``repos`` argument). The configured - package names in the ``repos`` list may contain wildcards to, for instance, - write a rule requiring a certain test must pass for all `python-*` - packages. - - This rule type can only be used if the policy's subject type is - ``koji_build``. - - ``FedoraAtomicCi`` is a backwards compatibility alias for this rule type. - .. _remote-rule: RemoteRule diff --git a/greenwave/policies.py b/greenwave/policies.py index 3718500..6b93a68 100644 --- a/greenwave/policies.py +++ b/greenwave/policies.py @@ -494,65 +494,6 @@ class PassingTestCaseRule(Rule): self.scenario, result['id']) -class PackageSpecificRule(Rule): - """ - This rule only applies itself to results which are for its configured - list of packages (called "repos"). - - This intermediary class should be considered abstract, and not used directly. - """ - - safe_yaml_attributes = { - 'test_case_name': SafeYAMLString(), - 'repos': SafeYAMLList(str), - } - - def check(self, policy, product_version, subject_identifier, results_retriever, waivers): - """ Check that the subject passes testcase for the given results, but - only if the subject is a build of a package name configured for - this rule, specified by "repos". Any of the repos may be a glob. - - If this rule is used in a policy for some subject type other than - "koji_build" (which makes no sense), the rule is considered satisfied - (ignored). - - Subjects whose package names (extracted from their NVR) do not match - any of the globs in the "repos" list of this rule are considered - satisfied (ignored). - """ - - if policy.subject_type != 'koji_build': - return [] - - pkg_name = subject_identifier.rsplit('-', 2)[0] - if not any(fnmatch(pkg_name, repo) for repo in self.repos): - return [] - - rule = PassingTestCaseRule() - # pylint: disable=attribute-defined-outside-init - rule.test_case_name = self.test_case_name - return rule.check(policy, product_version, subject_identifier, results_retriever, waivers) - - def matches(self, policy, **attributes): - testcase = attributes.get('testcase') - return not testcase or testcase == self.test_case_name - - def to_json(self): - return { - 'rule': self.__class__.__name__, - 'test_case_name': self.test_case_name, - 'repos': self.repos, - } - - -class FedoraAtomicCi(PackageSpecificRule): - yaml_tag = '!FedoraAtomicCi' - - -class PackageSpecificBuild(PackageSpecificRule): - yaml_tag = '!PackageSpecificBuild' - - class Policy(SafeYAMLObject): root_yaml_tag = '!Policy' @@ -565,6 +506,7 @@ class Policy(SafeYAMLObject): 'rules': SafeYAMLList(Rule), 'blacklist': SafeYAMLList(str, optional=True), 'excluded_packages': SafeYAMLList(str, optional=True), + 'packages': SafeYAMLList(str, optional=True), 'relevance_key': SafeYAMLString(optional=True), 'relevance_value': SafeYAMLString(optional=True), } @@ -602,6 +544,10 @@ class Policy(SafeYAMLObject): for exclude in self.excluded_packages: if fnmatch(name, exclude): return [ExcludedInPolicy(subject_identifier) for rule in self.rules] + if self.packages and not any(fnmatch(name, package) for package in self.packages): + # If the `packages` whitelist is set and this package isn't in the + # `packages` whitelist, then the policy doesn't apply to it + return [] answers = [] for rule in self.rules: response = rule.check( @@ -631,6 +577,7 @@ class RemotePolicy(Policy): 'rules': SafeYAMLList(Rule), 'blacklist': SafeYAMLList(str, optional=True), 'excluded_packages': SafeYAMLList(str, optional=True), + 'packages': SafeYAMLList(str, optional=True), } def validate(self): diff --git a/greenwave/tests/test_policies.py b/greenwave/tests/test_policies.py index 26ca419..bbe3c22 100644 --- a/greenwave/tests/test_policies.py +++ b/greenwave/tests/test_policies.py @@ -191,68 +191,6 @@ rules: assert isinstance(decision[0], TestResultFailed) -def test_package_specific_rule(tmpdir): - p = tmpdir.join('fedora.yaml') - p.write(""" ---- !Policy -id: "some_policy" -product_versions: - - rhel-9000 -decision_context: bodhi_update_push_stable -subject_type: koji_build -rules: - - !PackageSpecificBuild {test_case_name: sometest, repos: [nethack, python-*]} - """) - policies = load_policies(tmpdir.strpath) - policy = policies[0] - - # Ensure that we fail with no results - results, waivers = DummyResultsRetriever(), [] - decision = policy.check('rhel-9000', 'nethack-1.2.3-1.el9000', results, waivers) - assert len(decision) == 1 - assert isinstance(decision[0], TestResultMissing) - - # That a matching, failing result can fail - results = DummyResultsRetriever('nethack-1.2.3-1.el9000', 'sometest', 'FAILED') - decision = policy.check('rhel-9000', 'nethack-1.2.3-1.el9000', results, waivers) - assert len(decision) == 1 - assert isinstance(decision[0], TestResultFailed) - - # That a matching, passing result can pass - results = DummyResultsRetriever('nethack-1.2.3-1.el9000', 'sometest') - decision = policy.check('rhel-9000', 'nethack-1.2.3-1.el9000', results, waivers) - assert len(decision) == 1 - assert isinstance(decision[0], RuleSatisfied) - - # That a non-matching passing result is ignored. - results = DummyResultsRetriever('foobar-1.2.3-1.el9000', 'sometest') - decision = policy.check('rhel-9000', 'foobar-1.2.3-1.el9000', results, waivers) - assert decision == [] - - # That a non-matching failing result is ignored. - results = DummyResultsRetriever('foobar-1.2.3-1.el9000', 'sometest', 'FAILED') - decision = policy.check('rhel-9000', 'foobar-1.2.3-1.el9000', results, waivers) - assert decision == [] - - # Ensure that fnmatch globs work in absence - results, waivers = DummyResultsRetriever(), [] - decision = policy.check('rhel-9000', 'python-foobar-1.2.3-1.el9000', results, waivers) - assert len(decision) == 1 - assert isinstance(decision[0], TestResultMissing) - - # Ensure that fnmatch globs work in the negative. - results = DummyResultsRetriever('python-foobar-1.2.3-1.el9000', 'sometest', 'FAILED') - decision = policy.check('rhel-9000', 'python-foobar-1.2.3-1.el9000', results, waivers) - assert len(decision) == 1 - assert isinstance(decision[0], TestResultFailed) - - # Ensure that fnmatch globs work in the positive. - results = DummyResultsRetriever('python-foobar-1.2.3-1.el9000', 'sometest') - decision = policy.check('rhel-9000', 'python-foobar-1.2.3-1.el9000', results, waivers) - assert len(decision) == 1 - assert isinstance(decision[0], RuleSatisfied) - - def test_load_policies(): app = create_app('greenwave.config.TestingConfig') assert len(app.config['policies']) > 0 @@ -702,6 +640,36 @@ def test_policy_with_arbitrary_subject_type(tmpdir): assert isinstance(decision[0], TestResultPassed) +@pytest.mark.parametrize(('package', 'num_decisions'), [ + ('nethack', 1), + ('net*', 1), + ('python-requests', 0), +]) +def test_policy_with_packages_whitelist(tmpdir, package, num_decisions): + p = tmpdir.join('temp.yaml') + p.write(dedent(""" + --- !Policy + id: "some_policy" + product_versions: + - rhel-9000 + decision_context: test + subject_type: koji_build + packages: + - {} + rules: + - !PassingTestCaseRule {{test_case_name: sometest}} + """.format(package))) + policies = load_policies(tmpdir.strpath) + policy = policies[0] + + waivers = [] + results = DummyResultsRetriever('nethack-1.2.3-1.el9000', 'sometest', 'PASSED', 'koji_build') + decision = policy.check('rhel-9000', 'nethack-1.2.3-1.el9000', results, waivers) + assert len(decision) == num_decisions + if num_decisions: + assert isinstance(decision[0], TestResultPassed) + + def test_parse_policies_invalid_rule(): expected_error = "Policy 'test': Attribute 'rules': Expected list of Rule objects" with pytest.raises(SafeYAMLError, match=expected_error): @@ -866,6 +834,7 @@ def test_policies_to_json(): 'subject_type': 'compose', 'blacklist': [], 'excluded_packages': [], + 'packages': [], 'rules': [], 'relevance_key': None, 'relevance_value': None, diff --git a/greenwave/tests/test_rules.py b/greenwave/tests/test_rules.py index 127ee13..4284952 100644 --- a/greenwave/tests/test_rules.py +++ b/greenwave/tests/test_rules.py @@ -29,28 +29,6 @@ def test_match_passing_test_case_rule(): assert not rule.matches(policy, testcase='other_test_case') -def test_match_package_specific_rule(): - policy_yaml = dedent(""" - --- !Policy - id: "some_policy" - product_versions: [rhel-9000] - decision_context: bodhi_update_push_stable - subject_type: koji_build - rules: - - !PackageSpecificBuild {test_case_name: some_test_case, repos: [nethack]} - """) - policies = Policy.safe_load_all(policy_yaml) - assert len(policies) == 1 - - policy = policies[0] - assert len(policy.rules) == 1 - - rule = policy.rules[0] - assert rule.matches(policy) - assert rule.matches(policy, testcase='some_test_case') - assert not rule.matches(policy, testcase='other_test_case') - - @mock.patch('greenwave.resources.retrieve_yaml_remote_rule') @mock.patch('greenwave.resources.retrieve_scm_from_koji') def test_match_remote_rule(mock_retrieve_scm_from_koji, mock_retrieve_yaml_remote_rule):