Skip to content

KEP for Windows security context#972

Merged
k8s-ci-robot merged 5 commits intokubernetes:masterfrom
ddebroy:master
Apr 18, 2019
Merged

KEP for Windows security context#972
k8s-ci-robot merged 5 commits intokubernetes:masterfrom
ddebroy:master

Conversation

@ddebroy
Copy link
Copy Markdown
Contributor

@ddebroy ddebroy commented Apr 18, 2019

This is a KEP to cover the addition of the API enhancements related to GMSA and RunAsUsername to the pod security context and the container security context.

While the GMSA KEP covers the details around the GMSA fields, we did note that the specific API fields are speculative and may change. This KEP is where the structure and field names for the GMSA field as well as the RunAsUserName field will be finalized.

ddebroy added 3 commits April 18, 2019 03:02
Signed-off-by: Deep Debroy <ddebroy@docker.com>
Signed-off-by: Deep Debroy <ddebroy@docker.com>
Signed-off-by: Deep Debroy <ddebroy@docker.com>
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 18, 2019
@k8s-ci-robot k8s-ci-robot added kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/windows Categorizes an issue or PR as relevant to SIG Windows. labels Apr 18, 2019
Signed-off-by: Deep Debroy <ddebroy@docker.com>
@ddebroy
Copy link
Copy Markdown
Contributor Author

ddebroy commented Apr 18, 2019

/assign @liggitt @PatrickLang @michmike

Signed-off-by: Deep Debroy <ddebroy@docker.com>
@ddebroy
Copy link
Copy Markdown
Contributor Author

ddebroy commented Apr 18, 2019

/cc @wk8 @jsturtevant

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

@ddebroy: GitHub didn't allow me to request PR reviews from the following users: jsturtevant.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

/cc @wk8 @jsturtevant

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 requested a review from wk8 April 18, 2019 17:58
### Test Plan

Unit tests will be added around `DetermineEffectiveSecurityContext` to ensure for each field in `WindowsSecurityOptions` the values in a container's `SecurityContext.WindowsOptions` can override values of the fields in the pod's `PodSecurityContext.WindowsOptions`.

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.

We can also include a E2E test for this that will be labelled [sig-windows] and included in the existing test passes.

### Version Skew Strategy

## Implementation History

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.

kubernetes/kubernetes#73609 was held back from 1.14 since it was too late for API review. This PR will be rebased and adjusted to meet this KEP's proposed API.

@PatrickLang
Copy link
Copy Markdown
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 18, 2019
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ddebroy, PatrickLang

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

The pull request process is described here

Details 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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 18, 2019
@k8s-ci-robot k8s-ci-robot merged commit a67372e into kubernetes:master Apr 18, 2019

## Summary

In this KEP, we propose API enhancements in the Kubernetes pod spec to capture Windows OS specific security options from the perspective of Windows workload identity in containers. Initially the enhancements will cover fields pertinent to GMSA credential specs and the username with which to execute the container entry-point. More fields may be added in the future.
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.

Do we know of any possible fields that might get added?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am not aware of any. Can you think of any we will need?

In this KEP we propose a new field named `WindowsOptions` in the `PodSecurityContext` struct (associated with a pod spec) and the `SecurityContext` struct (associated with each container in a pod). `WindowsOptions` will be a pointer to a new struct of type `WindowsSecurityOptions`. The `WindowsSecurityOptions` struct will contain Windows OS specific security attributes scoped either at the pod level (applicable to all containers in the pod) or at the individual container level (that can override the pod level specification). Initially, the `WindowsSecurityOptions` struct will have the following fields:

- GMSACredentialSpecName: A string specifying the name of a GMSA credential spec custom resource
- GMSACredentialSpec: A string specifying the full credential spec JSON string associated with a GMSA credential spec
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.

How big is the json spec? This might be difficult to format properly?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point. In the GMSA KEP we detail how this field is auto populated by a webhook and not something the user needs to populate. Will clarify this.

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.

Is it needed in the public API then? I am not too familiar, but I know there is an internal model that Kubernetes maintains and that might be the better location for it. It seems confusing to have it on the public API surface if users do not use it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it needed in the public API then? I am not too familiar, but I know there is an internal model that Kubernetes maintains and that might be the better location for it. It seems confusing to have it on the public API surface if users do not use it.

There's no private API used to communicate between components. The "internal" API types are used as a hub to convert between the various serialized forms, but all serialization (via the REST API or to/from etcd) uses public APIs.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Additionally, the GMSA KEP (and current implementation) does call for users to be able to pass that info directly, even though the main use case should be to rely on the webhook for this.

@ddebroy
Copy link
Copy Markdown
Contributor Author

ddebroy commented Apr 19, 2019

Comments addressed/clarified in #975

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/windows Categorizes an issue or PR as relevant to SIG Windows. 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.

7 participants