Adding runAsUsername API proposal to Windows KEP #799
Conversation
k8s-ci-robot
added
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: PatrickLang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
kind/kep
| @@ -38,11 +38,13 @@ status: implementable | |||
| - [Non-Goals](#non-goals) | |||
| - [Proposal](#proposal) | |||
| - [What works today](#what-works-today) | |||
| - [What we need to test and verify if it works or not for GA (Some items will be documented as unsupported, others will be documented as supported, and some will be bugs that will be fixed for GA)](#what-we-need-to-test-and-verify-if-it-works-or-not-for-ga-some-items-will-be-documented-as-unsupported-others-will-be-documented-as-supported-and-some-will-be-bugs-that-will-be-fixed-for-ga) | |||
| - [What we need to test and verify if it works or not for GA](#what-we-need-to-test-and-verify-if-it-works-or-not-for-ga) | |||
There was a problem hiding this comment.
sorry about the excess TOC updates. It wasn't regenerated correctly before the last merge to master
|
@jsturtevant is prototyping in kubernetes/kubernetes#73609 |
|
Is this needed for Windows GA? What level of impact would it have on usability of windows containers? I don't want to rush an API change. Probably a separate KEP, as with WMSA, would be best. cc @yujuhong |
|
Alright, I'll put it on the agenda in the next SIG-Windows meeting so we can decide whether to split this into a new kep for 1.14, or delay to 1.15. |
| #### Add V1.SecurityContext.WindowsOptions.runAsUsername | ||
|
|
||
| Linux uses userID (UID) and groupID (GID) which are represented as integer types. User and group names are not canonical - they are just an alias in /etc/groups or /etc/passwd back to UID+GID. Windows uses a larger binary security identifier (SID) which is stored in the Windows Security Access Manager (SAM) database. This database is not shared between the host and containers, or between containers. | ||
|
|
There was a problem hiding this comment.
what is the implication of the last sentence ? Is it saying that since Windows uses SID, uid/gid is not relevant at all to Windows and only string user and group names matter ?
There was a problem hiding this comment.
Is it saying that since Windows uses SID, uid/gid is not relevant at all to Windows and only string user and group names matter ?
no, string user names matter as per the Windows OCI spec (next paragraph).
i guess this sentence might need a little clarification.
e.g. as in: https://github.com/kubernetes/kubernetes/pull/64009/files#r192259780
|
|
||
| On Windows, if `runAsUser` is set today, the container will not start with an error "run as uid (%d) is not supported on Windows" [src](https://github.com/kubernetes/kubernetes/blob/a5ade16abd07b938978d4876e4909b956d25c223/pkg/kubelet/kuberuntime/kuberuntime_container_windows.go#L88). This behavior will not change. Setting `runAsGroup` or `supplementalGroups` without `runAsUser` is invalid so the existing check is sufficient on Windows. | ||
|
|
||
| The new field `runAsUserName` would not be implemented on Linux. Although the field `run_as_username` exists in CRI and the OCI container spec for Linux, it was [intended to be deprecated](https://github.com/kubernetes/kubernetes/pull/36542#discussion_r87922802) from CRI. It was never hooked up in the Kubernetes API and dockershim. We propose leaving it in the state it's in today and not adding this as a new feature on Linux. We can add a check at the same time so that if it is set on a Linux container, an error "run as username is not supported on Linux". |
There was a problem hiding this comment.
From the outside it seems better to support it if we can, than to have it present but non-functional.
There was a problem hiding this comment.
iiuc, specifying a string username when starting a linux container doesn't actually give control over the user the container runs as, since it still goes through a layer of indirection via /etc/passwd under the container image's control.
Is there a reason not to stick with the minimal, canonical user specification for each platform (int uid for linux, string username for windows)?
|
|
||
| Linux uses userID (UID) and groupID (GID) which are represented as integer types. User and group names are not canonical - they are just an alias in /etc/groups or /etc/passwd back to UID+GID. Windows uses a larger binary security identifier (SID) which is stored in the Windows Security Access Manager (SAM) database. This database is not shared between the host and containers, or between containers. | ||
|
|
||
| The Windows OCI spec and CRI can handle a username instead of UID, which is interpreted inside the container to create a process as the intended user. [64009](https://github.com/kubernetes/kubernetes/pull/64009) added this to the CRI and initially proposed changing the existing `V1.Container.SecurityContext.runAsUser` field to IntStr in a later PR. More discussion on [#73387](https://github.com/kubernetes/kubernetes/issues/73387) showed that would not work, so we're proposing to add `SecurityContext.WindowsOptions.RunAsUserName` as an alternative to support Windows users as strings. As [GMSA](https://github.com/kubernetes/enhancements/blob/master/keps/sig-windows/20181221-windows-group-managed-service-accounts-for-container-identity.md) graduates to beta, the field `CredentialSpec` could be added to the same object as well. |
There was a problem hiding this comment.
As GMSA graduates to beta, the field
CredentialSpeccould be added to the same object as well.
What would the anticipated interaction between those two fields be? Is it possible to set both runAsUserName and credentialSpec?
| @@ -173,6 +179,19 @@ Windows nodes will adhere to Kubernetes version-skew policy (node to control pla | |||
| - [cncf-k8s-conformance thread](https://lists.cncf.io/g/cncf-k8s-conformance/topic/windows_conformance_tests/27913232) | |||
| - [kubernetes/enhancements proposal](https://github.com/kubernetes/features/issues/116) | |||
|
|
|||
| ### API Changes Proposed | |||
|
|
|||
| #### Add V1.SecurityContext.WindowsOptions.runAsUsername | |||
There was a problem hiding this comment.
There are pod-level and container-level securityContext stanzas. Both allow setting runAsUser for linux today (container-specific security context overrides pod-level). The GMSA design also described being able to set the credentialSpec pod-wide and per-container. Would windowsOptions.runAsUsername be added to both the pod and container-level securityContext structs?
|
|
||
| Linux uses userID (UID) and groupID (GID) which are represented as integer types. User and group names are not canonical - they are just an alias in /etc/groups or /etc/passwd back to UID+GID. Windows uses a larger binary security identifier (SID) which is stored in the Windows Security Access Manager (SAM) database. This database is not shared between the host and containers, or between containers. | ||
|
|
||
| The Windows OCI spec and CRI can handle a username instead of UID, which is interpreted inside the container to create a process as the intended user. [64009](https://github.com/kubernetes/kubernetes/pull/64009) added this to the CRI and initially proposed changing the existing `V1.Container.SecurityContext.runAsUser` field to IntStr in a later PR. More discussion on [#73387](https://github.com/kubernetes/kubernetes/issues/73387) showed that would not work, so we're proposing to add `SecurityContext.WindowsOptions.RunAsUserName` as an alternative to support Windows users as strings. As [GMSA](https://github.com/kubernetes/enhancements/blob/master/keps/sig-windows/20181221-windows-group-managed-service-accounts-for-container-identity.md) graduates to beta, the field `CredentialSpec` could be added to the same object as well. |
There was a problem hiding this comment.
nit: s/RunAsUserName/runAsUsername/g
|
/close |
|
@michmike: Closed this PR. In response to this:
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. |
As kubernetes/kubernetes#73387 was investigated, @liggitt called out that the existing
runAsUserfield, which is part of a stable API, cannot be changed. Instead, a new field would need to be added and go through API review.Because the full context of why this is needed is captured in the Windows KEP, I thought it would make sense to capture the investigation and proposed API change into the existing KEP.
Content aside, my first question for the release team and approvers (@bgrant0607 from SIG-Architecture, @dchen1107 from SIG-Node, @spiffxp from release) is if you think this is the right path forward, or should it be captured in it's own KEP? Should adding a new OS-specific field go through alpha/beta/stable process across 3 releases?
I thought having a draft of what it would look like would make the discussion easier, so now we're at this PR :)
SIG-Windows reviewers should be @jsturtevant @feiskyer @craiglpeters @michmike
/hold
/sig windows