Skip to content

Commit

Permalink
Drop blacklist attribute and use allowlist/blocklist where possible
Browse files Browse the repository at this point in the history
Drops deprecated policy attribute - it was replaced by
excluded_packages.
  • Loading branch information
hluk committed Sep 1, 2022
1 parent 188933e commit a636ec1
Show file tree
Hide file tree
Showing 15 changed files with 39 additions and 102 deletions.
10 changes: 2 additions & 8 deletions conf/policies/fedora.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
# Tasktron release-critical tasks rule
# https://github.com/fedora-infra/fmn/blob/develop/fmn/rules/taskotron.py#L5
--- !Policy
id: "taskotron_release_critical_tasks_with_blacklist"
id: "taskotron_release_critical_tasks_with_blocklist"
product_versions:
- fedora-26
decision_context: bodhi_update_push_stable
subject_type: koji_build
blacklist:
excluded_packages:
# see the excluded list for dist.abicheck
# https://infrastructure.fedoraproject.org/cgit/ansible.git/tree/roles/taskotron/taskotron-trigger/templates/trigger_rules.yml.j2#n17
- firefox
Expand All @@ -31,7 +31,6 @@ product_versions:
- fedora-*
decision_context: bodhi_update_push_testing
subject_type: koji_build
blacklist: []
rules:
- !PassingTestCaseRule {test_case_name: dist.rpmdeplint}
--- !Policy
Expand All @@ -40,7 +39,6 @@ product_versions:
- fedora-26
decision_context: bodhi_update_push_stable
subject_type: koji_build
blacklist: []
rules:
- !PassingTestCaseRule {test_case_name: dist.rpmdeplint}
- !PassingTestCaseRule {test_case_name: dist.upgradepath}
Expand All @@ -50,7 +48,6 @@ product_versions:
- fedora-rawhide
decision_context: rawhide_compose_sync_to_mirrors
subject_type: compose
blacklist: []
rules:
- !PassingTestCaseRule {test_case_name: compose.install_no_user, scenario: scenario1}
- !PassingTestCaseRule {test_case_name: compose.install_no_user, scenario: scenario2}
Expand All @@ -60,7 +57,6 @@ product_versions:
- fedora-26
decision_context: bodhi_update_push_stable_with_remoterule
subject_type: bodhi_update
blacklist: []
rules:
- !RemoteRule {}

Expand All @@ -70,7 +66,6 @@ product_versions:
- fedora-26
decision_context: bodhi_update_with_a_package_test
subject_type: bodhi_update
blacklist: []
rules:
- !PassingTestCaseRule {test_case_name: a_package_test}

Expand All @@ -90,7 +85,6 @@ product_versions:
- fedora-24
decision_context: bodhi_update_push_stable
subject_type: bodhi_update
blacklist: []
rules: []

# Policy for container-image tests
Expand Down
3 changes: 0 additions & 3 deletions conf/policies/redhat.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ product_versions:
- rhel-something
decision_context: osci_compose_gate
subject_type: koji_build
blacklist: []
excluded_packages:
- module-build*
packages:
Expand All @@ -25,7 +24,6 @@ product_versions:
- rhel-8
decision_context: osci_compose_gate_modules
subject_type: redhat-module
blacklist: []
rules:
- !PassingTestCaseRule {test_case_name: baseos-ci.redhat-module.tier0.functional}
--- !Policy
Expand All @@ -34,6 +32,5 @@ product_versions:
- rhel-something
decision_context: rtt_compose_gate
subject_type: compose
blacklist: []
rules:
- !PassingTestCaseRule {test_case_name: rtt.acceptance.validation}
1 change: 0 additions & 1 deletion docs/decision_requirements.rst
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,6 @@ Waived requirements have type ending with "-waived":
Other satisfied requirements types:

- ``fetched-gating-yaml``
- ``blacklisted`` (from ``blacklist`` in a policy)
- ``excluded`` (from ``excluded_packages`` in a policy)
- other types (can be extended in the future)

Expand Down
8 changes: 1 addition & 7 deletions docs/policies.rst
Original file line number Diff line number Diff line change
Expand Up @@ -115,14 +115,8 @@ The document is a map (dictionary) with the following keys:
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.

The blacklist only takes effect when Greenwave is making a decision about
subjects with ``"item": "koji_build"``.
``excluded_packages`` has a higher priority than ``packages``.

``excluded_packages`` (optional)
A list of binary RPM package names which are exempted from this policy.
Expand Down
6 changes: 3 additions & 3 deletions functional-tests/consumers/test_resultsdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,10 @@ def test_consume_new_result(
],
'subject_type': 'koji_build',
'subject_identifier': nvr,
'applicable_policies': ['taskotron_release_critical_tasks_with_blacklist',
'applicable_policies': ['taskotron_release_critical_tasks_with_blocklist',
'taskotron_release_critical_tasks'],
'previous': {
'applicable_policies': ['taskotron_release_critical_tasks_with_blacklist',
'applicable_policies': ['taskotron_release_critical_tasks_with_blocklist',
'taskotron_release_critical_tasks'],
'policies_satisfied': False,
'summary': '3 of 3 required test results missing',
Expand Down Expand Up @@ -389,7 +389,7 @@ def test_consume_legacy_result(
],
'subject_type': 'koji_build',
'subject_identifier': nvr,
'applicable_policies': ['taskotron_release_critical_tasks_with_blacklist',
'applicable_policies': ['taskotron_release_critical_tasks_with_blocklist',
'taskotron_release_critical_tasks'],
'previous': old_decision,
}
Expand Down
4 changes: 2 additions & 2 deletions functional-tests/consumers/test_waiverdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,12 @@ def test_consume_new_waiver(
)
actual_msgs_sent = [call[1][0].body for call in mock_fedora_messaging.mock_calls]
assert actual_msgs_sent[0] == {
'applicable_policies': ['taskotron_release_critical_tasks_with_blacklist',
'applicable_policies': ['taskotron_release_critical_tasks_with_blocklist',
'taskotron_release_critical_tasks'],
'policies_satisfied': True,
'decision_context': 'bodhi_update_push_stable',
'previous': {
'applicable_policies': ['taskotron_release_critical_tasks_with_blacklist',
'applicable_policies': ['taskotron_release_critical_tasks_with_blocklist',
'taskotron_release_critical_tasks'],
'policies_satisfied': False,
'summary': '1 of 3 required tests failed',
Expand Down
42 changes: 11 additions & 31 deletions functional-tests/test_api_v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ def test_make_a_decision_on_passed_result(requests_session, greenwave_server, te
res_data = r.json()
assert res_data['policies_satisfied'] is True
assert res_data['applicable_policies'] == [
'taskotron_release_critical_tasks_with_blacklist',
'taskotron_release_critical_tasks_with_blocklist',
'taskotron_release_critical_tasks',
]
expected_summary = 'All required tests passed'
Expand Down Expand Up @@ -369,7 +369,7 @@ def test_make_a_decision_on_failed_result_with_waiver(
res_data = r.json()
assert res_data['policies_satisfied'] is True
assert 'taskotron_release_critical_tasks' in res_data['applicable_policies']
assert 'taskotron_release_critical_tasks_with_blacklist' in res_data['applicable_policies']
assert 'taskotron_release_critical_tasks_with_blocklist' in res_data['applicable_policies']
expected_summary = 'All required tests passed'
assert res_data['summary'] == expected_summary

Expand All @@ -390,7 +390,7 @@ def test_make_a_decision_on_failed_result(requests_session, greenwave_server, te
res_data = r.json()
assert res_data['policies_satisfied'] is False
assert 'taskotron_release_critical_tasks' in res_data['applicable_policies']
assert 'taskotron_release_critical_tasks_with_blacklist' in res_data['applicable_policies']
assert 'taskotron_release_critical_tasks_with_blocklist' in res_data['applicable_policies']
expected_summary = '1 of 3 required tests failed, 2 results missing'
assert res_data['summary'] == expected_summary
expected_unsatisfied_requirements = [
Expand Down Expand Up @@ -436,7 +436,7 @@ def test_make_a_decision_on_queued_result(requests_session, greenwave_server, te
res_data = r.json()
assert res_data['policies_satisfied'] is False
assert 'taskotron_release_critical_tasks' in res_data['applicable_policies']
assert 'taskotron_release_critical_tasks_with_blacklist' in res_data['applicable_policies']
assert 'taskotron_release_critical_tasks_with_blocklist' in res_data['applicable_policies']
expected_summary = '3 of 3 required test results missing'
assert res_data['summary'] == expected_summary
expected_unsatisfied_requirements = [
Expand Down Expand Up @@ -482,7 +482,7 @@ def test_make_a_decision_on_running_result(requests_session, greenwave_server, t
res_data = r.json()
assert res_data['policies_satisfied'] is False
assert 'taskotron_release_critical_tasks' in res_data['applicable_policies']
assert 'taskotron_release_critical_tasks_with_blacklist' in res_data['applicable_policies']
assert 'taskotron_release_critical_tasks_with_blocklist' in res_data['applicable_policies']
expected_summary = '3 of 3 required test results missing'
assert res_data['summary'] == expected_summary
expected_unsatisfied_requirements = [
Expand Down Expand Up @@ -525,7 +525,7 @@ def test_make_a_decision_on_no_results(requests_session, greenwave_server, testd
res_data = r.json()
assert res_data['policies_satisfied'] is False
assert 'taskotron_release_critical_tasks' in res_data['applicable_policies']
assert 'taskotron_release_critical_tasks_with_blacklist' in res_data['applicable_policies']
assert 'taskotron_release_critical_tasks_with_blocklist' in res_data['applicable_policies']
expected_summary = '3 of 3 required test results missing'
assert res_data['summary'] == expected_summary
expected_unsatisfied_requirements = [
Expand Down Expand Up @@ -663,7 +663,7 @@ def test_bodhi_push_update_stable_policy(
res_data = r.json()
assert res_data['policies_satisfied'] is True
assert 'taskotron_release_critical_tasks' in res_data['applicable_policies']
assert 'taskotron_release_critical_tasks_with_blacklist' in res_data['applicable_policies']
assert 'taskotron_release_critical_tasks_with_blocklist' in res_data['applicable_policies']
expected_summary = 'All required tests passed'
assert res_data['summary'] == expected_summary
assert res_data['unsatisfied_requirements'] == []
Expand Down Expand Up @@ -722,7 +722,7 @@ def test_multiple_results_in_a_subject(
# The failed result should be taken into account.
assert res_data['policies_satisfied'] is False
assert 'taskotron_release_critical_tasks' in res_data['applicable_policies']
assert 'taskotron_release_critical_tasks_with_blacklist' in res_data['applicable_policies']
assert 'taskotron_release_critical_tasks_with_blocklist' in res_data['applicable_policies']
assert res_data['summary'] == '1 of 3 required tests failed'
expected_unsatisfied_requirements = [
{
Expand Down Expand Up @@ -984,9 +984,9 @@ def test_cached_false_positive(requests_session, greenwave_server, testdatabuild
assert res_data['policies_satisfied'] is True


def test_blacklist(requests_session, greenwave_server, testdatabuilder):
def test_blocklist(requests_session, greenwave_server, testdatabuilder):
"""
Test that packages on the blacklist will be excluded when applying the policy.
Test that packages on the blocklist will be excluded when applying the policy.
"""
nvr = 'firefox-1.0-1.el7'
testdatabuilder.create_result(item=nvr,
Expand Down Expand Up @@ -1068,26 +1068,6 @@ def test_validate_gating_yaml_valid(requests_session, greenwave_server):
assert result.status_code == 200


def test_validate_gating_yaml_deprecated_blacklist(requests_session, greenwave_server):
gating_yaml = dedent("""
--- !Policy
id: "test"
product_versions:
- fedora-26
decision_context: test
rules:
- !PassingTestCaseRule {test_case_name: test}
blacklist:
- python-requests
""")
result = requests_session.post(
greenwave_server + 'api/v1.0/validate-gating-yaml', data=gating_yaml)
assert result.json().get('message') == (
'The gating.yaml file is valid but it is using the deprecated '
'"blacklist" key. Please use "excluded_packages" instead.')
assert result.status_code == 200


def test_validate_gating_yaml_empty(requests_session, greenwave_server):
result = requests_session.post(greenwave_server + 'api/v1.0/validate-gating-yaml')
assert result.json().get('message') == 'No policies defined'
Expand All @@ -1111,7 +1091,7 @@ def test_validate_gating_yaml_obsolete_rule(requests_session, greenwave_server):
greenwave_server + 'api/v1.0/validate-gating-yaml', data=gating_yaml)
assert result.json().get('message') == (
'Policy \'test\': Attribute \'rules\': !PackageSpecificBuild is obsolete. '
'Please use the "packages" whitelist instead.')
'Please use the "packages" allowlist instead.')
assert result.status_code == 400


Expand Down
5 changes: 1 addition & 4 deletions greenwave/api_v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -410,10 +410,7 @@ def validate_gating_yaml_post():
raise BadRequest('No policies defined')

missing_decision_contexts = _missing_decision_contexts_in_parent_policies(policies)
if any(True for policy in policies if policy.blacklist):
msg = {'message': ('The gating.yaml file is valid but it is using the deprecated '
'"blacklist" key. Please use "excluded_packages" instead.')}
elif missing_decision_contexts:
if missing_decision_contexts:
msg = {'message': ('Greenwave could not find a parent policy(ies) for following decision'
' context(s): {}. Please change your policy so that it will match a '
'decision context in the parent policies.'.format(
Expand Down
28 changes: 3 additions & 25 deletions greenwave/policies.py
Original file line number Diff line number Diff line change
Expand Up @@ -445,23 +445,6 @@ def to_json(self):
return data


class BlacklistedInPolicy(RuleSatisfied):
"""
Package was blacklisted in policy.
"""
def __init__(self, subject_identifier, policy):
self.subject_identifier = subject_identifier
self.policy = policy

def to_json(self):
return {
'type': 'blacklisted',
'subject_identifier': self.subject_identifier,
'policy': self.policy.id,
'source': self.policy.source,
}


class ExcludedInPolicy(RuleSatisfied):
"""
Package was excluded in policy.
Expand Down Expand Up @@ -799,7 +782,7 @@ def check(self, policy, rule_context):

class PackageSpecificBuild(ObsoleteRule):
yaml_tag = '!PackageSpecificBuild'
advice = 'Please use the "packages" whitelist instead.'
advice = 'Please use the "packages" allowlist instead.'


class FedoraAtomicCi(PackageSpecificBuild):
Expand All @@ -816,7 +799,6 @@ class Policy(SafeYAMLObject):
'decision_contexts': SafeYAMLList(str, optional=True, default=list()),
'subject_type': SafeYAMLString(),
'rules': SafeYAMLList(Rule),
'blacklist': SafeYAMLList(str, optional=True),
'excluded_packages': SafeYAMLList(str, optional=True),
'packages': SafeYAMLList(str, optional=True),
'relevance_key': SafeYAMLString(optional=True),
Expand Down Expand Up @@ -865,17 +847,14 @@ def matches_sub_policy(self, sub_policy):
return set(sub_policy.all_decision_contexts).intersection(self.all_decision_contexts)

def check(self, rule_context):
# If an item is about a package and it is in the blacklist, return RuleSatisfied()
name = rule_context.subject.package_name
if name:
if name in self.blacklist:
return [BlacklistedInPolicy(rule_context.subject.identifier, self)]
for exclude in self.excluded_packages:
if fnmatch(name, exclude):
return [ExcludedInPolicy(rule_context.subject.identifier, self)]
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
# If the `packages` allowlist is set and this package isn't in the
# `packages` allowlist, then the policy doesn't apply to it
return []

answers = []
Expand Down Expand Up @@ -934,7 +913,6 @@ class RemotePolicy(Policy):
'decision_context': SafeYAMLString(optional=True),
'decision_contexts': SafeYAMLList(str, optional=True),
'rules': SafeYAMLList(Rule),
'blacklist': SafeYAMLList(str, optional=True),
'excluded_packages': SafeYAMLList(str, optional=True),
'packages': SafeYAMLList(str, optional=True),
}
Expand Down
2 changes: 1 addition & 1 deletion greenwave/request_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def get_requests_session():
connect=3,
backoff_factor=1,
status_forcelist=(500, 502, 503, 504),
method_whitelist=Retry.DEFAULT_METHOD_WHITELIST.union(('POST',)),
allowed_methods=Retry.DEFAULT_ALLOWED_METHODS.union(('POST',)),
)
adapter = HTTPAdapter(max_retries=retry)
session.mount('http://', adapter)
Expand Down
2 changes: 1 addition & 1 deletion greenwave/tests/test_listeners.py
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ def test_decision_change_for_modules(
- rhel-8
decision_context: osci_compose_gate_modules
subject_type: redhat-module
blacklist: []
excluded_packages: []
rules:
- !RemoteRule {}
"""
Expand Down

0 comments on commit a636ec1

Please sign in to comment.