Skip to content

Commit 6172dbd

Browse files
committed
docs: add comprehensive Google-style docstrings to unified_pdp
Add complete Args, Returns, Raises, and Attributes documentation to all public functions and methods in the unified_pdp plugin, matching the project's docstring style with full parameter descriptions. Files updated: - adapter.py: PolicyEvaluationError, PolicyEngineAdapter methods - cache.py: _build_cache_key, _CacheEntry, DecisionCache methods - pdp.py: PolicyDecisionPoint and all evaluation/combination methods - engines/cedar_engine.py: CedarEngineAdapter and all methods - engines/mac_engine.py: MACEngineAdapter and all methods - engines/native_engine.py: NativeRBACAdapter and all methods - engines/opa_engine.py: OPAEngineAdapter and all methods - unified_pdp.py: shutdown lifecycle method Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
1 parent 9719d6d commit 6172dbd

File tree

8 files changed

+697
-124
lines changed

8 files changed

+697
-124
lines changed

plugins/unified_pdp/adapter.py

Lines changed: 47 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,17 @@
3232

3333

3434
class PolicyEvaluationError(Exception):
35-
"""Raised when an engine fails to evaluate a request (network, timeout, …)."""
35+
"""Raised when an engine fails to evaluate a request (network, timeout, etc.).
36+
37+
Args:
38+
engine: The engine type that raised the error.
39+
message: Human-readable error description.
40+
cause: Optional underlying exception that caused this error.
41+
42+
Attributes:
43+
engine: The engine type that raised the error.
44+
cause: The underlying exception, if any.
45+
"""
3646

3747
def __init__(self, engine: EngineType, message: str, cause: Exception | None = None):
3848
self.engine = engine
@@ -67,7 +77,11 @@ class PolicyEngineAdapter(ABC):
6777
@property
6878
@abstractmethod
6979
def engine_type(self) -> EngineType:
70-
"""Which engine this adapter represents."""
80+
"""Return the engine type identifier for this adapter.
81+
82+
Returns:
83+
EngineType enum value identifying which policy engine this adapter wraps.
84+
"""
7185

7286
# ------------------------------------------------------------------
7387
# Core evaluation
@@ -81,18 +95,20 @@ async def evaluate(
8195
resource: Resource,
8296
context: Context,
8397
) -> EngineDecision:
84-
"""Evaluate a single access request.
85-
86-
Returns
87-
-------
88-
EngineDecision
89-
The engine's verdict. ``duration_ms`` should be populated by the
90-
concrete implementation (wrap the inner call with a timer).
91-
92-
Raises
93-
------
94-
PolicyEvaluationError
95-
On any failure – the PDP catches this and records it.
98+
"""Evaluate a single access request against this policy engine.
99+
100+
Args:
101+
subject: The authenticated user/principal requesting access.
102+
action: The action being performed (e.g., "tools.invoke.db-query").
103+
resource: The resource being accessed (tool, prompt, server, etc.).
104+
context: Request context including IP, timestamp, session info.
105+
106+
Returns:
107+
EngineDecision containing the verdict (ALLOW/DENY), reason,
108+
matching policies, and timing information.
109+
110+
Raises:
111+
PolicyEvaluationError: On any failure (network, timeout, invalid response).
96112
"""
97113

98114
# ------------------------------------------------------------------
@@ -102,8 +118,16 @@ async def evaluate(
102118
async def get_permissions(self, subject: Subject, context: Context) -> List[Permission]:
103119
"""Return all permissions the subject holds according to this engine.
104120
105-
The default implementation returns an empty list. Engines that can
106-
enumerate permissions (e.g. Native RBAC) should override.
121+
The default implementation returns an empty list. Engines that can
122+
enumerate permissions (e.g., Native RBAC) should override this method.
123+
124+
Args:
125+
subject: The authenticated user/principal to enumerate permissions for.
126+
context: Request context for conditional permission evaluation.
127+
128+
Returns:
129+
List of Permission objects representing all granted permissions.
130+
Empty list if the engine does not support enumeration.
107131
"""
108132
return []
109133

@@ -114,9 +138,13 @@ async def get_permissions(self, subject: Subject, context: Context) -> List[Perm
114138
async def health_check(self) -> EngineHealthReport:
115139
"""Probe the engine and return its health status.
116140
117-
The default implementation issues a trivial ``evaluate`` call with
118-
dummy data and times it. Subclasses that have a cheaper probe
119-
(e.g. OPA's ``/health`` endpoint) should override.
141+
The default implementation issues a trivial evaluate() call with
142+
dummy data and times it. Subclasses that have a cheaper probe
143+
(e.g., OPA's /health endpoint) should override this method.
144+
145+
Returns:
146+
EngineHealthReport containing status (HEALTHY/UNHEALTHY),
147+
latency in milliseconds, and optional detail message on failure.
120148
"""
121149
import time
122150
from .pdp_models import EngineStatus

plugins/unified_pdp/cache.py

Lines changed: 109 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,23 @@
3434

3535

3636
def _build_cache_key(subject: Subject, action: str, resource: Resource, context: Context) -> str:
37-
"""Produce a stable, collision-resistant cache key.
37+
"""Produce a stable, collision-resistant cache key via SHA-256 hash.
3838
3939
Includes all context fields that could affect policy decisions:
4040
- ip, session_id, user_agent for request identification
4141
- extra dict for policy-relevant metadata (e.g., MAC operation override)
4242
4343
Excludes only the timestamp so requests within the same TTL window
4444
can hit the cache.
45+
46+
Args:
47+
subject: The authenticated user/principal requesting access.
48+
action: The action being performed (e.g., "tools.invoke.db-query").
49+
resource: The resource being accessed.
50+
context: Request context (IP, user_agent, session, extra metadata).
51+
52+
Returns:
53+
A 64-character hex string (SHA-256 hash) suitable as a cache key.
4554
"""
4655
payload = json.dumps(
4756
{
@@ -60,24 +69,61 @@ def _build_cache_key(subject: Subject, action: str, resource: Resource, context:
6069

6170

6271
class _CacheEntry:
63-
"""Thin wrapper that pairs a value with its expiry epoch."""
72+
"""Thin wrapper that pairs a cached decision with its expiry epoch.
73+
74+
Attributes:
75+
value: The cached AccessDecision object.
76+
expires_at: Monotonic timestamp when this entry expires.
77+
"""
6478

6579
__slots__ = ("value", "expires_at")
6680

6781
def __init__(self, value: AccessDecision, ttl_seconds: int):
82+
"""Initialize a cache entry with TTL-based expiration.
83+
84+
Args:
85+
value: The AccessDecision to cache.
86+
ttl_seconds: Time-to-live in seconds from now.
87+
"""
6888
self.value = value
6989
self.expires_at = time.monotonic() + ttl_seconds
7090

7191
@property
7292
def expired(self) -> bool:
73-
"""Return True if this cache entry has exceeded its TTL."""
93+
"""Check if this cache entry has exceeded its TTL.
94+
95+
Returns:
96+
True if current time exceeds expires_at, False otherwise.
97+
"""
7498
return time.monotonic() > self.expires_at
7599

76100

77101
class DecisionCache:
78-
"""Two-tier cache: in-memory LRU + optional async Redis."""
102+
"""Two-tier decision cache: in-memory LRU + optional async Redis.
103+
104+
The in-memory layer uses an OrderedDict for LRU eviction. When Redis is
105+
configured, it acts as a write-through second tier for multi-node sharing.
106+
107+
Args:
108+
config: Cache configuration (enabled, ttl_seconds, max_entries).
109+
redis_url: Optional Redis connection URL for distributed caching.
110+
111+
Attributes:
112+
_config: The cache configuration.
113+
_store: In-memory LRU cache as OrderedDict.
114+
_redis_url: Redis connection URL or None.
115+
_redis: Lazy-initialized async Redis client.
116+
_hits: Counter for cache hits.
117+
_misses: Counter for cache misses.
118+
"""
79119

80120
def __init__(self, config: CacheConfig, redis_url: Optional[str] = None):
121+
"""Initialize the decision cache.
122+
123+
Args:
124+
config: Cache configuration specifying TTL, max entries, and enabled state.
125+
redis_url: Optional Redis URL for distributed caching across nodes.
126+
"""
81127
self._config = config
82128
self._store: OrderedDict[str, _CacheEntry] = OrderedDict()
83129
self._redis_url = redis_url
@@ -91,7 +137,15 @@ def __init__(self, config: CacheConfig, redis_url: Optional[str] = None):
91137
# ------------------------------------------------------------------
92138

93139
async def _get_redis(self): # pragma: no cover – integration test only
94-
"""Lazily initialize and return the Redis client, or None if unavailable."""
140+
"""Lazily initialize and return the async Redis client.
141+
142+
Creates the Redis connection on first call if redis_url is configured.
143+
Falls back to memory-only caching if the redis package is not installed.
144+
145+
Returns:
146+
Async Redis client instance, or None if Redis is not configured
147+
or the redis package is unavailable.
148+
"""
95149
if self._redis is None and self._redis_url:
96150
try:
97151
import redis.asyncio as aioredis
@@ -114,7 +168,20 @@ async def get(
114168
resource: Resource,
115169
context: Context,
116170
) -> Optional[AccessDecision]:
117-
"""Look up a cached decision. Returns ``None`` on miss or expiry."""
171+
"""Look up a cached decision by request parameters.
172+
173+
Checks in-memory cache first, then falls through to Redis if configured.
174+
Expired entries are lazily evicted on access. Cache hits update LRU order.
175+
176+
Args:
177+
subject: The authenticated user/principal requesting access.
178+
action: The action being performed.
179+
resource: The resource being accessed.
180+
context: Request context (IP, user_agent, session, extra).
181+
182+
Returns:
183+
Cached AccessDecision if found and not expired, None otherwise.
184+
"""
118185
if not self._config.enabled:
119186
return None
120187

@@ -155,7 +222,18 @@ async def put(
155222
context: Context,
156223
decision: AccessDecision,
157224
) -> None:
158-
"""Store a decision. Evicts LRU entries when the cap is reached."""
225+
"""Store a decision in both memory and Redis (if configured).
226+
227+
Evicts least-recently-used entries when max_entries is reached.
228+
Writes to Redis with TTL for distributed cache sharing.
229+
230+
Args:
231+
subject: The authenticated user/principal requesting access.
232+
action: The action being performed.
233+
resource: The resource being accessed.
234+
context: Request context (IP, user_agent, session, extra).
235+
decision: The AccessDecision to cache.
236+
"""
159237
if not self._config.enabled:
160238
return
161239

@@ -184,9 +262,19 @@ async def invalidate(
184262
action: Optional[str] = None,
185263
resource: Optional[Resource] = None,
186264
) -> int:
187-
"""Invalidate matching entries. Pass ``None`` for any field to match all.
265+
"""Invalidate cached entries matching the given filters.
188266
189-
Returns the number of entries removed.
267+
Pass None for any field to match all entries. Currently flushes the
268+
entire cache when any filter is provided (targeted invalidation is
269+
planned for a future release).
270+
271+
Args:
272+
subject: Filter by subject, or None to match all subjects.
273+
action: Filter by action, or None to match all actions.
274+
resource: Filter by resource, or None to match all resources.
275+
276+
Returns:
277+
Number of entries removed from the in-memory cache.
190278
"""
191279
removed = 0
192280
keys_to_delete = []
@@ -226,7 +314,18 @@ async def invalidate(
226314
# ------------------------------------------------------------------
227315

228316
def stats(self) -> Dict[str, Any]:
229-
"""Return hit/miss counters and current size."""
317+
"""Return cache statistics for monitoring and debugging.
318+
319+
Returns:
320+
Dictionary containing:
321+
- hits: Number of cache hits.
322+
- misses: Number of cache misses.
323+
- hit_rate: Ratio of hits to total requests (0.0-1.0).
324+
- size: Current number of entries in memory.
325+
- max_entries: Maximum allowed entries.
326+
- ttl_seconds: Time-to-live for cached entries.
327+
- redis_enabled: Whether Redis backing is configured.
328+
"""
230329
total = self._hits + self._misses
231330
return {
232331
"hits": self._hits,

0 commit comments

Comments
 (0)