Skip to content

Commit 6643093

Browse files
Copilotsimonkurtz-MSFT
Copilot
and
simonkurtz-MSFT
authored
Eliminate redundant Front Door lookup in get_endpoint() (#207)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonkurtz-MSFT <84809797+simonkurtz-MSFT@users.noreply.github.com>
1 parent c87c21e commit 6643093

4 files changed

Lines changed: 28 additions & 132 deletions

File tree

‎shared/python/azure_resources.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -991,7 +991,7 @@ def get_frontdoor_url(deployment_name: INFRASTRUCTURE, rg_name: str) -> str | No
991991

992992
if afd_endpoint_url:
993993
print_ok(f'Front Door Endpoint URL: {afd_endpoint_url}', blank_above=False)
994-
else:
994+
elif deployment_name == INFRASTRUCTURE.AFD_APIM_PE:
995995
print_warning('No Front Door endpoint URL found.')
996996

997997
return afd_endpoint_url

‎shared/python/utils.py‎

Lines changed: 4 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
# APIM Samples imports
2020
import azure_resources as az
2121
import logging_config
22-
from apimtypes import APIM_SKU, HTTP_VERB, INFRASTRUCTURE, Endpoints, Output, get_project_root
22+
from apimtypes import APIM_SKU, HTTP_VERB, INFRASTRUCTURE, Endpoints, Output, get_project_root # noqa: F401 (Endpoints re-exported for callers)
2323
from console import print_error, print_info, print_message, print_ok, print_plain, print_secret, print_val, print_warning
2424

2525
# Configure warning filter to suppress IPython exit warnings
@@ -1191,37 +1191,6 @@ def wait_for_apim_blob_permissions(apim_name: str, storage_account_name: str, re
11911191
return success
11921192

11931193

1194-
def test_url_preflight_check(deployment: INFRASTRUCTURE, rg_name: str, apim_gateway_url: str) -> str:
1195-
"""Check if the deployment uses Azure Front Door and return the appropriate endpoint URL."""
1196-
# Preflight: Check if the infrastructure architecture deployment uses Azure Front Door.
1197-
# If so, assume that APIM is not directly accessible and use the Front Door URL instead.
1198-
1199-
print_message('Checking if the infrastructure architecture deployment uses Azure Front Door.', blank_above=True)
1200-
1201-
afd_endpoint_url = az.get_frontdoor_url(deployment, rg_name)
1202-
1203-
if afd_endpoint_url:
1204-
endpoint_url = afd_endpoint_url
1205-
print_message(f'Using Azure Front Door URL: {afd_endpoint_url}', blank_above=True)
1206-
else:
1207-
endpoint_url = apim_gateway_url
1208-
print_message(f'Using APIM Gateway URL: {apim_gateway_url}', blank_above=True)
1209-
1210-
return endpoint_url
1211-
1212-
1213-
def get_endpoints(deployment: INFRASTRUCTURE, rg_name: str) -> Endpoints:
1214-
"""Identify and return all possible endpoints for the given infrastructure deployment."""
1215-
print_message(f'Identifying possible endpoints for infrastructure {deployment}...')
1216-
1217-
endpoints = Endpoints(deployment)
1218-
1219-
endpoints.afd_endpoint_url = az.get_frontdoor_url(deployment, rg_name)
1220-
endpoints.apim_endpoint_url = az.get_apim_url(rg_name)
1221-
endpoints.appgw_hostname, endpoints.appgw_public_ip = az.get_appgw_endpoint(rg_name)
1222-
1223-
return endpoints
1224-
12251194

12261195
def get_endpoint(deployment: INFRASTRUCTURE, rg_name: str, apim_gateway_url: str) -> Tuple[str, dict[str, str] | None, bool]:
12271196
"""Determine the endpoint URL, optional request headers, and TLS verification flag for test execution.
@@ -1232,7 +1201,7 @@ def get_endpoint(deployment: INFRASTRUCTURE, rg_name: str, apim_gateway_url: str
12321201
self-signed certificate that we create in the infrastructure deployment.
12331202
"""
12341203
# Determine endpoints, URLs, etc. prior to test execution
1235-
endpoints = get_endpoints(deployment, rg_name)
1204+
endpoints = az.get_endpoints(deployment, rg_name)
12361205
endpoint_url = None
12371206
request_headers = None
12381207
allow_insecure_tls = False
@@ -1244,9 +1213,8 @@ def get_endpoint(deployment: INFRASTRUCTURE, rg_name: str, apim_gateway_url: str
12441213
# during deployment, so TLS verification must be disabled for requests to succeed.
12451214
allow_insecure_tls = True
12461215
else:
1247-
# Preflight: Check if the deployment uses Azure Front Door.
1248-
# If so, assume APIM is not directly accessible and use the Front Door URL instead.
1249-
endpoint_url = test_url_preflight_check(deployment, rg_name, apim_gateway_url)
1216+
# Reuse the already-fetched Front Door URL, falling back to the APIM gateway URL.
1217+
endpoint_url = endpoints.afd_endpoint_url or apim_gateway_url
12501218

12511219
return endpoint_url, request_headers, allow_insecure_tls
12521220

‎tests/python/test_azure_resources.py‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -715,6 +715,17 @@ def test_get_frontdoor_url_wrong_infrastructure():
715715
mock_run.assert_not_called()
716716

717717

718+
def test_get_frontdoor_url_no_warning_for_non_afd(monkeypatch):
719+
"""Test that no 'No Front Door endpoint URL found.' warning is emitted for non-AFD infrastructures."""
720+
warnings = []
721+
monkeypatch.setattr('azure_resources.print_warning', lambda msg, **kw: warnings.append(msg))
722+
723+
for infra in [INFRASTRUCTURE.SIMPLE_APIM, INFRASTRUCTURE.APPGW_APIM, INFRASTRUCTURE.APPGW_APIM_PE, INFRASTRUCTURE.APIM_ACA]:
724+
warnings.clear()
725+
az.get_frontdoor_url(infra, 'test-rg')
726+
assert not any('No Front Door' in w for w in warnings), f'Unexpected warning for {infra}: {warnings}'
727+
728+
718729
def test_get_frontdoor_url_no_profile():
719730
"""Test Front Door URL when no profile found."""
720731

‎tests/python/test_utils.py‎

Lines changed: 12 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -434,22 +434,6 @@ def run_all():
434434
assert 'Test value' in output
435435

436436

437-
def test_test_url_preflight_check_with_frontdoor(monkeypatch, suppress_console):
438-
"""Test URL preflight check when Front Door is available."""
439-
monkeypatch.setattr(az, 'get_frontdoor_url', lambda x, y: 'https://test.azurefd.net')
440-
441-
result = utils.test_url_preflight_check(INFRASTRUCTURE.AFD_APIM_PE, 'test-rg', 'https://apim.com')
442-
assert result == 'https://test.azurefd.net'
443-
444-
445-
def test_test_url_preflight_check_no_frontdoor(monkeypatch, suppress_console):
446-
"""Test URL preflight check when Front Door is not available."""
447-
monkeypatch.setattr(az, 'get_frontdoor_url', lambda x, y: None)
448-
449-
result = utils.test_url_preflight_check(INFRASTRUCTURE.SIMPLE_APIM, 'test-rg', 'https://apim.com')
450-
assert result == 'https://apim.com'
451-
452-
453437
def test_determine_policy_path_filename_mode(monkeypatch):
454438
"""Test determine_policy_path with filename mode."""
455439

@@ -1838,38 +1822,9 @@ def test_notebookhelper_clean_up_jwt_failure(monkeypatch, caplog):
18381822

18391823

18401824
# ------------------------------
1841-
# get_endpoints TESTS
18421825
# ------------------------------
1843-
1844-
1845-
def test_get_endpoints_comprehensive(monkeypatch, suppress_console):
1846-
"""Test get_endpoints function."""
1847-
monkeypatch.setattr(az, 'get_frontdoor_url', lambda x, y: 'https://test-afd.azurefd.net')
1848-
monkeypatch.setattr(az, 'get_apim_url', lambda x: 'https://test-apim.azure-api.net')
1849-
monkeypatch.setattr(az, 'get_appgw_endpoint', lambda x: ('appgw.contoso.com', '1.2.3.4'))
1850-
1851-
endpoints = utils.get_endpoints(INFRASTRUCTURE.AFD_APIM_PE, 'test-rg')
1852-
1853-
assert endpoints.afd_endpoint_url == 'https://test-afd.azurefd.net'
1854-
assert endpoints.apim_endpoint_url == 'https://test-apim.azure-api.net'
1855-
assert endpoints.appgw_hostname == 'appgw.contoso.com'
1856-
assert endpoints.appgw_public_ip == '1.2.3.4'
1857-
1858-
1859-
def test_get_endpoints_no_frontdoor(monkeypatch, suppress_console):
1860-
"""Test get_endpoints when Front Door is not available."""
1861-
monkeypatch.setattr(az, 'get_frontdoor_url', lambda x, y: None)
1862-
monkeypatch.setattr(az, 'get_apim_url', lambda x: 'https://test-apim.azure-api.net')
1863-
monkeypatch.setattr(az, 'get_appgw_endpoint', lambda x: (None, None))
1864-
1865-
endpoints = utils.get_endpoints(INFRASTRUCTURE.SIMPLE_APIM, 'test-rg')
1866-
1867-
assert endpoints.afd_endpoint_url is None
1868-
assert endpoints.apim_endpoint_url == 'https://test-apim.azure-api.net'
1869-
1870-
1826+
# get_endpoint
18711827
# ------------------------------
1872-
# get_json TESTS
18731828
# ------------------------------
18741829

18751830

@@ -2053,20 +2008,6 @@ def test_does_infrastructure_exist_with_prompt_multiple_retries(monkeypatch, sup
20532008
assert result is True # Block deployment
20542009

20552010

2056-
def test_get_endpoints_with_none_values(monkeypatch, suppress_console):
2057-
"""Test get_endpoints when some endpoints are None."""
2058-
monkeypatch.setattr(az, 'get_frontdoor_url', lambda x, y: None)
2059-
monkeypatch.setattr(az, 'get_apim_url', lambda x: 'https://test-apim.azure-api.net')
2060-
monkeypatch.setattr(az, 'get_appgw_endpoint', lambda x: (None, None))
2061-
2062-
endpoints = utils.get_endpoints(INFRASTRUCTURE.SIMPLE_APIM, 'test-rg')
2063-
2064-
assert endpoints.afd_endpoint_url is None
2065-
assert endpoints.apim_endpoint_url == 'https://test-apim.azure-api.net'
2066-
assert endpoints.appgw_hostname is None
2067-
assert endpoints.appgw_public_ip is None
2068-
2069-
20702011
# ------------------------------
20712012
# get_endpoint
20722013
# ------------------------------
@@ -2080,7 +2021,7 @@ def test_get_endpoint_with_appgw_both_values(monkeypatch, suppress_console):
20802021
mock_endpoints.appgw_hostname = 'api.contoso.com'
20812022
mock_endpoints.appgw_public_ip = '1.2.3.4'
20822023

2083-
monkeypatch.setattr(utils, 'get_endpoints', lambda d, r: mock_endpoints)
2024+
monkeypatch.setattr(az, 'get_endpoints', lambda d, r: mock_endpoints)
20842025

20852026
endpoint_url, request_headers, allow_insecure_tls = utils.get_endpoint(INFRASTRUCTURE.APPGW_APIM, 'test-rg', 'https://apim.azure-api.net')
20862027

@@ -2098,8 +2039,7 @@ def test_get_endpoint_with_appgw_hostname_only(monkeypatch, suppress_console):
20982039
mock_endpoints.appgw_hostname = 'api.contoso.com'
20992040
mock_endpoints.appgw_public_ip = None
21002041

2101-
monkeypatch.setattr(utils, 'get_endpoints', lambda d, r: mock_endpoints)
2102-
monkeypatch.setattr(utils, 'test_url_preflight_check', lambda d, r, a: 'https://afd.azurefd.net')
2042+
monkeypatch.setattr(az, 'get_endpoints', lambda d, r: mock_endpoints)
21032043

21042044
endpoint_url, request_headers, allow_insecure_tls = utils.get_endpoint(INFRASTRUCTURE.APPGW_APIM, 'test-rg', 'https://apim.azure-api.net')
21052045

@@ -2116,8 +2056,7 @@ def test_get_endpoint_with_appgw_ip_only(monkeypatch, suppress_console):
21162056
mock_endpoints.appgw_hostname = None
21172057
mock_endpoints.appgw_public_ip = '1.2.3.4'
21182058

2119-
monkeypatch.setattr(utils, 'get_endpoints', lambda d, r: mock_endpoints)
2120-
monkeypatch.setattr(utils, 'test_url_preflight_check', lambda d, r, a: 'https://afd.azurefd.net')
2059+
monkeypatch.setattr(az, 'get_endpoints', lambda d, r: mock_endpoints)
21212060

21222061
endpoint_url, request_headers, allow_insecure_tls = utils.get_endpoint(INFRASTRUCTURE.APPGW_APIM, 'test-rg', 'https://apim.azure-api.net')
21232062

@@ -2126,16 +2065,15 @@ def test_get_endpoint_with_appgw_ip_only(monkeypatch, suppress_console):
21262065
assert allow_insecure_tls is False
21272066

21282067

2129-
def test_get_endpoint_with_no_appgw_uses_preflight(monkeypatch, suppress_console):
2130-
"""Test get_endpoint when no appgw values present, uses preflight check."""
2068+
def test_get_endpoint_with_no_appgw_falls_back_to_apim(monkeypatch, suppress_console):
2069+
"""Test get_endpoint when no appgw values and no AFD, falls back to APIM gateway URL."""
21312070
mock_endpoints = Endpoints(INFRASTRUCTURE.SIMPLE_APIM)
21322071
mock_endpoints.afd_endpoint_url = None
21332072
mock_endpoints.apim_endpoint_url = 'https://apim.azure-api.net'
21342073
mock_endpoints.appgw_hostname = None
21352074
mock_endpoints.appgw_public_ip = None
21362075

2137-
monkeypatch.setattr(utils, 'get_endpoints', lambda d, r: mock_endpoints)
2138-
monkeypatch.setattr(utils, 'test_url_preflight_check', lambda d, r, a: 'https://apim.azure-api.net')
2076+
monkeypatch.setattr(az, 'get_endpoints', lambda d, r: mock_endpoints)
21392077

21402078
endpoint_url, request_headers, allow_insecure_tls = utils.get_endpoint(INFRASTRUCTURE.SIMPLE_APIM, 'test-rg', 'https://apim.azure-api.net')
21412079

@@ -2144,16 +2082,15 @@ def test_get_endpoint_with_no_appgw_uses_preflight(monkeypatch, suppress_console
21442082
assert allow_insecure_tls is False
21452083

21462084

2147-
def test_get_endpoint_with_afd_via_preflight(monkeypatch, suppress_console):
2148-
"""Test get_endpoint returns AFD URL via preflight check."""
2085+
def test_get_endpoint_with_afd_url(monkeypatch, suppress_console):
2086+
"""Test get_endpoint returns AFD URL when afd_endpoint_url is set."""
21492087
mock_endpoints = Endpoints(INFRASTRUCTURE.AFD_APIM_PE)
21502088
mock_endpoints.afd_endpoint_url = 'https://myapp.azurefd.net'
21512089
mock_endpoints.apim_endpoint_url = None
21522090
mock_endpoints.appgw_hostname = None
21532091
mock_endpoints.appgw_public_ip = None
21542092

2155-
monkeypatch.setattr(utils, 'get_endpoints', lambda d, r: mock_endpoints)
2156-
monkeypatch.setattr(utils, 'test_url_preflight_check', lambda d, r, a: 'https://myapp.azurefd.net')
2093+
monkeypatch.setattr(az, 'get_endpoints', lambda d, r: mock_endpoints)
21572094

21582095
endpoint_url, request_headers, allow_insecure_tls = utils.get_endpoint(
21592096
INFRASTRUCTURE.AFD_APIM_PE, 'test-rg', 'https://apim-internal.azure-api.net'
@@ -2172,8 +2109,7 @@ def test_get_endpoint_appgw_with_empty_strings(monkeypatch, suppress_console):
21722109
mock_endpoints.appgw_hostname = ''
21732110
mock_endpoints.appgw_public_ip = ''
21742111

2175-
monkeypatch.setattr(utils, 'get_endpoints', lambda d, r: mock_endpoints)
2176-
monkeypatch.setattr(utils, 'test_url_preflight_check', lambda d, r, a: 'https://apim.azure-api.net')
2112+
monkeypatch.setattr(az, 'get_endpoints', lambda d, r: mock_endpoints)
21772113

21782114
endpoint_url, request_headers, allow_insecure_tls = utils.get_endpoint(INFRASTRUCTURE.APPGW_APIM, 'test-rg', 'https://apim.azure-api.net')
21792115

@@ -2198,8 +2134,7 @@ def test_get_endpoint_various_infrastructures(monkeypatch, suppress_console):
21982134
mock_endpoints.appgw_hostname = None
21992135
mock_endpoints.appgw_public_ip = None
22002136

2201-
monkeypatch.setattr(utils, 'get_endpoints', lambda d, r, m=mock_endpoints: m)
2202-
monkeypatch.setattr(utils, 'test_url_preflight_check', lambda d, r, a: 'https://apim.azure-api.net')
2137+
monkeypatch.setattr(az, 'get_endpoints', lambda d, r, m=mock_endpoints: m)
22032138

22042139
endpoint_url, request_headers, allow_insecure_tls = utils.get_endpoint(infra, 'test-rg', 'https://apim.azure-api.net')
22052140

@@ -2502,24 +2437,6 @@ def test_wait_for_apim_blob_permissions_with_custom_timeout(monkeypatch, suppres
25022437
mock_check.assert_called_once_with('test-apim', 'test-storage', 'test-rg', 5)
25032438

25042439

2505-
def test_test_url_preflight_check_with_afd_endpoint(monkeypatch, suppress_console):
2506-
"""Test test_url_preflight_check selects AFD when available."""
2507-
monkeypatch.setattr(az, 'get_frontdoor_url', lambda x, y: 'https://afd-endpoint.azurefd.net')
2508-
2509-
result = utils.test_url_preflight_check(INFRASTRUCTURE.AFD_APIM_PE, 'test-rg', 'https://apim.azure-api.net')
2510-
2511-
assert result == 'https://afd-endpoint.azurefd.net'
2512-
2513-
2514-
def test_test_url_preflight_check_without_afd(monkeypatch, suppress_console):
2515-
"""Test test_url_preflight_check uses APIM when no AFD."""
2516-
monkeypatch.setattr(az, 'get_frontdoor_url', lambda x, y: None)
2517-
2518-
result = utils.test_url_preflight_check(INFRASTRUCTURE.SIMPLE_APIM, 'test-rg', 'https://apim.azure-api.net')
2519-
2520-
assert result == 'https://apim.azure-api.net'
2521-
2522-
25232440
def test_get_json_with_nested_structure():
25242441
"""Test get_json with deeply nested JSON."""
25252442
nested_json = '{"level1": {"level2": {"level3": {"value": "deep"}}}}'

0 commit comments

Comments
 (0)