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

Documentation for CEL in Admission Control #37770

Merged
merged 2 commits into from
Dec 2, 2022
Merged

Documentation for CEL in Admission Control #37770

merged 2 commits into from
Dec 2, 2022

Conversation

cici37
Copy link
Contributor

@cici37 cici37 commented Nov 7, 2022 •
edited
Loading

Adding doc for CEL in Admission Control on KEP: kubernetes/enhancements#3488

Co-author: @jiahuif @DangerOnTheRanger

@k8s-ci-robot k8s-ci-robot added this to the 1.26 milestone Nov 7, 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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 7, 2022
@netlify
Copy link

netlify bot commented Nov 7, 2022 •
edited
Loading

👷 Deploy Preview for kubernetes-io-vnext-staging processing.

Name Link
🔨 Latest commit 98d41f2
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/638789273a5353000925a5f1

@k8s-ci-robot k8s-ci-robot added the sig/docs Categorizes an issue or PR as relevant to SIG Docs. label Nov 7, 2022
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Nov 7, 2022
@cici37 cici37 mentioned this pull request Nov 7, 2022
Open
16 tasks
@krol3
Copy link
Contributor

krol3 commented Nov 15, 2022

Hi @cici37 , Thank you for the draft doc PR here, Please update to Ready for Review, the deadline it's on Tuesday 15th November 2022. Thank you!

@k8s-ci-robot k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. language/en Issues or PRs related to English language and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 19, 2022
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 19, 2022
@cici37 cici37 changed the title [WIP] Placeholder pr for CEL in Admission Control Documentation for CEL in Admission Control Nov 19, 2022
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 19, 2022
@krol3
Copy link
Contributor

krol3 commented Nov 28, 2022

Hi @cici37! This PR needs a doc review by Mon Nov 28th to get this into the release. Please reach out to required SIGs to get their review. Thank you!

jpbetz
jpbetz approved these changes Nov 28, 2022
View reviewed changes
Copy link
Contributor

@jpbetz jpbetz 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 Nov 28, 2022
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: d428d2023c6027cba2643c5dc5d72258aae33129

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 28, 2022
@jpbetz
Copy link
Contributor

jpbetz commented Nov 28, 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 Nov 28, 2022
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: fb3f95094d8edb747f7de830e887b2f5a4f20de9

@krol3
Copy link
Contributor

krol3 commented Nov 29, 2022

@cici37 Please review the suggestions to merge this PR

nate-double-u
nate-double-u reviewed Nov 29, 2022
View reviewed changes
Copy link
Contributor

@nate-double-u nate-double-u left a comment

Choose a reason for hiding this comment

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

This lgtm from a docs perspective.

I've noted a couple nits in line, but none of those should hold up approval.

content/en/docs/reference/access-authn-authz/admission-controllers.md Outdated Show resolved Hide resolved
content/en/docs/reference/access-authn-authz/validating-admission-policy.md Outdated Show resolved Hide resolved
content/en/docs/reference/access-authn-authz/validating-admission-policy.md Outdated Show resolved Hide resolved
content/en/docs/reference/access-authn-authz/validating-admission-policy.md Outdated Show resolved Hide resolved
content/en/docs/reference/access-authn-authz/validating-admission-policy.md Outdated Show resolved Hide resolved
@jpbetz jpbetz mentioned this pull request Nov 30, 2022
Merged
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 30, 2022
sftim
sftim reviewed Nov 30, 2022
View reviewed changes
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.

More thoughts.

content/en/docs/reference/access-authn-authz/validating-admission-policy.md Outdated

Validating Admission Policy is part of the cluster control-plane. You should write and deploy them with great caution. The following describes how to quickly experiment with Validating Admission Policy.

Prerequisites
Copy link
Contributor

Choose a reason for hiding this comment

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

We'd typically have a section

Suggested change
Prerequisites
## {{% heading "prerequisites" %}}

You could also split this into (small) reference page and a task page, and document the prerequisites for the task only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggestion applied. Plan to add a task page in next release after some other planned work in. Thank you for the suggestion :)

content/en/docs/reference/access-authn-authz/admission-controllers.md Show resolved Hide resolved
content/en/docs/reference/access-authn-authz/validating-admission-policy.md Outdated Show resolved Hide resolved
content/en/docs/reference/access-authn-authz/validating-admission-policy.md Outdated Show resolved Hide resolved
content/en/docs/reference/access-authn-authz/validating-admission-policy.md Outdated Show resolved Hide resolved
content/en/docs/reference/access-authn-authz/validating-admission-policy.md Outdated Show resolved Hide resolved
@cici37
Copy link
Contributor Author

cici37 commented Nov 30, 2022

Thank you for the review! The comments have been addressed. Please have a look when have time. Thank you

@krol3
Copy link
Contributor

krol3 commented Nov 30, 2022

@sftim Could you take a look on it?

@leonardpahlke
Copy link
Member

K8s v1.26 is scheduled for less than a week, so we need to get this done as soon as possible! 🙏

@leonardpahlke
Copy link
Member

Thanks Cici :)

krol3
krol3 approved these changes Dec 2, 2022
View reviewed changes
Copy link
Contributor

@krol3 krol3 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 Dec 2, 2022
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 0f6bfe891f83ec8b670e566729be2368dc116044

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jpbetz, krol3

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 Dec 2, 2022
@k8s-ci-robot k8s-ci-robot merged commit 50246c2 into kubernetes:dev-1.26 Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nate-double-u nate-double-u nate-double-u left review comments

@andrewsykim andrewsykim andrewsykim left review comments

@sftim sftim sftim requested changes

@krol3 krol3 krol3 approved these changes

@daminisatya daminisatya Awaiting requested review from daminisatya

@onlydole onlydole Awaiting requested review from onlydole

@jiahuif jiahuif Awaiting requested review from jiahuif

@jpbetz jpbetz Awaiting requested review from jpbetz

Assignees

@jpbetz jpbetz

@jiahuif jiahuif

@krol3 krol3

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/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
1.26
Development

Successfully merging this pull request may close these issues.
9 participants
@cici37 @krol3 @k8s-ci-robot @jiahuif @jpbetz @leonardpahlke @nate-double-u @andrewsykim @sftim