feat(deployments): Add per-deployment OpenShell backend config - #1200
feat(deployments): Add per-deployment OpenShell backend config#1200JashG wants to merge 6 commits into
Conversation
Signed-off-by: Jash Gulabrai <jgulabrai@nvidia.com>
Signed-off-by: Jash Gulabrai <jgulabrai@nvidia.com>
Signed-off-by: Jash Gulabrai <jgulabrai@nvidia.com>
|
Signed-off-by: Jash Gulabrai <jgulabrai@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughOpenShell deployments now support optional per-deployment sandbox policy files. The API schema, Pydantic models, backend policy selection, sandbox creation, and unit tests were updated. ChangesOpenShell policy configuration
Sequence Diagram(s)sequenceDiagram
participant DeploymentAPI
participant create_deployment
participant _build_deployment_policy
participant OpenShellSandbox
DeploymentAPI->>create_deployment: Submit optional policyPath
create_deployment->>_build_deployment_policy: Build effective policy
_build_deployment_policy-->>create_deployment: Return executor or deployment policy
create_deployment->>OpenShellSandbox: Create sandbox with effective policy
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/nemo-deployments/tests/unit/backends/openshell/test_backend.py (1)
303-325: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAssert preserved platform egress.
This assertion only proves that the override differs from the executor policy. Add a test with
platform_egressconfigured and assert that the effective policy contains that egress rule. A missinginject_platform_egresscall would otherwise pass this test.🤖 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 `@plugins/nemo-deployments/tests/unit/backends/openshell/test_backend.py` around lines 303 - 325, The test test_create_uses_per_deployment_policy_when_policy_path_set must verify preserved platform egress, not only that the policy differs from executor_policy. Configure platform_egress on the backend or relevant policy setup, then assert the effective spec.policy contains the expected egress rule after create_deployment, covering the inject_platform_egress behavior.
🤖 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.
Inline comments:
In
`@plugins/nemo-deployments/src/nemo_deployments_plugin/backends/openshell/backend.py`:
- Line 273: The policy path currently resolves on the backend host before
CreateSandbox, so it cannot reference files available only inside the sandbox
image. In
plugins/nemo-deployments/src/nemo_deployments_plugin/backends/openshell/backend.py
lines 273-273, change the policy flow to use a backend-accessible source or
deliver and load the policy after sandbox creation. In
plugins/nemo-deployments/openapi/openapi.yaml lines 1219-1225 and
plugins/nemo-deployments/src/nemo_deployments_plugin/entities.py lines 177-185,
document and align the supported policy source with the finalized runtime
contract, then regenerate the OpenAPI schema.
---
Nitpick comments:
In `@plugins/nemo-deployments/tests/unit/backends/openshell/test_backend.py`:
- Around line 303-325: The test
test_create_uses_per_deployment_policy_when_policy_path_set must verify
preserved platform egress, not only that the policy differs from
executor_policy. Configure platform_egress on the backend or relevant policy
setup, then assert the effective spec.policy contains the expected egress rule
after create_deployment, covering the inject_platform_egress behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: eb4b56ff-afb2-409f-8b8e-7ff05aa930d0
📒 Files selected for processing (4)
plugins/nemo-deployments/openapi/openapi.yamlplugins/nemo-deployments/src/nemo_deployments_plugin/backends/openshell/backend.pyplugins/nemo-deployments/src/nemo_deployments_plugin/entities.pyplugins/nemo-deployments/tests/unit/backends/openshell/test_backend.py
Signed-off-by: Jash Gulabrai <jgulabrai@nvidia.com>
Summary
This PR enables individual OpenShell deployments to specify a custom sandbox policy YAML (via
policyPath) instead of inheriting the executor-level default.Related Issue
Changes
openshell: OpenShellDeploymentConfig | NonetoDeploymentBackendConfigalongside existingdockerandk8sfieldsdel backend_configstub inOpenShellDeploymentBackend.create_deploymentwith parsing of theopenshellsection_build_deployment_policytoOpenShellDeploymentBackend: returns the executor-level policy unchanged when nopolicyPathis set, otherwise builds a fresh policy from the specified file with the executor's egress rule always re-injectedType of Change
Quality Gates
policyPathoverride path follows the same policy-building logic already covered bytest_openshell_policy.py; the no-override path is unchanged behaviorVerification
Signed-off-by:traileruv run pre-commit run -apasses, or any blocked checks are identified belowTargeted validation:
uv run --frozen pytest plugins/nemo-deployments/tests/unit/ -q
Summary by CodeRabbit
policyPath.