Add nginx tests and other related improvements #691

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merged

pombredanne merged 40 commits into main from 643-nginx-tests

Apr 15, 2022

Member

pombredanne commented Apr 11, 2022

This PR adds nginx importer and improver tests

Along the way there are several other adjustments and refinements that have been applied

#643
Signed-off-by: Philippe Ombredanne pombredanne@nexb.com

pombredanne added 26 commits

April 10, 2022 09:38


          Remove duplicated split_markdown_front_matter

77b5600

Also move doctest to tests/

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Do not use .replace() method on a purl

039babf

The fact this is a namedtuple is an implementation detail that should
not be relied upon.

* Add new evolve_purl() helper function to help with this

Reference: package-url/packageurl-python#82
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Add constant aliases for each scoring system

cfcd01e

This makes these easier to reference and reuse in tests

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Add commoncode as a dependency for testing

49278ca

There are useful utilities to handle test data.

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Add tests for nginx importer

177c4c9

* Refactor code for testability
* Move complex doctest to plain test code
* Ensure that there is Reference for a CVE
* Use object (NginxAdvisory namedtuple) rather than plain tuple of
  values when passing results around

Reference: #643
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Load expected test results with JSON

018ae29

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Improve doctrsings and error messages

27af54a

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Do not use .replace() method on a purl

52f4c22

The fact this is a namedtuple is an implementation detail that should
not be relied upon.

* Add new evolve_purl() helper function to help with this

Reference: package-url/packageurl-python#82
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Sort ignored test alphabetically

fbeeecc

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Make tests fail for invalid template variables

b877816

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Remove unused import

8bdc72e

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Remove unused import

8bf4cb0

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Move model save() method to conventional location

bec79b0

By convention we put fields first, meta and then methods.

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Add NginxImporter importer test

e662a7b

* Make it easier to test importer with a fetch() method.
  This way a test mock object can subclass NginxImporter.fetch
  to avoid making network calls.

* Add database test using an isolated transaction that tests a dump
  of the created advisories against an expected JSON

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Complete "package_managers" migration

ad36f06

As part of testing nginx, it was easier to complete the migration of
all existing package version fetchers.

* Rename package_managers_2.py to package_managers.py
* Complete migration and add tests for all classes using mocks as needed
* Move test files in a common directory. Remove unused ones.
* Change semantics to yield PackageVreiosn and never cache by default.
  In practice for long running operations, caching is a problem and not
  a solution as it may hold to stale, our of date data.
* Create new helper to quety GitHub graphql API

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Rename archlinux scoring system

493c923

avgs is not super meaningful, archlinux is better

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Remove unused imports

2402b45

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Improve docstrings

5e96d5c

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Improve logging

f2de3d1

* A logging statement was not formatted correctly.
* Also prefix with _ an unused variable

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Adjust code with new imports

6174e68

As part of the importers migrations, adjust to use new class names
for not-yet migrated importers.

This is mostly harmless as the code has not been migrated yet, but at
least most code now uses the proper new class names where possible.

Reference: #597
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Remove unused variable

cd1318d

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Remove unused test file

2cfce61

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Improve Github importer

d7f34dd

* Use new helper function for GH graphql API calls
* Use lower case logger
* Use severity_systems constants rather than a mapping lookup

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Use severity_systems constants, not a lookup

2731a08

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Add test and refine nginx code

985ad19

* Do not use asyncio. Use the new simpler GitHubTagsAPI
* Extract fetching tags in a fetch_nginx_version_from_git_tags() method
* Add and improve importer and improver tests

Reference: #643
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Test nginx improver get_inferences() method

da7faf7

* Add AdvisoryData.to_dict() methods as convenience for testing
* Split NginxBasicImprover.get_inferences() in tow functions to ease
  testing

Reference: #643
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

pombredanne requested review from Hritik14, TG1999 and sbs2001

April 11, 2022 14:13


          Add missing format string prefix

d74425f

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

pombredanne added 8 commits

April 12, 2022 08:58


          Add missing licenses, remove unused imports

a7545e7

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Merge latest main branch

521c2d2

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Enable OpenSSL tests

179854e

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Remove unused imports

f889546

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Use new file-based testing and env-based regen

39b5fab

To regen expected files, set VULNERABLECODE_REGEN_TEST_FIXTURES to any
value

Reference: https://github.com/nexB/vulnerablecode/blob/a7545e7e6fa94d3a3f6382bfab42f90a72bce7d7/vulnerabilities/tests/util_tests.py#L36
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Generate scoring systems mapping from a list

a89aba2

Otherwise having a mapping with hardcode keys can drift in the future.

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Add missing license for nginx importer

eb69138

Use as a license the same as the overall license for the project, as
this is the only statement that exists there.

Reference: #614
Reported-by: Hritik Vijay <hritikxx8@gmail.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Fix docstring typo

5a457fc

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Member Author

pombredanne commented Apr 12, 2022 •

edited

Loading

We reviewed these changes extensively in weekly call today. I would like to merge this ASAP unless there is an objection

TG1999 reviewed

View reviewed changes

vulnerabilities/helpers.py Outdated Show resolved Hide resolved

Member Author

pombredanne commented Apr 12, 2022

Actually the nginx improver is a problem:

Import:31

vulnerabilities.improvers.default.DefaultImprover several times"
In [13]: f"{Vulnerability.objects.count()}, {Package.objects.count()}"
Out[13]: '31, 78'

vulnerabilities.importers.nginx.NginxBasicImprover once:
In [30]: f"{Vulnerability.objects.count()}, {Package.objects.count()}"
Out[30]: '31, 662'

pombredanne added 5 commits

April 12, 2022 15:49


          Remove noisy logging statement

38d0d5f

Reported-by: Tushar Goel <tushar.goel.dav@gmail.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Merge branch 'main' into 643-nginx-tests

024a149


          Rename archlinux scoring system: model update

ad9863e

avgs is not super meaningful, archlinux is better

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Do not attempt to fornat migrations

792c7d3

isort should not sort migrations

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>


          Adjust text example

d553201

* Use file-based test expectation for sanity. Otherwise updating test
  expectations to match code changes becomes a painful chore.

* TODO: the tests and data still need improvements for clarity and
  documentation

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

pombredanne force-pushed the 643-nginx-tests branch from 069909e to d553201 Compare

April 15, 2022 12:39

Member Author

pombredanne commented Apr 15, 2022

All green now .. merging !

TG1999 approved these changes

View reviewed changes

Contributor

TG1999 left a comment

LGTM!

pombredanne merged commit eed065c into main

pombredanne deleted the 643-nginx-tests branch

April 15, 2022 12:57

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet