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

Stop recommending people scrape auto-generated service account tokens #31845

Merged
merged 1 commit into from Feb 24, 2022

Conversation

liggitt
Copy link
Member

@liggitt liggitt commented Feb 22, 2022

/sig auth
/cc @smarterclayton @zshihang

@k8s-ci-robot k8s-ci-robot added sig/auth Categorizes an issue or PR as relevant to SIG Auth. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Feb 22, 2022
@netlify
Copy link

netlify bot commented Feb 22, 2022

✔️ Deploy Preview for kubernetes-io-main-staging ready!

🔨 Explore the source changes: 3d15ef3

🔍 Inspect the deploy log: https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/62167cc9bc5325000788ff67

😎 Browse the preview: https://deploy-preview-31845--kubernetes-io-main-staging.netlify.app

The tokens obtained using this method have bounded lifetimes, and are automatically
invalidated when the Pod they are mounted into is deleted.

Service account token secrets can still be created manually if you need a token that never expires,
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, service account token secrets support revocation, which I don't know that TokenRequest allows in a meaningful way. It's unfortunate that to get revocation, you need to have the contents visible to the client, but I guess it's a useful tradeoff.

Copy link
Member Author

@liggitt liggitt Feb 22, 2022

Choose a reason for hiding this comment

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

tokens returned by tokenrequest can be revoked by deleting the associated service account. A secondary object (pod or secret) can also be referenced in the token request, and deletion of that secondary object revokes the token as well

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there doc that describes how to bind the token request to a secret so it can be revoked? I guess not - I can open an issue to track that.

Copy link
Contributor

Choose a reason for hiding this comment

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

@smarterclayton yes please (to the new issue)

Copy link
Member Author

Choose a reason for hiding this comment

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

just the field-level API doc. there's a kubectl create token command coming in 1.24 that has CLI support for binding to a secret object... that might be nice to include in the doc

@zshihang
Copy link
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 Feb 22, 2022
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: e620c272170c8bda58875602f00abd940dd5edd4

Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

Thanks for this, that's a sound overall improvement. I've added inline feedback.

Also, a specific query:
Should the annotation kubernetes.io/service-account-name (or perhaps kubernetes.io/service-account.name) be listed in https://kubernetes.io/docs/reference/labels-annotations-taints/?

(I don't see it).
I'm trying to get to a point where all the annotations we either use in code or recommend in docs, that are inside kubernetes.io, are registered and documented.

Comment on lines +157 to +162
Automatic creation of API credentials in secrets to mount into running pods
is no longer used in v1.22 and newer versions. Instead, API credentials are
obtained directly by using the [TokenRequest](/docs/reference/kubernetes-api/authentication-resources/token-request-v1/) API,
and are mounted into Pods using a [projected volume](/docs/reference/access-authn-authz/service-accounts-admin/#bound-service-account-token-volume).
The tokens obtained using this method have bounded lifetimes, and are automatically
invalidated when the Pod they are mounted into is deleted.
Copy link
Contributor

Choose a reason for hiding this comment

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

Are TokenRequests a subresource of ServiceAccount? I think they are, but it's not 100% clear from my understanding of what counts as a subresource.

We should aim to clarify that in the reference for ServiceAccount: https://kubernetes.io/docs/reference/access-authn-authz/service-accounts-admin/

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, they are.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, then I recommend using the term subresource to make that more clear.

@sftim
Copy link
Contributor

sftim commented Feb 22, 2022

Also, what'd be really nice is a concept explanation for ServiceAccount; right now there isn't one.

I'd either

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

liggitt commented Feb 23, 2022

fixed up style issues

Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

Changes LGTM.

@jimangel
Copy link
Member

/lgtm
/approve

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

LGTM label has been added.

Git tree hash: 5aefc8e0a6a6c4af8989a5f4bafceb9091b7c5eb

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jimangel

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 24, 2022
@k8s-ci-robot k8s-ci-robot merged commit 9de0833 into kubernetes:main Feb 24, 2022
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. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants