From 5b0a223fc666963decdd7f3cbd7bbf6057ae4bce Mon Sep 17 00:00:00 2001 From: Ramakrishna Prabhu Date: Tue, 1 Sep 2026 16:12:07 -0500 Subject: [PATCH] RoutingClient: match Client's (host, port, tls=...) signature RoutingClient(target="host:port") diverged from the LP/MIP Client's (host, port, tls=...) even though it was added over 5 weeks after Client already had TLS support (#1525 vs #1597), and reuses the exact same grpc_python_client_t shim that already carried the TLS-options constructor overload -- it just never called it. Neither the constructor signature nor the missing TLS support came up anywhere in #1597's review, unlike every other follow-up from that review (which all got filed as tracked issues). Looks like an oversight from a POC-scoped first cut, not a deliberate design choice. RoutingClient(host, port, *, tls=None) now mirrors Client exactly, reusing the same _connect_options_from_tls() helper and TLS-aware grpc_python_client_t constructor overload Client already used. Fixes #1839. Co-Authored-By: Claude Sonnet 5 --- .../cuopt/cuopt/grpc/client/grpc_client.pyx | 23 +++-- python/cuopt/cuopt/grpc/routing/__init__.py | 2 +- .../tests/routing/test_routing_grpc_client.py | 9 +- .../routing/test_routing_grpc_client_tls.py | 84 +++++++++++++++++++ 4 files changed, 110 insertions(+), 8 deletions(-) create mode 100644 python/cuopt/cuopt/tests/routing/test_routing_grpc_client_tls.py diff --git a/python/cuopt/cuopt/grpc/client/grpc_client.pyx b/python/cuopt/cuopt/grpc/client/grpc_client.pyx index bdfd35963e..6e1909fc2f 100644 --- a/python/cuopt/cuopt/grpc/client/grpc_client.pyx +++ b/python/cuopt/cuopt/grpc/client/grpc_client.pyx @@ -970,13 +970,26 @@ cdef class RoutingClient: cdef unique_ptr[grpc_python_client_t] _client - def __cinit__(self, str target="localhost:50051"): - host, _, port = target.rpartition(":") - if not host: - host, port = target, "50051" + def __cinit__(self, str host, int port, *, tls=None): + """ + Connect to ``cuopt_grpc_server`` at ``host:port``. + + ``tls`` controls transport security, same as :class:`Client`: + + * ``None`` (default) — read ``CUOPT_TLS_*`` from the environment. + * ``False`` — plain TCP; ignore ``CUOPT_TLS_*``. + * :class:`TlsConfig` — explicit TLS/mTLS; omit ``root_certs`` to use the + system/default CA trust store. + """ + if tls is not None and tls is not False and not isinstance(tls, TlsConfig): + raise TypeError("tls must be None, False, or TlsConfig") + + cdef grpc_python_client_connect_options_t options cdef string host_cpp = host.encode("utf-8") cdef string err - self._client.reset(new grpc_python_client_t(host_cpp, int(port))) + + options = _connect_options_from_tls(tls) + self._client.reset(new grpc_python_client_t(host_cpp, port, options)) if not self._client.get().connect(err): raise RoutingSolveError( "failed to connect: " + err.decode("utf-8") diff --git a/python/cuopt/cuopt/grpc/routing/__init__.py b/python/cuopt/cuopt/grpc/routing/__init__.py index 9be5a4e525..a323b8664a 100644 --- a/python/cuopt/cuopt/grpc/routing/__init__.py +++ b/python/cuopt/cuopt/grpc/routing/__init__.py @@ -12,7 +12,7 @@ dm = routing.DataModel(n_locations, n_fleet) dm.add_cost_matrix(cost) ... - client = RoutingClient("gpu-host:50051") + client = RoutingClient("gpu-host", 50051) solution = client.solve(dm) """ diff --git a/python/cuopt/cuopt/tests/routing/test_routing_grpc_client.py b/python/cuopt/cuopt/tests/routing/test_routing_grpc_client.py index 5de3fec0cc..c57def0975 100644 --- a/python/cuopt/cuopt/tests/routing/test_routing_grpc_client.py +++ b/python/cuopt/cuopt/tests/routing/test_routing_grpc_client.py @@ -25,6 +25,11 @@ ) +def _client(): + host, _, port = (_SERVER or "").rpartition(":") + return grpc_routing.RoutingClient(host, int(port)) + + def _small_vrp(): dm = routing.DataModel(5, 2) cost = np.array( @@ -46,7 +51,7 @@ def test_remote_solve_matches_local(): settings.set_time_limit(2) local = routing.Solve(_small_vrp(), settings) - client = grpc_routing.RoutingClient(_SERVER) + client = _client() remote = client.solve(_small_vrp(), {"time_limit": 2.0}) assert remote["status"] == 0, remote["status_message"] @@ -57,7 +62,7 @@ def test_remote_solve_matches_local(): def test_submit_wait_result_lifecycle(): - client = grpc_routing.RoutingClient(_SERVER) + client = _client() job_id = client.submit(_small_vrp(), {"time_limit": 1.0}) assert job_id client.wait(job_id, timeout=30) diff --git a/python/cuopt/cuopt/tests/routing/test_routing_grpc_client_tls.py b/python/cuopt/cuopt/tests/routing/test_routing_grpc_client_tls.py new file mode 100644 index 0000000000..696ab8a3be --- /dev/null +++ b/python/cuopt/cuopt/tests/routing/test_routing_grpc_client_tls.py @@ -0,0 +1,84 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""TLS/mTLS coverage for the compiled VRP gRPC client (cuopt.grpc.routing). + +Unlike test_routing_grpc_client.py, these tests do not need +CUOPT_GRPC_SERVER -- the tls_server_info/mtls_server_info fixtures start +their own cuopt_grpc_server subprocess (skipped if the test TLS certs are +not found). +""" + +import os + +import numpy as np +import pytest + +from cuopt import routing +from cuopt.grpc.linear_programming import TlsConfig + +grpc_routing = pytest.importorskip("cuopt.grpc.routing") + + +def _small_vrp(): + dm = routing.DataModel(5, 2) + cost = np.array( + [ + [0, 1, 2, 2, 1], + [1, 0, 1, 2, 2], + [2, 1, 0, 1, 2], + [2, 2, 1, 0, 1], + [1, 2, 2, 1, 0], + ], + dtype=np.float32, + ) + dm.add_cost_matrix(cost) + return dm + + +def test_rejects_invalid_tls_argument(): + with pytest.raises(TypeError): + grpc_routing.RoutingClient("localhost", 1, tls="bogus") + + +@pytest.mark.xdist_group(name="grpc_server") +@pytest.mark.filterwarnings("ignore::DeprecationWarning") +class TestRoutingClientTls: + def test_submit_with_explicit_tls_config(self, tls_server_info): + cert_dir = tls_server_info["cert_dir"] + client = grpc_routing.RoutingClient( + "localhost", + tls_server_info["port"], + tls=TlsConfig(os.path.join(cert_dir, "ca.crt")), + ) + solution = client.solve(_small_vrp(), {"time_limit": 2.0}) + assert solution["status"] == 0, solution["status_message"] + + def test_tls_server_rejects_plain_client(self, tls_server_info): + with pytest.raises(grpc_routing.RoutingSolveError): + grpc_routing.RoutingClient( + "localhost", tls_server_info["port"], tls=False + ) + + def test_submit_with_explicit_mtls_config(self, mtls_server_info): + cert_dir = mtls_server_info["cert_dir"] + client = grpc_routing.RoutingClient( + "localhost", + mtls_server_info["port"], + tls=TlsConfig( + os.path.join(cert_dir, "ca.crt"), + client_cert=os.path.join(cert_dir, "client.crt"), + client_key=os.path.join(cert_dir, "client.key"), + ), + ) + solution = client.solve(_small_vrp(), {"time_limit": 2.0}) + assert solution["status"] == 0, solution["status_message"] + + def test_mtls_server_rejects_missing_client_cert(self, mtls_server_info): + cert_dir = mtls_server_info["cert_dir"] + with pytest.raises(grpc_routing.RoutingSolveError): + grpc_routing.RoutingClient( + "localhost", + mtls_server_info["port"], + tls=TlsConfig(os.path.join(cert_dir, "ca.crt")), + )