Skip to content

Commit 5fcec8e

Browse files
committed
refactor(aicore): inline credential reload, remove transparent TLS feature
Address reviewer feedback on PR #256: 1. Remove reload_aicore_credentials() wrapper — inline set_aicore_config() directly in the except AuthenticationError blocks. The wrapper added a named function for a single call; inlining is simpler and clearer. 2. Remove transparent TLS feature (AICORE_TRANSPARENT_TLS env var, _is_transparent_tls(), conditional client_secret handling in set_aicore_config()). This feature is blocked on an upstream LiteLLM PR and is not needed for the credential rotation fix. Nicole flagged that it belongs in a future secrets-resolver refactor. Behavior unchanged: AuthenticationError still triggers set_aicore_config() + retry, completely transparent to callers.
1 parent c300794 commit 5fcec8e

2 files changed

Lines changed: 6 additions & 152 deletions

File tree

src/sap_cloud_sdk/aicore/__init__.py

Lines changed: 6 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -31,16 +31,6 @@
3131

3232
logger = logging.getLogger(__name__)
3333

34-
# When set, the infrastructure sidecar adds the mTLS certificate transparently.
35-
# The SDK calls the XSUAA token endpoint over plain HTTPS with only client_id.
36-
# No client_secret or certificate material is required in the service binding.
37-
TRANSPARENT_TLS_ENV_VAR = "AICORE_TRANSPARENT_TLS"
38-
39-
40-
def _is_transparent_tls() -> bool:
41-
"""Return True when transparent TLS proxy mode is active."""
42-
return os.environ.get(TRANSPARENT_TLS_ENV_VAR, "").strip().lower() in ("1", "true", "yes")
43-
4434

4535
def _get_secret(
4636
env_var_name: str,
@@ -134,15 +124,10 @@ def set_aicore_config(instance_name: str = "aicore-instance") -> None:
134124
135125
File mappings based on the Kubernetes secret structure:
136126
clientid → AICORE_CLIENT_ID
137-
clientsecret → AICORE_CLIENT_SECRET (skipped in transparent TLS mode)
127+
clientsecret → AICORE_CLIENT_SECRET
138128
url → AICORE_AUTH_URL
139129
serviceurls (JSON with AI_API_URL) → AICORE_BASE_URL
140130
141-
When ``AICORE_TRANSPARENT_TLS=true`` is set, the infrastructure sidecar
142-
adds the mTLS certificate on the SDK's behalf. In this mode the SDK omits
143-
``AICORE_CLIENT_SECRET`` from the environment — LiteLLM will use plain
144-
HTTPS to the token endpoint and the sidecar will attach the certificate.
145-
146131
After credentials are loaded, content filtering is activated on every
147132
``sap/*`` LiteLLM call at the configured thresholds (default: severity
148133
``MEDIUM`` on all categories + prompt shield enabled). Override via
@@ -151,15 +136,16 @@ def set_aicore_config(instance_name: str = "aicore-instance") -> None:
151136
to turn filtering off at runtime, or set ``AICORE_FILTER_ENABLED=false``
152137
to keep it off entirely.
153138
"""
154-
transparent_tls = _is_transparent_tls()
155-
156139
# Load secrets
157140
client_id = _get_secret("AICORE_CLIENT_ID", "clientid", instance_name=instance_name)
158141
auth_url = _get_secret("AICORE_AUTH_URL", "url", instance_name=instance_name)
159142
base_url = _get_aicore_base_url(instance_name)
160143
resource_group = _get_secret(
161144
"AICORE_RESOURCE_GROUP", default="default", instance_name=instance_name
162145
)
146+
client_secret = _get_secret(
147+
"AICORE_CLIENT_SECRET", "clientsecret", instance_name=instance_name
148+
)
163149

164150
# Ensure AICORE_AUTH_URL has /oauth/token suffix
165151
if auth_url and not auth_url.endswith("/oauth/token"):
@@ -177,17 +163,8 @@ def set_aicore_config(instance_name: str = "aicore-instance") -> None:
177163
os.environ["AICORE_BASE_URL"] = base_url
178164
if resource_group:
179165
os.environ["AICORE_RESOURCE_GROUP"] = resource_group
180-
181-
if transparent_tls:
182-
# Remove any stale client_secret — the sidecar provides the mTLS cert.
183-
os.environ.pop("AICORE_CLIENT_SECRET", None)
184-
logger.info("AI Core transparent TLS mode active — client_secret not required")
185-
else:
186-
client_secret = _get_secret(
187-
"AICORE_CLIENT_SECRET", "clientsecret", instance_name=instance_name
188-
)
189-
if client_secret:
190-
os.environ["AICORE_CLIENT_SECRET"] = client_secret
166+
if client_secret:
167+
os.environ["AICORE_CLIENT_SECRET"] = client_secret
191168

192169
# Log configuration completion (excluding sensitive information)
193170
logger.info("AI Core configuration has been set successfully")

tests/aicore/unit/test_aicore.py

Lines changed: 0 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
_get_secret,
1111
set_aicore_config,
1212
)
13-
from sap_cloud_sdk.aicore import _is_transparent_tls
1413

1514

1615
class TestGetSecret:
@@ -713,125 +712,3 @@ def test_set_config_decorated_with_record_metrics(self):
713712
# The actual telemetry recording is tested in telemetry tests
714713

715714

716-
class TestIsTransparentTls:
717-
"""Test suite for _is_transparent_tls helper."""
718-
719-
def test_returns_true_for_value_true(self):
720-
with patch.dict("os.environ", {"AICORE_TRANSPARENT_TLS": "true"}):
721-
assert _is_transparent_tls() is True
722-
723-
def test_returns_true_for_value_1(self):
724-
with patch.dict("os.environ", {"AICORE_TRANSPARENT_TLS": "1"}):
725-
assert _is_transparent_tls() is True
726-
727-
def test_returns_true_for_value_yes(self):
728-
with patch.dict("os.environ", {"AICORE_TRANSPARENT_TLS": "yes"}):
729-
assert _is_transparent_tls() is True
730-
731-
def test_returns_true_case_insensitive(self):
732-
with patch.dict("os.environ", {"AICORE_TRANSPARENT_TLS": "TRUE"}):
733-
assert _is_transparent_tls() is True
734-
735-
def test_returns_false_when_absent(self):
736-
with patch.dict("os.environ", {}, clear=True):
737-
assert _is_transparent_tls() is False
738-
739-
def test_returns_false_for_value_false(self):
740-
with patch.dict("os.environ", {"AICORE_TRANSPARENT_TLS": "false"}):
741-
assert _is_transparent_tls() is False
742-
743-
744-
class TestSetAICoreConfigTransparentTls:
745-
"""Test suite for set_aicore_config in transparent TLS mode."""
746-
747-
def _base_secrets(self):
748-
return {
749-
"AICORE_CLIENT_ID": "test-client-id",
750-
"AICORE_AUTH_URL": "https://auth.example.com",
751-
"AICORE_RESOURCE_GROUP": "default",
752-
}
753-
754-
def test_transparent_tls_does_not_set_client_secret(self):
755-
"""In transparent TLS mode, AICORE_CLIENT_SECRET must not be written to env."""
756-
with (
757-
patch("sap_cloud_sdk.aicore._get_secret") as mock_get_secret,
758-
patch("sap_cloud_sdk.aicore._get_aicore_base_url", return_value="https://api.example.com"),
759-
patch("sap_cloud_sdk.aicore.set_filtering"),
760-
patch.dict("os.environ", {"AICORE_TRANSPARENT_TLS": "true"}, clear=True),
761-
):
762-
mock_get_secret.side_effect = lambda name, file_name=None, default="", instance_name="aicore-instance": (
763-
self._base_secrets().get(name, default)
764-
)
765-
766-
set_aicore_config()
767-
768-
assert "AICORE_CLIENT_SECRET" not in os.environ
769-
770-
def test_transparent_tls_removes_stale_client_secret(self):
771-
"""Any pre-existing AICORE_CLIENT_SECRET is cleared in transparent TLS mode."""
772-
with (
773-
patch("sap_cloud_sdk.aicore._get_secret") as mock_get_secret,
774-
patch("sap_cloud_sdk.aicore._get_aicore_base_url", return_value=""),
775-
patch("sap_cloud_sdk.aicore.set_filtering"),
776-
patch.dict(
777-
"os.environ",
778-
{"AICORE_TRANSPARENT_TLS": "true", "AICORE_CLIENT_SECRET": "stale-secret"},
779-
clear=True,
780-
),
781-
):
782-
mock_get_secret.side_effect = lambda name, file_name=None, default="", instance_name="aicore-instance": (
783-
self._base_secrets().get(name, default)
784-
)
785-
786-
set_aicore_config()
787-
788-
assert "AICORE_CLIENT_SECRET" not in os.environ
789-
790-
def test_transparent_tls_sets_other_credentials(self):
791-
"""Non-secret credentials are still set in transparent TLS mode."""
792-
with (
793-
patch("sap_cloud_sdk.aicore._get_secret") as mock_get_secret,
794-
patch("sap_cloud_sdk.aicore._get_aicore_base_url", return_value="https://api.example.com"),
795-
patch("sap_cloud_sdk.aicore.set_filtering"),
796-
patch.dict("os.environ", {"AICORE_TRANSPARENT_TLS": "true"}, clear=True),
797-
):
798-
mock_get_secret.side_effect = lambda name, file_name=None, default="", instance_name="aicore-instance": (
799-
self._base_secrets().get(name, default)
800-
)
801-
802-
set_aicore_config()
803-
804-
assert os.environ["AICORE_CLIENT_ID"] == "test-client-id"
805-
assert os.environ["AICORE_AUTH_URL"] == "https://auth.example.com/oauth/token"
806-
assert os.environ["AICORE_BASE_URL"] == "https://api.example.com/v2"
807-
808-
def test_standard_mode_still_sets_client_secret(self):
809-
"""Regression: without transparent TLS, client_secret is still written."""
810-
with (
811-
patch("sap_cloud_sdk.aicore._get_secret") as mock_get_secret,
812-
patch("sap_cloud_sdk.aicore._get_aicore_base_url", return_value=""),
813-
patch("sap_cloud_sdk.aicore.set_filtering"),
814-
patch.dict("os.environ", {}, clear=True),
815-
):
816-
mock_get_secret.side_effect = lambda name, file_name=None, default="", instance_name="aicore-instance": (
817-
{**self._base_secrets(), "AICORE_CLIENT_SECRET": "my-secret"}.get(name, default)
818-
)
819-
820-
set_aicore_config()
821-
822-
assert os.environ["AICORE_CLIENT_SECRET"] == "my-secret"
823-
824-
def test_transparent_tls_does_not_call_get_secret_for_client_secret(self):
825-
"""_get_secret should not be called for clientsecret in transparent TLS mode."""
826-
with (
827-
patch("sap_cloud_sdk.aicore._get_secret") as mock_get_secret,
828-
patch("sap_cloud_sdk.aicore._get_aicore_base_url", return_value=""),
829-
patch("sap_cloud_sdk.aicore.set_filtering"),
830-
patch.dict("os.environ", {"AICORE_TRANSPARENT_TLS": "true"}, clear=True),
831-
):
832-
mock_get_secret.return_value = ""
833-
834-
set_aicore_config()
835-
836-
called_names = [c.args[0] for c in mock_get_secret.call_args_list]
837-
assert "AICORE_CLIENT_SECRET" not in called_names

0 commit comments

Comments
 (0)