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 documentation for system:monitoring rbac policy #22715

Merged
merged 1 commit into from
Oct 20, 2020

Conversation

logicalhan
Copy link
Member

A new bootstrapping role for monitoring endpoints was introduced in kubernetes/kubernetes#93311 and this PR intends to add documentation to our bootstrapped default RBAC policies/roles/role-bindings/groups so that it reflects the new addition.

@logicalhan
Copy link
Member Author

/assign @liggitt

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

netlify bot commented Jul 24, 2020

Deploy preview for kubernetes-io-master-staging ready!

Built with commit d04fdb9

https://deploy-preview-22715--kubernetes-io-master-staging.netlify.app

@liggitt
Copy link
Member

liggitt commented Jul 24, 2020

one nit, lgtm otherwise

/hold
for 1.20 dev branch

@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 Jul 24, 2020
@logicalhan logicalhan force-pushed the monitoring branch 2 times, most recently from 9ba6d9e to 3169642 Compare July 24, 2020 19:04
@sftim
Copy link
Contributor

sftim commented Jul 25, 2020

/sig auth

@k8s-ci-robot k8s-ci-robot added the sig/auth Categorizes an issue or PR as relevant to SIG Auth. label Jul 25, 2020
@jimangel
Copy link
Member

/assign @savitharaghunathan

Given the comment that this should be for 1.20, I wonder if we should consider alternate ways to handling this or potentially close this PR - waiting for the 1.20 cycle to begin.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 27, 2020
@liggitt
Copy link
Member

liggitt commented Jul 27, 2020

potentially close this PR - waiting for the 1.20 cycle to begin.

why not hold and add to the 1.20 milestone?

@celestehorgan
Copy link
Contributor

Per @LiGgit's suggestion:

/milestone 1.20

@k8s-ci-robot k8s-ci-robot added this to the 1.20 milestone Aug 31, 2020
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 3, 2020
@savitharaghunathan
Copy link
Member

savitharaghunathan commented Sep 23, 2020

one nit, lgtm otherwise

/hold
for 1.20 dev branch

@logicalhan Can you update the target branch to dev-1.20 instead of master ?

@annajung
Copy link
Contributor

annajung commented Oct 2, 2020

Hi @logicalhan 1.20 docs lead here, a friendly reminder to update the target branch to dev-1.20.

@logicalhan logicalhan changed the base branch from master to dev-1.20 October 8, 2020 20:49
@logicalhan
Copy link
Member Author

Hi @logicalhan 1.20 docs lead here, a friendly reminder to update the target branch to dev-1.20.

Hi! Sorry have been out on paternity leave, updated the branch! Thanks!

@annajung
Copy link
Contributor

annajung commented Oct 8, 2020

Thank you @logicalhan!

Also, was just reviewing this and if you take a look at the preview https://deploy-preview-22715--kubernetes-io-master-staging.netlify.app/docs/reference/access-authn-authz/rbac/

it actually does not apply ` correctly, and looks like this. It looks like this can be solved using < tt >< /tt > (without spaces, added spaces so that it shows up in the comment) instead.

Screen Shot 2020-10-08 at 4 23 39 PM

@annajung
Copy link
Contributor

annajung commented Oct 8, 2020

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 8, 2020
@k8sio-netlify-preview-bot
Copy link
Collaborator

Deploy preview for kubernetes-io-vnext-staging processing.

Building with commit f37f473

https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/5f7f97dbc52f3c00071d9bee

@logicalhan
Copy link
Member Author

it actually does not apply ` correctly, and looks like this. It looks like this can be solved using < tt >< /tt > (without spaces, added spaces so that it shows up in the comment) instead.

Applied the change. Thanks for the quick review!

@annajung
Copy link
Contributor

Hey @zparnold Can I get your review on this?

I believe technical review from @liggitt stands, just need a review from sig docs and we can get this approved!

<tbody>
<tr>
<td><b>system:monitoring</b></td>
<td><b>system:monitoring</b> group</td>
Copy link
Contributor

Choose a reason for hiding this comment

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

Should group appear in bold?

<b>group</b>

I think the <tt> element is deprecated?
For code or endpoints, you can use backticks:
/healthz

The table data content could be formatted as:

Allows read access to control-plane monitoring endpoints such as the {{< glossary_tooltip term_id="kube-apiserver" text="kube-apiserver" >}} liveness and readiness endpoints (/healthz, /livez, /readyz), the individual health-check endpoints (/healthz/*, /livez/*, /readyz/*), and /metrics. Note that individual health check endpoints and the metric endpoint may expose sensitive information.

Copy link
Contributor

@annajung annajung Oct 20, 2020

Choose a reason for hiding this comment

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

Hi @kbhawkey, using backticks resulted in the following (#22715 (comment)) so I suggested to change to < tt >. It doesn't seem to be deprecated as the page renders as expected (https://5f7f97dbc52f3c00071d9bee--kubernetes-io-vnext-staging.netlify.app/docs/reference/access-authn-authz/rbac/). PTAL when you can, thank you!

@logicalhan pinging to review Karen's suggestions! :D

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, the HTML table looks good.
/lgtm

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

LGTM label has been added.

Git tree hash: cea016092bc86b692fcf48c660d1fe31023c0681

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.

/approve

<tr>
<td><b>system:monitoring</b></td>
<td><b>system:monitoring</b> group</td>
<td>Allows read access to control-plane monitoring endpoints (i.e. {{< glossary_tooltip term_id="kube-apiserver" text="kube-apiserver" >}} liveness and readiness endpoints (<tt>/healthz</tt>, <tt>/livez</tt>, <tt>/readyz</tt>), the individual health-check endpoints (<tt>/healthz/*</tt>, <tt>/livez/*</tt>, <tt>/readyz/*</tt>), and <tt>/metrics</tt>). Note that individual health check endpoints and the metric endpoint may expose sensitive information.</td>
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we prefer to avoid Latin abbreviations (”i.e.” and similar)

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: logicalhan, 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 Oct 20, 2020
@k8s-ci-robot k8s-ci-robot merged commit 7cfdee6 into kubernetes:dev-1.20 Oct 20, 2020
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/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants