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

Provide RunAsGroup feature for Containers in a Pod #213

Open
krmayankk opened this issue Mar 19, 2017 · 84 comments

Comments

@krmayankk
Copy link
Contributor

commented Mar 19, 2017

Feature Description

As a Kubernetes User, i should be able to specify both user id and group id for the containers running inside a pod on a per Container basis, similar to how docker allows that using docker run options -u, --user="" Username or UID (format: <name|uid>[:<group|gid>]) format. Currently kubernetes only allows us to control the primary user id and allows us to add supplemental groups. There is no way to control the primary group id of the running container which is always 0(root).
This feature would enable enterprises to run containers as non root(non zero uid and non zero gid) and hence improve the level of security for the running containers. More discussion and agreement was gathered in this issue 22179

List of Work Items:-

  • RunAsGroup Implementation
  • Add feature flag , mark it alpha and disable by default
  • PSP Implementation for RunAsGroup
  • Verify e2e and Unit test Coverage
  • Verify Containerd and cri-o Test coverage

Containerd and Cri-o Implementation PR's

Test Results for CRI-O PR with latest Kubernetes Master
https://k8s-testgrid.appspot.com/sig-node-cri-o#crio-e2e-fedora

@ghost

This comment has been minimized.

Copy link

commented May 2, 2017

Is the progress listed above accurate?

@krmayankk

This comment has been minimized.

Copy link
Contributor Author

commented May 2, 2017

@pineking

This comment has been minimized.

Copy link

commented May 24, 2017

@krmayankk any progress to update?

@krmayankk

This comment has been minimized.

Copy link
Contributor Author

commented Jun 7, 2017

@pineking i have the proposal , and the code almost ready. Will send out the proposal by Friday while i try to figure the unit tests and api changes.

@jduncan-rva

This comment has been minimized.

Copy link

commented Aug 10, 2017

@krmayankk is this still on your radar?

@krmayankk

This comment has been minimized.

Copy link
Contributor Author

commented Aug 11, 2017

@jduncan-rva yes the proposal is already out. I have some review comments which i will address. I should have a PR by next week.

@kincl

This comment has been minimized.

Copy link

commented Aug 24, 2017

@krmayankk any updates?

@krmayankk

This comment has been minimized.

Copy link
Contributor Author

commented Aug 28, 2017

@kincl the proposal is already out and nearing lgtm. We are waiting one more reviewer to review. I was out last week on vacation. I should have the actual PR this week

@krmayankk

This comment has been minimized.

Copy link
Contributor Author

commented Aug 30, 2017

Here is the proposal under review kubernetes/community#756

@php-coder

This comment has been minimized.

Copy link

commented Oct 10, 2017

Responsible SIGs: sig-node

Sounds like it falls into sig-auth area.

@php-coder

This comment has been minimized.

Copy link

commented Oct 10, 2017

For the history: here is an implementation of the proposal -- kubernetes/kubernetes#52077

@fejta-bot

This comment has been minimized.

Copy link

commented Jan 8, 2018

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

@fejta-bot

This comment has been minimized.

Copy link

commented Jan 8, 2018

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

1 similar comment
@fejta-bot

This comment has been minimized.

Copy link

commented Jan 8, 2018

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

@fejta-bot

This comment has been minimized.

Copy link

commented Feb 10, 2018

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten
/remove-lifecycle stale

@php-coder

This comment has been minimized.

Copy link

commented Feb 11, 2018

/remove-lifecycle rotten

@krmayankk

This comment has been minimized.

Copy link
Contributor Author

commented Mar 7, 2018

/sig auth

@krmayankk

This comment has been minimized.

Copy link
Contributor Author

commented Mar 14, 2018

/sig node

@justaugustus

This comment has been minimized.

Copy link
Member

commented Apr 17, 2018

@krmayankk
Any plans for this in 1.11?

If so, can you please ensure the feature is up-to-date with the appropriate:

  • Description
  • Milestone
  • Assignee(s)
  • Labels:
    • stage/{alpha,beta,stable}
    • sig/*
    • kind/feature

cc @idvoretskyi

@claurence

This comment has been minimized.

Copy link

commented Feb 22, 2019

@krmayankk looking over the KEP I don't see any testing plans - can someone help PR in testing plans for this enhancement? This information is helpful for knowing readiness of this feature for the release and is specifically useful for CI Signal.

@lledru

This comment has been minimized.

Copy link
Member

commented Feb 25, 2019

Hello, 1.14 enhancement shadow here. Code Freeze is March 7th and all PRs must be merged by then to your issue to make the 1.14 release. What open K/K PRs do you still have that need to merge? Thanks

@krmayankk

This comment has been minimized.

Copy link
Contributor Author

commented Feb 27, 2019

@krmayankk

This comment has been minimized.

Copy link
Contributor Author

commented Feb 27, 2019

@claurence the graduation criteria in https://github.com/kubernetes/enhancements/pull/835/files at the bottom should cover it. Are you looking for testing plan for making it stable or for making it beta ?

@claurence

This comment has been minimized.

Copy link

commented Feb 27, 2019

@mariantalla for CI signal is that enough for testing plans ^?

@lledru

This comment has been minimized.

Copy link
Member

commented Mar 5, 2019

Hi @krmayankk, the only PR not merged yet is #73691, correct? will this be merged before code freeze? Thanks

@krmayankk

This comment has been minimized.

Copy link
Contributor Author

commented Mar 6, 2019

@lledru that is correct

@claurence

This comment has been minimized.

Copy link

commented Mar 7, 2019

@krmayankk That PR isn't on the 1.14 milestone - if this PR is needed for 1.14 can I add it to the milestone?

@krmayankk

This comment has been minimized.

Copy link
Contributor Author

commented Mar 8, 2019

@claurence Please track this one kubernetes/kubernetes#75164 and ignore 73691

@jingweno

This comment has been minimized.

Copy link

commented Mar 12, 2019

I just ran into the issue that setting Containers[0].SecurityContext.RunAsUser = 3000 and Containers[0].SecurityContext.RunAsGroup = 3000 make the process running as 3000 but its group is still root (0). Is this expected?

@tallclair

This comment has been minimized.

Copy link
Member

commented Mar 13, 2019

@jingweno what version of Kubernetes? Is the RunAsGroup feature gate enabled?

@krmayankk

This comment has been minimized.

Copy link
Contributor Author

commented Mar 13, 2019

@jingweno it’s an alpha feature being enabled by default in 1.14(moving to beta ). There was also a bug in containerd that was fixed sometime back . So depends on which runtime are you using , which kubernetes version and whether the alpha feature flags are enabled

dgerd added a commit to dgerd/serving that referenced this issue Mar 13, 2019

Add conformance test for setting user
This change adds a conformance test to validate that a user set in the
securityContext is reflected in the container. This change also adds the
group information to the runtime test image, but we do not validate this
as 1. it is not currently part of the runtime contract and 2. Setting
group is currently an alpha feature that does not work on many Kubernetes
clusters. See kubernetes/enhancements#213

Related to: knative#3223

dgerd added a commit to dgerd/serving that referenced this issue Mar 13, 2019

Add conformance test for setting user
This change adds a conformance test to validate that a user set in the
securityContext is reflected in the container. This change also adds the
group information to the runtime test image, but we do not validate this
as 1. it is not currently part of the runtime contract and 2. Setting
group is currently an alpha feature that does not work on many Kubernetes
clusters. See kubernetes/enhancements#213

Related to: knative#3223

knative-prow-robot added a commit to knative/serving that referenced this issue Mar 27, 2019

Add conformance test for setting user (#3423)
This change adds a conformance test to validate that a user set in the
securityContext is reflected in the container. This change also adds the
group information to the runtime test image, but we do not validate this
as 1. it is not currently part of the runtime contract and 2. Setting
group is currently an alpha feature that does not work on many Kubernetes
clusters. See kubernetes/enhancements#213

Related to: #3223
@kacole2

This comment has been minimized.

Copy link
Member

commented Apr 12, 2019

Hello @krmayankk , I'm the Enhancement Lead for 1.15. Is this feature going to be graduating alpha/beta/stable stages in 1.15? Please let me know so it can be tracked properly and added to the spreadsheet.

Once coding begins, please list all relevant k/k PRs in this issue so they can be tracked properly.

@kacole2 kacole2 removed this from the v1.14 milestone Apr 12, 2019

@kacole2 kacole2 added tracked/no and removed tracked/yes labels Apr 12, 2019

@krmayankk

This comment has been minimized.

Copy link
Contributor Author

commented Apr 14, 2019

@kacole2 this feature is beta in 1.14. I will work and figure out what does it take to graduate to Stable in 1.15

@cjreyn

This comment has been minimized.

Copy link

commented Jun 20, 2019

Did this get added in 1.15?

@tallclair

This comment has been minimized.

Copy link
Member

commented Jun 20, 2019

It's beta as of 1.14
@krmayankk what is the plan for bringing this to GA?

@Elegant996

This comment has been minimized.

Copy link

commented Jun 21, 2019

Restricting runAsGroup would likely be a requirement for GA. I seem to recall my PSP providing an error when I attempted to set the field. It should also support MustRunAsNonRoot similar to the below. Thanks!

spec:
  runAsGroup:
    rule: MustRunAsNonRoot
@tallclair

This comment has been minimized.

Copy link
Member

commented Jun 21, 2019

It should work with PSP, if not please file an issue. For MustRunAsNonRoot (on the pod spec), there were a lot of concerns about all the different ways to attach groups to a process, and whether we could reliably assert that it wasn't using the root group. I'm not categorically against it, but also don't want to have the feature if it's not a strong guarantee. I also don't think it should block GA, as it's functionality that can be layered onto the current approach.

@krmayankk

This comment has been minimized.

Copy link
Contributor Author

commented Jun 22, 2019

@tallclair I think the main criteria is having the right test coverage for runtimes. This issue kubernetes/kubernetes#72253 covers that , but i have no idea what needs to be done there ? Can you advise ?

@krmayankk

This comment has been minimized.

Copy link
Contributor Author

commented Jun 22, 2019

@Elegant996 here is the discussion related to mustRunAsNonRoot on the pod spec kubernetes/kubernetes#62216 But we ditched in it favor of only doing it in PSP

@kacole2

This comment has been minimized.

Copy link
Member

commented Jul 9, 2019

Hi @krmayankk @tallclair , I'm the 1.16 Enhancement Lead/Shadow. Is this feature going to be graduating alpha/beta/stable stages in 1.16? Please let me know so it can be added to the 1.16 Tracking Spreadsheet. If not's graduating, I will remove it from the milestone and change the tracked label.

Once coding begins or if it already has, please list all relevant k/k PRs in this issue so they can be tracked properly.

Milestone dates are Enhancement Freeze 7/30 and Code Freeze 8/29.

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.