Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add hostUsers pod field for k8s 1.29 and newer #363

Merged
merged 1 commit into from
Dec 22, 2024

Conversation

solidDoWant
Copy link
Contributor

@solidDoWant solidDoWant commented Dec 10, 2024

Description of the change

This adds the hostUsers field when the chart is deployed to a k8s cluster that is 1.29 or newer. It defaults to true, k8s' default value.

Added

hostUsers field to pod spec

Benefits

Users of the chart can now specify that pods should run in their own user namespace, rather than reusing the host user namespace. Pods can run as root with reduced risk of container escape or privilege escalation.

Possible drawbacks

Requires k8s 1.29 or newer, as well as specific CRI versions and kernel versions. Setting this requires that several other fields (such as hostPID) are false.

Checklist

  • Title of the PR conforms to the Conventional Commits standard.
  • Scope of the of the PR title contains the chart name.
  • Chart version in Chart.yaml has been bumped according to Semantic Versioning.
  • Chart artifacthub.io/changes changelog annotation has been updated in Chart.yaml. See Artifact Hub documentation for more info.
  • Variables have been documented in the values.yaml file.

Signed-off-by: solidDoWant <fred.heinecke@yahoo.com>
@bjw-s bjw-s changed the base branch from main to common-3.6.0 December 22, 2024 09:17
@bjw-s
Copy link
Owner

bjw-s commented Dec 22, 2024

I'll go ahead and merge this into the common-3.6.0 branch and fix what needs fixing there. Thanks a lot for submitting the PR!

@bjw-s bjw-s merged commit 87ca145 into bjw-s:common-3.6.0 Dec 22, 2024
3 of 13 checks passed
@JuniorJPDJ
Copy link

JuniorJPDJ commented Dec 28, 2024

the feature needs featureFlag enabled on kubernetes and is disabled by default: https://kubernetes.io/docs/tasks/configure-pod-container/user-namespaces/
that means the check in chart is incorrect

it made my ArgoCD cry about always-here diff as this flag is being removed from manifests and chart generates it

@bjw-s
Copy link
Owner

bjw-s commented Dec 28, 2024

The fact that the CI install passes means that there is nothing inherently wrong with the check (the install would fail on k8s >= 1.29 otherwise). Instead this somehow reads to me as an issue specific to how ArgoCD renders the chart and/or compares it to the actual state 🤔

Could you please create a separate issue for this? Closed PR's aren't the best for visibility.

@halkeye
Copy link
Contributor

halkeye commented Dec 28, 2024

@JuniorJPDJ i found a workaround, made an issue and documented it @ #366

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants