-
Notifications
You must be signed in to change notification settings - Fork 134
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
pylxd/client: guard against cert=None #618
Conversation
simondeziel
commented
Dec 11, 2024
@hamistao while touching that part of the code, I noticed that |
I noticed the same thing, but I don't think we can do anything about it besides maybe putting a comment to explain what the funciton does. |
pylxd/client.py
Outdated
try: | ||
cert = open(self.api.session.cert[0]).read().encode("utf-8") | ||
except FileNotFoundError: | ||
raise exceptions.ClientConnectionFailed("Client certificate not found") |
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.
If the purpose is just to check if the file exists, just open(self.api.session.cert[0])
should suffice.
Also, I think we should close the file in case it exists.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #618 +/- ##
==========================================
- Coverage 96.24% 95.38% -0.86%
==========================================
Files 32 32
Lines 3165 3229 +64
==========================================
+ Hits 3046 3080 +34
- Misses 119 149 +30 ☔ View full report in Codecov by Sentry. |
``` >>> import pylxd >>> c = pylxd.Client(endpoint="https://127.0.0.1:8443", cert=None, verify=False) /opt/pylxd/.tox/integration/lib/python3.12/site-packages/urllib3/connectionpool.py:1099: InsecureRequestWarning: Unverified HTTPS request is being made to host '127.0.0.1'. Adding certificate verification is strongly advised. See: https://urllib3.readthedocs.io/en/latest/advanced-usage.html#tls-warnings warnings.warn( >>> c.authenticate("password") Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/opt/pylxd/.tox/integration/lib/python3.12/site-packages/pylxd/client.py", line 573, in authenticate cert = open(self.api.session.cert[0]).read().encode("utf-8") ~~~~~~~~~~~~~~~~~~~~~^^^ TypeError: 'NoneType' object is not subscriptable ``` Signed-off-by: Simon Deziel <[email protected]>
Signed-off-by: Simon Deziel <[email protected]>
Signed-off-by: Simon Deziel <[email protected]>
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.
Sorry for the sudden review, but this looks good to me!