Skip to content

Commit d9dae93

Browse files
Fix domain override validation to accept valid URLs
- Updated get_validated_domain_override() to accept both domain-only and full URL formats - Added URL parsing logic to validate http/https protocols - Updated URL construction in agent365_exporter.py to handle both formats - Added comprehensive unit tests for domain validation - Updated existing tests to reflect new validation behavior Co-authored-by: sergioescalera <[email protected]>
1 parent ecc6a0f commit d9dae93

File tree

4 files changed

+198
-11
lines changed

4 files changed

+198
-11
lines changed

libraries/microsoft-agents-a365-observability-core/microsoft_agents_a365/observability/core/exporters/agent365_exporter.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import time
1111
from collections.abc import Callable, Sequence
1212
from typing import Any, final
13+
from urllib.parse import urlparse
1314

1415
import requests
1516
from microsoft_agents_a365.runtime.power_platform_api_discovery import PowerPlatformApiDiscovery
@@ -94,12 +95,22 @@ def export(self, spans: Sequence[ReadableSpan]) -> SpanExportResult:
9495
else:
9596
discovery = PowerPlatformApiDiscovery(self._cluster_category)
9697
endpoint = discovery.get_tenant_island_cluster_endpoint(tenant_id)
98+
9799
endpoint_path = (
98100
f"/maven/agent365/service/agents/{agent_id}/traces"
99101
if self._use_s2s_endpoint
100102
else f"/maven/agent365/agents/{agent_id}/traces"
101103
)
102-
url = f"https://{endpoint}{endpoint_path}?api-version=1"
104+
105+
# Construct URL - if endpoint already has a scheme (http:// or https://), use it as-is
106+
# Otherwise, prepend https://
107+
parsed = urlparse(endpoint)
108+
if parsed.scheme:
109+
# Endpoint is a full URL, append path
110+
url = f"{endpoint}{endpoint_path}?api-version=1"
111+
else:
112+
# Endpoint is just a domain, prepend https://
113+
url = f"https://{endpoint}{endpoint_path}?api-version=1"
103114

104115
# Debug: Log endpoint being used
105116
logger.info(

libraries/microsoft-agents-a365-observability-core/microsoft_agents_a365/observability/core/exporters/utils.py

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import os
66
from collections.abc import Sequence
77
from typing import Any
8+
from urllib.parse import urlparse
89

910
from opentelemetry.sdk.trace import ReadableSpan
1011
from opentelemetry.trace import SpanKind, StatusCode
@@ -153,12 +154,37 @@ def get_validated_domain_override() -> str | None:
153154
if not domain_override:
154155
return None
155156

156-
# Basic validation: ensure domain doesn't contain protocol or path separators
157-
if "://" in domain_override or "/" in domain_override:
158-
logger.warning(
159-
f"Invalid domain override '{domain_override}': "
160-
"domain should not contain protocol (://) or path separators (/)"
161-
)
157+
# Validate that it's a valid URL
158+
try:
159+
parsed = urlparse(domain_override)
160+
161+
# If scheme is present and looks like a protocol (contains //)
162+
if parsed.scheme and "://" in domain_override:
163+
# Validate it's http or https
164+
if parsed.scheme not in ("http", "https"):
165+
logger.warning(
166+
f"Invalid domain override '{domain_override}': "
167+
f"scheme must be http or https, got '{parsed.scheme}'"
168+
)
169+
return None
170+
# Must have a netloc (hostname) when scheme is present
171+
if not parsed.netloc:
172+
logger.warning(
173+
f"Invalid domain override '{domain_override}': "
174+
"missing hostname"
175+
)
176+
return None
177+
else:
178+
# If no scheme with ://, it should be just a domain (no path)
179+
# Note: domain can contain : for port (e.g., example.com:8080)
180+
if "/" in domain_override:
181+
logger.warning(
182+
f"Invalid domain override '{domain_override}': "
183+
"domain without protocol should not contain path separators (/)"
184+
)
185+
return None
186+
except Exception as e:
187+
logger.warning(f"Invalid domain override '{domain_override}': {e}")
162188
return None
163189

164190
return domain_override

tests/observability/core/exporters/test_utils.py

Lines changed: 82 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
1-
# Copyright (c) Microsoft Corporation.
2-
# Licensed under the MIT License.
1+
# Copyright (c) Microsoft. All rights reserved.
32

3+
import os
44
import unittest
5+
from unittest.mock import patch
56

67
from microsoft_agents_a365.observability.core.exporters.utils import (
8+
get_validated_domain_override,
79
truncate_span,
810
)
911

@@ -64,5 +66,83 @@ def test_truncate_span_if_needed(self):
6466
self.assertEqual(result["attributes"][key], "TRUNCATED")
6567

6668

69+
class TestGetValidatedDomainOverride(unittest.TestCase):
70+
"""Unit tests for get_validated_domain_override function."""
71+
72+
def test_returns_none_when_env_var_not_set(self):
73+
"""Test that function returns None when environment variable is not set."""
74+
with patch.dict(os.environ, {}, clear=True):
75+
result = get_validated_domain_override()
76+
self.assertIsNone(result)
77+
78+
def test_returns_none_when_env_var_is_empty(self):
79+
"""Test that function returns None when environment variable is empty."""
80+
with patch.dict(os.environ, {"A365_OBSERVABILITY_DOMAIN_OVERRIDE": ""}):
81+
result = get_validated_domain_override()
82+
self.assertIsNone(result)
83+
84+
def test_returns_none_when_env_var_is_whitespace(self):
85+
"""Test that function returns None when environment variable is only whitespace."""
86+
with patch.dict(os.environ, {"A365_OBSERVABILITY_DOMAIN_OVERRIDE": " "}):
87+
result = get_validated_domain_override()
88+
self.assertIsNone(result)
89+
90+
def test_accepts_valid_domain(self):
91+
"""Test that function accepts a valid domain without protocol."""
92+
with patch.dict(os.environ, {"A365_OBSERVABILITY_DOMAIN_OVERRIDE": "example.com"}):
93+
result = get_validated_domain_override()
94+
self.assertEqual(result, "example.com")
95+
96+
def test_accepts_valid_domain_with_port(self):
97+
"""Test that function accepts a valid domain with port."""
98+
with patch.dict(os.environ, {"A365_OBSERVABILITY_DOMAIN_OVERRIDE": "example.com:8080"}):
99+
result = get_validated_domain_override()
100+
self.assertEqual(result, "example.com:8080")
101+
102+
def test_accepts_valid_https_url(self):
103+
"""Test that function accepts a valid URL with https protocol."""
104+
with patch.dict(
105+
os.environ, {"A365_OBSERVABILITY_DOMAIN_OVERRIDE": "https://example.com"}
106+
):
107+
result = get_validated_domain_override()
108+
self.assertEqual(result, "https://example.com")
109+
110+
def test_accepts_valid_http_url(self):
111+
"""Test that function accepts a valid URL with http protocol."""
112+
with patch.dict(os.environ, {"A365_OBSERVABILITY_DOMAIN_OVERRIDE": "http://example.com"}):
113+
result = get_validated_domain_override()
114+
self.assertEqual(result, "http://example.com")
115+
116+
def test_accepts_valid_http_url_with_port(self):
117+
"""Test that function accepts a valid URL with http protocol and port."""
118+
with patch.dict(
119+
os.environ, {"A365_OBSERVABILITY_DOMAIN_OVERRIDE": "http://localhost:8080"}
120+
):
121+
result = get_validated_domain_override()
122+
self.assertEqual(result, "http://localhost:8080")
123+
124+
def test_rejects_invalid_protocol(self):
125+
"""Test that function rejects URLs with invalid protocols (not http/https)."""
126+
with patch.dict(
127+
os.environ, {"A365_OBSERVABILITY_DOMAIN_OVERRIDE": "ftp://example.com"}
128+
):
129+
result = get_validated_domain_override()
130+
self.assertIsNone(result)
131+
132+
def test_rejects_domain_with_path(self):
133+
"""Test that function rejects domain-only format with path separator."""
134+
with patch.dict(
135+
os.environ, {"A365_OBSERVABILITY_DOMAIN_OVERRIDE": "example.com/path"}
136+
):
137+
result = get_validated_domain_override()
138+
self.assertIsNone(result)
139+
140+
def test_rejects_protocol_without_hostname(self):
141+
"""Test that function rejects URLs with protocol but no hostname."""
142+
with patch.dict(os.environ, {"A365_OBSERVABILITY_DOMAIN_OVERRIDE": "https://"}):
143+
result = get_validated_domain_override()
144+
self.assertIsNone(result)
145+
146+
67147
if __name__ == "__main__":
68148
unittest.main()

tests/observability/core/test_agent365_exporter.py

Lines changed: 72 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -507,10 +507,80 @@ def test_export_ignores_empty_domain_override(self):
507507
# Verify PowerPlatformApiDiscovery was called (override was ignored)
508508
mock_discovery_class.assert_called_once_with("test")
509509

510+
def test_export_uses_valid_url_override_with_https(self):
511+
"""Test that domain override with https:// protocol is accepted and used correctly."""
512+
# Arrange
513+
os.environ["A365_OBSERVABILITY_DOMAIN_OVERRIDE"] = "https://override.example.com"
514+
515+
# Create exporter after setting environment variable
516+
exporter = _Agent365Exporter(
517+
token_resolver=self.mock_token_resolver, cluster_category="test"
518+
)
519+
520+
spans = [self._create_mock_span("test_span")]
521+
522+
# Mock the PowerPlatformApiDiscovery class (should NOT be called since override is valid)
523+
with patch(
524+
"microsoft_agents_a365.observability.core.exporters.agent365_exporter.PowerPlatformApiDiscovery"
525+
) as mock_discovery_class:
526+
# Mock the _post_with_retries method
527+
with patch.object(exporter, "_post_with_retries", return_value=True) as mock_post:
528+
# Act
529+
result = exporter.export(spans)
530+
531+
# Assert
532+
self.assertEqual(result, SpanExportResult.SUCCESS)
533+
mock_post.assert_called_once()
534+
535+
# Verify the call arguments - should use override URL without duplicating protocol
536+
args, kwargs = mock_post.call_args
537+
url, body, headers = args
538+
539+
expected_url = "https://override.example.com/maven/agent365/agents/test-agent-456/traces?api-version=1"
540+
self.assertEqual(url, expected_url)
541+
542+
# Verify PowerPlatformApiDiscovery was not called
543+
mock_discovery_class.assert_not_called()
544+
545+
def test_export_uses_valid_url_override_with_http(self):
546+
"""Test that domain override with http:// protocol is accepted and used correctly."""
547+
# Arrange
548+
os.environ["A365_OBSERVABILITY_DOMAIN_OVERRIDE"] = "http://localhost:8080"
549+
550+
# Create exporter after setting environment variable
551+
exporter = _Agent365Exporter(
552+
token_resolver=self.mock_token_resolver, cluster_category="test"
553+
)
554+
555+
spans = [self._create_mock_span("test_span")]
556+
557+
# Mock the PowerPlatformApiDiscovery class (should NOT be called since override is valid)
558+
with patch(
559+
"microsoft_agents_a365.observability.core.exporters.agent365_exporter.PowerPlatformApiDiscovery"
560+
) as mock_discovery_class:
561+
# Mock the _post_with_retries method
562+
with patch.object(exporter, "_post_with_retries", return_value=True) as mock_post:
563+
# Act
564+
result = exporter.export(spans)
565+
566+
# Assert
567+
self.assertEqual(result, SpanExportResult.SUCCESS)
568+
mock_post.assert_called_once()
569+
570+
# Verify the call arguments - should use override URL with http protocol
571+
args, kwargs = mock_post.call_args
572+
url, body, headers = args
573+
574+
expected_url = "http://localhost:8080/maven/agent365/agents/test-agent-456/traces?api-version=1"
575+
self.assertEqual(url, expected_url)
576+
577+
# Verify PowerPlatformApiDiscovery was not called
578+
mock_discovery_class.assert_not_called()
579+
510580
def test_export_ignores_invalid_domain_with_protocol(self):
511-
"""Test that domain override containing protocol is ignored."""
581+
"""Test that domain override with invalid protocol is ignored."""
512582
# Arrange
513-
os.environ["A365_OBSERVABILITY_DOMAIN_OVERRIDE"] = "https://invalid.example.com"
583+
os.environ["A365_OBSERVABILITY_DOMAIN_OVERRIDE"] = "ftp://invalid.example.com"
514584

515585
# Create exporter after setting environment variable
516586
exporter = _Agent365Exporter(

0 commit comments

Comments
 (0)