Introduce Impersonate-UID header #99961

margocrawf · 2021-03-08T21:37:57Z

What type of PR is this?

/kind-bug

What this PR does / why we need it:

This introduces an Impersonate-Uid header to server side code.
While UserInfo contains a uid field alongside groups, username and extra, it was not possible to pass a UID through as an impersonation header like you can with Impersonate-Group, Impersonate-User and Impersonate-Extra.

This PR contains:

Changes to impersonation.go to parse the Impersonate-Uid header and authorize uid impersonation
Unit tests for allowed and disallowed impersonation cases
An integration test that creates a CertificateSigningRequest using impersonation, and ensures that the API server populates the correct impersonated spec.uid upon creation.

Which issue(s) this PR fixes:

This PR is a step towards fixing #93699.

Does this PR introduce a user-facing change?

Kube API server accepts Impersonate-Uid header to impersonate a user with a specific UID, in the same way that you can currently use Impersonate-User, Impersonate-Group and Impersonate-Extra

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot · 2021-03-08T21:38:05Z

Welcome @margocrawf!

It looks like this is your first PR to kubernetes/kubernetes 🎉. 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/kubernetes 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 · 2021-03-08T21:38:06Z

Hi @margocrawf. 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.

fejta-bot · 2021-03-08T22:16:12Z

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.

fedebongio · 2021-03-09T21:14:24Z

/assign @deads2k
/triage accepted

enj

Mostly minor stuff.

staging/src/k8s.io/api/authentication/v1/types.go

staging/src/k8s.io/apiserver/pkg/endpoints/filters/impersonation.go

enj · 2021-03-25T21:15:54Z

staging/src/k8s.io/apiserver/pkg/endpoints/filters/impersonation.go

@@ -67,6 +67,7 @@ func WithImpersonation(handler http.Handler, a authorizer.Authorizer, s runtime.
 		username := ""
 		groups := []string{}
 		userExtra := map[string][]string{}
+		uid := ""


Maybe in a separate PR, can you move groupsSpecified to its single point of use and update the comment to note that it only applies to service accounts (the newer group logic below has superseded this comment).

staging/src/k8s.io/apiserver/pkg/endpoints/filters/impersonation.go

test/integration/auth/auth_test.go

enj · 2021-03-26T13:47:34Z

test/integration/auth/auth_test.go

+			t.Errorf("body should contain a uid, but it didn't.")
+		}
+	}()
+


Suggested change

staging/src/k8s.io/apiserver/pkg/endpoints/filters/impersonation_test.go

enj · 2021-03-26T14:12:05Z

/ok-to-test
/assign
/kind feature
/priority important-longterm
/milestone v1.22

@margocrawf

Add a more descriptive commit message + PR message (dev facing - can be the same), ex: 9b23f22, 30eb668
Release note (end user facing), ex: Fix GC uid races and handling of conflicting ownerReferences #92743 (comment)
Squash into one commit
There are probably docs that need updating - see if you can find them and open a PR to update them.

margocrawf · 2021-07-02T20:07:54Z

/retest

UserInfo contains a uid field alongside groups, username and extra. This change makes it possible to pass a UID through as an impersonation header like you can with Impersonate-Group, Impersonate-User and Impersonate-Extra. This PR contains: * Changes to impersonation.go to parse the Impersonate-Uid header and authorize uid impersonation * Unit tests for allowed and disallowed impersonation cases * An integration test that creates a CertificateSigningRequest using impersonation, and ensures that the API server populates the correct impersonated spec.uid upon creation.

This change goes with kubernetes/kubernetes#99961 in the Kubernetes repo.

enj · 2021-07-06T17:29:01Z

/lgtm
/approve

margocrawf · 2021-07-06T17:57:08Z

/retest

margocrawf · 2021-07-06T18:04:24Z

/retest

deads2k · 2021-07-06T18:50:20Z

/approve

margocrawf · 2021-07-06T19:05:45Z

/retest

liggitt · 2021-07-06T19:21:27Z

/approve

k8s-ci-robot · 2021-07-06T19:21:49Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, enj, liggitt, margocrawf

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:

~~staging/src/k8s.io/api/OWNERS~~ [liggitt]
~~staging/src/k8s.io/apiserver/OWNERS~~ [deads2k,liggitt]
~~test/OWNERS~~ [deads2k,enj,liggitt]

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

enj · 2021-07-06T20:04:42Z

/retest

Unrelated quota integration test flake.

This change goes with kubernetes/kubernetes#99961 in the Kubernetes repo.

This corresponds to previous work to allow impersonating UIDs: * Introduce Impersonate-UID header: kubernetes#99961 * Add UID to client-go impersonation config kubernetes#104483 Signed-off-by: Margo Crawford <margaretc@vmware.com>

k8s-ci-robot added do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Mar 8, 2021

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 8, 2021

k8s-ci-robot requested review from enj and SataQiu March 8, 2021 21:38

k8s-ci-robot assigned deads2k Mar 9, 2021

k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Mar 9, 2021

enj suggested changes Mar 26, 2021

View reviewed changes

k8s-ci-robot assigned enj Mar 26, 2021

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. kind/feature Categorizes issue or PR as related to a new feature. labels Mar 26, 2021

margocrawf force-pushed the master branch from 96a18bd to 74f5ed6 Compare July 6, 2021 17:13

margocrawf added a commit to margocrawf/website that referenced this pull request Jul 6, 2021

Add Impersonate-Uid description to Authentication docs page.

023e3b5

This change goes with kubernetes/kubernetes#99961 in the Kubernetes repo.

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 6, 2021

liggitt self-assigned this Jul 6, 2021

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 6, 2021

k8s-ci-robot merged commit e1acbbd into kubernetes:master Jul 7, 2021

SIG Auth Old automation moved this from In Review (v1.22) to Closed / Done Jul 7, 2021

liggitt mentioned this pull request Jul 8, 2021

pull-kubernetes-integration timing out #103512

Closed

margocrawf added a commit to margocrawf/website that referenced this pull request Jul 12, 2021

Add Impersonate-Uid description to Authentication docs page.

d773681

This change goes with kubernetes/kubernetes#99961 in the Kubernetes repo.

enj mentioned this pull request Jul 23, 2021

UserInfo.UID is not passed by client-go #93699

Open

margocrawf mentioned this pull request Aug 20, 2021

Add UID to client-go impersonation config #104483

Merged

margocrawf mentioned this pull request Oct 20, 2021

--as-uid flag in kubectl and kubeconfigs. #105794

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce Impersonate-UID header #99961

Introduce Impersonate-UID header #99961

margocrawf commented Mar 8, 2021 •

edited

k8s-ci-robot commented Mar 8, 2021

k8s-ci-robot commented Mar 8, 2021

fejta-bot commented Mar 8, 2021

fedebongio commented Mar 9, 2021

enj left a comment

enj Mar 25, 2021

enj Mar 26, 2021

enj commented Mar 26, 2021

margocrawf commented Jul 2, 2021

enj commented Jul 6, 2021

margocrawf commented Jul 6, 2021

margocrawf commented Jul 6, 2021

deads2k commented Jul 6, 2021

margocrawf commented Jul 6, 2021

liggitt commented Jul 6, 2021

k8s-ci-robot commented Jul 6, 2021

enj commented Jul 6, 2021

Introduce Impersonate-UID header #99961

Introduce Impersonate-UID header #99961

Conversation

margocrawf commented Mar 8, 2021 • edited

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot commented Mar 8, 2021

k8s-ci-robot commented Mar 8, 2021

fejta-bot commented Mar 8, 2021

fedebongio commented Mar 9, 2021

enj left a comment

Choose a reason for hiding this comment

enj Mar 25, 2021

Choose a reason for hiding this comment

enj Mar 26, 2021

Choose a reason for hiding this comment

enj commented Mar 26, 2021

margocrawf commented Jul 2, 2021

enj commented Jul 6, 2021

margocrawf commented Jul 6, 2021

margocrawf commented Jul 6, 2021

deads2k commented Jul 6, 2021

margocrawf commented Jul 6, 2021

liggitt commented Jul 6, 2021

k8s-ci-robot commented Jul 6, 2021

enj commented Jul 6, 2021

margocrawf commented Mar 8, 2021 •

edited