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.

**Decision responses** will always include policies (in
`applicable_policies`) that contain RemoteRule without checking if the
remote gating file contains any matching policy.

**Decision change messages** will still only be published if there are
any required tests.

Fixes https://pagure.io/greenwave/issue/668
JIRA: RHELWF-9620
  • Loading branch information
hluk committed Sep 5, 2023
1 parent 3bc630e commit f6adf1b
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 30 deletions.
1 change: 1 addition & 0 deletions greenwave/decision.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ def check(self, subject, policies, results_retriever):
if policy.matches(
decision_context=self.decision_context,
product_version=self.product_version,
match_any_remote_rule=True,
subject=subject)
]

Expand Down
3 changes: 3 additions & 0 deletions greenwave/policies.py
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,9 @@ def check(self, policy, rule_context):
return answers

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

subject = attributes.get('subject')
if not subject:
return True
Expand Down
68 changes: 43 additions & 25 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,38 +192,37 @@ 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_no_applicable_policies(mock_results, mock_waivers, make_decision):
mock_results.return_value = []
mock_waivers.return_value = []
policies = """
--- !Policy
id: "test_policy"
product_versions:
- fedora-rawhide
decision_contexts:
- test_policies_2
subject_type: koji_build
rules:
- !PassingTestCaseRule {test_case_name: sometest}
"""
response = make_decision(policies=policies)
assert 404 == response.status_code
assert response.json['message'] == (
'Found no applicable policies for koji_build subjects at gating '
'point(s) test_policies in fedora-rawhide'
)
mock_waivers.assert_not_called()


def test_make_decision_with_missing_required_gating_yaml(mock_results, mock_waivers, make_decision):
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
6 changes: 2 additions & 4 deletions greenwave/tests/test_policies.py
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,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 @@ -741,9 +741,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
6 changes: 6 additions & 0 deletions greenwave/tests/test_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,12 @@ def test_match_remote_rule(mock_retrieve_scm_from_koji, mock_retrieve_yaml_remot
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',
match_any_remote_rule=True,
)


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

0 comments on commit f6adf1b

Please sign in to comment.