fix(server): notify watchers after atomic policy commits - #1
Open
NaveCohenMonday wants to merge 20 commits into
Open
fix(server): notify watchers after atomic policy commits#1NaveCohenMonday wants to merge 20 commits into
NaveCohenMonday wants to merge 20 commits into
Conversation
Signed-off-by: Evan Lezar <elezar@nvidia.com>
NaveCohenMondayforce-pushed
the
2517-notify-atomic-policy-watchers/nc
branch
from
July 28, 2026 15:40
4674448 to
b9917c4CompareReplace yield_now() spin-loop with sleep(10ms) polling in delete_handler_ends_telemetry_for_the_resolved_sandbox_id. The single-threaded tokio runtime starves the spawn_blocking threads used by SQLite when yield_now() burns 100% CPU waiting for the delete gate entry count. Increase the timeout from 1s to 5s for consistency with similar guard tests. Signed-off-by: Derek Carr <decarr@redhat.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
* feat(examples): add supervisor middleware content guard Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * docs(examples): refine middleware preview warning Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(examples): add middleware policy version Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * refactor(supervisor-middleware): simplify service endpoints Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(examples): adapt content guard to middleware enums Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(examples): align content guard with merged middleware Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * feat(examples): add content guard smoke flow Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * chore(examples): remove smoke launcher test Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(examples): align content guard smoke with main Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(examples): address content guard review feedback Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * refactor(examples): parse cargo metadata with jq Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(examples): prioritize longest content matches Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * docs(examples): use GitHub warning alert Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(examples): merge overlapping content matches Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * docs(examples): render preview warning on GitHub Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> --------- Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
…#2463) The Kubernetes driver's default workspace PVC never set storageClassName, so on clusters with no default StorageClass the PVC stayed Pending and sandbox creation failed. Add a workspace_storage_class option to KubernetesComputeConfig, wired through SandboxPodParams into the generated volumeClaimTemplates. When non-empty it sets storageClassName; empty preserves the current behavior of relying on the cluster default StorageClass. Expose it via the OPENSHELL_K8S_WORKSPACE_STORAGE_CLASS env var on both the standalone driver and the embedded gateway runtime defaults, and via the server.workspaceStorageClass Helm value. ClosesNVIDIA#2442 Signed-off-by: lr90 <qiuweimin@matrixorigin.cn>
* feat(sandbox): use policy-first OCI image identity ClosesNVIDIA#2331 Preserve per-field policy omission, derive Docker and Podman fallbacks from the inspected immutable image, and resolve the final numeric identity before starting agent children. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fix(sandbox): preserve declared process identities Keep explicit policy values and OCI-declared names intact, defer passwd lookup until a primary GID is required, and refresh stale policy examples. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fix(supervisor): reuse resolved OCI identity Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fix(supervisor): allow Linux pre-exec arguments Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fix(kubernetes): protect resolved sandbox identity Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fix(sandbox): prepare workspace for OCI identity Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * refactor(sandbox): own only workspace root Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fix(sandbox): harden partial identity drops Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * test(sandbox): scope OCI image e2e to Docker Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fix(sandbox): narrow OCI identity fallback scope Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * test(podman): cover OCI identity launch Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fix(podman): exercise OCI fallback in E2E Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> --------- Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
…#2446) parse_duration_to_ms was moved to commands/common.rs since the original PR was opened, but it still split the last byte of the input with split_at(s.len() - 1), which panics when the final character is multi-byte UTF-8 (e.g. 'openshell logs my-sandbox --since 5€'). Split off the last character using its UTF-8 length instead, so invalid units surface the intended 'unknown duration unit' error. Add regression tests in commands/common.rs. Signed-off-by: Andrew White <andrewh@cdw.com>
Bumps [docker/login-action](https://github.com/docker/login-action) from 4.5.1 to 4.5.2. - [Release notes](https://github.com/docker/login-action/releases) - [Commits](docker/login-action@abd2ef4...371161b) --- updated-dependencies: - dependency-name: docker/login-action dependency-version: 4.5.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…ivers (NVIDIA#2153) * refactor(compute): extract create_sandbox_record and update_sandbox_record helpers Split apply_sandbox_update_locked into named helpers to make the two distinct paths explicit: create_sandbox_record for first-observation events and update_sandbox_record for subsequent driver snapshots on existing sandboxes. The dispatcher now uses a match on the existing record rather than an early-return guard. No behavior change. Signed-off-by: Evan Lezar <elezar@nvidia.com> * refactor(compute): make sandbox readiness gateway-owned across all drivers Introduce compute_phase_components and apply_readiness_conditions to centralise the gateway's phase composition logic. The public SandboxPhase is now determined by combining the backend phase reported by the driver with supervisor session presence, independent of the driver implementation. Remove SupervisorReadiness from the driver contract. Running containers always report BackendReady; the gateway owns the Ready decision. Rename the dispatcher match to three arms so that status-less events for existing sandboxes are a documented no-op rather than a silent pass-through. Drop backend_ready_no_session and the SupervisorNotConnected condition. The BackendReady driver condition plus the Provisioning phase already communicates that the backend is up but the supervisor has not connected. The redundant condition added noise without new information. ClosesNVIDIA#1951 Signed-off-by: Evan Lezar <elezar@nvidia.com> * fix(compute): expose disconnected supervisor readiness Signed-off-by: Drew Newberry <anewberry@nvidia.com> * docs(sandboxes): clarify supervisor readiness lifecycle Signed-off-by: Evan Lezar <elezar@nvidia.com> --------- Signed-off-by: Evan Lezar <elezar@nvidia.com> Signed-off-by: Drew Newberry <anewberry@nvidia.com> Co-authored-by: Drew Newberry <anewberry@nvidia.com>
Bumps [actions/stale](https://github.com/actions/stale) from 10.4.0 to 11.0.0. - [Release notes](https://github.com/actions/stale/releases) - [Changelog](https://github.com/actions/stale/blob/main/CHANGELOG.md) - [Commits](actions/stale@1e223db...4391f3d) --- updated-dependencies: - dependency-name: actions/stale dependency-version: 11.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…DIA#2523) The subprocess-based integration tests inherit the full parent environment. If the developer has OPENSHELL_GATEWAY_INSECURE=true set in their shell, it leaks into the spawned CLI process and causes it to connect with .with_no_client_auth(), skipping the mTLS client certificate. The test server requires mTLS, so it responds with CertificateRequired and the test fails. Strip OPENSHELL_GATEWAY_INSECURE, OPENSHELL_GATEWAY, OPENSHELL_GATEWAY_ENDPOINT, and OPENSHELL_WORKSPACE from the subprocess environment. The test already sets --gateway and --gateway-endpoint explicitly via CLI args, so these env vars should not influence the subprocess behavior. Reported-by: Seth Jennings Signed-off-by: Roland Huß <rhuss@redhat.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
ForwardSpec accepts IPv6 bind addresses, but ssh_forward_arg() emitted them unbracketed (e.g. ::1:8080:127.0.0.1:8080), which OpenSSH rejects as a bad local forwarding specification. access_url() likewise produced invalid URLs like http://::1:8080/. Extract a shared bracket_ipv6_host() helper and use it in ssh_forward_arg(), access_url() (via format_gateway_url), and format_gateway_url() so IPv6 literals are bracketed consistently. FixesNVIDIA#2279 Signed-off-by: Russell Bryant <rbryant@redhat.com>
* test(vm): add composable QEMU test guests Signed-off-by: Drew Newberry <anewberry@nvidia.com> * docs(vm): describe test VM directory structure Signed-off-by: Drew Newberry <anewberry@nvidia.com> * refactor(vm): replace shell catalog functions Signed-off-by: Drew Newberry <anewberry@nvidia.com> * test(vm): add Fedora release guest support Signed-off-by: Drew Newberry <anewberry@nvidia.com> * test(vm): enable rootless Podman socket Signed-off-by: Drew Newberry <anewberry@nvidia.com> * feat(test-guest): add OCI-backed image caching Signed-off-by: Drew Newberry <anewberry@nvidia.com> * perf(test-guest): accelerate cached guest startup Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(test-guest): address review feedback Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(test-guest): verify OCI cache provenance Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(test-guest): harden cached guest reuse Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(test-guest): refresh runtime setup state Signed-off-by: Drew Newberry <anewberry@nvidia.com> * feat(test-guest): support E2E runner inputs Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(test-guest): harden runner and OCI reuse Signed-off-by: Drew Newberry <anewberry@nvidia.com> * feat(test-guest): prepare Podman E2E artifacts Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(test-guest): address review findings Signed-off-by: Drew Newberry <anewberry@nvidia.com> * revert(test-guest): remove recent Podman artifact changes Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(test-guest): canonicalize scp source paths Signed-off-by: Drew Newberry <anewberry@nvidia.com> * refactor(test-guest): provision artifacts with Ansible Signed-off-by: Drew Newberry <anewberry@nvidia.com> * feat(test-guest): populate missing caches on startup Signed-off-by: Drew Newberry <anewberry@nvidia.com> --------- Signed-off-by: Drew Newberry <anewberry@nvidia.com>
* feat(auth): implement RFC 0011 Phase 2 workspace authorization Signed-off-by: Derek Carr <decarr@redhat.com> * fix(auth): address PR review feedback on workspace authorization - Docker e2e: add --health-port and switch readiness probe from `openshell status` to `curl /healthz`, fixing a false-positive readiness check in OIDC mode where the CLI exited 0 without actually contacting the gateway - ListWorkspaces: move membership filtering from post-query N+1 lookups into a SQL EXISTS subquery so pagination applies to the visible set, not the global ordering. Add generic list_with_membership to the persistence layer. - Descriptor validator: reject role/scope fields on unauthenticated and sandbox auth modes, and allow-list workspace_role as user/admin and global_role as platform_admin to catch typos at startup Signed-off-by: Derek Carr <decarr@redhat.com> * fix(server): use authed request in delete telemetry test The workspace authorization added by the Phase 2 auth changes requires a Principal on every delete request. The delete-telemetry test was still using a bare Request::new, so extract_principal failed before the handler could acquire the delete gate, causing a 5-second timeout flake. Signed-off-by: Derek Carr <decarr@redhat.com> * fix(auth): address gator review findings for workspace authorization - Inject unauthenticated-local-dev principal in no-auth gateway mode so handlers that call extract_principal() always find one. - Cap label-selector membership query at MAX_PAGE_SIZE instead of u32::MAX to bound the in-memory read. - Authorize workspace membership before resolving workspace existence in all sandbox RPCs to prevent workspace-name enumeration by non-members. - Remove dead_code allow on AuthorizedWorkspace.workspace now that callers use the normalized name from the authz result. Signed-off-by: Derek Carr <decarr@redhat.com> * fix(auth): close workspace-name oracle and label-selector truncation Swap authorize-before-resolve ordering in 27 handlers across provider.rs, service.rs, policy.rs, and workspace.rs to prevent CWE-203 workspace-name enumeration by non-members. Add combined membership+label SQL query (list_with_membership_and_selector) to both persistence backends so ListWorkspaces with label selectors no longer silently drops results beyond the first page of membership matches. Signed-off-by: Derek Carr <decarr@redhat.com> * test(auth): add non-member rejection and membership+label persistence tests Add comprehensive test coverage for workspace authorization changes: - Non-member rejection tests across all 44 workspace-scoped handlers (sandbox, provider, service, policy, workspace, inference) verifying PERMISSION_DENIED is returned instead of NOT_FOUND to prevent CWE-203 workspace-name oracle - Persistence test for list_with_membership_and_selector verifying SQL-level membership EXISTS + label filtering, multiple predicates, no-match cases, and pagination Signed-off-by: Derek Carr <decarr@redhat.com> * fix(auth): format merged import line in sandbox tests Signed-off-by: Derek Carr <decarr@redhat.com> * fix(auth): address gator re-review findings on workspace authorization - Fix TUI unconditionally setting providers_v2_enabled after provider refresh; read the actual gateway setting via GetGatewayConfig at startup instead - Fix SQLite json_extract with dotted label keys (e.g. example.com/env) by quoting the key in the JSON path - Add authed_request wrappers to upstream OCI identity tests that were missing a principal after rebase - Add test proving GetGatewayConfig is accessible without Platform Admin - Add test for dotted/prefixed Kubernetes-style label key filtering Signed-off-by: Derek Carr <decarr@redhat.com> * fix(auth): address second gator re-review findings - Loosen GetGatewayConfig from platform_admin to scope-only so workspace users can discover providers_v2_enabled during sandbox creation with inferred-provider commands; update proto descriptor, descriptor validation, and RFC 0011 access table - Add validate_label_selector to handle_list_workspaces and escape single quotes in SQLite json_extract interpolation (CWE-89 defense-in-depth) - Re-fetch providers_v2_enabled after TUI gateway switch so the new gateway's capability is reflected - Add e2e test for workspace user with inferred-provider command - Add persistence test for adversarial label keys with SQL injection attempts - Add handler test for invalid label selector rejection in ListWorkspaces Signed-off-by: Derek Carr <decarr@redhat.com> * fix(auth): address third gator review findings - Cap label selector pairs at 64 (CWE-400) to bound SQLite dynamic SQL - Add SCOPE_ONLY_METHODS allowlist for scope-without-role RPCs (CWE-863) - Normalize ID-based data-plane handlers to return NOT_FOUND for unauthorized sandboxes, closing the cross-workspace oracle (CWE-203) - Fix TUI provider profile cache lookup key mismatch for legacy providers with empty profile_workspace - Add whoami to CLI skill reference command tree - Update TUI skill doc with workspace, provider, and settings coverage - Document scope/workspace orthogonality on GetGatewayConfig proto Signed-off-by: Derek Carr <decarr@redhat.com> * fix(auth): extend CWE-203 normalization to policy.rs sandbox handlers GetSandboxConfig and GetSandboxLogs in policy.rs had the same fetch-before-authorize pattern that leaked cross-workspace sandbox existence. Promote fetch_and_authorize_sandbox to pub(super) and use it from both sandbox.rs and policy.rs handlers. Signed-off-by: Derek Carr <decarr@redhat.com> * test(auth): update assertions for CWE-203 sandbox ID normalization Cross-workspace sandbox access via ID-based handlers now returns NOT_FOUND instead of PERMISSION_DENIED to prevent existence inference. Update the unit test and OIDC e2e assertion to match. Signed-off-by: Derek Carr <decarr@redhat.com> * fix(auth): narrow CWE-203 error mapping and correct whoami output formats Only remap PERMISSION_DENIED to NOT_FOUND in fetch_and_authorize_sandbox and RevokeSshSession, letting INTERNAL and UNAUTHENTICATED propagate as-is. Fix whoami --output format values in cli-reference.md to match the actual CLI (table/json/yaml, not text/json). Signed-off-by: Derek Carr <decarr@redhat.com> * fix(ci): share network namespace with Keycloak in containerized CI In GitHub Actions job containers, Docker port publishing lands on the host, not inside the job container. Detect this environment and attach Keycloak to the job container's network namespace instead, with hardened defaults (cap-drop ALL, no-new-privileges, loopback-only listener). Signed-off-by: Derek Carr <decarr@redhat.com> --------- Signed-off-by: Derek Carr <decarr@redhat.com>
* fix(gator): preserve resolved review feedback Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(gator): make follow-up reviews pragmatic Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(gator): enforce review convergence Signed-off-by: Drew Newberry <anewberry@nvidia.com> --------- Signed-off-by: Drew Newberry <anewberry@nvidia.com>
* test(vm): add composable QEMU test guests Signed-off-by: Drew Newberry <anewberry@nvidia.com> * docs(vm): describe test VM directory structure Signed-off-by: Drew Newberry <anewberry@nvidia.com> * refactor(vm): replace shell catalog functions Signed-off-by: Drew Newberry <anewberry@nvidia.com> * test(vm): add Fedora release guest support Signed-off-by: Drew Newberry <anewberry@nvidia.com> * test(vm): enable rootless Podman socket Signed-off-by: Drew Newberry <anewberry@nvidia.com> * feat(test-guest): add OCI-backed image caching Signed-off-by: Drew Newberry <anewberry@nvidia.com> * perf(test-guest): accelerate cached guest startup Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(test-guest): address review feedback Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(test-guest): verify OCI cache provenance Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(test-guest): harden cached guest reuse Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(test-guest): refresh runtime setup state Signed-off-by: Drew Newberry <anewberry@nvidia.com> * feat(test-guest): support E2E runner inputs Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(test-guest): harden runner and OCI reuse Signed-off-by: Drew Newberry <anewberry@nvidia.com> * feat(test-guest): prepare Podman E2E artifacts Signed-off-by: Drew Newberry <anewberry@nvidia.com> * feat(e2e): add host and test VM runner Signed-off-by: Drew Newberry <anewberry@nvidia.com> * test(e2e): add VM-backed Podman shutdown suite Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(e2e): use renamed test guest app Signed-off-by: Drew Newberry <anewberry@nvidia.com> * chore(e2e): rename runner task Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(e2e): make guest smoke examples portable Signed-off-by: Drew Newberry <anewberry@nvidia.com> * test(e2e): remove Podman shutdown suite Signed-off-by: Drew Newberry <anewberry@nvidia.com> * refactor(e2e): use test guest Podman setup Signed-off-by: Drew Newberry <anewberry@nvidia.com> * refactor(e2e): run Rust suites directly Signed-off-by: Drew Newberry <anewberry@nvidia.com> * refactor(e2e): simplify suite runner Signed-off-by: Drew Newberry <anewberry@nvidia.com> * refactor(e2e): isolate runner runtime state Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(e2e): prepare Podman VM runtime Signed-off-by: Drew Newberry <anewberry@nvidia.com> * perf(e2e): speed up cached guest startup Signed-off-by: Drew Newberry <anewberry@nvidia.com> * perf(e2e): streamline guest gateway startup Signed-off-by: Drew Newberry <anewberry@nvidia.com> --------- Signed-off-by: Drew Newberry <anewberry@nvidia.com>
* refactor(server): isolate gateway listener context Signed-off-by: Evan Lezar <elezar@nvidia.com> * refactor(server): preserve gateway listener binding Signed-off-by: Evan Lezar <elezar@nvidia.com> * fix(server): preserve covered listener callback scope Signed-off-by: Evan Lezar <elezar@nvidia.com> * refactor(server): reuse gateway listener spec Signed-off-by: Evan Lezar <elezar@nvidia.com> * ci(e2e): pin rootless podman packages Signed-off-by: Drew Newberry <anewberry@nvidia.com> --------- Signed-off-by: Evan Lezar <elezar@nvidia.com> Signed-off-by: Drew Newberry <anewberry@nvidia.com> Co-authored-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Nave Cohen <nave@monday.com>
NaveCohenMondayforce-pushed
the
2517-notify-atomic-policy-watchers/nc
branch
from
July 30, 2026 10:18
b9917c4 to
2870236CompareNaveCohenMonday pushed a commit
that referenced
this pull request
Aug 9, 2026
…VIDIA#2271) * feat(sdk/go): add Go SDK foundation, types, and sandbox client (A) Add the Go SDK module with the full API contract and a working sandbox client as the first vertical slice. All other resource clients are present as stubs returning Unimplemented errors, to be replaced with real implementations in subsequent PRs. Contents: - Module setup (go.mod, Makefile, mise.toml) - All domain types (types/ package) - Full ClientInterface with all sub-client accessors - Shared infrastructure (errors, auth, gRPC connection, logging) - Sandbox client with converter and tests (fully functional) - Stub clients for remaining resources (exec, file, health, provider, profile, config, refresh, policy, service, ssh, tcp) Part of the Go SDK decomposition plan (NVIDIA#2270). Implements NVIDIA#2044. * fix(sdk/go): address review feedback on PR NVIDIA#2271 - Make scheme parsing drive transport selection: http:// uses plaintext gRPC, https:// or no scheme uses TLS. Add regression tests. - Add Resources and DriverConfig fields to SandboxTemplate and update both converter directions (SandboxFromProto/SandboxSpecToProto). - Regenerate proto bindings from current canonical proto sources to eliminate drift (SigV4/MCP fields, params matchers, reserved fields). - Run gofmt/goimports on all handwritten Go files. Signed-off-by: Roland Huß <rhuss@redhat.com> * fix(sdk/go): address principal engineer review findings - Remove dead boolCount function that would fail golangci-lint (#1) - Emit EventAdded for the first watch event instead of EventModified, matching k8s watch semantics (NVIDIA#7) - Add mutex locking to all mock server methods that access the shared sandboxes map, fixing latent race conditions (NVIDIA#12) - Skip HealthCheck integration test that calls an unimplemented stub (NVIDIA#13) - Scope doc.go examples: mark sections for sub-clients not yet available in this PR with "available in a future release" (NVIDIA#4) - Document Config.Timeout/RetryPolicy/Logger and WatchOptions fields as reserved for future use (#2, NVIDIA#6) Signed-off-by: Roland Huß <rhuss@redhat.com> * refactor(sdk/go): migrate mise config to centralized task include Move Go SDK mise configuration from standalone sdk/go/mise.toml into the project's centralized pattern: - Add Go tools (go, golangci-lint, protoc-gen-go, protoc-gen-go-grpc) to root mise.toml [tools] section - Create tasks/go.toml with all SDK tasks using go: namespace prefix and dir=sdk/go for working directory - Update sdk/go/Makefile to reference namespaced task names - Update proto:sync default path for monorepo layout Addresses review feedback from drew on PR NVIDIA#2271 regarding mise convention alignment. Signed-off-by: Roland Huß <rhuss@redhat.com> * refactor(sdk/go): remove UPSTREAM_VERSION standalone repo artifact Remove sdk/go/proto/UPSTREAM_VERSION file and its exclusion from proto:check. This was a leftover from the standalone repo prototype. In a monorepo, proto drift is detectable via git diff between sdk/go/proto/ and proto/ directly. Signed-off-by: Roland Huß <rhuss@redhat.com> * refactor(sdk/go): switch proto generation from protoc to buf Replace raw protoc invocations with buf for Go SDK proto code generation, aligning with the TS SDK approach (PR NVIDIA#2122). - Add repo-level buf.yaml declaring proto/ as the buf module with lint and breaking change detection config - Add sdk/go/buf.gen.yaml configuring buf to generate Go code directly from root proto/ (no more vendored .proto copies) - Delete vendored .proto source files from sdk/go/proto/ - Rewrite go:proto:gen and go:proto:check mise tasks to use buf - Remove go:proto:sync and go:proto:clean tasks (no longer needed) - Add proto target to sdk/go/Makefile - Add buf 1.72.0 to root mise.toml tool dependencies - Include options.proto in generation (was stripped from vendored copies) - Regenerate all .pb.go files via the new buf pipeline Signed-off-by: Roland Huß <rhuss@redhat.com> * test(sdk/go): add proto-converter field coverage detection Use protobuf reflection to enumerate all fields on key proto messages (SandboxSpec, SandboxTemplate, SandboxStatus, SandboxCondition, SandboxPolicy) and compare against explicit handled/skipped sets in the converter tests. Unhandled fields produce warnings (t.Log), not failures, so proto contributors are not forced to fix SDK converters in the same PR. Stale entries in the handled set (removed proto fields) do fail, since they indicate the converter references something that no longer exists. A follow-up CI workflow will create GitHub issues when converter drift lands on main. Signed-off-by: Roland Huß <rhuss@redhat.com> * fix(sdk/go): bump Go to 1.26 and fix errcheck lint violations The upstream go.mod now has `toolchain go1.26.4`, which requires Go 1.26 to build golangci-lint. Bump the mise.toml Go version from 1.25 to 1.26 and wrap deferred Close() calls in test helpers to satisfy errcheck. Assisted-By: 🤖 Claude Code * feat(sdk/go): add ObjectMeta fields (annotations, workspace, deletion_timestamp) Add three new proto ObjectMeta fields to Sandbox and Provider domain types: Annotations (map), Workspace (string), and DeletionTimestamp (*time.Time). Update converters in both directions, deep-copy maps at the proto/SDK boundary, and add TimeFromMillisPtr/MillisFromTimePtr helper functions. Assisted-By: 🤖 Claude Code * chore(sdk/go): regenerate proto bindings after rebase Pick up workspace fields from upstream PR NVIDIA#2445 (Wire authorization into workspace model). All request messages now include workspace parameter in the generated Go bindings. Assisted-By: 🤖 Claude Code * feat(sdk/go): add workspace scoping to all RPC interfaces Add workspace parameter to every sandbox-scoped RPC method across all interfaces (Sandbox, Exec, File, Service, SSH, TCP, Config, Policy, Provider, Profile, Refresh). The workspace string is passed as the second parameter after ctx, following the convention workspace then resource-name. Key changes: - SandboxInterface: all 10 methods gain workspace parameter - sandbox_client.go: passes Workspace field in every proto request - ListOptions: add AllWorkspaces field for cross-workspace queries - All stub interfaces updated to match new signatures - All sandbox client tests updated with "default" workspace Assisted-By: 🤖 Claude Code * chore(sdk/go): remove coverage.out from tracking Assisted-By: 🤖 Claude Code * fix(sdk/go): address review feedback from mrunalp - Add RefreshStrategyAWSStsAssumeRole to match proto enum value 6, fulfilling the "all domain types upfront" contract - Wrap context.DeadlineExceeded and context.Canceled in StatusError so IsDeadlineExceeded() and IsCancelled() helpers work correctly - Return error from mapToStruct/SandboxSpecToProto instead of silently discarding structpb.NewStruct failures on invalid template maps Signed-off-by: Roland Huss <rhuss@redhat.com> * fix(sdk/go): address remaining review items - Wire go:ci into root ci task so SDK is tested in repository CI - Fix gofmt formatting on converter files - Add goimports to mise.toml tools - Add coverage.out to .gitignore - Add Go SDK section to AGENTS.md and CONTRIBUTING.md - Add regression tests for context-error wrapping (IsDeadlineExceeded, IsCancelled) and invalid template map rejection - Remove panic from SandboxToProto, return error instead Signed-off-by: Roland Huss <rhuss@redhat.com> * fix(sdk/go): pin goimports version and update lockfile Pin goimports to 0.48.0 instead of "latest" and regenerate mise.lock to include the new entry. Signed-off-by: Roland Huss <rhuss@redhat.com> * fix(sdk/go): TLS.Insecure means skip-verify, not plaintext Align TLS.Insecure semantics with the Rust SDK: Insecure: true now uses TLS with InsecureSkipVerify (skip cert verification) instead of switching to plaintext. Only the http:// scheme triggers plaintext. This fixes token auth against dev/k3d gateways: StaticToken and RefreshableToken require transport security, which real TLS (even with InsecureSkipVerify) satisfies, but plaintext does not. For http:// + token auth (dev gateways without TLS), wrap the auth provider to override RequireTransportSecurity, matching the Rust SDK's behavior where http:// accepts any auth mode. Transport decision table (matches Rust SDK crates/openshell-sdk): http:// + any TLS config -> plaintext (TLS config ignored) https:// + Insecure: true -> TLS, skip cert verify https:// + Insecure: false -> TLS, full verification no scheme -> same as https:// Signed-off-by: Roland Huss <rhuss@redhat.com> * feat(sdk/go): add missing policy proto fields Add 6 previously silently dropped fields to the network policy types and converters, preventing security-relevant data loss on round-trip: NetworkEndpoint fields 19-23: - CredentialSigning: SigV4 re-signing mode - SigningService: AWS service name for SigV4 - SigningRegion: AWS region override for SigV4 - JsonRpcMaxBodyBytes: JSON-RPC body inspection limit - Mcp: MCP-specific policy options (new McpOptions type) L7Allow and L7DenyRule field 9: - Params: MCP params matcher map for tools/call filtering New type McpOptions with StrictToolNames and AllowAllKnownMcpMethods optional booleans matching the proto definitions. Signed-off-by: Roland Huss <rhuss@redhat.com> * fix(sdk/go): enforce coverage test and extend to policy messages Change coverage_test.go from t.Logf (silent) to t.Errorf so that unhandled proto fields fail the test immediately. Add coverage tests for NetworkEndpoint (23 fields), L7Allow (8 fields), L7DenyRule (8 fields), and McpOptions (2 fields). Any new proto field that is not in the handled set or explicitly skipped now breaks the build, closing the silent-drift gap. Signed-off-by: Roland Huss <rhuss@redhat.com> * ci(sdk/go): add Go SDK job to branch-checks workflow Add a Go SDK job to branch-checks.yml that runs mise run go:ci (lint, build, test, proto-check, docs-check) on every PR. This ensures the SDK is tested in CI, not just locally. Signed-off-by: Roland Huss <rhuss@redhat.com> * fix(sdk/go): address should-fix review items NVIDIA#6 Fix broken godoc examples: add workspace parameter to all method calls in doc.go that were broken after workspace scoping. NVIDIA#7 Add Err field to Event[T]: Watch error events now carry the underlying error instead of discarding it. NVIDIA#8 Separate Unauthenticated from PermissionDenied: add ErrorUnauthenticated code and IsUnauthenticated() helper. gRPC Unauthenticated (401) now maps to its own code instead of collapsing into PermissionDenied (403). NVIDIA#9 Add Unwrap to StatusError: replace dead Details field with Cause error field. StatusError.Unwrap() returns Cause, enabling errors.Is/As unwrapping. FromGRPCError and contextError both populate Cause. Signed-off-by: Roland Huss <rhuss@redhat.com> * ci(sdk/go): add go:format:check to CI pipeline Add gofmt format verification to go:ci. Catches unformatted Go files before they reach the PR. Fix formatting on coverage_test.go. Signed-off-by: Roland Huss <rhuss@redhat.com> * chore(sdk/go): remove Makefile in favor of mise tasks All build, lint, test, and proto-gen tasks are already defined in tasks/go.toml and invoked via mise. The Makefile was a leftover that duplicated this and raised questions in review. Signed-off-by: Roland Huß <rhuss@redhat.com> * feat(sdk/go): sync proto bindings and add credential handle support Regenerate Go proto bindings after rebase to pick up new CredentialHandle message and Provider.credential_handles and profile_workspace fields from upstream. Add domain types, converter support, and proto field coverage tests for Provider and CredentialHandle. Signed-off-by: Roland Huß <rhuss@redhat.com> * fix(sdk/go): reject plaintext auth leak and fix watch error handling Reject http:// addresses when the auth provider requires transport security instead of silently stripping the requirement. Remove the insecureAuthWrapper that overrode RequireTransportSecurity. Fix watch stream error handling: use blocking send for terminal errors so they are never silently dropped when the channel is full, and wrap mid-stream errors with converter.FromGRPCError so SDK error helpers like IsUnavailable work on watch Event.Err. Signed-off-by: Roland Huß <rhuss@redhat.com> * fix(sdk/go): address review findings from multi-agent code review - WaitReady now detects SandboxDeleting phase and returns immediately instead of polling indefinitely - Watch goroutine defers streamCancel() to prevent context leaks - Fix StopOnTerminal=false test to keep stream open (was wrong-reason pass due to stream ending, not StopOnTerminal logic) - Add EventDeleted test covering the Deleting phase branch - Add provider converter unit tests for CredentialHandle round-trip, nil handling, and empty maps Signed-off-by: Roland Huß <rhuss@redhat.com> --------- Signed-off-by: Roland Huß <rhuss@redhat.com> Signed-off-by: Roland Huss <rhuss@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
10 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Notify sandbox watchers after an atomic
UpdateConfigpolicy revision commits.Without this notification, live
WatchSandbox(follow_status=true)consumers cancontinue showing stale sandbox state until another event occurs. Sandbox
supervisors poll
GetSandboxConfigindependently and are not affected by thisnotification gap.
Related Issue
FixesNVIDIA#2517
Changes
put_policy_revision_atomicsucceeds.one committed revision emits exactly one watcher notification.
notify observers.
Testing
CARGO_INCREMENTAL=0 mise run pre-commitpassesFocused test:
Checklist