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

Add exec as user KEP #1224

Closed
wants to merge 3 commits into from
Closed

Add exec as user KEP #1224

wants to merge 3 commits into from

Conversation

max88991
Copy link

Signed-off-by: Lei Gong lgong@alauda.io

Signed-off-by: Lei Gong <lgong@alauda.io>
@k8s-ci-robot
Copy link
Contributor

Welcome @max88991!

It looks like this is your first PR to kubernetes/enhancements 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/enhancements has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 29, 2019
@k8s-ci-robot
Copy link
Contributor

Hi @max88991. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 29, 2019
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/node Categorizes an issue or PR as relevant to SIG Node. labels Aug 29, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: max88991
To complete the pull request process, please assign dchen1107
You can assign the PR to them by writing /assign @dchen1107 in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@max88991
Copy link
Author

/assign @dchen1107

@max88991
Copy link
Author

/sig nodes

@max88991
Copy link
Author

@mattjmcnaughton @smarterclayton please take a look at this

Copy link
Contributor

@mattjmcnaughton mattjmcnaughton left a comment

Choose a reason for hiding this comment

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

Thanks for your kep here!

Left some initial thoughts here - interested to know what other members of sig-node and sig-auth think.

- sig-cli
- sig-auth
reviewers:
- "@smarterclayton"
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we please add a reviewer from @kubernetes/sig-auth-api-reviews ?


> In the Kubernetes 1.5 release, we are proud to introduce the Container Runtime Interface (CRI) – a plugin interface which enables kubelet to use a wide variety of container runtimes, without the need to recompile. CRI consists of a protocol buffers and gRPC API, and libraries, with additional specifications and tools under active development. CRI is being released as Alpha in Kubernetes 1.5.

But There are many container runtime implementation, e.g Docker, Kata, gVisior, most of which provide a way to enter the container as a special `user`.
Copy link
Contributor

Choose a reason for hiding this comment

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

To be sure I'm understanding, are you saying that this exec as user feature will only work for certain container runtime implementations (i.e. Docker, Kata...)?

That's interesting to me - my sense is that k8s is trying to move towards less special casing of certain container runtime implementations.

I'd be interested to hear from others in the community about the risk of increasing featuring between different container runtime implementations.


> User specifies specific user (and group) information for the container process

It brings convenience to debugging, and users are used to it. Adding a `user` option with `exec` in Kubernetes seems reasonable.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you share a little bit more about the debugging experience?

What are the specific types of problems this would allow you to debug that you couldn't debug before?

I'm also curious about if some of the ideas proposed in the Ephemeral Containers KEP could be helpful w.r.t. these types of debugging (cc @verb ).

Copy link
Contributor

Choose a reason for hiding this comment

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

Currently we don't allow changing user in an ephemeral container, so what you get is what is in the dockerfile of the ephemeral container image. Likely some of the use cases here could be solved with an ephemeral debugging container (for example troubleshooting as root when a container image specifies a non-root user), but I see value in both approaches. They would be complementary.

I, too, would like to hear more about the debugging experience and agree that adding examples of things one cannot currently accomplish would help quite a bit.

Since the container does not have a uniform `exec` format, the format of `user` should be just a string.

#### Story 2
By providing the `user` option for `exec`, user can do the auit inside the container.
Copy link
Contributor

Choose a reason for hiding this comment

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

Apologies for not knowing this :) but what is auit?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's a typo :)

@k8s-ci-robot k8s-ci-robot added sig/auth Categorizes an issue or PR as relevant to SIG Auth. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API labels Aug 31, 2019
@fejta-bot
Copy link

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

#### Story 2
By providing the `user` option for `exec`, user can do the auit inside the container.

## Design Details
Copy link
Contributor

Choose a reason for hiding this comment

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

There’s a lot of additional security protection which is required to make arbitrary exec safe, especially by username (which is only mappable inside a container).

Anyone using PSP or similar systems would need a way to control which users are allowed to exec.

The use of username is contrary to how kubernetes pods run so it needs substantially more justification.

The design would also have to take into account how windows exec would support exec, and how runtimes that don’t support exec changes would communicate that to users.

Co-Authored-By: Slava Semushin <slava.semushin@gmail.com>
@k8s-ci-robot k8s-ci-robot removed the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 25, 2019
@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Sep 25, 2019
Co-Authored-By: Slava Semushin <slava.semushin@gmail.com>

### User Stories [optional]

As a Kubernetes User, I should be able to control how people can enter the container, by default kubernetes use the default user in docker image.
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't control how people access the containers. They'll have to specify user name when exec'ing, or are you suggesting using PSP to further restrict users?

-u, --user="" Username or UID (format: <string>]) format
```

Since the container does not have a uniform `exec` format, the format of `user` should be just a string.
Copy link
Contributor

Choose a reason for hiding this comment

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

Windows does not support uid. For Linux, kubelet uses uid when starting the containers.
The current design may be confusing for cross-platform support.

@fejta-bot
Copy link

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.

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

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 24, 2019
@fejta-bot
Copy link

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

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jan 23, 2020
@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

@k8s-ci-robot
Copy link
Contributor

@fejta-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@mikebrow
Copy link
Member

mikebrow commented Jun 1, 2022

@max88991 are you still working this KEP?

@zqzten
Copy link
Member

zqzten commented Aug 14, 2022

@mikebrow There's a new on-going KEP: #3372. I've been watching on that but it also seems to be inactive for some while.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/node Categorizes an issue or PR as relevant to SIG Node. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.