Skip to content

Commit

Permalink
Make RemoteRule more consistent
Browse files Browse the repository at this point in the history
Previously, a Greenwave policy with just a RemoteRule would cause
"Cannot find any applicable policies" 404 error if the remote policy
file does not contain a policy with the requested decision context. But
that was not the case if the Greenwave policy would contain at least one
PassingTestCaseRule.

New behavior is to pass the decision with "no tests are required"
instead of the 404 error.

Additionally, policy with RemoteRule would be always included in
decision response/message in `applicable_policies`.

Fixes https://pagure.io/greenwave/issue/668
JIRA: RHELWF-9620
  • Loading branch information
hluk committed Aug 29, 2023
1 parent 6afad9f commit e80f958
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 50 deletions.
18 changes: 1 addition & 17 deletions greenwave/policies.py
Original file line number Diff line number Diff line change
Expand Up @@ -657,23 +657,7 @@ def check(self, policy, rule_context):
return answers

def matches(self, policy, **attributes):
subject = attributes.get('subject')
if not subject:
return True

sub_policies = []
sub_policies, answers = self._get_sub_policies(policy, subject)

# Include policy if remote rule file is missing.
if not answers:
return True

# Include any failure fetching/parsing remote rule file in the
# decision.
if any(not answer.is_satisfied for answer in answers):
return True

return any(sub_policy.matches(**attributes) for sub_policy in sub_policies)
return True

def to_json(self):
return {
Expand Down
47 changes: 21 additions & 26 deletions greenwave/tests/test_api_v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,27 @@ def test_make_decision_with_no_tests_required_and_missing_gating_yaml(
mock_waivers.assert_not_called()


@pytest.mark.parametrize(
"remote_gating_yaml",
(
dedent("""
--- !Policy
decision_contexts:
- test_policies
- abc
rules: [ ]
"""),
dedent("""
--- !Policy
decision_contexts:
- foo
- bar
rules: [ ]
"""),
)
)
def test_make_decision_with_no_tests_required_and_empty_remote_rules(
mock_results, mock_waivers, make_decision):
mock_results, mock_waivers, make_decision, remote_gating_yaml):
mock_results.return_value = []
mock_waivers.return_value = []
policies = """
Expand All @@ -173,39 +192,15 @@ def test_make_decision_with_no_tests_required_and_empty_remote_rules(
- !RemoteRule {}
"""

remote_fragment1 = dedent("""
--- !Policy
decision_contexts:
- test_policies
- abc
rules: [ ]
""")

remote_fragment2 = dedent("""
--- !Policy
decision_contexts:
- foo
- bar
rules: [ ]
""")

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 = remote_fragment1
f.return_value = remote_gating_yaml
response = make_decision(policies=policies)
assert 200 == response.status_code
assert 'no tests are required' == response.json['summary']
mock_waivers.assert_not_called()

with mock.patch('greenwave.resources.retrieve_yaml_remote_rule') as f:
f.return_value = remote_fragment2
response = make_decision(policies=policies)
assert 404 == response.status_code
assert 'Found no applicable policies for koji_build subjects at gating ' \
'point(s) test_policies in fedora-rawhide' == response.json['message']
mock_waivers.assert_not_called()


def test_make_decision_with_missing_required_gating_yaml(mock_results, mock_waivers, make_decision):
mock_results.return_value = []
Expand Down
13 changes: 12 additions & 1 deletion greenwave/tests/test_listeners.py
Original file line number Diff line number Diff line change
Expand Up @@ -539,16 +539,27 @@ def test_remote_rule_decision_change(


def test_remote_rule_decision_change_not_matching(
mock_retrieve_decision,
mock_retrieve_yaml_remote_rule,
mock_retrieve_scm_from_koji,
mock_retrieve_results,
mock_connection,
koji_proxy,
):
"""
Test publishing decision change message for test cases mentioned in
Test not publishing decision change message for test cases not mentioned in
gating.yaml.
"""
def retrieve_decision(data, _config):
return {
"policies_satisfied": True,
"summary": "no tests are required",
"satisfied_requirements": [],
"unsatisfied_requirements": [],
}

mock_retrieve_decision.side_effect = retrieve_decision

gating_yaml = dedent(
"""
--- !Policy
Expand Down
9 changes: 4 additions & 5 deletions greenwave/tests/test_policies.py
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ def test_remote_rule_policy_old_config(tmpdir):
'http://localhost.localdomain/nethack/'
'c3c47a08a66451cb9686c49f040776ed35a0d1bb/gating.yaml'
)
assert f.mock_calls == [call, call]
assert f.mock_calls == [call]
finally:
Config.REMOTE_RULE_POLICIES = config_remote_rules_backup

Expand Down Expand Up @@ -740,9 +740,7 @@ def test_get_sub_policies_multiple_urls(tmpdir):
*scm.return_value
)
)
assert session.request.mock_calls == [
expected_call1, expected_call2,
expected_call1, expected_call2]
assert session.request.mock_calls == [expected_call1, expected_call2]
assert answer_types(decision.answers) == ['missing-gating-yaml']
assert not decision.answers[0].is_satisfied
assert decision.answers[0].subject.identifier == subject.identifier
Expand Down Expand Up @@ -1432,7 +1430,6 @@ def test_on_demand_policy_match(two_rules, koji_proxy):

policy_matches = policy.matches(subject=subject)

koji_proxy.getBuild.assert_called_once()
assert policy_matches

results = DummyResultsRetriever(
Expand All @@ -1443,6 +1440,8 @@ def test_on_demand_policy_match(two_rules, koji_proxy):
if two_rules:
assert answer_types(decision.answers) == ['test-result-passed']

koji_proxy.getBuild.assert_called_once()


def test_remote_rule_policy_on_demand_policy_required():
""" Testing the RemoteRule with the koji interaction when on_demand policy is given.
Expand Down
2 changes: 1 addition & 1 deletion greenwave/tests/test_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def test_match_remote_rule(mock_retrieve_scm_from_koji, mock_retrieve_yaml_remot
assert rule.matches(policy)
assert rule.matches(policy, subject=subject)
assert rule.matches(policy, subject=subject, testcase='some_test_case')
assert not rule.matches(policy, subject=subject, testcase='other_test_case')
assert rule.matches(policy, subject=subject, testcase='other_test_case')


@mock.patch('greenwave.resources.retrieve_yaml_remote_rule')
Expand Down

0 comments on commit e80f958

Please sign in to comment.