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

Improvement: Corrected the link for Authorization. #30430

Merged
merged 1 commit into from
Nov 30, 2021
Merged

Improvement: Corrected the link for Authorization. #30430

merged 1 commit into from
Nov 30, 2021

Conversation

Shubham82
Copy link
Contributor

This PR fixes the link of Authorization under the API Access Extensions in the Extending Kubernetes docs.

Fixes: #30429

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. language/en Issues or PRs related to English language labels Nov 10, 2021
@k8s-ci-robot k8s-ci-robot added the sig/docs Categorizes an issue or PR as relevant to SIG Docs. label Nov 10, 2021
@Shubham82
Copy link
Contributor Author

Hi @cheftako @lavalamp @bradtopol
PTAL!

@Shubham82 Shubham82 mentioned this pull request Nov 10, 2021
Closed
@netlify
Copy link

netlify bot commented Nov 10, 2021 •
edited
Loading

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

🔨 Explore the source changes: 349be77

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

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

@seokho-son
Copy link
Member

@Shubham82 Thanks !

/lgtm

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

LGTM label has been added.

Git tree hash: afdb041194424bafab14699505672b74daabd799

@Shubham82
Copy link
Contributor Author

Thanks, @seokho-son for taking look at this.

mehabhalodiya
mehabhalodiya reviewed Nov 11, 2021
View reviewed changes
Copy link
Contributor

@mehabhalodiya mehabhalodiya left a comment

Choose a reason for hiding this comment

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

LGTM!
Thanks, @Shubham82 for improvement :)

@Shubham82
Copy link
Contributor Author

LGTM! Thanks, @Shubham82 for improvement :)

You're welcome! @mehabhalodiya

kbhawkey
kbhawkey reviewed Nov 11, 2021
View reviewed changes
content/en/docs/concepts/extend-kubernetes/_index.md Outdated
@@ -145,7 +145,7 @@ Kubernetes provides several built-in authentication methods, and an [Authenticat

### Authorization

[Authorization](/docs/reference/access-authn-authz/webhook/) determines whether specific users can read, write, and do other operations on API resources. It works at the level of whole resources -- it doesn't discriminate based on arbitrary object fields. If the built-in authorization options don't meet your needs, and [Authorization webhook](/docs/reference/access-authn-authz/webhook/) allows calling out to user-provided code to make an authorization decision.
[Authorization](/docs/reference/access-authn-authz/authorization/) determines whether specific users can read, write, and do other operations on API resources. It works at the level of whole resources -- it doesn't discriminate based on arbitrary object fields. If the built-in authorization options don't meet your needs, and [Authorization webhook](/docs/reference/access-authn-authz/webhook/) allows calling out to user-provided code to make an authorization decision.
Copy link
Contributor

Choose a reason for hiding this comment

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

HI @Shubham82 . The link update looks great.
What do you think about removing the word, "and" before the second link text (Authorization webhook)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @kbhawkey, Thanks for your suggestion
It seems good to me, I will update it soon.
Have you any suggestions other than that?

@kbhawkey
Copy link
Contributor

Page preview:
https://deploy-preview-30430--kubernetes-io-main-staging.netlify.app/docs/concepts/extend-kubernetes/

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 15, 2021
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 15, 2021 •
edited
Loading

CLA Not Signed

@Shubham82
Copy link
Contributor Author

Hi @kbhawkey

What do you think about removing the word, "and" before the second link text (Authorization webhook)?

Addressed the comments, PTAL!

@jihoon-seo
Copy link
Member

/lgtm

@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 74a2edb70350d3d9536a2243bdf61ab7c046e0dd

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 16, 2021
RinkiyaKeDad
RinkiyaKeDad approved these changes Nov 17, 2021
View reviewed changes
Copy link
Member

@RinkiyaKeDad RinkiyaKeDad left a comment

Choose a reason for hiding this comment

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

LGTM too!

@RinkiyaKeDad
Copy link
Member

@Shubham82 you'll have to sign the CLA before we can merge this 🙂

@Shubham82
Copy link
Contributor Author

Hi, @RinkiyaKeDad I have already signed the CLA. This EasyCLA is optional.
Please refer to this: kubernetes/test-infra#24381

@Shubham82
Copy link
Contributor Author

HI @seokho-son @jihoon-seo
If everything seems good then Could you please approve this PR?
Thanks

@kbhawkey
Copy link
Contributor

/label tide/merge-method-squash

@k8s-ci-robot k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Nov 18, 2021
@kbhawkey
Copy link
Contributor

kbhawkey commented Nov 18, 2021 •
edited
Loading

Hi, @Shubham82 .
The changes look good.
I see that the CNCF CLA label has been applied. However, do you need to click the EasyCLA to authorize (and enable all CI/CD checks)?
Another idea, could you rebase to a single commit (one of the commits is attached to the EasyCLA)?
Thanks.

@Shubham82
Copy link
Contributor Author

Hi @kbhawkey

However, do you need to click the EasyCLA to authorize (and enable all CI/CD checks)?

I don't think I need to authorize EasyCLA to merge this PR. It is an optional thing.

Another idea, could you rebase to a single commit (one of the commits is attached to the EasyCLA)?

yes, I will squash it to a single commit.

@Shubham82
Improvement for Authorization in Extending Kubernetes docs.
349be77 
Improvement: Corrected the link for Authorization.

Fix Typo
@Shubham82 Shubham82 force-pushed the Correct-link-Authorization branch from 52ded0e to 349be77 Compare November 23, 2021 11:36
@Shubham82
Copy link
Contributor Author

Hi @kbhawkey

PTAL!

@tengqm
Copy link
Contributor

tengqm commented Nov 24, 2021

@Shubham82 CLA signing is mandatory, not optional.

@Shubham82
Copy link
Contributor Author

Hi @tengqm
I already signed the CLA. This EasyCLA is a different thing that is optional and ignored by Kubernetes as of now.
Ref: kubernetes/test-infra#24381

@Shubham82
Copy link
Contributor Author

/easycla

@RA489
Copy link

RA489 commented Nov 29, 2021

kubernetes/org#3068
/easycla

@Shubham82
Copy link
Contributor Author

Hi, @kbhawkey @sftim @tengqm Looking for approval level.

@Shubham82 Shubham82 mentioned this pull request Nov 29, 2021
Closed
@mrbobbytables
Copy link
Member

EasyCLA is in non-blocking mode to surface various issues between the old system and the new one, it's okay that it's reporting an error as long as its reported in the tracking thread (it is).
See the announcement about it here: https://groups.google.com/g/kubernetes-dev/c/-mdBNXLYZ_Y/m/Bp7B4W7IAAAJ
Feedback thread here: kubernetes/org#3068

@sftim
Copy link
Contributor

sftim commented Nov 30, 2021

/remove-label tide/merge-method-squash
/approve

@k8s-ci-robot k8s-ci-robot removed the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Nov 30, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RinkiyaKeDad, sftim

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 Nov 30, 2021
@k8s-ci-robot k8s-ci-robot merged commit 0f4eb18 into kubernetes:main Nov 30, 2021
@Shubham82 Shubham82 mentioned this pull request Jan 8, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mehabhalodiya mehabhalodiya mehabhalodiya left review comments

@kbhawkey kbhawkey kbhawkey left review comments

@RinkiyaKeDad RinkiyaKeDad RinkiyaKeDad approved these changes

@cheftako cheftako Awaiting requested review from cheftako

@lavalamp lavalamp Awaiting requested review from lavalamp

Assignees

@seokho-son seokho-son

@jihoon-seo jihoon-seo

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/docs Categorizes an issue or PR as relevant to SIG Docs. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Wrong Link for Authorization.