From 44248b5f12b9d73876437165379f12f4d48a6818 Mon Sep 17 00:00:00 2001 From: Tushar Goel Date: Fri, 25 Nov 2022 19:19:47 +0530 Subject: [PATCH 1/4] Make bulk search fast Signed-off-by: Tushar Goel --- vulnerabilities/api.py | 36 ++++++++----------- .../migrations/0034_package_package.py | 18 ++++++++++ .../migrations/0035_auto_20221125_1257.py | 28 +++++++++++++++ vulnerabilities/models.py | 11 ++++++ 4 files changed, 72 insertions(+), 21 deletions(-) create mode 100644 vulnerabilities/migrations/0034_package_package.py create mode 100644 vulnerabilities/migrations/0035_auto_20221125_1257.py diff --git a/vulnerabilities/api.py b/vulnerabilities/api.py index 85d62bafb..3a2f69756 100644 --- a/vulnerabilities/api.py +++ b/vulnerabilities/api.py @@ -238,32 +238,26 @@ def bulk_search(self, request): """ Lookup for vulnerable packages using many Package URLs at once. """ - response = [] + purls = request.data.get("purls", []) or [] + purl_only = request.data.get("purl_only", False) if not purls or not isinstance(purls, list): return Response( status=400, - data={"Error": "A non-empty 'purls' list of package URLs is required."}, + data={"Error": "A non-empty 'purls' list of PURLs is required."}, ) - for purl in request.data["purls"]: - try: - purl_string = purl - purl = PackageURL.from_string(purl) - except ValueError: - return Response(status=400, data={"Error": f"Invalid Package URL: {purl}"}) - lookups = get_purl_query_lookups(purl) - purl_data = Package.objects.filter(**lookups) - purl_response = {} - if purl_data: - purl_response = PackageSerializer(purl_data[0], context={"request": request}).data - else: - purl_response = purl.to_dict() - purl_response["unresolved_vulnerabilities"] = [] - purl_response["resolved_vulnerabilities"] = [] - purl_response["purl"] = purl_string - response.append(purl_response) - - return Response(response) + + query = Package.objects.filter(package__in=purls) + + if not purl_only: + return Response( + PackageSerializer(query.distinct(), many=True, context={"request": request}).data + ) + + vulnerable_purls = ( + query.filter(packagerelatedvulnerability__fix=False).only("package").distinct() + ) + return Response(data=vulnerable_purls) @action(detail=False, methods=["get"], throttle_scope="vulnerable_packages") def all(self, request): diff --git a/vulnerabilities/migrations/0034_package_package.py b/vulnerabilities/migrations/0034_package_package.py new file mode 100644 index 000000000..9c7ab36a3 --- /dev/null +++ b/vulnerabilities/migrations/0034_package_package.py @@ -0,0 +1,18 @@ +# Generated by Django 4.0.7 on 2022-11-25 12:57 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('vulnerabilities', '0033_alter_vulnerabilityseverity_scoring_system'), + ] + + operations = [ + migrations.AddField( + model_name='package', + name='package', + field=models.CharField(blank=True, help_text='The Package URL for this package.', max_length=255), + ), + ] diff --git a/vulnerabilities/migrations/0035_auto_20221125_1257.py b/vulnerabilities/migrations/0035_auto_20221125_1257.py new file mode 100644 index 000000000..aac109736 --- /dev/null +++ b/vulnerabilities/migrations/0035_auto_20221125_1257.py @@ -0,0 +1,28 @@ +# Generated by Django 4.0.7 on 2022-11-25 12:57 + +from django.db import migrations +from packageurl import PackageURL + +class Migration(migrations.Migration): + + def save_package(apps, schema_editor): + Package = apps.get_model("vulnerabilities", "Package") + for package in Package.objects.all(): + purl = PackageURL( + type=package.type, + namespace=package.namespace, + name=package.name, + version=package.version, + qualifiers=package.qualifiers, + subpath=package.subpath, + ) + package.package = str(purl) + package.save() + + dependencies = [ + ('vulnerabilities', '0034_package_package'), + ] + + operations = [ + migrations.RunPython(save_package, reverse_code=migrations.RunPython.noop), + ] diff --git a/vulnerabilities/models.py b/vulnerabilities/models.py index a2b8b5498..895d5c39b 100644 --- a/vulnerabilities/models.py +++ b/vulnerabilities/models.py @@ -531,8 +531,19 @@ class Package(PackageURLMixin): to="Vulnerability", through="PackageRelatedVulnerability" ) + package = models.CharField( + max_length=255, + blank=True, + null=False, + help_text="The Package URL for this package.", + ) + objects = PackageQuerySet.as_manager() + def save(self, *args, **kwargs): + self.package = self.purl + super().save(*args, **kwargs) + @property def purl(self): return self.package_url From 17317ca8c9370e389e3a915c2b8c58782e361ae8 Mon Sep 17 00:00:00 2001 From: Tushar Goel Date: Fri, 25 Nov 2022 23:34:44 +0530 Subject: [PATCH 2/4] Address review comments Signed-off-by: Tushar Goel --- vulnerabilities/api.py | 6 ++--- ...package.py => 0034_package_package_url.py} | 6 ++--- ...py => 0035_add_package_url_to_packages.py} | 22 ++++++++++++------- .../0036_alter_package_package_url.py | 18 +++++++++++++++ vulnerabilities/models.py | 14 +++++++++--- 5 files changed, 49 insertions(+), 17 deletions(-) rename vulnerabilities/migrations/{0034_package_package.py => 0034_package_package_url.py} (58%) rename vulnerabilities/migrations/{0035_auto_20221125_1257.py => 0035_add_package_url_to_packages.py} (51%) create mode 100644 vulnerabilities/migrations/0036_alter_package_package_url.py diff --git a/vulnerabilities/api.py b/vulnerabilities/api.py index 3a2f69756..745e4b818 100644 --- a/vulnerabilities/api.py +++ b/vulnerabilities/api.py @@ -247,7 +247,7 @@ def bulk_search(self, request): data={"Error": "A non-empty 'purls' list of PURLs is required."}, ) - query = Package.objects.filter(package__in=purls) + query = Package.objects.filter(package_url__in=purls) if not purl_only: return Response( @@ -264,8 +264,8 @@ def all(self, request): """ Return the Package URLs of all packages known to be vulnerable. """ - vulnerable_packages = Package.objects.vulnerable().only(*PackageURL._fields).distinct() - vulnerable_purls = [str(package.purl) for package in vulnerable_packages] + vulnerable_packages = Package.objects.vulnerable().only("package_url").distinct() + vulnerable_purls = [str(package.package_url) for package in vulnerable_packages] return Response(vulnerable_purls) diff --git a/vulnerabilities/migrations/0034_package_package.py b/vulnerabilities/migrations/0034_package_package_url.py similarity index 58% rename from vulnerabilities/migrations/0034_package_package.py rename to vulnerabilities/migrations/0034_package_package_url.py index 9c7ab36a3..7d32eeb53 100644 --- a/vulnerabilities/migrations/0034_package_package.py +++ b/vulnerabilities/migrations/0034_package_package_url.py @@ -1,4 +1,4 @@ -# Generated by Django 4.0.7 on 2022-11-25 12:57 +# Generated by Django 4.0.7 on 2022-11-25 17:28 from django.db import migrations, models @@ -12,7 +12,7 @@ class Migration(migrations.Migration): operations = [ migrations.AddField( model_name='package', - name='package', - field=models.CharField(blank=True, help_text='The Package URL for this package.', max_length=255), + name='package_url', + field=models.CharField(blank=True, db_index=True, help_text='The Package URL for this package.', max_length=255), ), ] diff --git a/vulnerabilities/migrations/0035_auto_20221125_1257.py b/vulnerabilities/migrations/0035_add_package_url_to_packages.py similarity index 51% rename from vulnerabilities/migrations/0035_auto_20221125_1257.py rename to vulnerabilities/migrations/0035_add_package_url_to_packages.py index aac109736..b27055183 100644 --- a/vulnerabilities/migrations/0035_auto_20221125_1257.py +++ b/vulnerabilities/migrations/0035_add_package_url_to_packages.py @@ -1,12 +1,11 @@ -# Generated by Django 4.0.7 on 2022-11-25 12:57 - from django.db import migrations from packageurl import PackageURL class Migration(migrations.Migration): - def save_package(apps, schema_editor): + def save_purls(apps, schema_editor): Package = apps.get_model("vulnerabilities", "Package") + updatables = [] for package in Package.objects.all(): purl = PackageURL( type=package.type, @@ -16,13 +15,20 @@ def save_package(apps, schema_editor): qualifiers=package.qualifiers, subpath=package.subpath, ) - package.package = str(purl) - package.save() + package.package_url = str(purl) + updatables.append(package) + + updated = Package.objects.bulk_update( + objs = updatables, + fields=["package_url"], + batch_size=500, + ) + print(f"Migrated {updated} packages with package_url") dependencies = [ - ('vulnerabilities', '0034_package_package'), + ('vulnerabilities', '0034_package_package_url'), ] operations = [ - migrations.RunPython(save_package, reverse_code=migrations.RunPython.noop), - ] + migrations.RunPython(save_purls, reverse_code=migrations.RunPython.noop), + ] \ No newline at end of file diff --git a/vulnerabilities/migrations/0036_alter_package_package_url.py b/vulnerabilities/migrations/0036_alter_package_package_url.py new file mode 100644 index 000000000..a02752dc9 --- /dev/null +++ b/vulnerabilities/migrations/0036_alter_package_package_url.py @@ -0,0 +1,18 @@ +# Generated by Django 4.0.7 on 2022-11-25 17:34 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('vulnerabilities', '0035_add_package_url_to_packages'), + ] + + operations = [ + migrations.AlterField( + model_name='package', + name='package_url', + field=models.CharField(db_index=True, help_text='The Package URL for this package.', max_length=255), + ), + ] diff --git a/vulnerabilities/models.py b/vulnerabilities/models.py index 895d5c39b..e06b89cf5 100644 --- a/vulnerabilities/models.py +++ b/vulnerabilities/models.py @@ -531,17 +531,25 @@ class Package(PackageURLMixin): to="Vulnerability", through="PackageRelatedVulnerability" ) - package = models.CharField( + package_url = models.CharField( max_length=255, - blank=True, null=False, help_text="The Package URL for this package.", + db_index=True, ) objects = PackageQuerySet.as_manager() def save(self, *args, **kwargs): - self.package = self.purl + purl_object = PackageURL( + type=self.type, + namespace=self.namespace, + name=self.name, + version=self.version, + qualifiers=self.qualifiers, + subpath=self.subpath, + ) + self.package_url = str(purl_object) super().save(*args, **kwargs) @property From c22de9cbd69577916aa56311470d00010ce49c8e Mon Sep 17 00:00:00 2001 From: Tushar Goel Date: Mon, 28 Nov 2022 19:24:16 +0530 Subject: [PATCH 3/4] Add plain_purl option and add tests Signed-off-by: Tushar Goel --- vulnerabilities/api.py | 42 +++++++++++++++---- ..._package_url_package_plain_package_url.py} | 9 +++- .../0035_add_package_url_to_packages.py | 11 ++++- ...036_alter_package_package_url_and_more.py} | 9 +++- vulnerabilities/models.py | 16 ++++++- vulnerabilities/tests/test_fix_api.py | 31 ++++++++++++-- 6 files changed, 100 insertions(+), 18 deletions(-) rename vulnerabilities/migrations/{0034_package_package_url.py => 0034_package_package_url_package_plain_package_url.py} (50%) rename vulnerabilities/migrations/{0036_alter_package_package_url.py => 0036_alter_package_package_url_and_more.py} (51%) diff --git a/vulnerabilities/api.py b/vulnerabilities/api.py index 745e4b818..9c115bbaa 100644 --- a/vulnerabilities/api.py +++ b/vulnerabilities/api.py @@ -241,22 +241,50 @@ def bulk_search(self, request): purls = request.data.get("purls", []) or [] purl_only = request.data.get("purl_only", False) + plain_purl = request.data.get("plain_purl", False) if not purls or not isinstance(purls, list): return Response( status=400, data={"Error": "A non-empty 'purls' list of PURLs is required."}, ) - query = Package.objects.filter(package_url__in=purls) + if plain_purl: + purl_objects = [PackageURL.from_string(purl) for purl in purls] + plain_purl_objects = [ + PackageURL( + type=purl.type, + namespace=purl.namespace, + name=purl.name, + version=purl.version, + ) + for purl in purl_objects + ] + plain_purls = [str(purl) for purl in plain_purl_objects] + + query = ( + Package.objects.filter(plain_package_url__in=plain_purls) + .order_by("plain_package_url") + .distinct("plain_package_url") + ) + + if not purl_only: + return Response( + PackageSerializer(query, many=True, context={"request": request}).data + ) + + # using order by and distinct because there will be + # many fully qualified purl for a single plain purl + vulnerable_purls = query.vulnerable().only("plain_package_url") + vulnerable_purls = [str(package.plain_package_url) for package in vulnerable_purls] + return Response(data=vulnerable_purls) + + query = Package.objects.filter(package_url__in=purls).distinct() if not purl_only: - return Response( - PackageSerializer(query.distinct(), many=True, context={"request": request}).data - ) + return Response(PackageSerializer(query, many=True, context={"request": request}).data) - vulnerable_purls = ( - query.filter(packagerelatedvulnerability__fix=False).only("package").distinct() - ) + vulnerable_purls = query.vulnerable().only("package_url") + vulnerable_purls = [str(package.package_url) for package in vulnerable_purls] return Response(data=vulnerable_purls) @action(detail=False, methods=["get"], throttle_scope="vulnerable_packages") diff --git a/vulnerabilities/migrations/0034_package_package_url.py b/vulnerabilities/migrations/0034_package_package_url_package_plain_package_url.py similarity index 50% rename from vulnerabilities/migrations/0034_package_package_url.py rename to vulnerabilities/migrations/0034_package_package_url_package_plain_package_url.py index 7d32eeb53..65d6d599a 100644 --- a/vulnerabilities/migrations/0034_package_package_url.py +++ b/vulnerabilities/migrations/0034_package_package_url_package_plain_package_url.py @@ -1,4 +1,4 @@ -# Generated by Django 4.0.7 on 2022-11-25 17:28 +# Generated by Django 4.0.7 on 2022-11-28 12:58 from django.db import migrations, models @@ -13,6 +13,11 @@ class Migration(migrations.Migration): migrations.AddField( model_name='package', name='package_url', - field=models.CharField(blank=True, db_index=True, help_text='The Package URL for this package.', max_length=255), + field=models.CharField(blank=True, db_index=True, help_text='The Package URL for this package.', max_length=1000), + ), + migrations.AddField( + model_name='package', + name='plain_package_url', + field=models.CharField(blank=True, db_index=True, help_text='The Package URL for this package without qualifiers and subpath.', max_length=1000), ), ] diff --git a/vulnerabilities/migrations/0035_add_package_url_to_packages.py b/vulnerabilities/migrations/0035_add_package_url_to_packages.py index b27055183..ea18aa325 100644 --- a/vulnerabilities/migrations/0035_add_package_url_to_packages.py +++ b/vulnerabilities/migrations/0035_add_package_url_to_packages.py @@ -15,18 +15,25 @@ def save_purls(apps, schema_editor): qualifiers=package.qualifiers, subpath=package.subpath, ) + plain_purl = PackageURL( + type=package.type, + namespace=package.namespace, + name=package.name, + version=package.version, + ) package.package_url = str(purl) + package.plain_package_url = str(plain_purl) updatables.append(package) updated = Package.objects.bulk_update( objs = updatables, - fields=["package_url"], + fields=["package_url", "plain_package_url"], batch_size=500, ) print(f"Migrated {updated} packages with package_url") dependencies = [ - ('vulnerabilities', '0034_package_package_url'), + ("vulnerabilities", "0034_package_package_url_package_plain_package_url"), ] operations = [ diff --git a/vulnerabilities/migrations/0036_alter_package_package_url.py b/vulnerabilities/migrations/0036_alter_package_package_url_and_more.py similarity index 51% rename from vulnerabilities/migrations/0036_alter_package_package_url.py rename to vulnerabilities/migrations/0036_alter_package_package_url_and_more.py index a02752dc9..8374a0137 100644 --- a/vulnerabilities/migrations/0036_alter_package_package_url.py +++ b/vulnerabilities/migrations/0036_alter_package_package_url_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 4.0.7 on 2022-11-25 17:34 +# Generated by Django 4.0.7 on 2022-11-28 13:00 from django.db import migrations, models @@ -13,6 +13,11 @@ class Migration(migrations.Migration): migrations.AlterField( model_name='package', name='package_url', - field=models.CharField(db_index=True, help_text='The Package URL for this package.', max_length=255), + field=models.CharField(db_index=True, help_text='The Package URL for this package.', max_length=1000), + ), + migrations.AlterField( + model_name='package', + name='plain_package_url', + field=models.CharField(db_index=True, help_text='The Package URL for this package without qualifiers and subpath.', max_length=1000), ), ] diff --git a/vulnerabilities/models.py b/vulnerabilities/models.py index e06b89cf5..ac59dcbbc 100644 --- a/vulnerabilities/models.py +++ b/vulnerabilities/models.py @@ -532,12 +532,19 @@ class Package(PackageURLMixin): ) package_url = models.CharField( - max_length=255, + max_length=1000, null=False, help_text="The Package URL for this package.", db_index=True, ) + plain_package_url = models.CharField( + max_length=1000, + null=False, + help_text="The Package URL for this package without qualifiers and subpath.", + db_index=True, + ) + objects = PackageQuerySet.as_manager() def save(self, *args, **kwargs): @@ -549,7 +556,14 @@ def save(self, *args, **kwargs): qualifiers=self.qualifiers, subpath=self.subpath, ) + plain_purl = PackageURL( + type=self.type, + namespace=self.namespace, + name=self.name, + version=self.version, + ) self.package_url = str(purl_object) + self.plain_package_url = str(plain_purl) super().save(*args, **kwargs) @property diff --git a/vulnerabilities/tests/test_fix_api.py b/vulnerabilities/tests/test_fix_api.py index 294ad3f49..0497fe160 100644 --- a/vulnerabilities/tests/test_fix_api.py +++ b/vulnerabilities/tests/test_fix_api.py @@ -358,6 +358,19 @@ def setUp(self): attrs = {k: v for k, v in purl.to_dict().items() if v} Package.objects.create(**attrs) + vulnerable_packages = [ + "pkg:nginx/nginx@1.0.15?foo=bar", + "pkg:nginx/nginx@1.0.15?foo=baz", + ] + + vuln = Vulnerability.objects.create(summary="test") + + for package in vulnerable_packages: + purl = PackageURL.from_string(package) + attrs = {k: v for k, v in purl.to_dict().items() if v} + pkg = Package.objects.create(**attrs) + PackageRelatedVulnerability.objects.create(package=pkg, vulnerability=vuln) + def test_bulk_api_response(self): request_body = { "purls": self.packages, @@ -370,9 +383,7 @@ def test_bulk_api_response(self): assert len(response) == 13 def test_bulk_api_response_with_ignoring_qualifiers(self): - request_body = { - "purls": ["pkg:nginx/nginx@1.0.15?qualifiers=dev"], - } + request_body = {"purls": ["pkg:nginx/nginx@1.0.15?qualifiers=dev"], "plain_purl": True} response = self.csrf_client.post( "/api/packages/bulk_search", data=json.dumps(request_body), @@ -382,8 +393,20 @@ def test_bulk_api_response_with_ignoring_qualifiers(self): assert response[0]["purl"] == "pkg:nginx/nginx@1.0.15" def test_bulk_api_response_with_ignoring_subpath(self): + request_body = {"purls": ["pkg:nginx/nginx@1.0.15#dev/subpath"], "plain_purl": True} + response = self.csrf_client.post( + "/api/packages/bulk_search", + data=json.dumps(request_body), + content_type="application/json", + ).json() + assert len(response) == 1 + assert response[0]["purl"] == "pkg:nginx/nginx@1.0.15" + + def test_bulk_api_with_purl_only_option(self): request_body = { "purls": ["pkg:nginx/nginx@1.0.15#dev/subpath"], + "purl_only": True, + "plain_purl": True, } response = self.csrf_client.post( "/api/packages/bulk_search", @@ -391,7 +414,7 @@ def test_bulk_api_response_with_ignoring_subpath(self): content_type="application/json", ).json() assert len(response) == 1 - assert response[0]["purl"] == "pkg:nginx/nginx@1.0.15" + assert response[0] == "pkg:nginx/nginx@1.0.15" class BulkSearchAPICPE(TestCase): From 44939b0a986fb195a6bc860c594bf3d3855a46d0 Mon Sep 17 00:00:00 2001 From: Tushar Goel Date: Mon, 28 Nov 2022 19:28:47 +0530 Subject: [PATCH 4/4] Add CHANGELOG Signed-off-by: Tushar Goel --- CHANGELOG.rst | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 397e104cb..79ef622ad 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -8,7 +8,13 @@ Version v30.3.2 - We re-enabled support for the PostgreSQL securities advisories importer. - We fixed the API key request form UI and made it consistent with rest of UI. - +- We made bulk search faster by pre-computing + `package_url` and `plain_package_url` in Package model. + And provided two options in package bulk search + ``purl_only`` option to get only vulnerable + purls without any extra details, ``plain_purl`` option + to filter purls without qualifiers and + subpath and also return them without qualifiers and subpath. Version v30.3.1 ----------------