Skip to content

Commit 54a7d8b

Browse files
committed
Address PR Review Comments #1953
Signed-off-by: Michael Ehab Mikhail <michael.ehab@hotmail.com>
1 parent c1909f5 commit 54a7d8b

File tree

9 files changed

+129
-89
lines changed

9 files changed

+129
-89
lines changed

docker-compose.yml

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ services:
2323
vulnerablecode:
2424
build: .
2525
command: /bin/sh -c "
26-
./manage.py migrate &&
27-
./manage.py collectstatic --no-input --verbosity 0 --clear &&
28-
gunicorn vulnerablecode.wsgi:application -u nobody -g nogroup --bind :8000 --timeout 600 --workers 8"
26+
./manage.py migrate &&
27+
./manage.py collectstatic --no-input --verbosity 0 --clear &&
28+
gunicorn vulnerablecode.wsgi:application -u nobody -g nogroup --bind :8000 --timeout 600 --workers 8"
2929
env_file:
3030
- docker.env
3131
expose:
@@ -60,6 +60,17 @@ services:
6060
- db
6161
- vulnerablecode
6262

63+
vulnerablecode_rqworker_live:
64+
build: .
65+
command: wait-for-it web:8000 -- python ./manage.py rqworker live
66+
env_file:
67+
- docker.env
68+
volumes:
69+
- /etc/vulnerablecode/:/etc/vulnerablecode/
70+
depends_on:
71+
- vulnerablecode_redis
72+
- db
73+
- vulnerablecode
6374

6475
nginx:
6576
image: nginx
@@ -75,9 +86,7 @@ services:
7586
depends_on:
7687
- vulnerablecode
7788

78-
7989
volumes:
8090
db_data:
8191
static:
8292
vulnerablecode_redis_data:
83-

docker.env

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,6 @@ POSTGRES_PASSWORD=vulnerablecode
55
VULNERABLECODE_DB_HOST=db
66
VULNERABLECODE_STATIC_ROOT=/var/vulnerablecode/static/
77

8-
VULNERABLECODE_REDIS_HOST=vulnerablecode_redis
8+
VULNERABLECODE_REDIS_HOST=vulnerablecode_redis
9+
10+
VULNERABLECODE_ENABLE_LIVE_EVALUATION_API=false

vulnerabilities/api_v2.py

Lines changed: 10 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1304,6 +1304,7 @@ class LiveEvaluationSerializer(serializers.Serializer):
13041304

13051305
class LiveEvaluationViewSet(viewsets.GenericViewSet):
13061306
serializer_class = LiveEvaluationSerializer
1307+
throttle_classes = [AnonRateThrottle, PermissionBasedUserRateThrottle]
13071308

13081309
@extend_schema(
13091310
request=LiveEvaluationSerializer,
@@ -1346,34 +1347,15 @@ def evaluate(self, request):
13461347
status=status.HTTP_400_BAD_REQUEST,
13471348
)
13481349

1349-
# Create a single LivePipelineRun to represent this evaluation
1350-
from vulnerabilities.models import LivePipelineRun
1351-
1352-
live_run = LivePipelineRun.objects.create(purl=purl_string)
1353-
runs = []
1354-
for importer in importers:
1355-
importer_name = getattr(importer, "pipeline_id", importer.__name__)
1356-
run_id = enqueue_ad_hoc_pipeline(importer_name, inputs={"purl": purl})
1357-
# Attach each PipelineRun to the LivePipelineRun
1358-
from vulnerabilities.models import PipelineRun
1359-
1360-
try:
1361-
run_obj = PipelineRun.objects.get(run_id=run_id)
1362-
run_obj.live_pipeline = live_run
1363-
run_obj.save()
1364-
except PipelineRun.DoesNotExist:
1365-
pass
1366-
runs.append(
1367-
{
1368-
"importer": importer_name,
1369-
"run_id": str(run_id) if run_id else None,
1370-
}
1371-
)
1350+
# Enqueue all selected importers together and link runs to a new LivePipelineRun
1351+
importer_ids = [getattr(imp, "pipeline_id", imp.__name__) for imp in importers]
1352+
live_run_id, run_ids = enqueue_ad_hoc_pipeline(importer_ids, inputs={"purl": purl})
1353+
runs = [
1354+
{"importer": importer_ids[idx], "run_id": str(rid)} for idx, rid in enumerate(run_ids)
1355+
]
13721356

13731357
request_obj = request
1374-
status_path = reverse(
1375-
"live-evaluation-status", kwargs={"live_run_id": str(live_run.run_id)}
1376-
)
1358+
status_path = reverse("live-evaluation-status", kwargs={"live_run_id": str(live_run_id)})
13771359

13781360
if hasattr(request_obj, "build_absolute_uri"):
13791361
status_url = request_obj.build_absolute_uri(status_path)
@@ -1382,7 +1364,7 @@ def evaluate(self, request):
13821364

13831365
return Response(
13841366
{
1385-
"live_run_id": str(live_run.run_id),
1367+
"live_run_id": str(live_run_id),
13861368
"runs": runs,
13871369
"status_url": status_url,
13881370
},
@@ -1431,6 +1413,7 @@ def status(self, request, live_run_id=None):
14311413
"live_run_id": str(live_run.run_id),
14321414
"overall_status": live_run.status,
14331415
"created_date": live_run.created_date,
1416+
"started_date": getattr(live_run, "started_date", None),
14341417
"completed_date": live_run.completed_date,
14351418
"purl": live_run.purl,
14361419
"importers": importer_statuses,

vulnerabilities/models.py

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1985,17 +1985,37 @@ def is_finished(self):
19851985
return self.status == "finished"
19861986

19871987
def update_status(self):
1988-
if not self.pipelineruns.exists():
1988+
runs = list(self.pipelineruns.all())
1989+
if not runs:
19891990
self.status = "queued"
1990-
elif all(run.status == PipelineRun.Status.SUCCESS for run in self.pipelineruns.all()):
1991+
self.completed_date = None
1992+
self.save(update_fields=["status", "completed_date"])
1993+
return
1994+
1995+
# Determine aggregate status
1996+
if all(r.status == PipelineRun.Status.SUCCESS for r in runs):
19911997
self.status = "finished"
1992-
self.completed_date = timezone.now()
1993-
elif any(run.status == PipelineRun.Status.FAILURE for run in self.pipelineruns.all()):
1998+
elif any(r.status == PipelineRun.Status.FAILURE for r in runs):
19941999
self.status = "failed"
1995-
self.completed_date = timezone.now()
1996-
else:
2000+
elif any(r.status == PipelineRun.Status.RUNNING for r in runs):
19972001
self.status = "running"
1998-
self.save()
2002+
else:
2003+
# queued or mixed queued
2004+
self.status = "queued"
2005+
2006+
end_times = [r.run_end_date for r in runs if r.run_end_date]
2007+
completed = None
2008+
if self.status in ("finished", "failed") and end_times:
2009+
completed = max(end_times)
2010+
self.completed_date = completed
2011+
self.save(update_fields=["status", "completed_date"])
2012+
2013+
@property
2014+
def started_date(self):
2015+
"""Return earliest run_start_date among child runs, if any."""
2016+
runs = self.pipelineruns.all()
2017+
start_times = [r.run_start_date for r in runs if r.run_start_date]
2018+
return min(start_times) if start_times else None
19992019

20002020
class Meta:
20012021
ordering = ["-created_date"]
@@ -2281,10 +2301,8 @@ def append_to_log(self, message, is_multiline=False):
22812301
message = message.strip()
22822302
if not is_multiline:
22832303
message = message.replace("\n", "").replace("\r", "")
2284-
2285-
new_log = (self.log or "") + message + "\n"
2286-
type(self).objects.filter(run_id=self.run_id).update(log=new_log)
2287-
self.log = new_log
2304+
self.log = (self.log or "") + message + "\n"
2305+
self.save(update_fields=["log"])
22882306

22892307
def dequeue(self):
22902308
from vulnerabilities.tasks import dequeue_job

vulnerabilities/tasks.py

Lines changed: 46 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
logger = logging.getLogger(__name__)
2323

2424
default_queue = django_rq.get_queue("default")
25+
live_queue = django_rq.get_queue("live")
2526

2627

2728
def execute_pipeline(pipeline_id, run_id, inputs=None):
@@ -132,35 +133,59 @@ def enqueue_pipeline(pipeline_id):
132133
)
133134

134135

135-
def enqueue_ad_hoc_pipeline(pipeline_id, *, inputs=None):
136-
"""Enqueue a one-off execution for the given pipeline_id with optional inputs.
136+
def enqueue_ad_hoc_pipeline(pipeline_ids, *, inputs=None):
137+
"""Enqueue one-off executions for the given pipeline_ids with optional inputs.
137138
138-
Returns the created run_id or None if the pipeline cannot be enqueued.
139+
When multiple pipeline IDs are provided, this will create a single LivePipelineRun and attach
140+
each created PipelineRun to it. Returns a tuple of (live_run_id, run_ids).
141+
142+
If a single pipeline ID (str) is provided, it will be wrapped into a list.
139143
"""
144+
inputs = inputs or {}
145+
# Normalize to list
146+
if isinstance(pipeline_ids, str):
147+
pipeline_ids = [pipeline_ids]
148+
149+
# Create a LivePipelineRun to group these ad-hoc runs, if any inputs (such as purl) are given
150+
purl_val = inputs.get("purl")
140151
try:
141-
pipeline_schedule = models.PipelineSchedule.objects.get(pipeline_id=pipeline_id)
142-
except models.PipelineSchedule.DoesNotExist:
143-
pipeline_schedule = models.PipelineSchedule.objects.create(
144-
pipeline_id=pipeline_id,
145-
is_active=False,
146-
)
152+
# accept PackageURL instance as well as string
153+
purl_str = str(purl_val) if purl_val is not None else None
154+
except Exception:
155+
purl_str = None
156+
157+
live_run = models.LivePipelineRun.objects.create(purl=purl_str)
158+
159+
run_ids = []
160+
for pipeline_id in pipeline_ids:
161+
try:
162+
pipeline_schedule = models.PipelineSchedule.objects.get(pipeline_id=pipeline_id)
163+
except models.PipelineSchedule.DoesNotExist:
164+
pipeline_schedule = models.PipelineSchedule.objects.create(
165+
pipeline_id=pipeline_id,
166+
is_active=False,
167+
)
147168

148-
run = models.PipelineRun.objects.create(pipeline=pipeline_schedule)
169+
run = models.PipelineRun.objects.create(pipeline=pipeline_schedule, live_pipeline=live_run)
170+
171+
# Enqueue on the dedicated live queue
172+
live_queue.enqueue(
173+
execute_pipeline,
174+
pipeline_id,
175+
run.run_id,
176+
inputs,
177+
job_id=str(run.run_id),
178+
on_failure=set_run_failure,
179+
job_timeout=f"{pipeline_schedule.execution_timeout}h",
180+
)
181+
run_ids.append(run.run_id)
149182

150-
live_queue = django_rq.get_queue("live")
151-
job = live_queue.enqueue(
152-
execute_pipeline,
153-
pipeline_id,
154-
run.run_id,
155-
inputs or {},
156-
job_id=str(run.run_id),
157-
on_failure=set_run_failure,
158-
job_timeout=f"{pipeline_schedule.execution_timeout}h",
159-
)
160-
return run.run_id
183+
return live_run.run_id, run_ids
161184

162185

163186
def dequeue_job(job_id):
164187
"""Remove a job from queue if it hasn't been executed yet."""
165188
if job_id in default_queue.jobs:
166189
default_queue.remove(job_id)
190+
if job_id in live_queue.jobs:
191+
live_queue.remove(job_id)

vulnerabilities/tests/test_api_v2.py

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -913,27 +913,15 @@ def setUp(self):
913913

914914
@patch("vulnerabilities.api_v2.LIVE_IMPORTERS_REGISTRY")
915915
@patch("vulnerabilities.api_v2.enqueue_ad_hoc_pipeline")
916-
@patch("vulnerabilities.models.PipelineRun.objects.get")
917-
@patch("vulnerabilities.models.LivePipelineRun.objects.create")
918916
@patch("django.urls.reverse")
919-
def test_evaluate_success(
920-
self, mock_reverse, mock_live_create, mock_pipeline_get, mock_enqueue, mock_registry
921-
):
917+
def test_evaluate_success(self, mock_reverse, mock_enqueue, mock_registry):
922918
class MockImporter:
923919
pipeline_id = "pypa_live_importer_v2"
924920
supported_types = ["pypi"]
925921

926922
mock_registry.values.return_value = [MockImporter]
927923
valid_uuid = "00000000-0000-0000-0000-000000000001"
928-
mock_live_run = type("MockLiveRun", (), {"run_id": valid_uuid})()
929-
mock_live_create.return_value = mock_live_run
930-
mock_enqueue.return_value = "mock-run-id"
931-
mock_pipeline_run = type(
932-
"MockPipelineRun",
933-
(),
934-
{"run_id": "mock-run-id", "live_pipeline": None, "save": lambda self: None},
935-
)()
936-
mock_pipeline_get.return_value = mock_pipeline_run
924+
mock_enqueue.return_value = (valid_uuid, ["mock-run-id"])
937925
mock_reverse.return_value = f"/api/v2/live-evaluation/status/{valid_uuid}"
938926

939927
data = {"purl": "pkg:pypi/django@3.2"}
@@ -942,7 +930,7 @@ class MockImporter:
942930
assert isinstance(response.data, dict)
943931
assert response.data["live_run_id"] is not None
944932
assert response.data["runs"][0]["importer"] == "pypa_live_importer_v2"
945-
assert response.data["runs"][0]["run_id"] is not None
933+
assert response.data["runs"][0]["run_id"] == "mock-run-id"
946934
assert "status_url" in response.data
947935
assert response.data["status_url"].endswith(f"/api/v2/live-evaluation/status/{valid_uuid}")
948936

vulnerabilities/views.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -697,6 +697,15 @@ def get_context_data(self, **kwargs):
697697
context["disabled_pipeline_count"] = PipelineSchedule.objects.filter(
698698
pipeline_id__in=live_pipeline_ids, is_active=False
699699
).count()
700+
701+
# Update status of recent LivePipelineRun groups for freshness
702+
try:
703+
from vulnerabilities.models import LivePipelineRun
704+
705+
for lpr in LivePipelineRun.objects.order_by("-created_date")[:50]:
706+
lpr.update_status()
707+
except Exception:
708+
pass
700709
return context
701710

702711

vulnerablecode/settings.py

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,10 @@
181181
"VULNERABLECODEIO_REQUIRE_AUTHENTICATION", default=False
182182
)
183183

184+
VULNERABLECODE_ENABLE_LIVE_EVALUATION_API = env.bool(
185+
"VULNERABLECODE_ENABLE_LIVE_EVALUATION_API", default=False
186+
)
187+
184188
LOGIN_REDIRECT_URL = "/"
185189
LOGOUT_REDIRECT_URL = "/"
186190

@@ -383,12 +387,11 @@
383387
"PORT": env.str("VULNERABLECODE_REDIS_PORT", default="6379"),
384388
"PASSWORD": env.str("VULNERABLECODE_REDIS_PASSWORD", default=""),
385389
"DEFAULT_TIMEOUT": env.int("VULNERABLECODE_REDIS_DEFAULT_TIMEOUT", default=3600),
386-
}
387-
}
388-
389-
RQ_QUEUES["live"] = {
390-
"HOST": env.str("VULNERABLECODE_REDIS_HOST", default=RQ_QUEUES["default"]["HOST"]),
391-
"PORT": env.str("VULNERABLECODE_REDIS_PORT", default=RQ_QUEUES["default"]["PORT"]),
392-
"PASSWORD": env.str("VULNERABLECODE_REDIS_PASSWORD", default=RQ_QUEUES["default"]["PASSWORD"]),
393-
"DEFAULT_TIMEOUT": env.int("VULNERABLECODE_LIVE_REDIS_DEFAULT_TIMEOUT", default=3600),
390+
},
391+
"live": {
392+
"HOST": env.str("VULNERABLECODE_REDIS_HOST", default="localhost"),
393+
"PORT": env.str("VULNERABLECODE_REDIS_PORT", default="6379"),
394+
"PASSWORD": env.str("VULNERABLECODE_REDIS_PASSWORD", default=""),
395+
"DEFAULT_TIMEOUT": env.int("VULNERABLECODE_LIVE_REDIS_DEFAULT_TIMEOUT", default=3600),
396+
},
394397
}

vulnerablecode/urls.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
from vulnerabilities.views import VulnerabilitySearch
4747
from vulnerablecode.settings import DEBUG
4848
from vulnerablecode.settings import DEBUG_TOOLBAR
49+
from vulnerablecode.settings import VULNERABLECODE_ENABLE_LIVE_EVALUATION_API
4950

5051

5152
# See the comment at https://stackoverflow.com/a/46163870.
@@ -71,7 +72,9 @@ def __init__(self, *args, **kwargs):
7172
api_v2_router.register("codefixes", CodeFixViewSet, basename="codefix")
7273
api_v2_router.register("pipelines", PipelineScheduleV2ViewSet, basename="pipelines")
7374
api_v2_router.register("advisory-codefixes", CodeFixV2ViewSet, basename="advisory-codefix")
74-
api_v2_router.register("live-evaluation", LiveEvaluationViewSet, basename="live-evaluation")
75+
76+
if VULNERABLECODE_ENABLE_LIVE_EVALUATION_API:
77+
api_v2_router.register("live-evaluation", LiveEvaluationViewSet, basename="live-evaluation")
7578

7679

7780
urlpatterns = [

0 commit comments

Comments
 (0)