-
Notifications
You must be signed in to change notification settings - Fork 64
Add: Support for the openvasd HTTP API #1215
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.
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
base: main
Are you sure you want to change the base?
Conversation
Dependency ReviewThe following issues were found:
Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. License Issuespoetry.lock
Allowed Licenses: 0BSD, AGPL-3.0-or-later, Apache-2.0, BlueOak-1.0.0, BSD-2-Clause, BSD-3-Clause-Clear, BSD-3-Clause, BSL-1.0, CAL-1.0, CC-BY-3.0, CC-BY-4.0, CC-BY-SA-4.0, CC0-1.0, EPL-2.0, GPL-1.0-or-later, GPL-2.0-only, GPL-2.0-or-later, GPL-2.0, GPL-3.0-only, GPL-3.0-or-later, GPL-3.0, ISC, LGPL-2.0-only, LGPL-2.0-or-later, LGPL-2.1-only, LGPL-2.1-or-later, LGPL-2.1, LGPL-3.0-only, LGPL-3.0, LGPL-3.0-or-later, MIT, MIT-CMU, MPL-1.1, MPL-2.0, OFL-1.1, PSF-2.0, Python-2.0, Python-2.0.1, Unicode-DFS-2016, Unlicense, Zlib, ZPL-2.1 OpenSSF Scorecard
Scanned Files
|
Conventional Commits Report
🚀 Conventional commits found. |
e09dc48
to
2a75e5a
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1215 +/- ##
==========================================
+ Coverage 97.74% 97.78% +0.03%
==========================================
Files 71 76 +5
Lines 4967 5139 +172
Branches 895 915 +20
==========================================
+ Hits 4855 5025 +170
Misses 76 76
- Partials 36 38 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
A new subpackage for sending requests to HTTP APIs has been added which also includes the first version of the openvasd API.
Sphinx documentation pages and some additional docstrings have been added. The "api" module in gvm.http.core is renamed to "_api" and url_join is now a class method of HttpApiConnector.
9cda0bb
to
ec59911
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Timo, could you replace requests with httpx please? requests doesn't support asyncio and httpx has sync and asyncio APIs very similar to requests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And I think it should be gvm/protocols/http as osp and gmp can be found in gvm.protocols currently.
4eef46b
to
ae6a432
Compare
ae6a432
to
6dbe31d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finally I took some time to review the PR. I am not sure if we should provide an abstraction over http like done in gvm.protocols.core for GMP. The gvm.protocols.core module is modeled like http libraries work internally and to provide a similar low level API.
IMHO the code in gvm.protocols.http.core is not really necessary. We could use the httpx classes (for example the HttpClient) directly. An Http abstraction should not be necessary. I am also not sure if we should provide access to http internals at all. Do we need such a low level access or should we abstract the scanner-api on a higher level? If we abstract the scanner-api I would go for returning json based dicts from the OpenvasdHttpApiV1 methods or even more advanced some models bases on dataclasses. An inspiration could be our GitHub API or the python docker libarary.
Maybe we even should split the API into several sub classes similar to https://github.com/greenbone/pontos/blob/main/pontos/github/api/api.py so we get something like openvasdapi.health.get_health_alive()
openvasdapi.notus.get_notus_os_list()
. This is something I also wanted for GMP for a long time.
What
A new subpackage for sending requests to HTTP APIs has been added which
also includes the first version of the openvasd API.
Why
References
GEA-939
Checklist