From 58a7ff41a163e982d01ab5462b2e958b76f22ee5 Mon Sep 17 00:00:00 2001 From: Valerij Maljulin Date: Aug 25 2020 13:53:38 +0000 Subject: Multiple decision contexts ('decision_contexts' field) updates This fixes: #467 JIRA: RHELWF-413 --- diff --git a/docs/policies.rst b/docs/policies.rst index 0293919..4ba36bb 100644 --- a/docs/policies.rst +++ b/docs/policies.rst @@ -21,8 +21,8 @@ Here is an example policy: --- !Policy id: taskotron_release_critical_tasks - decision_context: bodhi_update_push_stable decision_contexts: + - bodhi_update_push_stable - bodhi_update_context1 - bodhi_update_context2 subject_type: bodhi_update @@ -52,7 +52,13 @@ The document is a map (dictionary) with the following keys: This is optional in ``gating.yaml`` files (see :ref:`remote-rule`). -``decision_context`` +``decision_contexts`` + Allows to specify many decision contexts for one policy. Previous + parameter `decision_context` was kept for backward compatibility + and its value is being used if this parameter is not specified. + However only one parameter can be used in the same policy. + +``decision_context`` (obsolete) This is an arbitrary string identifying the "context" of the decisions where this policy is applicable. In other words, if Greenwave is making decisions at gating points in a pipeline, this is how we identify which @@ -64,11 +70,6 @@ The document is a map (dictionary) with the following keys: passes this value when it asks Greenwave to decide whether a Bodhi update is ready to be pushed to the stable repositories. -``decision_contexts`` - Allows to specify many decision contexts for one policy. Previous - parameter was kept for backward compatibility and its value is being - added to this list if provided. - .. _subject_type: ``subject_type`` diff --git a/greenwave/policies.py b/greenwave/policies.py index 9684746..0420e3b 100644 --- a/greenwave/policies.py +++ b/greenwave/policies.py @@ -675,7 +675,7 @@ class Policy(SafeYAMLObject): safe_yaml_attributes = { 'id': SafeYAMLString(), 'product_versions': SafeYAMLList(str), - 'decision_context': SafeYAMLString(), + 'decision_context': SafeYAMLString(optional=True), 'decision_contexts': SafeYAMLList(str, optional=True, default=list()), 'subject_type': SafeYAMLString(), 'rules': SafeYAMLList(Rule), @@ -686,6 +686,15 @@ class Policy(SafeYAMLObject): 'relevance_value': SafeYAMLString(optional=True), } + def validate(self): + if not self.decision_context and not self.decision_contexts: + raise SafeYAMLError('No decision contexts provided') + if self.decision_context and self.decision_contexts: + raise SafeYAMLError( + 'Both properties "decision_contexts" and "decision_context" were set' + ) + super().validate() + def matches(self, **attributes): """ Returns True only if policy matches provided attributes. @@ -753,12 +762,11 @@ class Policy(SafeYAMLObject): @property def all_decision_contexts(self): - rv = [] if self.decision_contexts: - rv.extend(self.decision_contexts) - if self.decision_context and self.decision_context not in rv: - rv.append(self.decision_context) - return rv + return self.decision_contexts + if self.decision_context: + return [self.decision_context] + raise SafeYAMLError('No decision contexts provided') class OnDemandPolicy(Policy): @@ -837,10 +845,11 @@ def applicable_decision_context_product_version_pairs(policies, **attributes): def _missing_decision_contexts_in_parent_policies(policies): missing_decision_contexts = set() + parent_dcs = set() + for parent_policy in current_app.config['policies']: + parent_dcs.update(set(parent_policy.all_decision_contexts)) for policy in policies: - # Assume a parent policy is not present for a policy in the remote rule - for parent_policy in current_app.config['policies']: - missing_decision_contexts.update( - set(parent_policy.all_decision_contexts).difference(policy.all_decision_contexts) - ) + missing_decision_contexts.update( + set(policy.all_decision_contexts).difference(parent_dcs) + ) return list(missing_decision_contexts) diff --git a/greenwave/tests/test_policies.py b/greenwave/tests/test_policies.py index 06d4384..643c913 100644 --- a/greenwave/tests/test_policies.py +++ b/greenwave/tests/test_policies.py @@ -879,7 +879,7 @@ def test_parse_policies_missing_product_versions(): def test_parse_policies_missing_decision_context(): - expected_error = "Policy 'test': Attribute 'decision_context' is required" + expected_error = "No decision contexts provided" with pytest.raises(SafeYAMLError, match=expected_error): Policy.safe_load_all(dedent(""" --- !Policy @@ -892,6 +892,24 @@ def test_parse_policies_missing_decision_context(): """)) +def test_parse_policies_both_decision_contexts_set(): + expected_error = 'Both properties "decision_contexts" and "decision_context" were set' + with pytest.raises(SafeYAMLError, match=expected_error): + Policy.safe_load_all(dedent(""" + --- !Policy + id: test + product_versions: [fedora-rawhide] + subject_type: compose + blacklist: [] + decision_context: test1 + decision_contexts: + - test1 + - test2 + rules: + - !PassingTestCaseRule {test_case_name: compose.cloud.all} + """)) + + def test_policy_with_arbitrary_subject_type(tmpdir): p = tmpdir.join('fedora.yaml') p.write(dedent(""" @@ -921,20 +939,6 @@ def test_policy_all_decision_contexts(tmpdir): id: "some_policy1" product_versions: - rhel-9000 - decision_context: test1 - decision_contexts: - - test1 - - test2 - - test3 - subject_type: kind-of-magic - rules: - - !PassingTestCaseRule {test_case_name: sometest} - - --- !Policy - id: "some_policy2" - product_versions: - - rhel-9000 - decision_context: test4 decision_contexts: - test1 - test2 @@ -957,9 +961,6 @@ def test_policy_all_decision_contexts(tmpdir): assert len(policy.all_decision_contexts) == 3 assert set(policy.all_decision_contexts) == {'test1', 'test2', 'test3'} policy = policies[1] - assert len(policy.all_decision_contexts) == 4 - assert set(policy.all_decision_contexts) == {'test1', 'test2', 'test3', 'test4'} - policy = policies[2] assert len(policy.all_decision_contexts) == 1 assert policy.all_decision_contexts == ['test4']