feat(jobs): auth e2e testing and http_client DI

[ironcommit]

Allow callers of get_platform_sdk, get_task_sdk, and PlatformSDKProvider to pass an explicit sync httpx.Client, enabling task code to be tested in-process without spawning a real HTTP server.

Add integration tests in services/core/jobs that verify the full NMP_PRINCIPAL → on-behalf-of auth propagation path: a task using get_task_sdk can access secrets as the job creator, and is correctly denied when the creator lacks permission. Register standard pytest markers in the jobs pyproject.toml to support these test categories.

Summary by CodeRabbit

Release Notes

• New Features

  • SDK factory and provider now support optional dependency-injected synchronous HTTP clients for both platform and task SDK creation.

• Tests

  • Added an integration test covering task-side authentication propagation and delegated access behavior.
  • Added a unit test verifying explicit HTTP client injection is preserved.
  • Introduced custom pytest markers for clearer test categorization.

• Documentation

  • Updated inline guidance on the HTTP client injection and migration path.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds an optional http_client: httpx.Client | None parameter to get_platform_sdk, get_task_sdk, and both matching PlatformSDKProvider methods. When provided, the injected client replaces the shared sync client. A unit test and two integration tests validate the injection path and task-side auth propagation.

Changes

Sync HTTP Client Injection

Layer / File(s)	Summary
SDK factory signature and implementation packages/nmp_common/src/nmp/common/sdk_factory.py	get_platform_sdk and get_task_sdk gain http_client: httpx.Client | None = None; get_platform_sdk resolves http_client or shared_sync_http_client() when constructing NeMoPlatform; get_task_sdk forwards the parameter; module-level comment updated.
PlatformSDKProvider wiring packages/nmp_common/src/nmp/common/sdk_factory.py	PlatformSDKProvider.get_task_sdk and get_platform_sdk updated to accept and pass through the optional http_client.
Unit test for sync client injection packages/nmp_common/tests/sdk_factory/test_sdk.py	test_get_task_sdk_uses_explicit_sync_http_client passes a fastapi.testclient.TestClient into get_task_sdk and asserts sdk._client is client.
Integration tests for task auth runtime services/core/jobs/pyproject.toml, services/core/jobs/tests/integration/test_task_auth_runtime.py	TestTaskRuntimeAuthPropagation exercises get_task_sdk with NMP_PRINCIPAL set, verifying on_behalf_of propagation in the access log for both authorized and permission-denied cases; pytest markers added to pyproject.toml.
Testing client comment update packages/nmp_testing/src/nmp/testing/client.py	Comment updated to reference both get_async_platform_sdk() and get_platform_sdk() injection paths.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 46.15% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	Title mentions both http_client DI and auth e2e testing, but the summarized changes show http_client DI is equally significant to the testing—it's the core enabler. The PR objectives confirm a reviewer noted the title underemphasizes the DI mechanism.	Consider whether 'http_client DI and auth e2e testing' better reflects the dual contribution of enabling dependency injection and adding integration tests.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch auth-tests/rsadler

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

packages/nmp_common/src/nmp/common/sdk_factory.py (1)

134-151: Add keyword-only marker to align with provider interface.

The module-level get_platform_sdk lacks the * keyword-only marker that its provider implementations already have. While the codebase uses only keyword arguments (no breaking calls exist), enforce consistency:

Fix

def get_platform_sdk(
    as_service: str | None = None,
    internal: bool = False,
+    *,
    http_client: httpx.Client | None = None,
    on_behalf_of: str | Principal | None = None,
) -> NeMoPlatform:

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/nmp_common/src/nmp/common/sdk_factory.py` around lines 134 - 151,
Add a `*` keyword-only marker to the `get_platform_sdk` function signature
immediately after the opening parenthesis to enforce that all parameters
(as_service, internal, http_client, on_behalf_of) must be passed as keyword
arguments. This aligns the module-level function with its provider interface
implementations that already enforce keyword-only arguments.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/nmp_common/src/nmp/common/sdk_factory.py`:
- Around line 134-151: Add a `*` keyword-only marker to the `get_platform_sdk`
function signature immediately after the opening parenthesis to enforce that all
parameters (as_service, internal, http_client, on_behalf_of) must be passed as
keyword arguments. This aligns the module-level function with its provider
interface implementations that already enforce keyword-only arguments.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 368139f3-12b1-4f44-b36c-eaa0ada2f844

📥 Commits

Reviewing files that changed from the base of the PR and between d36974f and 24ae7a2.

📒 Files selected for processing (5)

• packages/nmp_common/src/nmp/common/sdk_factory.py
• packages/nmp_common/tests/sdk_factory/test_sdk.py
• packages/nmp_testing/src/nmp/testing/client.py
• services/core/jobs/pyproject.toml
• services/core/jobs/tests/integration/test_task_auth_runtime.py

[mckornfield]

kind of a nit on the PR title, this is an integration test, not e2e, yeah?

[github-actions]

Suite	Lines Covered	Line Rate	Branch Rate
Unit Tests	20036/26510	75.6%	60.9%
Integration Tests	11692/25282	46.2%	19.8%

[mckornfield]

also might be worth including in the pr title the http client DI you added