Skip to content

Commit 47738a4

Browse files
committed
Make the default SSL option configurable
Signed-off-by: Leandro Lucarella <[email protected]>
1 parent 111aacd commit 47738a4

File tree

3 files changed

+32
-8
lines changed

3 files changed

+32
-8
lines changed

RELEASE_NOTES.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
## Upgrading
88

99
- The `parse_grpc_uri` function (and `BaseApiClient` constructor) now enables SSL by default (`ssl=false` should be passed to disable it).
10+
- 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.
1011

1112
## New Features
1213

src/frequenz/client/base/channel.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,19 @@ def _to_bool(value: str) -> bool:
2323

2424

2525
def parse_grpc_uri(
26-
uri: str, channel_type: type[ChannelT], /, *, default_port: int = 9090
26+
uri: str,
27+
channel_type: type[ChannelT],
28+
/,
29+
*,
30+
default_port: int = 9090,
31+
default_ssl: bool = True,
2732
) -> ChannelT:
2833
"""Create a grpclib client channel from a URI.
2934
3035
The URI must have the following format:
3136
3237
```
33-
grpc://hostname[:port][?ssl=true]
38+
grpc://hostname[:port][?ssl=<bool>]
3439
```
3540
3641
A few things to consider about URI components:
@@ -39,14 +44,15 @@ def parse_grpc_uri(
3944
- If the port is omitted, the `default_port` is used.
4045
- If a query parameter is passed many times, the last value is used.
4146
- The only supported query parameter is `ssl`, which must be a boolean value and
42-
defaults to `true`.
47+
defaults to the `default_ssl` argument if not present.
4348
- Boolean query parameters can be specified with the following values
4449
(case-insensitive): `true`, `1`, `on`, `false`, `0`, `off`.
4550
4651
Args:
4752
uri: The gRPC URI specifying the connection parameters.
4853
channel_type: The type of channel to create.
4954
default_port: The default port number to use if the URI does not specify one.
55+
default_ssl: The default SSL setting to use if the URI does not specify one.
5056
5157
Returns:
5258
A grpclib client channel object.
@@ -69,7 +75,8 @@ def parse_grpc_uri(
6975
)
7076

7177
options = {k: v[-1] for k, v in parse_qs(parsed_uri.query).items()}
72-
ssl = _to_bool(options.pop("ssl", "true"))
78+
ssl_option = options.pop("ssl", None)
79+
ssl = _to_bool(ssl_option) if ssl_option is not None else default_ssl
7380
if options:
7481
raise ValueError(
7582
f"Unexpected query parameters {options!r} in the URI '{uri}'",

tests/test_channel.py

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,18 +32,23 @@
3232

3333
class _CreateChannelKwargs(TypedDict):
3434
default_port: NotRequired[int]
35+
default_ssl: NotRequired[bool]
3536

3637

3738
@pytest.mark.parametrize("uri, host, port, ssl", VALID_URLS)
3839
@pytest.mark.parametrize(
3940
"default_port", [None, 9090, 1234], ids=lambda x: f"default_port={x}"
4041
)
41-
def test_grpclib_parse_uri_ok(
42+
@pytest.mark.parametrize(
43+
"default_ssl", [None, True, False], ids=lambda x: f"default_ssl={x}"
44+
)
45+
def test_grpclib_parse_uri_ok( # pylint: disable=too-many-arguments
4246
uri: str,
4347
host: str,
4448
port: int,
4549
ssl: bool,
4650
default_port: int | None,
51+
default_ssl: bool | None,
4752
) -> None:
4853
"""Test successful parsing of gRPC URIs using grpclib."""
4954

@@ -56,28 +61,36 @@ class _FakeChannel:
5661
kwargs = _CreateChannelKwargs()
5762
if default_port is not None:
5863
kwargs["default_port"] = default_port
64+
if default_ssl is not None:
65+
kwargs["default_ssl"] = default_ssl
5966

6067
expected_port = port if f":{port}" in uri or default_port is None else default_port
68+
expected_ssl = ssl if "ssl" in uri or default_ssl is None else default_ssl
69+
6170
with mock.patch(
6271
"frequenz.client.base.channel._grpchacks.grpclib_create_channel",
6372
return_value=_FakeChannel(host, port, ssl),
6473
) as create_channel_mock:
6574
channel = parse_grpc_uri(uri, _grpchacks.GrpclibChannel, **kwargs)
6675

6776
assert isinstance(channel, _FakeChannel)
68-
create_channel_mock.assert_called_once_with(host, expected_port, ssl)
77+
create_channel_mock.assert_called_once_with(host, expected_port, expected_ssl)
6978

7079

7180
@pytest.mark.parametrize("uri, host, port, ssl", VALID_URLS)
7281
@pytest.mark.parametrize(
7382
"default_port", [None, 9090, 1234], ids=lambda x: f"default_port={x}"
7483
)
75-
def test_grpcio_parse_uri_ok(
84+
@pytest.mark.parametrize(
85+
"default_ssl", [None, True, False], ids=lambda x: f"default_ssl={x}"
86+
)
87+
def test_grpcio_parse_uri_ok( # pylint: disable=too-many-arguments,too-many-locals
7688
uri: str,
7789
host: str,
7890
port: int,
7991
ssl: bool,
8092
default_port: int | None,
93+
default_ssl: bool | None,
8194
) -> None:
8295
"""Test successful parsing of gRPC URIs using grpcio."""
8396
expected_channel = mock.MagicMock(
@@ -87,10 +100,13 @@ def test_grpcio_parse_uri_ok(
87100
name="mock_credentials", spec=_grpchacks.GrpcioChannel
88101
)
89102
expected_port = port if f":{port}" in uri or default_port is None else default_port
103+
expected_ssl = ssl if "ssl" in uri or default_ssl is None else default_ssl
90104

91105
kwargs = _CreateChannelKwargs()
92106
if default_port is not None:
93107
kwargs["default_port"] = default_port
108+
if default_ssl is not None:
109+
kwargs["default_ssl"] = default_ssl
94110

95111
with (
96112
mock.patch(
@@ -110,7 +126,7 @@ def test_grpcio_parse_uri_ok(
110126

111127
assert channel == expected_channel
112128
expected_target = f"{host}:{expected_port}"
113-
if ssl:
129+
if expected_ssl:
114130
ssl_channel_credentials_mock.assert_called_once_with()
115131
secure_channel_mock.assert_called_once_with(
116132
expected_target, expected_credentials

0 commit comments

Comments
 (0)