Allow passing SSL options via server URL (#73)

llucax · web-flow · commit 76f2bdd9464e · 2024-08-20T14:29:04.000Z
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
@@ -12,11 +12,15 @@
 
 - The `parse_grpc_uri` function (and `BaseApiClient` constructor) now enables SSL by default (`ssl=false` should be passed to disable it).
 
-- The `parse_grpc_uri` function now accepts an optional `default_ssl` parameter to set the default value for the `ssl` parameter when not present in the URI.
+- The `parse_grpc_uri` and `BaseApiClient` function now accepts a set of defaults to use when the URI does not specify a value for a given option.
 
 ## New Features
 
-<!-- Here goes the main new features and examples or instructions on how to use them -->
+- The connection URI can now have a few new SSL options:
+
+  * `ssl_root_certificates_path` to specify the path to the root certificates file.
+  * `ssl_private_key_path` to specify the path to the private key file.
+  * `ssl_certificate_chain_path` to specify the path to the certificate chain file.
 
 ## Bug Fixes
 
diff --git a/pyproject.toml b/pyproject.toml
@@ -56,7 +56,7 @@ dev-mkdocs = [
   "mkdocs-macros-plugin == 1.0.5",
   "mkdocs-material == 9.5.31",
   "mkdocstrings[python] == 0.25.2",
-  "mkdocstrings-python == 1.9.2",
+  "mkdocstrings-python == 1.10.8",
   "frequenz-repo-config[lib] == 0.10.0",
   "frequenz-client-base",
 ]
diff --git a/src/frequenz/client/base/channel.py b/src/frequenz/client/base/channel.py
@@ -3,50 +3,90 @@
 
 """Handling of gRPC channels."""
 
+import dataclasses
+import pathlib
+from typing import assert_never
 from urllib.parse import parse_qs, urlparse
 
 from grpc import ssl_channel_credentials
 from grpc.aio import Channel, insecure_channel, secure_channel
 
 
-def _to_bool(value: str) -> bool:
-    value = value.lower()
-    if value in ("true", "on", "1"):
-        return True
-    if value in ("false", "off", "0"):
-        return False
-    raise ValueError(f"Invalid boolean value '{value}'")
+@dataclasses.dataclass(frozen=True)
+class SslOptions:
+    """SSL options for a gRPC channel."""
+
+    enabled: bool = True
+    """Whether SSL should be enabled."""
+
+    root_certificates: pathlib.Path | bytes | None = None
+    """The PEM-encoded root certificates.
+
+    This can be a path to a file containing the certificates, a byte string, or None to
+    retrieve them from a default location chosen by gRPC runtime.
+    """
+
+    private_key: pathlib.Path | bytes | None = None
+    """The PEM-encoded private key.
+
+    This can be a path to a file containing the key, a byte string, or None if no key
+    should be used.
+    """
+
+    certificate_chain: pathlib.Path | bytes | None = None
+    """The PEM-encoded certificate chain.
+
+    This can be a path to a file containing the chain, a byte string, or None if no
+    chain should be used.
+    """
+
+
+@dataclasses.dataclass(frozen=True)
+class ChannelOptions:
+    """Options for a gRPC channel."""
+
+    port: int = 9090
+    """The port number to connect to."""
+
+    ssl: SslOptions = SslOptions()
+    """SSL options for the channel."""
 
 
 def parse_grpc_uri(
     uri: str,
     /,
-    *,
-    default_port: int = 9090,
-    default_ssl: bool = True,
+    defaults: ChannelOptions = ChannelOptions(),
 ) -> Channel:
     """Create a client channel from a URI.
 
     The URI must have the following format:
 
     ```
-    grpc://hostname[:port][?ssl=<bool>]
+    grpc://hostname[:port][?param=value&...]
     ```
 
     A few things to consider about URI components:
 
     - If any other components are present in the URI, a [`ValueError`][] is raised.
     - If the port is omitted, the `default_port` is used.
     - If a query parameter is passed many times, the last value is used.
-    - The only supported query parameter is `ssl`, which must be a boolean value and
-      defaults to the `default_ssl` argument if not present.
     - Boolean query parameters can be specified with the following values
       (case-insensitive): `true`, `1`, `on`, `false`, `0`, `off`.
 
+    Supported query parameters:
+
+    - `ssl` (bool): Enable or disable SSL. Defaults to `default_ssl`.
+    - `ssl_root_certificates_path` (str): Path to the root certificates file. Only
+      valid if SSL is enabled. Will raise a `ValueError` if the file cannot be read.
+    - `ssl_private_key_path` (str): Path to the private key file. Only valid if SSL is
+      enabled. Will raise a `ValueError` if the file cannot be read.
+    - `ssl_certificate_chain_path` (str): Path to the certificate chain file. Only
+      valid if SSL is enabled. Will raise a `ValueError` if the file cannot be read.
+
     Args:
         uri: The gRPC URI specifying the connection parameters.
-        default_port: The default port number to use if the URI does not specify one.
-        default_ssl: The default SSL setting to use if the URI does not specify one.
+        defaults: The default options use to create the channel when not specified in
+            the URI.
 
     Returns:
         A client channel object.
@@ -68,18 +108,143 @@ def parse_grpc_uri(
                 uri,
             )
 
-    options = {k: v[-1] for k, v in parse_qs(parsed_uri.query).items()}
+    options = _parse_query_params(uri, parsed_uri.query)
+
+    host = parsed_uri.hostname
+    port = parsed_uri.port or defaults.port
+    target = f"{host}:{port}"
+
+    ssl = defaults.ssl.enabled if options.ssl is None else options.ssl
+    if ssl:
+        return secure_channel(
+            target,
+            ssl_channel_credentials(
+                root_certificates=_get_contents(
+                    "root certificates",
+                    options.ssl_root_certificates_path,
+                    defaults.ssl.root_certificates,
+                ),
+                private_key=_get_contents(
+                    "private key",
+                    options.ssl_private_key_path,
+                    defaults.ssl.private_key,
+                ),
+                certificate_chain=_get_contents(
+                    "certificate chain",
+                    options.ssl_certificate_chain_path,
+                    defaults.ssl.certificate_chain,
+                ),
+            ),
+        )
+    return insecure_channel(target)
+
+
+def _to_bool(value: str) -> bool:
+    value = value.lower()
+    if value in ("true", "on", "1"):
+        return True
+    if value in ("false", "off", "0"):
+        return False
+    raise ValueError(f"Invalid boolean value '{value}'")
+
+
+@dataclasses.dataclass(frozen=True)
+class _QueryParams:
+    ssl: bool | None
+    ssl_root_certificates_path: pathlib.Path | None
+    ssl_private_key_path: pathlib.Path | None
+    ssl_certificate_chain_path: pathlib.Path | None
+
+
+def _parse_query_params(uri: str, query_string: str) -> _QueryParams:
+    """Parse query parameters from a URI.
+
+    Args:
+        uri: The URI from which the query parameters were extracted.
+        query_string: The query string to parse.
+
+    Returns:
+        A `_QueryParams` object with the parsed query parameters.
+
+    Raises:
+        ValueError: If the query string contains unexpected components.
+    """
+    options = {k: v[-1] for k, v in parse_qs(query_string).items()}
     ssl_option = options.pop("ssl", None)
-    ssl = _to_bool(ssl_option) if ssl_option is not None else default_ssl
+    ssl: bool | None = None
+    if ssl_option is not None:
+        ssl = _to_bool(ssl_option)
+
+    ssl_opts = {
+        k: options.pop(k, None)
+        for k in (
+            "ssl_root_certificates_path",
+            "ssl_private_key_path",
+            "ssl_certificate_chain_path",
+        )
+    }
+
+    if ssl is False:
+        erros = []
+        for opt_name, opt in ssl_opts.items():
+            if opt is not None:
+                erros.append(opt_name)
+        if erros:
+            raise ValueError(
+                f"Option(s) {', '.join(erros)} found in URI {uri!r}, but SSL is disabled",
+            )
+
     if options:
+        names = ", ".join(options)
         raise ValueError(
-            f"Unexpected query parameters {options!r} in the URI '{uri}'",
+            f"Unexpected query parameters [{names}] in the URI '{uri}'",
             uri,
         )
 
-    host = parsed_uri.hostname
-    port = parsed_uri.port or default_port
-    target = f"{host}:{port}"
-    if ssl:
-        return secure_channel(target, ssl_channel_credentials())
-    return insecure_channel(target)
+    return _QueryParams(
+        ssl=ssl,
+        **{k: pathlib.Path(v) if v is not None else None for k, v in ssl_opts.items()},
+    )
+
+
+def _get_contents(
+    name: str, source: pathlib.Path | None, default: pathlib.Path | bytes | None
+) -> bytes | None:
+    """Get the contents of a file or use a default value.
+
+    If the `source` is `None`, the `default` value is used instead. If the source (or
+    default) is a path, the contents of the file are returned. If the source is a byte
+    string (or default) the byte string is returned without doing any reading.
+
+    Args:
+        name: The name of the contents (used for error messages).
+        source: The source of the contents.
+        default: The default value to use if the source is None.
+
+    Returns:
+        The contents of the source file or the default value.
+    """
+    file_path: pathlib.Path
+    match source:
+        case None:
+            match default:
+                case None:
+                    return None
+                case bytes() as default_bytes:
+                    return default_bytes
+                case pathlib.Path() as file_path:
+                    return _read_bytes(name, file_path)
+                case unexpected:
+                    assert_never(unexpected)
+        case pathlib.Path() as file_path:
+            return _read_bytes(name, file_path)
+        case unexpected:
+            assert_never(unexpected)
+
+
+def _read_bytes(name: str, source: pathlib.Path) -> bytes:
+    """Read the contents of a file as bytes."""
+    try:
+        return source.read_bytes()
+    except OSError as exc:
+        raise ValueError(f"Failed to read {name} from '{source}': {exc}") from exc
diff --git a/src/frequenz/client/base/client.py b/src/frequenz/client/base/client.py
@@ -10,7 +10,7 @@
 
 from grpc.aio import AioRpcError, Channel
 
-from .channel import parse_grpc_uri
+from .channel import ChannelOptions, parse_grpc_uri
 from .exception import ApiClientError, ClientNotConnected
 
 StubT = TypeVar("StubT")
@@ -103,7 +103,7 @@ async def main():
                     break
         ```
 
-        !!! Note
+        Note:
             * In this case a very simple `GrpcStreamBroadcaster` is used, asuming that
                 each call to `example_stream` will stream the same data. If the request
                 is more complex, you will probably need to have some kind of map from
@@ -117,6 +117,7 @@ def __init__(
         create_stub: Callable[[Channel], StubT],
         *,
         connect: bool = True,
+        channel_defaults: ChannelOptions = ChannelOptions(),
     ) -> None:
         """Create an instance and connect to the server.
 
@@ -127,9 +128,12 @@ def __init__(
                 created. If `False`, the client will not connect to the server until
                 [connect()][frequenz.client.base.client.BaseApiClient.connect] is
                 called.
+            channel_defaults: The default options for the gRPC channel to create using
+                the server URL.
         """
         self._server_url: str = server_url
         self._create_stub: Callable[[Channel], StubT] = create_stub
+        self._channel_defaults: ChannelOptions = channel_defaults
         self._channel: Channel | None = None
         self._stub: StubT | None = None
         if connect:
@@ -156,6 +160,11 @@ def channel(self) -> Channel:
             raise ClientNotConnected(server_url=self.server_url, operation="channel")
         return self._channel
 
+    @property
+    def channel_defaults(self) -> ChannelOptions:
+        """The default options for the gRPC channel."""
+        return self._channel_defaults
+
     @property
     def stub(self) -> StubT:
         """The underlying gRPC stub.
@@ -192,7 +201,7 @@ def connect(self, server_url: str | None = None) -> None:
             self._server_url = server_url
         elif self.is_connected:
             return
-        self._channel = parse_grpc_uri(self._server_url)
+        self._channel = parse_grpc_uri(self._server_url, self._channel_defaults)
         self._stub = self._create_stub(self._channel)
 
     async def disconnect(self) -> None:
diff --git a/tests/test_channel.py b/tests/test_channel.py
diff --git a/tests/test_client.py b/tests/test_client.py

Original file line number	Diff line number	Diff line change
`@@ -56,7 +56,7 @@ dev-mkdocs = [`
`56`	`56`	`"mkdocs-macros-plugin == 1.0.5",`
`57`	`57`	`"mkdocs-material == 9.5.31",`
`58`	`58`	`"mkdocstrings[python] == 0.25.2",`
`59`		`- "mkdocstrings-python == 1.9.2",`
	`59`	`+ "mkdocstrings-python == 1.10.8",`
`60`	`60`	`"frequenz-repo-config[lib] == 0.10.0",`
`61`	`61`	`"frequenz-client-base",`
`62`	`62`	`]`