opendatahub-io / caikit-nlp-client

A grpc and http client library for caikit-nlp
Apache License 2.0
3 stars 6 forks source link

http client: cannot provide a CA certificate path #70

Closed dtrifiro closed 11 months ago

dtrifiro commented 11 months ago

Logic for validating TLS arguments in http client is currently broken:

host = ...
url = "https://{host}"
http = HttpClient(url, ca_cert_path="bundle.pem")

Results in


File caikit-nlp-client/src/caikit_nlp_client/http_client.py:67, in HttpClient.__init__(self, base_url, ca_cert_path, client_cert_path, client_key_path)
     56 self._ca_cert_path = ca_cert_path
     57 if (
     58     any(
     59         (
   (...)
     65     and not self._mtls_configured
     66 ):
---> 67     raise ValueError(
     68         "Must provide both ca_cert_path, client_cert_path, client_key_path "
     69         "for mTLS"
     70     )

ValueError: Must provide both ca_cert_path, client_cert_path, client_key_path for mTLS

this is a valid usecase, in which the user provides a custom ca bundle to handle self-signed certificates.

dtrifiro commented 11 months ago

@vaibhavjainwiz

diff --git a/src/caikit_nlp_client/http_client.py b/src/caikit_nlp_client/http_client.py
index bda36d6..b4db375 100644
--- a/src/caikit_nlp_client/http_client.py
+++ b/src/caikit_nlp_client/http_client.py
@@ -55,13 +55,7 @@ class HttpClient:
         self._client_key_path = client_key_path
         self._ca_cert_path = ca_cert_path
         if (
-            any(
-                (
-                    self._client_key_path,
-                    self._client_cert_path,
-                    self._ca_cert_path,
-                )
-            )
+            any((self._client_key_path, self._client_cert_path))
             and not self._mtls_configured
         ):
             raise ValueError(

plus the Exception message below