Skip to content

Commit

Permalink
Make mlbf testable (#22896)
Browse files Browse the repository at this point in the history
* Make mlbf testable

* Disable manual upload in prod

* FIx tests and disable in production

* Make it a task

* Disable if waffle disabled

* Fix comments

* Use form post request
  • Loading branch information
KevinMind committed Nov 27, 2024
1 parent f6e4f21 commit 02bb35c
Show file tree
Hide file tree
Showing 12 changed files with 159 additions and 15 deletions.
2 changes: 2 additions & 0 deletions settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,3 +192,5 @@ def insert_debug_toolbar_middleware(middlewares):
},
'PERSIST_AUTH': True,
}

ENABLE_ADMIN_MLBF_UPLOAD = True
29 changes: 29 additions & 0 deletions src/olympia/blocklist/admin.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
from django import http
from django.conf import settings
from django.contrib import admin, auth, contenttypes, messages
from django.core.exceptions import PermissionDenied
from django.http import HttpResponseNotAllowed
from django.shortcuts import get_object_or_404, redirect
from django.template.loader import render_to_string
from django.template.response import TemplateResponse
from django.urls import path, reverse
from django.utils.html import format_html

import waffle

from olympia.activity.models import ActivityLog
from olympia.addons.models import Addon
from olympia.amo.admin import AMOModelAdmin
Expand Down Expand Up @@ -68,6 +72,11 @@ def get_urls(self):
self.admin_site.admin_view(self.add_from_addon_pk_view),
name='blocklist_block_addaddon',
),
path(
'upload_mlbf/',
self.admin_site.admin_view(self.upload_mlbf_view),
name='blocklist_block_upload_mlbf',
),
]
return my_urls + super().get_urls()

Expand Down Expand Up @@ -141,6 +150,26 @@ def add_from_addon_pk_view(self, request, pk, **kwargs):
+ f'?guids={addon.addonguid_guid}&{request.GET.urlencode()}'
)

def upload_mlbf_view(self, request):
if request.method != 'POST':
return HttpResponseNotAllowed(['POST'])
if (
not settings.ENABLE_ADMIN_MLBF_UPLOAD
or not request.user.has_perm('blocklist.change_block')
or not waffle.switch_is_active('blocklist_mlbf_submit')
):
raise PermissionDenied
force_base = request.GET.get('force_base', 'false').lower() == 'true'
from .tasks import upload_mlbf_to_remote_settings_task

upload_mlbf_to_remote_settings_task.delay(force_base=force_base)
messages.add_message(
request,
messages.SUCCESS,
'MLBF upload to remote settings has been triggered.',
)
return redirect('admin:blocklist_block_changelist')


@admin.register(BlocklistSubmission)
class BlocklistSubmissionAdmin(AMOModelAdmin):
Expand Down
9 changes: 7 additions & 2 deletions src/olympia/blocklist/mlbf.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,18 @@
from olympia.amo.utils import SafeStorage
from olympia.blocklist.models import BlockType, BlockVersion
from olympia.blocklist.utils import datetime_to_ts
from olympia.constants.blocklist import BASE_REPLACE_THRESHOLD
from olympia.constants.blocklist import BASE_REPLACE_THRESHOLD_KEY
from olympia.versions.models import Version
from olympia.zadmin.models import get_config


log = olympia.core.logger.getLogger('z.amo.blocklist')


def get_base_replace_threshold():
return get_config(BASE_REPLACE_THRESHOLD_KEY, int_value=True, default=5_000)


def ordered_diff_lists(
previous: List[str], current: List[str]
) -> Tuple[List[str], List[str], int]:
Expand Down Expand Up @@ -366,7 +371,7 @@ def should_upload_filter(
self.blocks_changed_since_previous(
block_type=block_type, previous_mlbf=previous_mlbf
)
> BASE_REPLACE_THRESHOLD
> get_base_replace_threshold()
)

def should_upload_stash(
Expand Down
7 changes: 7 additions & 0 deletions src/olympia/blocklist/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ def BLOCKLIST_RECORD_MLBF_BASE(block_type: BlockType):
raise ValueError(f'Unknown block type: {block_type}')


@task
def upload_mlbf_to_remote_settings_task(force_base=False):
from .cron import upload_mlbf_to_remote_settings

upload_mlbf_to_remote_settings(force_base=force_base)


@task
@use_primary_db
def process_blocklistsubmission(multi_block_submit_id, **kw):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,19 @@
Delete Multiple {{ cl.opts.verbose_name_plural }}
</a>
</li>
<li>
{% url cl.opts|admin_urlname:'upload_mlbf' as upload_mlbf_url %}
<form method="post" action="{% add_preserved_filters upload_mlbf_url %}">
{% csrf_token %}
<button type="submit" class="button">
Upload MLBF
</button>
</form>
<form method="post" action="{% add_preserved_filters upload_mlbf_url %}?force_base=true">
{% csrf_token %}
<button type="submit" class="button">
Upload MLBF (Force Base)
</button>
</form>
</li>
{% endblock %}
67 changes: 67 additions & 0 deletions src/olympia/blocklist/tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@
from django.conf import settings
from django.contrib.admin.models import ADDITION, LogEntry
from django.contrib.contenttypes.models import ContentType
from django.test.utils import override_settings
from django.urls import reverse

import responses
from freezegun import freeze_time
from pyquery import PyQuery as pq
from waffle.testutils import override_switch

from olympia import amo
from olympia.activity.models import ActivityLog
Expand Down Expand Up @@ -283,6 +285,71 @@ def test_soften_disabled_only_soft_blocked_versions_already(self):
assert 'disabled' not in doc('.hardenlink').attr('class')
assert 'disabled' in doc('.softenlink').attr('class')

def _test_upload_mlbf_disabled(self):
user = user_factory(email='[email protected]')
self.client.force_login(user)
response = self.client.post(
reverse('admin:blocklist_block_upload_mlbf'), follow=True
)
assert response.status_code == 403

def test_upload_mlbf_disabled(self):
self._test_upload_mlbf_disabled()

@override_switch('blocklist_mlbf_submit', active=True)
@override_settings(ENABLE_ADMIN_MLBF_UPLOAD=False)
def test_upload_mlbf_disabled_setting(self):
self._test_upload_mlbf_disabled()

@override_switch('blocklist_mlbf_submit', active=False)
@override_settings(ENABLE_ADMIN_MLBF_UPLOAD=True)
def test_upload_mlbf_disabled_switch(self):
self._test_upload_mlbf_disabled()

@override_switch('blocklist_mlbf_submit', active=True)
@override_settings(ENABLE_ADMIN_MLBF_UPLOAD=True)
def test_upload_mlbf_disabled_permission(self):
self._test_upload_mlbf_disabled()

@override_switch('blocklist_mlbf_submit', active=True)
@override_settings(ENABLE_ADMIN_MLBF_UPLOAD=True)
def test_upload_mlf_get_request_not_allowed(self):
user = user_factory(email='[email protected]')
self.client.force_login(user)
response = self.client.get(
reverse('admin:blocklist_block_upload_mlbf'), follow=True
)
assert response.status_code == 405

def _test_upload_mlbf_enabled(self, mock_upload, force_base=False):
user = user_factory(email='[email protected]')
self.grant_permission(user, 'Blocklist:Create')
self.client.force_login(user)
url = reverse('admin:blocklist_block_upload_mlbf')
if force_base:
url += '?force_base=true'
response = self.client.post(url, follow=True)
assert response.status_code == 200
assert mock_upload.called
assert mock_upload.call_args == mock.call(force_base=force_base)
messages = list(response.context['messages'])
assert len(messages) == 1
assert str(messages[0]) == (
'MLBF upload to remote settings has been triggered.'
)

@override_switch('blocklist_mlbf_submit', active=True)
@override_settings(ENABLE_ADMIN_MLBF_UPLOAD=True)
@mock.patch('olympia.blocklist.tasks.upload_mlbf_to_remote_settings_task.delay')
def test_upload_mlbf_enabled(self, mock_upload):
self._test_upload_mlbf_enabled(mock_upload, force_base=False)

@override_switch('blocklist_mlbf_submit', active=True)
@override_settings(ENABLE_ADMIN_MLBF_UPLOAD=True)
@mock.patch('olympia.blocklist.tasks.upload_mlbf_to_remote_settings_task.delay')
def test_upload_mlbf_enabled_force_base(self, mock_upload):
self._test_upload_mlbf_enabled(mock_upload, force_base=True)


def check_checkbox(checkbox, version):
assert checkbox.attrib['value'] == str(version.id)
Expand Down
17 changes: 11 additions & 6 deletions src/olympia/blocklist/tests/test_cron.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@
from olympia.blocklist.models import Block, BlocklistSubmission, BlockType, BlockVersion
from olympia.blocklist.tasks import upload_filter
from olympia.blocklist.utils import datetime_to_ts
from olympia.constants.blocklist import MLBF_BASE_ID_CONFIG_KEY, MLBF_TIME_CONFIG_KEY
from olympia.constants.blocklist import (
BASE_REPLACE_THRESHOLD_KEY,
MLBF_BASE_ID_CONFIG_KEY,
MLBF_TIME_CONFIG_KEY,
)
from olympia.zadmin.models import set_config


Expand Down Expand Up @@ -490,14 +494,15 @@ def test_upload_stash_unless_missing_base_filter(self):
)
) in self.mocks['olympia.blocklist.cron.upload_filter.delay'].call_args_list

@mock.patch('olympia.blocklist.mlbf.BASE_REPLACE_THRESHOLD', 1)
@mock.patch('olympia.blocklist.mlbf.get_base_replace_threshold')
@override_switch('enable-soft-blocking', active=True)
def test_upload_stash_unless_enough_changes(self):
block_type = BlockType.BLOCKED
def test_upload_stash_unless_enough_changes(self, mock_get_base_replace_threshold):
"""
When there are new blocks, upload either a stash or a filter depending on
whether we have surpased the BASE_REPLACE_THRESHOLD amount.
whether we have surpased the threshold amount.
"""
mock_get_base_replace_threshold.return_value = 1
block_type = BlockType.BLOCKED
for _block_type in BlockType:
self._block_version(is_signed=True, block_type=_block_type)

Expand Down Expand Up @@ -541,13 +546,13 @@ def test_upload_stash_unless_enough_changes(self):
assert len(new_mlbf.data.blocked_items) == 2
assert len(data['softblocked']) == 1

@mock.patch('olympia.blocklist.mlbf.BASE_REPLACE_THRESHOLD', 1)
def _test_upload_stash_and_filter(
self,
enable_soft_blocking: bool,
expected_stash: dict | None,
expected_filters: List[BlockType],
):
set_config(BASE_REPLACE_THRESHOLD_KEY, 1)
with override_switch('enable-soft-blocking', active=enable_soft_blocking):
upload_mlbf_to_remote_settings()

Expand Down
19 changes: 13 additions & 6 deletions src/olympia/blocklist/tests/test_mlbf.py
Original file line number Diff line number Diff line change
Expand Up @@ -550,8 +550,9 @@ def test_block_soft_to_hard(self):
'unblocked': [],
}

@mock.patch('olympia.blocklist.mlbf.BASE_REPLACE_THRESHOLD', 2)
def test_hard_to_soft_multiple(self):
@mock.patch('olympia.blocklist.mlbf.get_base_replace_threshold')
def test_hard_to_soft_multiple(self, mock_get_base_replace_threshold):
mock_get_base_replace_threshold.return_value = 2
addon, block = self._blocked_addon()
block_versions = [
self._block_version(block, self._version(addon)) for _ in range(2)
Expand Down Expand Up @@ -584,8 +585,11 @@ def test_hard_to_soft_multiple(self):
'unblocked': [],
}

@mock.patch('olympia.blocklist.mlbf.BASE_REPLACE_THRESHOLD', 1)
def test_stash_is_empty_if_uploading_new_filter(self):
@mock.patch('olympia.blocklist.mlbf.get_base_replace_threshold')
def test_stash_is_empty_if_uploading_new_filter(
self, mock_get_base_replace_threshold
):
mock_get_base_replace_threshold.return_value = 1
mlbf = MLBF.generate_from_db('test')

# No changes yet so no new filter and empty stash
Expand Down Expand Up @@ -866,8 +870,11 @@ def test_generate_stash_returns_expected_stash(self):
'unblocked': MLBF.hash_filter_inputs(expected_unblocked),
}

@mock.patch('olympia.blocklist.mlbf.BASE_REPLACE_THRESHOLD', 2)
def test_generate_empty_stash_when_all_items_in_filter(self):
@mock.patch('olympia.blocklist.mlbf.get_base_replace_threshold')
def test_generate_empty_stash_when_all_items_in_filter(
self, mock_get_base_replace_threshold
):
mock_get_base_replace_threshold.return_value = 2
# Add a hard blocked version and 2 soft blocked versions
addon, block = self._blocked_addon(
file_kw={'is_signed': True}, block_type=BlockType.BLOCKED
Expand Down
2 changes: 2 additions & 0 deletions src/olympia/conf/dev/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,5 @@
FXA_PROFILE_HOST = 'https://profile.stage.mozaws.net/v1'

SITEMAP_DEBUG_AVAILABLE = True

ENABLE_ADMIN_MLBF_UPLOAD = True
2 changes: 2 additions & 0 deletions src/olympia/conf/stage/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,5 @@
)

CINDER_QUEUE_PREFIX = 'amo-stage-'

ENABLE_ADMIN_MLBF_UPLOAD = True
2 changes: 1 addition & 1 deletion src/olympia/constants/blocklist.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from olympia.blocklist.models import BlockType


BASE_REPLACE_THRESHOLD = 5_000
BASE_REPLACE_THRESHOLD_KEY = 'blocklist_base_replace_threshold'

# Config keys used to track recent mlbf ids
MLBF_TIME_CONFIG_KEY = 'blocklist_mlbf_generation_time'
Expand Down
3 changes: 3 additions & 0 deletions src/olympia/lib/settings_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -954,6 +954,7 @@ def get_db_config(environ_var, atomic_requests=True):
'olympia.scanners.tasks.run_yara_query_rule': {'queue': 'zadmin'},
'olympia.scanners.tasks.run_yara_query_rule_on_versions_chunk': {'queue': 'zadmin'},
'olympia.zadmin.tasks.celery_error': {'queue': 'zadmin'},
'olympia.blocklist.tasks.upload_mlbf_to_remote_settings_task': {'queue': 'zadmin'},
}

# See PEP 391 for formatting help.
Expand Down Expand Up @@ -1606,3 +1607,5 @@ def read_only_mode(env):
# Set to True in settings_test.py
# This controls the behavior of migrations
TESTING_ENV = False

ENABLE_ADMIN_MLBF_UPLOAD = False

0 comments on commit 02bb35c

Please sign in to comment.