feat(kubernetes): add cni-sidecar supervisor topology - #2606
Open
russellb wants to merge 19 commits into
Open
Conversation
Add a cni-sidecar supervisor topology that keeps the sidecar runtime model but installs pod-network bypass-prevention rules through a privileged OpenShell chained CNI plugin instead of a pod-local network init container. A new openshell-cni crate provides the chained plugin and node installer. The Kubernetes driver emits openshell.ai/* pod annotations consumed by the plugin during CNI ADD, omits the network init container in this topology, and reuses the sidecar network-only process supervision path. Helm gains a privileged CNI installer DaemonSet gated on cni.enabled and a supervisor.topology=cni-sidecar option. Reconstructed on current main: uses the renamed topology config field and drops the abandoned proxy-pod topology. Signed-off-by: Russell Bryant <rbryant@redhat.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com>
On OpenShift the binary-aware network sidecar must run as UID 0 with SYS_PTRACE and DAC_READ_SEARCH to inspect cross-UID /proc, which the restricted-v2 SCC forbids. Add a gated minimal SecurityContextConstraints (restricted-v2 baseline plus exactly those two capabilities and the image volume type) with a ClusterRole and ClusterRoleBinding granting it to the sandbox ServiceAccount. Controlled by sandboxServiceAccount.openshift.binaryAwareSCC (default false); the cni-sidecar OpenShift overlay enables it. Signed-off-by: Russell Bryant <rbryant@redhat.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com>
Remediate merge-blocking review findings on the cni-sidecar supervisor topology: - driver: mount the gateway client mTLS bundle directly onto the network sidecar (cni-sidecar omits network-init, so the bundle was never seeded and the sidecar crash-looped on policy fetch) - driver: annotate the effective sidecar proxy UID (0 in binary-aware mode) so the node CNI exempts the right identity instead of redirecting the sidecar's own egress - cni: fail closed when ip6tables is missing in the iptables fallback so a dual-stack sandbox cannot bypass policy over IPv6 - cni: emit a CNI-spec error object on stdout so the runtime surfaces plugin failures instead of an opaque crash - helm: re-patch the chained plugin on the reconcile tick when absent so a CNI config rewrite or restart cannot drop egress enforcement - helm: keep the component label out of the immutable gateway selector so existing releases can upgrade - helm: mirror supervisorImage repository/tag fallbacks for the CNI image - docs: document the sidecar privilege profile and cold-start window; trim RFC non-goals Signed-off-by: Russell Bryant <rbryant@redhat.com>
Address the second-round review of the cni-sidecar topology: - cni/driver: add a per-node readiness scheduling gate. The installer labels its node openshell.ai/cni-ready via a new `openshell-cni node-ready` subcommand (minimal cluster-scoped nodes get/patch RBAC), clearing it on shutdown or when a reconcile tick cannot restore the plugin; the driver sets a required nodeAffinity on that label for cni-sidecar sandbox pods, so a pod cannot schedule before per-node egress enforcement is active. - driver: run the untrusted workspace-init container as the sandbox UID under binary-aware policy instead of root, so it cannot inherit the CNI's UID-0 egress exemption and bypass policy before the sidecar enforces it. - helm: gate the conflist/chain-conf mv on jq success and non-empty output so a jq failure cannot truncate the host CNI config and break node networking. - helm: give the CNI DaemonSet a distinct app name (openshell-cni) and revert the gateway Service selector to base-only, so a helm upgrade no longer blanks gateway endpoints for pods that predate the component label. - cni: only require ip6tables in the iptables fallback when the pod has a routable IPv6 address (from prevResult), so IPv4-only nodes are not rejected. - build: add crates/openshell-cni/BUILD.bazel so Bazel and the aggregate rustfmt suite cover the crate. - docs: document the scheduling gate and corrected sidecar privilege profile across architecture, published docs, crate READMEs, values, AGENTS, and the debug-openshell-cluster skill. Signed-off-by: Russell Bryant <rbryant@redhat.com>
russellb
changed the title
Address the third review round of the cni-sidecar topology: - cni/helm: fence before repair and stop failing open on teardown. The reconcile loop clears the readiness label the instant enforcement is not verifiably present, then repairs and re-marks. Enforcement lives in the host CNI config and survives pod restarts, so preStop no longer strips it on an ordinary restart/rolling update: a new `openshell-cni daemonset-active` check removes it only when the owning DaemonSet is confirmed terminating, and fences the node first. Reconcile interval 300s -> 30s. - cni/helm: stamp an owner (<namespace>/<release>) on the chained plugin entry and refuse to overwrite an entry owned by a different release (fail closed). Only one OpenShell release per cluster is supported for cni-sidecar today; documented in the RFC. - helm: validate the multus chain conf content (type, owner, kubeconfig, non-empty namespaces) before marking the node ready, instead of accepting any existing file. - driver: harden the untrusted workspace-init container (allowPrivilegeEscalation=false, drop ALL capabilities, runAsNonRoot when non-root) so a setuid binary cannot regain the exempt UID 0; restrict the non-root init to the Sidecar/CniSidecar topologies so combined keeps root. - cni: determine IPv6 enforcement by probing /proc/net/if_inet6 in the pod netns (fail closed on indeterminate; link-local counts) instead of trusting prevResult, keeping IPv4-only nodes working without ip6tables. - build: stage openshell-cni in the prebuilt 'all' target. - rbac: add get on the installer's own DaemonSet (resourceNames-scoped). Signed-off-by: Russell Bryant <rbryant@redhat.com>
Address the fourth review round of the cni-sidecar topology: - cni/helm: convert the CNI installer to a cluster singleton. The chained plugin enforces any pod carrying the OpenShell annotations (set only by a gateway on its own sandbox pods), so a single installation serves every release across all namespaces. The plugin config uses an empty sandboxNamespaces allowlist and a fixed 'openshell' owner; the installer resources use release-independent names; pods get RBAC becomes cluster-scoped. This fixes the multi-release fail-open: a second release no longer relies on a namespace list that another release could overwrite. Additional gateway releases set cni.enabled=false + cni.external=true to share the singleton. - helm: bind node readiness to a configVersion stamp so a stale-version entry is repaired before the node is re-marked ready; fence before the initial patch on startup; retry mark_unready so a transient API error does not leave stale readiness. - helm: create the host SA token atomically with install -m 0600 (no briefly world-readable node-patch credential). - driver: keep default root capabilities for the combined-topology workspace init (UID 0 is not CNI-exempt there) so it retains DAC read access; apply the drop-ALL/no-privilege-escalation/runAsNonRoot hardening only to the non-root exempt (Sidecar/CniSidecar binary-aware) case. - docs: add a cni-sidecar enable-and-verify walkthrough to the OpenShift page, add user-facing example values files under deploy/helm/openshell/examples/ (replacing the ci/ fixture in docs), and update topology, compute-runtimes, and the RFC to the singleton model. Signed-off-by: Russell Bryant <rbryant@redhat.com>
… entry in place The singleton installer refused to overwrite an openshell-cni chained plugin entry whose owner value differed from the fixed "openshell" owner. That guard was redundant — the conflist patch already preserves every non-openshell-cni plugin and replaces only the openshell-cni entry, and the plugin type is itself the ownership signal — and it broke in-place upgrades from an earlier owner format (e.g. "<namespace>/<release>"), crash-looping the installer. Treat any existing openshell-cni entry as ours to upgrade in place. Signed-off-by: Russell Bryant <rbryant@redhat.com>
Address the fifth review round of the cni-sidecar topology: - driver: harden the untrusted workspace-init in EVERY sidecar topology, not only the binary-aware case. A non-binary-aware Sidecar/CniSidecar init still needs UID 0 but now drops all capabilities and disables privilege escalation, so it cannot CAP_SETUID to the exempt proxy UID or use CAP_NET_RAW to bypass egress. Only the Combined topology (no separate exempt sidecar) keeps default root capabilities for DAC read access. - cni/helm: add cni.sandboxNamespaces (default: the release's sandbox namespace). The plugin passes through pods in unlisted namespaces WITHOUT a Kubernetes API lookup, so control-plane/RBAC problems no longer block unrelated workloads' pod creation. Multi-release deployments list every sandbox namespace on the singleton. Bound into configVersion. - cni/helm: close the node-reboot fail-open window. openshell-cni node-ready now removes a boot-time NoSchedule taint (openshell.ai/cni-not-ready) once enforcement is ready (optimistic-concurrency JSON patch); the DaemonSet tolerates it. Boot-time application is operator node config — an OpenShift MachineConfig example ships under examples/ with caveats. conflist mode is disk-backed and unaffected by reboot. - helm: make the CNI singleton name a genuinely fixed constant (openshell-cni), not derived from nameOverride, so releases cannot install competing singletons over shared host state. - docs: document cni.external and the singleton in setup.mdx and the debug-openshell-cluster skill; update the RFC and architecture docs. Signed-off-by: Russell Bryant <rbryant@redhat.com>
…ngleton docs Address the sixth review round (docs/examples only): - Fix the additional-release workflow: the singleton only enforces namespaces in its cni.sandboxNamespaces allowlist, so an extra cni.external release whose namespace is not added to the owner's allowlist runs unenforced. The extra-release example and the OpenShift docs now require (and show) the two-step flow — add the namespace to the singleton owner and let nodes reconcile, THEN install the additional gateway — with prominent warnings. - Harden the boot-taint MachineConfig example: retry-until-tainted loop, Restart=on-failure, kubelet --register-with-taints for the initial join, and an honest note that a small residual reboot race remains (no chart-shippable mechanism is fully race-free; conflist mode is unaffected). - Correct stale docs to match the implementation: topology and architecture docs described an empty all-namespaces allowlist and a foreign-owner refusal, but the plugin uses an explicit sandboxNamespaces allowlist and upgrades any openshell-cni entry in place. Add cni.sandboxNamespaces and boot-taint diagnosis to the debug-openshell-cluster skill. Signed-off-by: Russell Bryant <rbryant@redhat.com>
…owlist Round seven: make external-release enforcement machine-verifiable, and correct the remaining docs. - cni: add `openshell-cni label-namespace` and `list-sandbox-namespaces` subcommands. Each cni-sidecar gateway release runs a helm hook that labels its sandbox namespace openshell.ai/sandbox=true; the CNI singleton's reconcile aggregates every labeled namespace (unioned with the optional static cni.sandboxNamespaces) into the plugin allowlist and rebinds configVersion. An additional cni.external release is therefore discovered and enforced within one reconcile — no manual owner-allowlist edit, closing the silent-bypass footgun. - helm: add the namespace-label hook (Job + minimal namespaces get/patch RBAC scoped to the release's sandbox namespace) and grant the CNI ClusterRole namespaces list. On discovery failure the reconcile keeps the installed allowlist rather than shrinking it (never fails a running namespace open). - docs: stop claiming the boot taint 'closes' the reboot window (it narrows it; a residual race remains — conflist mode is unaffected). Correct topology, architecture, RFC, and the debug skill to describe the explicit auto-aggregated allowlist and in-place upgrade of any openshell-cni entry, and fix stale RFC claims (cni.enabled-or-external, 30s reconcile). Signed-off-by: Russell Bryant <rbryant@redhat.com>
Round eight: close the registration race and give registration a proper lifecycle. - helm/cni: replace the namespace-label hook with a Helm-owned marker ConfigMap (openshell.ai/cni-registration=true) in each release's sandbox namespace. The installer discovers namespaces by listing marker ConfigMaps (RBAC: configmaps list). Because the marker is a normal Helm resource, uninstalling a release or changing its sandboxNamespace removes the registration automatically — no orphaned allowlist entries. This also drops the hook Job, its namespace-patch RBAC, and the per-release ClusterRole name collision. - cni/helm: close the discovery-window race. The installer publishes on each node the CSV of namespaces it enforces (openshell.ai/cni-sandbox-namespaces annotation, via set-node-coverage); every gateway runs a wait-coverage init container that blocks serving until every cni-ready node acknowledges the gateway's namespace. A newly-registered release therefore cannot create sandboxes before enforcement is confirmed cluster-wide. - docs: update topology, architecture, RFC, values, and the debug skill to the marker + wait-for-ack model; fix the RFC boot-taint 'closes' -> 'narrows' and the design spec 300s -> 30s reconcile interval. Signed-off-by: Russell Bryant <rbryant@redhat.com>
Contributor
Author
|
I think this will need more review/fix iterations, but it's fine to review and try out now. |
russellb
marked this pull request as ready for review
August 4, 2026 23:29
russellb
requested review from
a team,
derekwaynecarr,
maxamillion and
mrunalp
as code owners
August 4, 2026 23:29
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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
Adds
cni-sidecar, a third Kubernetes supervisor topology for OpenShell sandboxpods. It keeps the split-supervisor model of the
sidecartopology but movespod-network rule installation out of the sandbox pod: a privileged, node-level
OpenShell CNI DaemonSet installs a chained CNI plugin that programs the
bypass-prevention rules during CNI
ADD, removing the per-pod privileged networkinit container. Includes OpenShift/Multus enablement (multus-chain install mode,
purpose-built SCCs) and a per-node readiness scheduling gate.
Related Issue
Related: #899 (restricted SCC support for
managed Kubernetes). Design is captured in the in-branch RFC
(
rfc/cni-sidecar-topology-DRAFT.md).Changes
cni-sidecarSupervisorTopologyin the Kubernetes driver;omits the in-pod network-init container and annotates sandbox pods for the
chained plugin.
openshell-cni): chained plugin that installs nftables/iptablesbypass-prevention rules in the pod netns during CNI
ADD; fail-closed on jqand IPv6-enforcement errors; structured CNI-spec error objects on stdout.
openshell-cni node-readysubcommand labels thenode
openshell.ai/cni-ready(minimal cluster-scopednodes get;patchRBAC),cleared on shutdown/repair-failure; the driver sets a required
nodeAffinityso sandbox pods cannot schedule before per-node enforcement is active.
workspace-initcontainer runs as the sandboxUID (non-exempt) under binary-aware policy, so it cannot inherit the CNI's UID-0
egress exemption.
the gateway Service selector stays base-only (no upgrade endpoint gap);
OpenShift
multus-chainmode and cni-sidecar values overlay; minimalbinary-aware sandbox SCC.
compute-runtimes.md), published docs(
docs/kubernetes/topology.mdx), crate READMEs, and thedebug-openshell-clusterskill.Testing
Platform coverage so far: on-cluster E2E has been performed on OpenShift
(Multus / OVN-Kubernetes) using
multus-chainmode only. The defaultconflistmode (vanilla Kubernetes / k3s) is covered by unit tests but has notyet been validated on a live cluster.
mise run pre-commitpassestests) — covers both
conflistandmultus-chainpathsmultus-chainonly: node labeled
cni-ready, sandbox pod scheduled 2/2 with the requirednodeAffinity,
workspace-initrunning as the sandbox UID, sidecarauthenticated to the gateway, and default-deny egress enforced.
conflistmode) — not yet doneChecklist