Kubelet & API changes for Windows GMSA support#75459
Kubelet & API changes for Windows GMSA support#75459k8s-ci-robot merged 4 commits intokubernetes:masterfrom
Conversation
|
Putting on hold until the webhook is upated too. |
|
/hold |
|
/sig windows |
ddebroy
left a comment
There was a problem hiding this comment.
Overall looks good. Once comment about removing the feature flag. While the link to the KEP is useful, can you update the PR description please with the API objects changed: PodSecurityContext and SecurityContext) and the new fields introduced?
|
Looks like the following will need to be run: |
|
/milestone v1.15 |
|
@liggitt : thanks for the review, amended as requested. |
690c0c2 to
c74cdad
Compare
liggitt
left a comment
There was a problem hiding this comment.
thanks for the update. you can squash the Review comment: renaming GMSA go variables files into the right commits.
found a couple potential NPEs in pkg/securitycontext/util.go
the update to dropDisabledFields() (#75459 (comment)) is still outstanding as well
looking good otherwise
This patch comprises the API changes outlined in the Windows GMSA KEP (https://github.com/kubernetes/enhancements/blob/master/keps/sig-windows/20181221-windows-group-managed-service-accounts-for-container-identity.md) to add GMSA support to Windows workloads. It includes validation, as well as dropping fields if the `WindowsGMSA` feature flag is not set, both with unit tests. Signed-off-by: Jean Rouge <rougej+github@gmail.com>
Signed-off-by: Jean Rouge <rougej+github@gmail.com>
This patch comprises the auto-generated changes for the API changes outlined in the Windows GMSA KEP (https://github.com/kubernetes/enhancements/blob/master/keps/sig-windows/20181221-windows-group-managed-service-accounts-for-container-identity.md) to add GMSA support to Windows workloads. Signed-off-by: Jean Rouge <rougej+github@gmail.com>
|
@liggitt @ddebroy : amended as requested, including #75459 (comment), could you please take another look? |
This patch comprises the kubelet changes outlined in the Windows GMSA KEP (https://github.com/kubernetes/enhancements/blob/master/keps/sig-windows/20181221-windows-group-managed-service-accounts-for-container-identity.md) to add GMSA support to Windows workloads. Updated tests. Signed-off-by: Jean Rouge <rougej+github@gmail.com>
|
/lgtm |
|
/priority important-soon |
|
/cc @yujuhong for cri approval |
| string run_as_username = 1; | ||
|
|
||
| // The contents of the GMSA credential spec to use to run this container. | ||
| string credential_spec = 2; |
There was a problem hiding this comment.
The API change LGTM. We may need to change the comment to make it clear if we ever supports non-GMSA spects.
There was a problem hiding this comment.
@yujuhong : sorry if I'm misunderstanding, but that field's name was requested to not include "GMSA" in #75459 (comment), and the comment makes it clear it's for GMSA for now; so not sure what else we should do here?
There was a problem hiding this comment.
I think this is fine for now. The field name matches the oci field name, and the description is reasonably consistent with the oci doc.
|
@yujuhong - can you approve or find one from SIG-Node? |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: liggitt, wk8, yujuhong The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind feature
What this PR does / why we need it:
This patch comprises the kubelet & API changes outlined in the Windows GMSA KEP
(https://github.com/kubernetes/enhancements/blob/master/keps/sig-windows/20181221-windows-group-managed-service-accounts-for-container-identity.md)
to add GMSA support to Windows workloads.
This essentially means moving the GMSA cred spec names & contents
from annotations to dedicated API fields.
More specifically, this is affecting
PodSecurityContextandSecurityContextstructs, adding a
WindowsOptionsobject to both.Updated tests.
Special notes for your reviewer:
Still a WIP, will also update the webhook
(https://github.com/kubernetes-sigs/windows-gmsa)
to ensure all is in order before merging.
Does this PR introduce a user-facing change?: