-
Notifications
You must be signed in to change notification settings - Fork 6
Add plan to implement INFP-504 #885
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| # Filter Interface Contracts | ||
|
|
||
| **Feature**: INFP-504 | **Date**: 2026-03-20 | ||
|
|
||
| ## Jinja2 Filter Signatures | ||
|
|
||
| ### artifact_content | ||
|
|
||
| ```python | ||
| async def artifact_content(storage_id: str) -> str | ||
| ``` | ||
|
|
||
| | Input | Output | Error | | ||
| | ----- | ------ | ----- | | ||
| | Valid storage_id string | Raw artifact content (text) | — | | ||
| | `None` | — | `JinjaFilterError("artifact_content", "storage_id is null", hint="...")` | | ||
| | `""` (empty) | — | `JinjaFilterError("artifact_content", "storage_id is empty", hint="...")` | | ||
| | Non-existent storage_id | — | `JinjaFilterError("artifact_content", "content not found: {id}")` | | ||
| | Permission denied (401/403) | — | `JinjaFilterError("artifact_content", "permission denied for storage_id: {id}")` | | ||
| | No client provided | — | `JinjaFilterError("artifact_content", "requires InfrahubClient", hint="pass client via Jinja2Template(client=...)")` | | ||
|
|
||
| **Validation**: Blocked in `CORE` context. Allowed in `WORKER` context. | ||
|
|
||
| ### file_object_content | ||
|
|
||
| ```python | ||
| async def file_object_content(storage_id: str) -> str | ||
| ``` | ||
|
|
||
| | Input | Output | Error | | ||
| | ----- | ------ | ----- | | ||
| | Valid storage_id (text file) | Raw file content (text) | — | | ||
| | Valid storage_id (binary file) | — | `JinjaFilterError("file_object_content", "binary content not supported for storage_id: {id}")` | | ||
| | `None` | — | `JinjaFilterError("file_object_content", "storage_id is null", hint="...")` | | ||
| | `""` (empty) | — | `JinjaFilterError("file_object_content", "storage_id is empty", hint="...")` | | ||
| | Non-existent storage_id | — | `JinjaFilterError("file_object_content", "content not found: {id}")` | | ||
| | Permission denied (401/403) | — | `JinjaFilterError("file_object_content", "permission denied for storage_id: {id}")` | | ||
| | No client provided | — | `JinjaFilterError("file_object_content", "requires InfrahubClient", hint="pass client via Jinja2Template(client=...)")` | | ||
|
|
||
| **Validation**: Blocked in `CORE` context. Allowed in `WORKER` context. | ||
|
|
||
| ### from_json | ||
|
|
||
| ```python | ||
| def from_json(value: str) -> dict | list | ||
| ``` | ||
|
|
||
| | Input | Output | Error | | ||
| | ----- | ------ | ----- | | ||
| | Valid JSON string | Parsed dict or list | — | | ||
| | `""` (empty) | `{}` | — | | ||
| | Malformed JSON | — | `JinjaFilterError("from_json", "invalid JSON: {error_detail}")` | | ||
|
|
||
| **Validation**: Allowed in all contexts (`ALL`). | ||
|
|
||
| ### from_yaml | ||
|
|
||
| ```python | ||
| def from_yaml(value: str) -> dict | list | ||
| ``` | ||
|
|
||
| | Input | Output | Error | | ||
| | ----- | ------ | ----- | | ||
| | Valid YAML string | Parsed dict, list, or scalar | — | | ||
| | `""` (empty) | `{}` | — | | ||
| | Malformed YAML | — | `JinjaFilterError("from_yaml", "invalid YAML: {error_detail}")` | | ||
|
|
||
| **Validation**: Allowed in all contexts (`ALL`). | ||
|
|
||
| ## ObjectStore API Contract | ||
|
|
||
| ### GET /api/storage/object/{identifier} (existing) | ||
|
|
||
| Used by `artifact_content`. Returns plain text content. | ||
|
|
||
| ### GET /api/files/by-storage-id/{storage_id} (new) | ||
|
|
||
| Used by `file_object_content`. Returns file content with appropriate content-type header. | ||
|
|
||
| **Accepted content-types** (text-based): | ||
|
|
||
| - `text/*` | ||
| - `application/json` | ||
| - `application/yaml` | ||
| - `application/x-yaml` | ||
|
|
||
| **Rejected**: All other content-types → `JinjaFilterError` with binary content message. | ||
|
|
||
| ## Validation Contract | ||
|
|
||
| ### validate() method | ||
|
|
||
| ```python | ||
| def validate( | ||
| self, | ||
| restricted: bool = True, | ||
| context: ExecutionContext | None = None, | ||
| ) -> None | ||
| ``` | ||
|
|
||
| | Context | Trusted filters | Worker filters | Untrusted filters | | ||
| | ------- | :-: | :-: | :-: | | ||
| | `CORE` | allowed | blocked | blocked | | ||
| | `WORKER` | allowed | allowed | blocked | | ||
| | `LOCAL` | allowed | allowed | allowed | | ||
|
|
||
| **Backward compat**: `restricted=True` → `CORE`, `restricted=False` → `LOCAL`. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,166 @@ | ||
| # Data Model: Artifact Content Composition | ||
|
|
||
| **Feature**: INFP-504 | **Date**: 2026-03-20 | ||
|
|
||
| ## New Entities | ||
|
|
||
| ### ExecutionContext (Flag enum) | ||
|
|
||
| **Location**: `infrahub_sdk/template/filters.py` | ||
|
|
||
| ```python | ||
| class ExecutionContext(Flag): | ||
| CORE = auto() # API server computed attributes — most restrictive | ||
| WORKER = auto() # Prefect background workers | ||
| LOCAL = auto() # Local CLI / unrestricted rendering | ||
| ALL = CORE | WORKER | LOCAL | ||
| ``` | ||
|
|
||
| **Semantics**: Represents where template code executes. A filter's `allowed_contexts` flags are an allowlist — fewer flags means less trusted. | ||
|
|
||
| ### FilterDefinition (modified) | ||
|
|
||
| **Location**: `infrahub_sdk/template/filters.py` | ||
|
|
||
| ```python | ||
| @dataclass | ||
| class FilterDefinition: | ||
| name: str | ||
| allowed_contexts: ExecutionContext | ||
| source: str | ||
|
|
||
| @property | ||
| def trusted(self) -> bool: | ||
| """Backward compatibility: trusted means allowed in all contexts.""" | ||
| return self.allowed_contexts == ExecutionContext.ALL | ||
| ``` | ||
|
|
||
| **Migration**: | ||
|
|
||
| | Current | New | | ||
| | ------- | --- | | ||
| | `FilterDefinition("abs", trusted=True, source="jinja2")` | `FilterDefinition("abs", allowed_contexts=ExecutionContext.ALL, source="jinja2")` | | ||
| | `FilterDefinition("safe", trusted=False, source="jinja2")` | `FilterDefinition("safe", allowed_contexts=ExecutionContext.LOCAL, source="jinja2")` | | ||
|
|
||
| ### JinjaFilterError (new exception) | ||
|
|
||
| **Location**: `infrahub_sdk/template/exceptions.py` | ||
|
|
||
| ```python | ||
| class JinjaFilterError(JinjaTemplateError): | ||
| def __init__(self, filter_name: str, message: str, hint: str | None = None) -> None: | ||
| self.filter_name = filter_name | ||
| self.hint = hint | ||
| full_message = f"Filter '{filter_name}': {message}" | ||
| if hint: | ||
| full_message += f" — {hint}" | ||
| super().__init__(full_message) | ||
| ``` | ||
|
|
||
| **Inheritance**: `Error` → `JinjaTemplateError` → `JinjaFilterError` | ||
|
|
||
| ### InfrahubFilters (new class) | ||
|
|
||
| **Location**: `infrahub_sdk/template/infrahub_filters.py` (new file) | ||
|
|
||
| ```python | ||
| class InfrahubFilters: | ||
| def __init__(self, client: InfrahubClient) -> None: | ||
| self.client = client | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure that it's correct, but the way I saw this was that a |
||
|
|
||
| async def artifact_content(self, storage_id: str) -> str: | ||
| """Retrieve artifact content by storage_id.""" | ||
| ... | ||
|
|
||
| async def file_object_content(self, storage_id: str) -> str: | ||
| """Retrieve file object content by storage_id.""" | ||
| ... | ||
| ``` | ||
|
|
||
| **Key design decisions**: | ||
|
|
||
| - Methods are `async` — Jinja2's `auto_await` handles them in async rendering mode | ||
| - Holds an `InfrahubClient` (async only), not `InfrahubClientSync` | ||
| - Each method validates inputs and catches `AuthenticationError` to wrap in `JinjaFilterError` | ||
|
|
||
| ## Modified Entities | ||
|
|
||
| ### Jinja2Template (modified constructor) | ||
|
|
||
| **Location**: `infrahub_sdk/template/__init__.py` | ||
|
|
||
| ```python | ||
| def __init__( | ||
| self, | ||
| template: str | Path, | ||
| template_directory: Path | None = None, | ||
| filters: dict[str, Callable] | None = None, | ||
| client: InfrahubClient | None = None, # NEW | ||
| ) -> None: | ||
| ``` | ||
|
|
||
| **Changes**: | ||
|
|
||
| - New optional `client` parameter | ||
| - When `client` provided: instantiate `InfrahubFilters`, register `artifact_content` and `file_object_content` | ||
| - Always register `from_json` and `from_yaml` (no client needed) | ||
| - File-based environment must add `enable_async=True` for async filter support | ||
|
|
||
| ### Jinja2Template.validate() (modified signature) | ||
|
|
||
| ```python | ||
| def validate(self, restricted: bool = True, context: ExecutionContext | None = None) -> None: | ||
| ``` | ||
|
|
||
| **Changes**: | ||
|
|
||
| - New optional `context` parameter (takes precedence over `restricted` when provided) | ||
| - Backward compat: `restricted=True` → `ExecutionContext.CORE`, `restricted=False` → `ExecutionContext.LOCAL` | ||
| - Validation logic: filter allowed if `filter.allowed_contexts & context` is truthy | ||
|
|
||
| ### ObjectStore (new method) | ||
|
|
||
| **Location**: `infrahub_sdk/object_store.py` | ||
|
|
||
| ```python | ||
| async def get_file_by_storage_id(self, storage_id: str, tracker: str | None = None) -> str: | ||
| """Retrieve file object content by storage_id. | ||
|
|
||
| Raises error if content-type is not text-based. | ||
| """ | ||
| ... | ||
| ``` | ||
|
|
||
| **API endpoint**: `GET /api/files/by-storage-id/{storage_id}` | ||
|
|
||
| **Content-type check**: Allow `text/*`, `application/json`, `application/yaml`, `application/x-yaml`. Reject all others. | ||
|
|
||
| ## New Filter Registrations | ||
|
|
||
| ```python | ||
| # In AVAILABLE_FILTERS: | ||
|
|
||
| # Infrahub client-dependent filters (worker context only) | ||
| FilterDefinition("artifact_content", allowed_contexts=ExecutionContext.WORKER, source="infrahub"), | ||
| FilterDefinition("file_object_content", allowed_contexts=ExecutionContext.WORKER, source="infrahub"), | ||
|
|
||
| # Parsing filters (trusted, all contexts) | ||
| FilterDefinition("from_json", allowed_contexts=ExecutionContext.ALL, source="infrahub"), | ||
| FilterDefinition("from_yaml", allowed_contexts=ExecutionContext.ALL, source="infrahub"), | ||
| ``` | ||
|
|
||
| ## Relationships | ||
|
|
||
| ```text | ||
| Jinja2Template | ||
| ├── has-a → InfrahubFilters (when client provided) | ||
| ├── uses → FilterDefinition registry (for validation) | ||
| └── uses → ExecutionContext (for context-aware validation) | ||
|
|
||
| InfrahubFilters | ||
| ├── has-a → InfrahubClient | ||
| └── uses → ObjectStore (for content retrieval) | ||
|
|
||
| JinjaFilterError | ||
| └── extends → JinjaTemplateError → Error | ||
| ``` | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| # Implementation Plan: Artifact Content Composition | ||
|
|
||
| **Branch**: `infp-504-artifact-composition` | **Date**: 2026-03-20 | **Spec**: [spec.md](spec.md) | ||
| **Jira**: INFP-504 | **Epic**: IFC-2275 | ||
|
|
||
| ## Summary | ||
|
|
||
| Enable Jinja2 templates to reference and inline rendered content from other artifacts and file objects via new filters (`artifact_content`, `file_object_content`, `from_json`, `from_yaml`). Requires evolving the filter trust model from a binary boolean to a flag-based execution context system, creating a new `InfrahubFilters` class to hold client-dependent filter logic, and extending `Jinja2Template` with an optional client parameter. | ||
|
|
||
| ## Technical Context | ||
|
|
||
| **Language/Version**: Python 3.10-3.13 | ||
| **Primary Dependencies**: jinja2, httpx, pydantic >=2.0, PyYAML (already available via netutils) | ||
| **Storage**: Infrahub object store (REST API) | ||
| **Testing**: pytest (`uv run pytest tests/unit/`) | ||
| **Target Platform**: SDK library consumed by Prefect workers, CLI, and API server | ||
| **Project Type**: Single Python package | ||
| **Constraints**: No new external dependencies. Must maintain async/sync dual pattern. Must not break existing filter behavior. | ||
|
|
||
| ## Key Technical Decisions | ||
|
|
||
| ### 1. Async Filters via Jinja2 native support (R-001) | ||
|
|
||
| The `SandboxedEnvironment` already uses `enable_async=True`. Jinja2's `auto_await` automatically awaits filter return values during `render_async()`. The new content-fetching filters can be `async def` — no bridging needed. | ||
|
|
||
| **Required change**: Add `enable_async=True` to the file-based environment (`_get_file_based_environment()`) so async filters work for file-based templates too. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This “required change” appears already implemented in the current SDK.
🤖 Prompt for AI Agents |
||
|
|
||
| ### 2. Flag-based trust model (R-004) | ||
|
|
||
| Replace `FilterDefinition.trusted: bool` with `allowed_contexts: ExecutionContext` using Python's `Flag` enum. Three contexts: `CORE` (most restrictive), `WORKER`, `LOCAL` (least restrictive). A backward-compatible `trusted` property preserves existing API. | ||
|
|
||
| ### 3. Content-type checking for file objects (R-003) | ||
|
|
||
| New `ObjectStore.get_file_by_storage_id()` method checks response `content-type` header. Text-based types are allowed; binary types are rejected with a descriptive error. | ||
|
|
||
| ## Project Structure | ||
|
|
||
| ### Documentation (this feature) | ||
|
|
||
| ```text | ||
| dev/specs/infp-504-artifact-composition/ | ||
| ├── spec.md # Feature specification | ||
| ├── plan.md # This file | ||
| ├── research.md # Phase 0 research findings | ||
| ├── data-model.md # Entity definitions | ||
| ├── quickstart.md # Usage examples | ||
| ├── contracts/ | ||
| │ └── filter-interfaces.md # Filter I/O contracts | ||
| └── checklists/ | ||
| └── requirements.md # Quality checklist | ||
| ``` | ||
|
|
||
| ### Source Code (files to create or modify) | ||
|
|
||
| ```text | ||
| infrahub_sdk/ | ||
| ├── template/ | ||
| │ ├── __init__.py # MODIFY: Jinja2Template (client param, validate context) | ||
| │ ├── filters.py # MODIFY: ExecutionContext enum, FilterDefinition migration | ||
| │ ├── exceptions.py # MODIFY: Add JinjaFilterError | ||
| │ └── infrahub_filters.py # CREATE: InfrahubFilters class | ||
| ├── object_store.py # MODIFY: Add get_file_by_storage_id() | ||
| ``` | ||
|
|
||
| ```text | ||
| tests/unit/ | ||
| ├── template/ | ||
| │ ├── test_filters.py # MODIFY: Tests for new filters and trust model | ||
| │ └── test_infrahub_filters.py # CREATE: Tests for InfrahubFilters | ||
| ``` | ||
|
|
||
| ## Implementation Order | ||
|
|
||
| The 13 Jira tasks under IFC-2275 follow this dependency graph: | ||
|
|
||
| ```text | ||
| Phase 1 (Foundation — no dependencies, can be parallel): | ||
| IFC-2367: JinjaFilterError exception | ||
| IFC-2368: Flag-based trust model (ExecutionContext + FilterDefinition migration) | ||
| IFC-2373: ObjectStore.get_file_by_storage_id() | ||
|
|
||
| Phase 2 (Filters — depend on Phase 1): | ||
| IFC-2369: from_json filter (depends on IFC-2367) | ||
| IFC-2370: from_yaml filter (depends on IFC-2367) | ||
| IFC-2371: InfrahubFilters class (depends on IFC-2367) | ||
|
|
||
| Phase 3 (Content filters — depend on Phase 2): | ||
| IFC-2372: artifact_content filter (depends on IFC-2371) | ||
| IFC-2374: file_object_content filter (depends on IFC-2371, IFC-2373) | ||
|
|
||
| Phase 4 (Integration — depend on Phase 3): | ||
| IFC-2375: Jinja2Template client param + wiring (depends on IFC-2368, IFC-2371, IFC-2372) | ||
| IFC-2376: Filter registration with correct contexts (depends on IFC-2368, IFC-2369, IFC-2370, IFC-2372, IFC-2374) | ||
|
|
||
| Phase 5 (Documentation + Server — depend on Phase 4): | ||
| IFC-2377: Documentation (depends on IFC-2376) | ||
| IFC-2378: integrator.py threading [Infrahub server] (depends on IFC-2375) | ||
| IFC-2379: Schema validation [Infrahub server] (depends on IFC-2368) | ||
| ``` | ||
|
|
||
| ## Risk Register | ||
|
|
||
| | Risk | Likelihood | Impact | Mitigation | | ||
| | ---- | --------- | ------ | ---------- | | ||
| | Jinja2 `auto_await` doesn't work as expected for filters | Low | High | Verify with a minimal test before building on the assumption. Fallback: sync wrapper with thread executor. | | ||
| | File-based environment breaks with `enable_async=True` | Low | Medium | File-based env change is isolated and testable. Existing tests will catch regressions. | | ||
| | ObjectStore API returns incorrect content-type for file objects | Medium | Low | Already flagged by @wvandeun. The filter will use best-effort content-type checking; can be refined when API is fixed. | | ||
| | `validate()` backward compat breaks existing callers | Low | High | Keep `restricted` param with deprecation path. Test all existing call sites. | | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validation matrix contradicts WORKER-only filter policy.
This table allows “Worker filters” in
LOCAL, which conflicts with the spec’s WORKER-only rule forartifact_content/file_object_content. Please align the matrix to one policy across docs.🤖 Prompt for AI Agents