mirror of
https://github.com/netbox-community/netbox.git
synced 2026-04-20 16:01:34 +02:00
fix(extras): Handle username fallback for job events
Fallback to the associated user when username is missing from job lifecycle event contexts. Add a regression test to ensure JOB_COMPLETED webhooks are enqueued without a request context. Fixes #21371
This commit is contained in:
committed by
Jeremy Stretch
parent
af6e18b7d4
commit
bdd23f3d17
@@ -113,6 +113,17 @@ def enqueue_event(queue, instance, request, event_type):
|
|||||||
def process_event_rules(event_rules, object_type, event):
|
def process_event_rules(event_rules, object_type, event):
|
||||||
"""
|
"""
|
||||||
Process a list of EventRules against an event.
|
Process a list of EventRules against an event.
|
||||||
|
|
||||||
|
Notes on event sources:
|
||||||
|
- Object change events (created/updated/deleted) are enqueued via
|
||||||
|
enqueue_event() during an HTTP request.
|
||||||
|
These events include a request object and legacy request
|
||||||
|
attributes (e.g. username, request_id) for backward compatibility.
|
||||||
|
- Job lifecycle events (JOB_STARTED/JOB_COMPLETED) are emitted by
|
||||||
|
job_start/job_end signal handlers and may not include a request
|
||||||
|
context.
|
||||||
|
Consumers must not assume that fields like `username` are always
|
||||||
|
present.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
for event_rule in event_rules:
|
for event_rule in event_rules:
|
||||||
@@ -132,16 +143,22 @@ def process_event_rules(event_rules, object_type, event):
|
|||||||
queue_name = get_config().QUEUE_MAPPINGS.get('webhook', RQ_QUEUE_DEFAULT)
|
queue_name = get_config().QUEUE_MAPPINGS.get('webhook', RQ_QUEUE_DEFAULT)
|
||||||
rq_queue = get_queue(queue_name)
|
rq_queue = get_queue(queue_name)
|
||||||
|
|
||||||
|
# For job lifecycle events, `username` may be absent because
|
||||||
|
# there is no request context.
|
||||||
|
# Prefer the associated user object when present, falling
|
||||||
|
# back to the legacy username attribute.
|
||||||
|
username = getattr(event.get('user'), 'username', None) or event.get('username')
|
||||||
|
|
||||||
# Compile the task parameters
|
# Compile the task parameters
|
||||||
params = {
|
params = {
|
||||||
"event_rule": event_rule,
|
'event_rule': event_rule,
|
||||||
"object_type": object_type,
|
'object_type': object_type,
|
||||||
"event_type": event['event_type'],
|
'event_type': event['event_type'],
|
||||||
"data": event_data,
|
'data': event_data,
|
||||||
"snapshots": event.get('snapshots'),
|
'snapshots': event.get('snapshots'),
|
||||||
"timestamp": timezone.now().isoformat(),
|
'timestamp': timezone.now().isoformat(),
|
||||||
"username": event['username'],
|
'username': username,
|
||||||
"retry": get_rq_retry()
|
'retry': get_rq_retry(),
|
||||||
}
|
}
|
||||||
if 'request' in event:
|
if 'request' in event:
|
||||||
# Exclude FILES - webhooks don't need uploaded files,
|
# Exclude FILES - webhooks don't need uploaded files,
|
||||||
@@ -158,11 +175,12 @@ def process_event_rules(event_rules, object_type, event):
|
|||||||
|
|
||||||
# Enqueue a Job to record the script's execution
|
# Enqueue a Job to record the script's execution
|
||||||
from extras.jobs import ScriptJob
|
from extras.jobs import ScriptJob
|
||||||
|
|
||||||
params = {
|
params = {
|
||||||
"instance": event_rule.action_object,
|
'instance': event_rule.action_object,
|
||||||
"name": script.name,
|
'name': script.name,
|
||||||
"user": event['user'],
|
'user': event['user'],
|
||||||
"data": event_data
|
'data': event_data,
|
||||||
}
|
}
|
||||||
if 'snapshots' in event:
|
if 'snapshots' in event:
|
||||||
params['snapshots'] = event['snapshots']
|
params['snapshots'] = event['snapshots']
|
||||||
@@ -179,7 +197,7 @@ def process_event_rules(event_rules, object_type, event):
|
|||||||
object_type=object_type,
|
object_type=object_type,
|
||||||
object_id=event_data['id'],
|
object_id=event_data['id'],
|
||||||
object_repr=event_data.get('display'),
|
object_repr=event_data.get('display'),
|
||||||
event_type=event['event_type']
|
event_type=event['event_type'],
|
||||||
)
|
)
|
||||||
|
|
||||||
else:
|
else:
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
import json
|
import json
|
||||||
import uuid
|
import uuid
|
||||||
from unittest.mock import patch
|
from unittest.mock import Mock, patch
|
||||||
|
|
||||||
import django_rq
|
import django_rq
|
||||||
from django.http import HttpResponse
|
from django.http import HttpResponse
|
||||||
@@ -15,7 +15,8 @@ from dcim.choices import SiteStatusChoices
|
|||||||
from dcim.models import Site
|
from dcim.models import Site
|
||||||
from extras.choices import EventRuleActionChoices
|
from extras.choices import EventRuleActionChoices
|
||||||
from extras.events import enqueue_event, flush_events, serialize_for_event
|
from extras.events import enqueue_event, flush_events, serialize_for_event
|
||||||
from extras.models import EventRule, Tag, Webhook
|
from extras.models import EventRule, Script, Tag, Webhook
|
||||||
|
from extras.signals import process_job_end_event_rules
|
||||||
from extras.webhooks import generate_signature, send_webhook
|
from extras.webhooks import generate_signature, send_webhook
|
||||||
from netbox.context_managers import event_tracking
|
from netbox.context_managers import event_tracking
|
||||||
from utilities.testing import APITestCase
|
from utilities.testing import APITestCase
|
||||||
@@ -395,6 +396,36 @@ class EventRuleTest(APITestCase):
|
|||||||
with patch.object(Session, 'send', dummy_send):
|
with patch.object(Session, 'send', dummy_send):
|
||||||
send_webhook(**job.kwargs)
|
send_webhook(**job.kwargs)
|
||||||
|
|
||||||
|
def test_job_completed_webhook_username_fallback(self):
|
||||||
|
"""
|
||||||
|
Ensure job_end event processing can enqueue a webhook even when the EventContext
|
||||||
|
lacks legacy request attributes (e.g. `username`).
|
||||||
|
|
||||||
|
The job_start/job_end signal receivers only populate `user` and `data`, so webhook
|
||||||
|
processing must derive the username from the user object (or tolerate it being unset).
|
||||||
|
"""
|
||||||
|
script_type = ObjectType.objects.get_for_model(Script)
|
||||||
|
webhook_type = ObjectType.objects.get_for_model(Webhook)
|
||||||
|
webhook = Webhook.objects.get(name='Webhook 1')
|
||||||
|
event_rule = EventRule.objects.create(
|
||||||
|
name='Event Rule Job Completed',
|
||||||
|
event_types=[JOB_COMPLETED],
|
||||||
|
action_type=EventRuleActionChoices.WEBHOOK,
|
||||||
|
action_object_type=webhook_type,
|
||||||
|
action_object_id=webhook.pk,
|
||||||
|
)
|
||||||
|
event_rule.object_types.set([script_type])
|
||||||
|
# Mimic the `core.job_end` signal sender expected by extras.signals.process_job_end_event_rules
|
||||||
|
# (notably: no request, and thus no legacy `username`)
|
||||||
|
sender = Mock(object_type=script_type, data={}, user=self.user)
|
||||||
|
process_job_end_event_rules(sender)
|
||||||
|
self.assertEqual(self.queue.count, 1)
|
||||||
|
job = self.queue.jobs[0]
|
||||||
|
self.assertEqual(job.kwargs['event_rule'], event_rule)
|
||||||
|
self.assertEqual(job.kwargs['event_type'], JOB_COMPLETED)
|
||||||
|
self.assertEqual(job.kwargs['object_type'], script_type)
|
||||||
|
self.assertEqual(job.kwargs['username'], self.user.username)
|
||||||
|
|
||||||
def test_duplicate_triggers(self):
|
def test_duplicate_triggers(self):
|
||||||
"""
|
"""
|
||||||
Test for erroneous duplicate event triggers resulting from saving an object multiple times
|
Test for erroneous duplicate event triggers resulting from saving an object multiple times
|
||||||
|
|||||||
Reference in New Issue
Block a user