Skip to content

Commit 555ebbf

Browse files
committed
fix(sdk-review): FP-V-01 breaking-detector skips test classes
PR #217 (Betina) triggered BREAKING-01/02 + VER-01 because the breaking detector treated removal of test methods in TestSecretResolver as a public API deletion. Test classes (Test*, *Test, *TestCase) are not SDK API — their methods can be freely added, removed, or renamed without breaking consumers. Filter: extract_public_class_methods now skips class names matching the test class convention before extracting the method table.
1 parent f173521 commit 555ebbf

2 files changed

Lines changed: 97 additions & 0 deletions

File tree

.claude/scripts/lib/ast_python_checks.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,10 @@ def extract_public_class_methods(
395395
continue
396396
if node.name.startswith("_"):
397397
continue
398+
# FP-V-01: skip test classes (Test*, *Test, *TestCase) — they are not
399+
# public API and removing/renaming test methods never breaks SDK consumers.
400+
if node.name.startswith("Test") or node.name.endswith("Test") or node.name.endswith("TestCase"):
401+
continue
398402
methods: dict = {}
399403
for item in node.body:
400404
if not isinstance(item, (ast.FunctionDef, ast.AsyncFunctionDef)):
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
"""Resolver that reads service binding secrets from a mounted volume path."""
2+
3+
import os
4+
from typing import Any
5+
6+
from sap_cloud_sdk.core.secret_resolver._mapping import _get_field_map
7+
from sap_cloud_sdk.core.secret_resolver.constants import (
8+
BASE_MOUNT_PATH,
9+
SERVICE_BINDING_ROOT,
10+
)
11+
12+
13+
class MountResolver:
14+
"""Resolves bindings from a mounted volume path.
15+
16+
Reads secret files at ``{base_volume_mount}/{module}/{instance}/{field_key}``.
17+
Respects the ``SERVICE_BINDING_ROOT`` environment variable (servicebinding.io
18+
spec) as an override for ``base_volume_mount``.
19+
20+
Args:
21+
base_volume_mount: Base path for mounted secrets. Defaults to
22+
``/etc/secrets/appfnd``.
23+
"""
24+
25+
def __init__(self, base_volume_mount: str = BASE_MOUNT_PATH) -> None:
26+
self._base_volume_mount = base_volume_mount
27+
28+
def resolve(self, module: str, instance: str, target: Any) -> None:
29+
"""Load secrets from the mounted volume path."""
30+
effective_base = resolve_base_mount(self._base_volume_mount)
31+
_load_from_mount(effective_base, module, instance, target)
32+
33+
34+
def resolve_base_mount(base_volume_mount: str = BASE_MOUNT_PATH) -> str:
35+
"""Resolve the base mount path for service binding discovery.
36+
37+
Checks the ``SERVICE_BINDING_ROOT`` environment variable first (as defined
38+
by the `servicebinding.io <https://servicebinding.io/spec/core/1.1.0/>`_
39+
specification). Falls back to ``base_volume_mount`` when the env var is
40+
absent.
41+
42+
Args:
43+
base_volume_mount: Default base path used when ``SERVICE_BINDING_ROOT``
44+
is not set. Defaults to ``/etc/secrets/appfnd``.
45+
46+
Returns:
47+
The effective base path for secret mount resolution.
48+
"""
49+
return os.environ.get(SERVICE_BINDING_ROOT, base_volume_mount)
50+
51+
52+
def _load_from_mount(
53+
base_volume_mount: str, module: str, instance: str, target: Any
54+
) -> None:
55+
"""
56+
Load secrets from files at:
57+
{base_volume_mount}/{module}/{instance}/{field_key}
58+
59+
Sets string attributes directly on the dataclass instance.
60+
"""
61+
secret_dir = os.path.join(base_volume_mount, module, instance)
62+
_validate_path(secret_dir)
63+
64+
field_map = _get_field_map(target)
65+
for key, (attr_name, _) in field_map.items():
66+
file_path = os.path.join(secret_dir, key)
67+
try:
68+
# Read entire file content as text; do not strip newlines to match Go behavior
69+
with open(file_path, "r", encoding="utf-8") as f:
70+
content = f.read()
71+
except FileNotFoundError as e:
72+
# Align with Go: surface precise file error
73+
raise FileNotFoundError(
74+
f"failed to read secret file {file_path}: {e}"
75+
) from e
76+
except OSError as e:
77+
raise OSError(f"failed to read secret file {file_path}: {e}") from e
78+
79+
# Set target field (string only)
80+
setattr(target, attr_name, content)
81+
82+
83+
def _validate_path(path: str) -> None:
84+
"""Validate that the given path exists and is a directory."""
85+
try:
86+
_st = os.stat(path)
87+
except FileNotFoundError as e:
88+
raise FileNotFoundError(f"path does not exist: {path}") from e
89+
except OSError as e:
90+
raise OSError(f"cannot access path {path}: {e}") from e
91+
# If exists, ensure it's a directory
92+
if not os.path.isdir(path):
93+
raise NotADirectoryError(f"path is not a directory: {path}")

0 commit comments

Comments
 (0)