2707 configurable uid gid range/ebusto - #2737
Conversation
|
All contributors have signed the DCO ✍️ ✅ |
|
I have read the DCO document and I hereby sign the DCO. |
drew
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Validation: Project-valid implementation of validated issue #2707; the operator-configurable lower identity floor preserves the default of 1000 and rejects root.
Head SHA: b1b4bb32705432815f1cd0f9a1313698dbd942e9
Base SHA: c5498239e66581d5842b50773eff07ace65726f1
Merge base SHA: c5498239e66581d5842b50773eff07ace65726f1
Patch ID: a4928771b3cd59dd91c7902df8e2f7eda9552bd7
Gator payload: 4
Review mode: initial
Previous reviewed SHA: none
Review budget exhausted: no
Maintainer decision required: no
Blocking findings:
GATOR-b1b4bb32-01: Kubernetes still resolves and validates its authoritative workload identity against the fixed 1000 floor, so the advertised configured lower minima cannot survive that supported path.GATOR-b1b4bb32-02: VM gateways accept policies using the lowered global minima but do not propagate the minima to the guest supervisor or reject that configuration.
Carried findings:
- None
Non-blocking suggestions:
- None
Docs: docs/reference/gateway-config.mdx is updated, but its stated backend coverage exposes the unresolved VM contract described above.
Next state: gator:in-review
BlockedHead SHA: Gator is blocked because this PR is currently a draft. Thanks @ebusto — I saw your update that you are taking a pass at VM and Kubernetes support, and the latest head includes changes in both driver paths. The independent follow-up review of those changes will resume once the PR is ready for review. Next action: @ebusto, mark the PR ready for review when the implementation is ready for another pass. |
|
Added support for both the VM and Kubernetes drivers, and tested locally. |
|
Label |
|
Label |
drew
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Validation: Project-valid implementation of validated issue #2707; the operator-configurable lower identity floor preserves the default of 1000 and rejects root.
Head SHA: 2d85bfdac97ec5911e219a5a88a1cbcb4fdeb406
Base SHA: c5498239e66581d5842b50773eff07ace65726f1
Merge base SHA: c5498239e66581d5842b50773eff07ace65726f1
Patch ID: 50892fef3f32f3547d6cba711be65c9c375a358d
Gator payload: 4
Review mode: follow_up
Previous reviewed SHA: b1b4bb32705432815f1cd0f9a1313698dbd942e9
Review budget exhausted: no
Maintainer decision required: no
Thanks @ebusto. I checked your VM and Kubernetes update against the two prior identity-propagation findings. The current delta now passes the configured minima into the VM guest, uses the configured minima for Kubernetes identity resolution, and adds render-level coverage proving UID 500/GID 30 survives the Kubernetes path.
Blocking findings:
- No blocking findings remain.
Carried findings:
GATOR-b1b4bb32-01: Resolved by the Kubernetes minima propagation and render regression coverage.GATOR-b1b4bb32-02: Resolved by VM driver argument and guest-environment propagation.
Docs: docs/reference/gateway-config.mdx documents the configured minima and supported backend propagation.
Required tests: test:e2e and test:e2e-kubernetes applied; test-dispatch helper runs are queued, but the actual required workflows are not yet confirmed queued.
Next state: gator:in-review pending required test dispatch.
|
/ok to test 2d85bfd |
|
Regarding the failing checks, I don't think this has anything to do with my changes, as nothing credentials related was touched. Analysis from my buddies Cursor and Grok: The failing check is not the 500/30 identity work. What failed
Gateway RPCs all returned 200:
The gateway pod was What that means That test expects The harness is supposed to layer Relation to this PR The committed UID/GID changes do not touch credential-driver code or the Helm credential templates. This is a credential-storage wiring failure, not an identity-minima failure. The suite never reached the Vault pass; it died on the first ( Most likely cause The installed kubectl -n openshell get cm openshell-config -o jsonpath='{.data.gateway\.toml}'You want |
2d85bfd to
61eecb5
Compare
Operators can lower min_sandbox_uid and min_sandbox_gid in gateway.toml so enterprise system IDs such as GID 30 are accepted. Root remains rejected. Signed-off-by: Eric Busto <ebusto@nvidia.com>
Signed-off-by: Eric Busto <ebusto@nvidia.com>
Network-policy merge retries and agent proposals re-validated the full policy with the default UID/GID minimum, so a persisted run_as_user of 500 was rejected even when gateway.toml lowered the range. Signed-off-by: Eric Busto <ebusto@nvidia.com>
Production paths now pass configured identity limits, so the default wrapper was unused in non-test builds and triggered dead_code. Signed-off-by: Eric Busto <ebusto@nvidia.com>
Rename from_mins parameters to avoid similar_names, and use RangeInclusive::contains when parsing supervisor env minima. Signed-off-by: Eric Busto <ebusto@nvidia.com>
Rename apply_identity_limits_from_file parameters so pre-commit clippy -D warnings passes. Signed-off-by: Eric Busto <ebusto@nvidia.com>
Pass [openshell.gateway] min_sandbox_uid/gid to the VM driver subprocess and into the guest supervisor environment so a lowered range is enforced inside the microVM, matching Docker and Podman. Testing: - cargo test -p openshell-driver-vm --lib -- validate_sandbox_identity build_guest_environment_sets_supervisor_defaults build_guest_environment_includes_configured_identity_limits - cargo test -p openshell-server --lib -- vm_driver_command_includes_identity_limits - mise run gateway:vm with min_sandbox_uid/gid = 1; sandbox policy process.run_as_user/group 500; guest `id` reported uid=500 gid=500 Signed-off-by: Eric Busto <ebusto@nvidia.com>
Use configured min_sandbox_uid/gid for driver identity, OpenShift SCC start IDs, sidecar proxy_uid, and network-init so a lowered range is enforced in combined and sidecar pods, matching Docker, Podman, and VM. Testing: - cargo test -p openshell-driver-kubernetes --lib -- identity OpenShift proxy_uid combined_and_sidecar_inject parse_openshift validate_sandbox_identity validate_proxy_uid resolve_sandbox_uid_uses_openshift - cargo check -p openshell-driver-kubernetes --bins - mise run helm:skaffold:dev with min_sandbox_uid/gid = 1 and namespace annotations openshift.io/sa.scc.uid-range=500/50000 and supplemental-groups=500/50000; sandbox id reported uid=500 gid=500 - mise run helm:skaffold:dev:sidecar with the same mins and annotations; agent ran as UID 500, network-init --proxy-gid 500 succeeded Signed-off-by: Eric Busto <ebusto@nvidia.com>
Add validation, SCC, and combined/sidecar render regressions so a configured pair below the old 1000 floor is accepted and emitted instead of falling back to 1000/1000. Testing: - cargo test -p openshell-driver-kubernetes --lib -- validate_sandbox_identity_config_accepts_uid_500 parse_openshift_supplemental combined_and_sidecar_render_resolved_identity_500_30 Signed-off-by: Eric Busto <ebusto@nvidia.com>
61eecb5 to
b2a3dd7
Compare
|
/ok to test b2a3dd7 |
Summary
Operators can lower the sandbox UID/GID minima in
gateway.tomlso enterprisesystem identities below 1000 (for example GID 30 or UID 500) are accepted.
Defaults stay 1000, and root (
0) is still rejected.Related Issue
Fixes #2707
Changes
[openshell.gateway] min_sandbox_uidandmin_sandbox_gid(TOML only;default 1000, minimum 1).
retry, draft-chunk approval, and agent proposals.
OPENSHELL_MIN_SANDBOX_UID/OPENSHELL_MIN_SANDBOX_GIDinto Docker,Podman, and Kubernetes sandbox environments so the supervisor and OPA engine
use the same range.
docs/reference/gateway-config.mdx.Testing
mise run pre-commitpassesUnit coverage includes default-min rejection,
min=1accepting GID 30 / UID500, supervisor env parsing, OPA
from_protowith env limits, and merge /agent-proposal paths that previously re-validated against the default 1000
floor. Farm VM check: gateway with
min_sandbox_uid = 1/min_sandbox_gid = 1and a UID/GID 500 user.
Checklist