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.

Sign up for GitHub

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

Jump to bottom

Reword / improve authentication reference #50003

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Reword / improve authentication reference #50003

wants to merge 2 commits into from

Conversation

sftim
Copy link
Contributor

@sftim sftim commented Mar 4, 2025 •
edited
Loading

Make https://kubernetes.io/docs/reference/access-authn-authz/authentication/ better [preview]

  • fix the page title
  • improve the page overview / introduction
  • organize the structure better
  • mention that system:masters is privileged
  • write "command line argument" not "flag" (outside of the Go community, command line flags are typically booleans)
  • link to the ServiceAccount concept
  • link to the reference for kubectl auth whoami
  • modernize the content
  • add some callouts (caution and warning blocks)
  • follow the style guide for logical API verbs

/sig auth
/language en

@k8s-ci-robot k8s-ci-robot added sig/auth Categorizes an issue or PR as relevant to SIG Auth. language/en Issues or PRs related to English language cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 4, 2025
@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Mar 4, 2025
@sftim sftim mentioned this pull request Mar 4, 2025
Merged
@netlify Netlify
Copy link

netlify bot commented Mar 4, 2025 •
edited
Loading

✅ Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit 62171b4
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/67d9e0aaf78e85000891e680
😎 Deploy Preview https://deploy-preview-50003--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@sftim
Copy link
Contributor Author

sftim commented Mar 10, 2025

@windsonsea would you be willing to review this for SIG Docs?

windsonsea
windsonsea reviewed Mar 11, 2025
View reviewed changes
Copy link
Member

@windsonsea windsonsea left a comment

Choose a reason for hiding this comment

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

This may need a technical review.

@sftim
Copy link
Contributor Author

sftim commented Mar 11, 2025

Oops, year in branch name is wrong.

@sftim sftim force-pushed the 20240304_authn_reference_redo branch from 9b54aa7 to b3fe8c8 Compare March 11, 2025 11:24
@sftim
Copy link
Contributor Author

sftim commented Mar 11, 2025

@kubernetes/sig-auth-pr-reviews does this change look right? Any concerns?

windsonsea
windsonsea approved these changes Mar 12, 2025
View reviewed changes
Copy link
Member

@windsonsea windsonsea left a comment

Choose a reason for hiding this comment

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

/lgtm

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

LGTM label has been added.

Git tree hash: 6e871e6b19ebd823fedbc7040e839b3757412ac8

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: windsonsea
Once this PR has been reviewed and has the lgtm label, please ask for approval from sftim. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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 needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 12, 2025
@Shubham82
Copy link
Contributor

Thanks @sftim for this PR, please resolve the merge conflict.

@sftim sftim force-pushed the 20240304_authn_reference_redo branch from b3fe8c8 to cb6e375 Compare March 12, 2025 17:39
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 12, 2025
@k8s-ci-robot k8s-ci-robot requested a review from windsonsea March 12, 2025 17:39
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 12, 2025
@Shubham82
Copy link
Contributor

The changes look good to me, need a review from sig-auth.

@sftim sftim requested a review from Copilot March 14, 2025 12:16
Copilot
Copilot AI reviewed Mar 14, 2025
View reviewed changes

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refines the authentication reference documentation by updating wording and structure for clarity and consistency.

  • Fixed/improved the page title and introduction
  • Enhanced the structure of the content
  • Added clarification on ServiceAccounts authentication details
Ritikaa96
Ritikaa96 suggested changes Mar 18, 2025
View reviewed changes
Copy link
Contributor

@Ritikaa96 Ritikaa96 left a comment

Choose a reason for hiding this comment

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

small grammatical nits

content/en/docs/reference/access-authn-authz/authentication.md
(if you [extend Kubernetes with custom APIs](/docs/concepts/extend-kubernetes/api-extension/),
then that is a different situation and you could use your own custom API for user management).

Kubernetes is designed around an assumption that you have some mechanism to manage normal
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Kubernetes is designed around an assumption that you have some mechanism to manage normal
Kubernetes is designed around the assumption that you have some mechanism to manage normal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, there are other assumptions.

@sftim @windsonsea @Ritikaa96
Reword authentication reference
102f084 
Co-authored-by: Michael Yao <haifeng.yao@daocloud.io>
Co-authored-by: Ritika <52399571+Ritikaa96@users.noreply.github.com>
@sftim sftim force-pushed the 20240304_authn_reference_redo branch from 68a87ef to 62171b4 Compare March 18, 2025 21:07
@Ritikaa96
Copy link
Contributor

The PR LGTM.
/lgtm

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

LGTM label has been added.

Git tree hash: 7b69fc6050cc9c10e3eacbda67a86d90e36518be

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

Copilot code review Copilot Copilot left review comments

@Ritikaa96 Ritikaa96 Ritikaa96 requested changes

@liggitt liggitt Awaiting requested review from liggitt

@windsonsea windsonsea Awaiting requested review from windsonsea

Assignees

@Ritikaa96 Ritikaa96

@windsonsea windsonsea

Labels
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. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
SIG Auth
Status: Changes Requested
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@sftim @k8s-ci-robot @Shubham82 @Ritikaa96 @windsonsea