Skip to content

feat(kubernetes): configure per-pool user namespaces - #4650

Closed
MichelJansen91 wants to merge 3 commits into
cozystack:mainfrom
MichelJansen91:feat/per-pool-user-namespaces-release-1.6
Closed

feat(kubernetes): configure per-pool user namespaces#4650
MichelJansen91 wants to merge 3 commits into
cozystack:mainfrom
MichelJansen91:feat/per-pool-user-namespaces-release-1.6

Conversation

@MichelJansen91

@MichelJansen91 MichelJansen91 commented Oct 1, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown

What this PR does

Adds the optional KubernetesNodes.spec.talos.userNamespacesLimit setting for a single worker pool. The official Talos gVisor extension requires user namespaces, which Talos disables by default as a KSPP hardening measure. This exposes only that prerequisite, not arbitrary sysctls or machine configuration.

This branch is now based on main. Worker pools on main are separate KubernetesNodes applications, so the feature lives in packages/apps/kubernetes-nodes, rather than the release-1.6 Kubernetes nodeGroups API.

  • Default zero omits the override and preserves the existing rendered configuration and bootstrap hash.
  • A positive integer emits only machine.sysctls.user.max_user_namespaces inside this pool’s worker VMs. Negative values fail rendering.
  • The management hosts, control plane and other worker pools are unchanged.
  • The existing content-hashed TalosConfigTemplate mechanism changes the MachineDeployment bootstrap reference when this setting changes, including when returning to zero. No new rollout mechanism is introduced.
  • Installing the Talos gVisor extension, selecting/enforcing its RuntimeClass and applying workload/network restrictions remain separate deployment steps. This setting alone is not an AI sandbox security solution.

Validation

  • helm unittest packages/apps/kubernetes-nodes: 38 suites, 227 tests, 12 existing snapshots passed without updating snapshots.
  • COZYTEST_TRACE=0 sh hack/cozytest.sh hack/check-talosconfigtemplate-name-agreement.bats: all 5 tests passed, including the new opt-in/zero test for matching Job, RBAC and MachineDeployment template names and changed/restored bootstrap references.
  • Package make generate with the CI-pinned cozyvalues-gen v1.7.0: generated schema, README, Go types and ApplicationDefinition; a repeated run left no diff.
  • Root make generate and make manifests completed without unrelated tracked changes.
  • go test ./kubernetesnodes and go vet ./kubernetesnodes in api/apps/v1alpha1 completed; this generated API package has no test files.
  • Commit-trailer and whitespace checks passed.

No live-cluster rollout or gVisor execution was tested. Standalone helm lint is blocked by the chart’s existing relative icon URL and cluster-dependent/default-release assumptions; the chart rendering is exercised by the tests above. gopls is unavailable locally, so Go validation used the compiler and vet instead.

Downstream rollout

The downstream deployment is tracked in wherobv/whero-gitops#131. It must remain gated until a package containing this feature is installed, and must be adapted to the KubernetesNodes API before use on main. A release-1.6 backport would require a separate adaptation; this PR does not claim drop-in compatibility with the old nodeGroups path. Actual worker replacement and rollback still require staging-cluster validation.

Screenshots

Not applicable; no UI changes.

Downstream repositories

The remaining trigger-map entries are unaffected: no host prerequisites, package names, controller contracts, tooling or release conventions change.

Release note

feat(kubernetes): add an opt-in KubernetesNodes talos.userNamespacesLimit setting for guest worker user namespaces, supporting the Talos gVisor prerequisite while preserving defaults and using the existing per-pool bootstrap rollout mechanism.

Summary by CodeRabbit

  • New Features
    • Added an optional per-worker-pool limit for Talos user namespaces. Positive values set the maximum; the default value of 0 preserves Talos defaults.
    • Changing the limit updates worker configuration and rolls existing workers in the pool. Negative values are rejected.
  • Documentation
    • Clarified worker-pool kernel module selection: NVIDIA modules are selected by default for pools with NVIDIA GPUs; otherwise, none are selected. A non-empty list overrides the selection, while [] disables modules.

@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 22a9329 and bc6390a.

Files selected for processing: 2
  • api/apps/v1alpha1/kubernetesnodes/types.go
  • packages/system/kubernetes-nodes-rd/cozyrds/kubernetes-nodes.yaml

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Walkthrough

Adds talos.userNamespacesLimit to worker-pool settings. Zero omits the sysctl override. Positive values set machine.sysctls.user.max_user_namespaces. Changes update the bootstrap template hash and roll existing workers in the pool.

Changes: Worker user namespace limit

Layer / File(s) Summary
Expose the Talos setting
api/apps/v1alpha1/kubernetesnodes/types.go, packages/apps/kubernetes-nodes/values.yaml, packages/apps/kubernetes-nodes/values.schema.json, packages/system/kubernetes-nodes-rd/cozyrds/kubernetes-nodes.yaml, packages/apps/kubernetes-nodes/README.md		 Adds talos.userNamespacesLimit with a default of zero to the Talos type and chart values. Exposes the setting in the application schema and dashboard ordering, and documents its behavior.
Render and validate the limit
packages/apps/kubernetes-nodes/templates/_talosconfigtemplate.tpl, packages/apps/kubernetes-nodes/tests/user_namespaces_test.yaml, hack/check-talosconfigtemplate-name-agreement.bats		 Rejects negative values and emits the sysctl only when the limit is positive. Tests check default and zero omission, positive-value rendering, validation, and TalosConfigTemplate reference changes.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature

Merge Risk: Minimal · up to bc639

The feature is internally consistent and introduces no established merge-blocking risk.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: configuring user namespaces per Kubernetes worker pool.
Docstring Coverage Passed Docstring coverage is inconclusive: files from earlier reviews still need to be rechecked within the check's analysis limits. Not blocking.
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.
Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@github-actions github-actions Bot added area/kubernetes Issues or PRs related to the tenant Kubernetes app kind/feature Categorizes issue or PR as related to a new feature size/L This PR changes 100-499 lines, ignoring generated files labels Oct 1, 2026
@lexfrei

Aleksei Sviridkin (lexfrei) commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

MichelJansen91 this is a feature against release-1.6, and release branches only take backports. If you still want per-pool user namespaces, please retarget it to main. If there is no activity in the next two weeks, the stale bot will close it; reopening later is always fine.

@lexfrei Aleksei Sviridkin (lexfrei) added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale label Oct 10, 2026
@MichelJansen91
MichelJansen91 changed the base branch from release-1.6 to main October 19, 2026 11:36
@MichelJansen91 MichelJansen91 mentioned this pull request Oct 19, 2026
Open
@MichelJansen91
MichelJansen91 force-pushed the feat/per-pool-user-namespaces-release-1.6 branch from 99be0f0 to 22a9329 Compare October 19, 2026 12:07
@github-actions github-actions Bot added size/M This PR changes 30-99 lines, ignoring generated files and removed size/L This PR changes 100-499 lines, ignoring generated files lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale labels Oct 19, 2026
@MichelJansen91

MichelJansen91 commented Oct 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Aleksei Sviridkin (@lexfrei) Thanks for pointing this out! The PR now targets main and the setting moved to KubernetesNodes.
The goal is to enable gVisor inside a tenant cluster without changing the defaults or adding a broader sandbox setup to Cozystack.

Tested the chart-generated setting on Talos 1.14.4: gVisor failed with the default limit, worked after enabling it, and failed again after removing it. Couldn’t test the worker rollout locally unfortunately. It seems that the emulated x86 setup on my Mac was too slow to get the test cluster running reliably.

Would this scope make sense?

@MichelJansen91
MichelJansen91 marked this pull request as ready for review October 21, 2026 12:24
Michel Jansen added 3 commits November 2, 2026 10:46
Michel Jansen
feat(kubernetes): expose worker user namespace limit
0c4fc83 
The official Talos gVisor extension needs user namespaces enabled in the
worker guest. Keep that hardening trade-off opt-in and scoped to one pool.

Assisted-by: LLM
Signed-off-by: Michel Jansen <[email protected]>
Michel Jansen
feat(kubernetes): configure worker user namespace limit
84ba6a9 
An unconfigured pool must retain its existing bootstrap config and hash.
Render only a positive opt-in into the shared Talos template so CAPI and
the reconciliation Job agree on the pool configuration.

Assisted-by: LLM
Signed-off-by: Michel Jansen <[email protected]>
Michel Jansen
test(kubernetes): verify user namespace rollout references
bc6390a 
Changing only a bootstrap template body does not roll existing Machines.
Guard the changed MachineDeployment reference and matching Job permissions,
including restoration of the default reference when the opt-in is removed.

Assisted-by: LLM
Signed-off-by: Michel Jansen <[email protected]>
@MichelJansen91
MichelJansen91 force-pushed the feat/per-pool-user-namespaces-release-1.6 branch from 22a9329 to bc6390a Compare November 2, 2026 09:55
@MichelJansen91

MichelJansen91 commented Nov 2, 2026

Copy link
Copy Markdown
Author

Fixed merge conflicts again

@myasnikovdaniil

myasnikovdaniil commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

MichelJansen91 I'd prefer universal talos.sysctls map instead of single userNamespacesLimit field, rendered into machine.sysctls. Then gVisor setup is sysctls: {user.max_user_namespaces: "11255"}, and next sysctl someone needs doesn't mean new field in API types, schema, README and ApplicationDefinition again.

Keys and values need render-time validation (or escaping like registryMirrors has), because free-form tenant input ends up in the unquoted heredoc of the reconcile Job, see the INVARIANT comment in _talosconfigtemplate.tpl (#3513).

It is just my idea, I aim for scalable solution and other maintainers may see blockers for it. If you want to try, you can do it as a new PR replacing this one.

@MichelJansen91 MichelJansen91 mentioned this pull request Nov 9, 2026
Open
1 of 12 tasks
lexfrei
Aleksei Sviridkin (lexfrei) reviewed Nov 13, 2026
View reviewed changes

@lexfrei Aleksei Sviridkin (lexfrei) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MichelJansen91 #5254 replaces this one: talos.sysctls covers user.max_user_namespaces and any later sysctl, and its description says so. Please close this PR so the two don't both get reviewed and merged. Both add a field to the same Talos type and the same TalosConfigTemplate block.

@MichelJansen91

MichelJansen91 commented Nov 13, 2026

Copy link
Copy Markdown
Author

Closing with #5254 as replacement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lexfrei Aleksei Sviridkin (lexfrei) lexfrei left review comments
@kvaps Andrei Kvapil (kvaps) Awaiting requested review from kvaps kvaps is a code owner
@lllamnyp Timofei Larkin (lllamnyp) Awaiting requested review from lllamnyp lllamnyp is a code owner
@myasnikovdaniil myasnikovdaniil Awaiting requested review from myasnikovdaniil myasnikovdaniil is a code owner
@IvanHunters IvanHunters Awaiting requested review from IvanHunters IvanHunters is a code owner
@matthieu-robin Matthieu ROBIN (matthieu-robin) Awaiting requested review from matthieu-robin matthieu-robin is a code owner
@scooby87 scooby87 Awaiting requested review from scooby87 scooby87 is a code owner

Assignees

No one assigned

Labels

area/kubernetes Issues or PRs related to the tenant Kubernetes app kind/feature Categorizes issue or PR as related to a new feature size/M This PR changes 30-99 lines, ignoring generated files

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@MichelJansen91 @lexfrei @myasnikovdaniil