Skip to content

Conversation

@WanLinghao
Copy link

@WanLinghao WanLinghao commented Aug 8, 2018

add description about container-level serviceaccount, it could be useful when user want to
give containers different api-server access. The basic idea is make serviceaccount token acquired
by token projected volume contains container name. Authorizer would use this info with pod-name, pod-uid together to confirm which <pod, container> the request comes from.
ref:
kubernetes/kubernetes#66020
kubernetes/kubernetes#61858

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. kind/design Categorizes issue or PR as related to design. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/storage Categorizes an issue or PR as relevant to SIG Storage. labels Aug 8, 2018
@k8s-ci-robot k8s-ci-robot requested review from childsb and liggitt August 8, 2018 10:07
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 8, 2018
@k8s-ci-robot k8s-ci-robot requested a review from mikedanese August 8, 2018 10:15
@WanLinghao WanLinghao changed the title add description about container-level serviceaccount add proposal about container-level serviceaccount Aug 8, 2018
Path string
// IsContainerLevel determines if the token should include information about
// which container the request comes from.
IsContainerLevel bool
Copy link
Member

Choose a reason for hiding this comment

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

this would mean the kubelet would have to fan out and maintain N tokens for a pod with N containers... is that what we want?

Copy link
Author

Choose a reason for hiding this comment

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

@liggitt For most use cases who don't need container-level serviceaccount feature, they just launch a token projection volume with IsContainerLevel being false. In this case, all the containers could share single serviceaccount token.

Copy link
Member

@liggitt liggitt left a comment

Choose a reason for hiding this comment

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

I'd recommend considering this as an improvement after the token projection feature graduates, but will defer to @mikedanese who is driving that work

object will be embedded as claims in the issued token. A token bound to an
object will only be valid for as long as that object exists.
object will only be valid for as long as that object exists. The token will contain
which container the request comes from if ContainerName was specified. ContainerName
Copy link
Member

Choose a reason for hiding this comment

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

without a consistent enforcement mechanism to guarantee these tokens are only used in requests from that container, I don't think we can make that claim

Copy link
Author

Choose a reason for hiding this comment

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

kubernetes/kubernetes#61858 insert pod information into user.Info.Extra() , and we could not make sure the tokens are only used in requests from that pod neither. So what's the difference between the container information and pod information. I am a little confused about this.

Copy link
Member

Choose a reason for hiding this comment

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

those assert what pod the token was created for, they do not assert that is where the request is coming from

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: WanLinghao
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approvers: enj, saad-ali

If they are not already assigned, you can assign the PR to them by writing /assign @enj @saad-ali in a comment when ready.

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@WanLinghao
Copy link
Author

@liggitt I have improved the description as you commented, PTAL

@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 May 12, 2019
@krmayankk
Copy link

@WanLinghao what are some good use cases which require per container service account tokens which cannot be fulfilled by separating those containers into different pods ? It would be good to highlight those. Also it would be good to start a different KEP if this makes sense .

@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 Jun 12, 2019
@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.

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

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/design Categorizes issue or PR as related to design. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/storage Categorizes an issue or PR as relevant to SIG Storage. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants