Skip to content

Commit f51df3d

Browse files
authored
To fix the SSL issue for TLS enablement b/w service and chatbot (#1756)
* fix wca ssl fix * fix codeQL suggestions and pre-commit failure * fix review * fix test * fix comment * fix UTs * fix uts Signed-off-by: Sumit Jaiswal <[email protected]> * fix uts * fix healthchek mocking ut * fix chatview UTs * pre-commit * fix utc * fix review * address ci issues * fix sonar cloud grading * fix review * fix ci and review --------- Signed-off-by: Sumit Jaiswal <[email protected]>
1 parent 4b3a778 commit f51df3d

20 files changed

+2888
-1245
lines changed

ansible_ai_connect/ai/api/model_pipelines/http/pipelines.py

Lines changed: 61 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
import copy
1616
import json
1717
import logging
18-
import ssl
1918
from json import JSONDecodeError
2019
from typing import Any, AsyncGenerator
2120

@@ -57,6 +56,7 @@
5756
HealthCheckSummary,
5857
HealthCheckSummaryException,
5958
)
59+
from ansible_ai_connect.main.ssl_manager import ssl_manager
6060

6161
logger = logging.getLogger(__name__)
6262

@@ -66,7 +66,9 @@ class HttpMetaData(MetaData[HttpConfiguration]):
6666

6767
def __init__(self, config: HttpConfiguration):
6868
super().__init__(config=config)
69-
self.session = requests.Session()
69+
# Use centralized SSL manager for all HTTP requests
70+
self.session = ssl_manager.get_requests_session()
71+
7072
self.headers = {"Content-Type": "application/json"}
7173
i = self.config.timeout
7274
self._timeout = int(i) if i is not None else None
@@ -97,9 +99,6 @@ def invoke(self, params: CompletionsParameters) -> CompletionsResponse:
9799
headers=self.headers,
98100
json=model_input,
99101
timeout=self.task_gen_timeout(task_count),
100-
verify=(
101-
self.config.ca_cert_file if self.config.ca_cert_file else self.config.verify_ssl
102-
),
103102
)
104103
result.raise_for_status()
105104
response = json.loads(result.text)
@@ -117,11 +116,8 @@ def self_test(self) -> HealthCheckSummary:
117116
}
118117
)
119118
try:
120-
res = requests.get(
119+
res = self.session.get(
121120
url,
122-
verify=(
123-
self.config.ca_cert_file if self.config.ca_cert_file else self.config.verify_ssl
124-
),
125121
timeout=1,
126122
)
127123
res.raise_for_status()
@@ -151,13 +147,10 @@ def self_test(self) -> HealthCheckSummary:
151147
)
152148
try:
153149
headers = {"Content-Type": "application/json"}
154-
r = requests.get(
150+
r = self.session.get(
155151
self.config.inference_url + "/readiness",
156152
headers=headers,
157153
timeout=1,
158-
verify=(
159-
self.config.ca_cert_file if self.config.ca_cert_file else self.config.verify_ssl
160-
),
161154
)
162155
r.raise_for_status()
163156

@@ -178,6 +171,20 @@ def self_test(self) -> HealthCheckSummary:
178171
)
179172
return summary
180173

174+
def _safe_parse_error_detail(self, response_text: str) -> str:
175+
"""
176+
Safely parse error detail from response text.
177+
If JSON parsing fails, return the raw text or a default message.
178+
"""
179+
if not response_text:
180+
return "No error details available"
181+
try:
182+
parsed = json.loads(response_text)
183+
return parsed.get("detail", response_text)
184+
except (json.JSONDecodeError, TypeError):
185+
# Return raw text if JSON parsing fails, but limit length for safety
186+
return response_text[:500] if len(response_text) <= 500 else response_text[:500] + "..."
187+
181188

182189
@Register(api_type="http")
183190
class HttpChatBotPipeline(HttpChatBotMetaData, ModelPipelineChatBot[HttpConfiguration]):
@@ -209,12 +216,11 @@ def invoke(self, params: ChatBotParameters) -> ChatBotResponse:
209216
if params.mcp_headers:
210217
headers["MCP-HEADERS"] = json.dumps(params.mcp_headers)
211218

212-
response = requests.post(
219+
response = self.session.post(
213220
self.config.inference_url + "/v1/query",
214-
headers=self.headers,
221+
headers=headers,
215222
json=data,
216223
timeout=self.task_gen_timeout(1),
217-
verify=self.config.ca_cert_file if self.config.ca_cert_file else self.config.verify_ssl,
218224
)
219225

220226
if response.status_code == 200:
@@ -228,19 +234,19 @@ def invoke(self, params: ChatBotParameters) -> ChatBotResponse:
228234
return data
229235

230236
elif response.status_code == 401:
231-
detail = json.loads(response.text).get("detail", "")
237+
detail = self._safe_parse_error_detail(response.text)
232238
raise ChatbotUnauthorizedException(detail=detail)
233239
elif response.status_code == 403:
234-
detail = json.loads(response.text).get("detail", "")
240+
detail = self._safe_parse_error_detail(response.text)
235241
raise ChatbotForbiddenException(detail=detail)
236242
elif response.status_code == 413:
237-
detail = json.loads(response.text).get("detail", "")
243+
detail = self._safe_parse_error_detail(response.text)
238244
raise ChatbotPromptTooLongException(detail=detail)
239245
elif response.status_code == 422:
240-
detail = json.loads(response.text).get("detail", "")
246+
detail = self._safe_parse_error_detail(response.text)
241247
raise ChatbotValidationException(detail=detail)
242248
else:
243-
detail = json.loads(response.text).get("detail", "")
249+
detail = self._safe_parse_error_detail(response.text)
244250
raise ChatbotInternalServerException(detail=detail)
245251

246252

@@ -275,14 +281,42 @@ def send_schema1_event(self, ev):
275281

276282
send_schema1_event(ev)
277283

284+
def _get_aiohttp_connector(self, verify_ssl: bool = True) -> aiohttp.TCPConnector:
285+
"""Create aiohttp connector with proper SSL configuration.
286+
- aiohttp.TCPConnector does NOT accept ssl=None
287+
- ssl=True: Uses system default SSL verification (SECURE)
288+
- ssl=False: Disables SSL verification completely (INSECURE needed for dev/test)
289+
- ssl=SSLContext: Uses custom SSL context (SECURE with custom CAs)
290+
Args:
291+
verify_ssl: Whether SSL verification should be enabled
292+
Returns:
293+
TCPConnector with consistent SSL behavior:
294+
- verify_ssl=True + custom context: ssl=SSLContext (infrastructure CA bundle)
295+
- verify_ssl=True + no context: ssl=True (system default CAs)
296+
- verify_ssl=False: ssl=False (DISABLED - consistent with requests.Session)
297+
Raises:
298+
ssl.SSLError: If SSL context creation fails when verify_ssl=True
299+
"""
300+
if not verify_ssl:
301+
# SECURITY NOTE: This disables SSL verification
302+
# should only be used in development/testing
303+
# This matches requests.Session.verify=False behavior for consistency
304+
return aiohttp.TCPConnector(ssl=False)
305+
306+
# Get SSL context from centralized SSL manager
307+
ssl_context = ssl_manager.get_ssl_context()
308+
309+
if ssl_context is not None:
310+
# Use custom SSL context from SSL manager (infrastructure CA bundle)
311+
return aiohttp.TCPConnector(ssl=ssl_context)
312+
else:
313+
# Use system default SSL verification (fallback when no custom CA bundle)
314+
return aiohttp.TCPConnector(ssl=True)
315+
278316
async def async_invoke(self, params: StreamingChatBotParameters) -> AsyncGenerator:
279317

280-
# Configure SSL context based on verify_ssl setting
281-
if self.config.ca_cert_file:
282-
ssl_context = ssl.create_default_context(cafile=self.config.ca_cert_file)
283-
connector = aiohttp.TCPConnector(ssl=ssl_context)
284-
else:
285-
connector = aiohttp.TCPConnector(ssl=self.config.verify_ssl)
318+
# Create connector with proper SSL handling
319+
connector = self._get_aiohttp_connector(verify_ssl=self.config.verify_ssl)
286320

287321
async with aiohttp.ClientSession(raise_for_status=True, connector=connector) as session:
288322
headers = {

ansible_ai_connect/ai/api/model_pipelines/http/tests/test_healthcheck.py

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,13 @@
3131
TestModelPipelineHealthCheck,
3232
)
3333
from ansible_ai_connect.ai.api.model_pipelines.tests.test_wca_client import MockResponse
34+
from ansible_ai_connect.main import ssl_manager
3435

3536

3637
@override_settings(ANSIBLE_AI_MODEL_MESH_CONFIG=mock_config("http"))
3738
class TestModelPipelineFactory(TestModelPipelineHealthCheck):
3839

39-
@mock.patch("requests.get", return_value=MockResponse(None, 200))
40+
@mock.patch("requests.Session.get", return_value=MockResponse(None, 200))
4041
def test_completions_healthcheck(self, *args, **kwargs):
4142
self.assert_ok(ModelPipelineCompletions, "http")
4243

@@ -55,10 +56,16 @@ def test_playbook_explanation_healthcheck(self):
5556
def test_role_explanation_healthcheck(self):
5657
self.assert_skipped(ModelPipelineRoleExplanation, "nop")
5758

58-
@mock.patch("requests.get", return_value=MockResponse({"ready": True}, 200))
59-
def test_chatbot_healthcheck(self, *args, **kwargs):
59+
@mock.patch.object(ssl_manager.ssl_manager, "get_requests_session")
60+
def test_chatbot_healthcheck(self, mock_session_factory):
61+
mock_session = mock.Mock()
62+
mock_session.get.return_value = MockResponse({"ready": True}, 200)
63+
mock_session_factory.return_value = mock_session
6064
self.assert_ok(ModelPipelineChatBot, "http")
6165

62-
@mock.patch("requests.get", return_value=MockResponse({"ready": True}, 200))
63-
def test_streaming_chatbot_healthcheck(self, *args, **kwargs):
66+
@mock.patch.object(ssl_manager.ssl_manager, "get_requests_session")
67+
def test_streaming_chatbot_healthcheck(self, mock_session_factory):
68+
mock_session = mock.Mock()
69+
mock_session.get.return_value = MockResponse({"ready": True}, 200)
70+
mock_session_factory.return_value = mock_session
6471
self.assert_ok(ModelPipelineStreamingChatBot, "http")

0 commit comments

Comments
 (0)