Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add command to request a bound service account token #107880

Merged
merged 3 commits into from
Feb 9, 2022

Conversation

liggitt
Copy link
Member

@liggitt liggitt commented Jan 31, 2022

What type of PR is this?

/kind feature

What this PR does / why we need it:

Default RBAC policy change:

  • Include TokenRequest create permission in the default edit and admin roles. These roles already have full access to service account credentials via secrets, and will be able to get tokens via manually created token secrets in the future, and can create pods that exfiltrate injected tokens. Giving them direct access to tokenrequest is not a practical escalation from their current permissions.

TokenRequest endpoint changes:

  • Ensures any name/namespace/uid specified in the token request matches the service account
  • Ensures name/namespace/uid is populated in the returned tokenrequest object
  • Ensures the tokenrequest passed to validating admission is valid (instead of running in-process validation after calling validation webhooks)
  • Populate creationTimestamp in the returned object with the creation time as seen by the server
  • Adds a server-side warning if the requested expiration lifetime is shortened by the server

Kubectl changes:

  • Adds a command to kubectl to request a bound service account token. This will help ease the transition from scraping generated service account tokens with commands like kubectl get secret "$(kubectl get serviceaccount default -o jsonpath='{.secrets[0].name}')"

xref https://github.com/kubernetes/enhancements/tree/master/keps/sig-auth/2799-reduction-of-secret-based-service-account-token

TODO:

  • relocate to kubectl create token
  • consider whether we want to warn about not-honored requested spec.expirationSeconds server-side rather than client-side
  • think about whether end users with edit or admin roles in a namespace would expect to be able to do this (currently no user-facing roles include serviceaccounts/token permissions)
  • add tests

Does this PR introduce a user-facing change?

`kubectl create token` can now be used to request a service account token, and permission to request service account tokens is added to the `edit` and `admin` RBAC roles

/cc @deads2k @zshihang @soltysh
/sig auth cli

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/cli Categorizes an issue or PR as relevant to SIG CLI. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jan 31, 2022
@liggitt liggitt changed the title Add command to request a bound service account token WIP - Add command to request a bound service account token Jan 31, 2022
@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. approved Indicates a PR has been approved by an approver from all required OWNERS files. area/kubectl labels Jan 31, 2022
@enj enj added this to Needs Triage in SIG Auth Old Jan 31, 2022
@liggitt
Copy link
Member Author

liggitt commented Jan 31, 2022

/retest

@liggitt
Copy link
Member Author

liggitt commented Jan 31, 2022

unsure whether we want this here or under kubectl create token, etc

@liggitt
Copy link
Member Author

liggitt commented Feb 2, 2022

/assign @micahhausler
who volunteered to review

(wait to review until I relocate this to kubectl create and rework it in that form ... will probably be next week)

@liggitt liggitt force-pushed the kubectl-auth-token branch 6 times, most recently from b358c71 to 9900fec Compare February 6, 2022 20:29
@k8s-ci-robot k8s-ci-robot added area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Feb 6, 2022
@liggitt liggitt moved this from Needs Triage to In Review in SIG Auth Old Feb 6, 2022
@liggitt liggitt added this to the v1.24 milestone Feb 6, 2022
@liggitt liggitt added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Feb 6, 2022
@k8s-ci-robot k8s-ci-robot removed the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Feb 6, 2022
@zshihang
Copy link
Contributor

zshihang commented Feb 7, 2022

this new command would be very useful.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 7, 2022
@liggitt
Copy link
Member Author

liggitt commented Feb 7, 2022

/hold
want review of the server-side change by @deads2k

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 7, 2022
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 9, 2022
@liggitt
Copy link
Member Author

liggitt commented Feb 9, 2022

dropped uid precondition / population bits

@deads2k
Copy link
Contributor

deads2k commented Feb 9, 2022

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 9, 2022
@liggitt liggitt removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 9, 2022
@k8s-ci-robot k8s-ci-robot merged commit e74c42a into kubernetes:master Feb 9, 2022
SIG Auth Old automation moved this from In Review to Closed / Done Feb 9, 2022
@liggitt liggitt deleted the kubectl-auth-token branch February 10, 2022 17:37
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. area/dependency Issues or PRs related to dependency changes area/kubectl area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
Archived in project
SIG Auth Old
Closed / Done
Development

Successfully merging this pull request may close these issues.

None yet

7 participants