search

kubernetes / website

Kubernetes website and documentation repo:
https://kubernetes.io
Creative Commons Attribution 4.0 International
4.44k stars 14.32k forks source link

Improve the ValidationAdmissionPolicy docs #38504

Open samos123 opened 1 year ago

samos123 commented 1 year ago

This is a Feature Request

What would you like to be added

comment from @tengqm: Issues like this would be easy to detect if we make the manifest an example YAML. For example:

  1. move the YAML snippet into a separate file content/en/examples/policy/validating-admission-ex.yaml and,
  2. reference it as {{< codenew file="policy/validating-admission-ex.yaml" >}} in the markdown page,
  3. (optionally) amend content/en/examples/examples_test.go so that it can be validated against current and future k8s releases.

Why is this needed Current docs was hard to follow for a new user like myself

jimmyraywv commented 1 year ago

So, more documentation about what the admission controller receives from the API would also be helpful. Is it the customary AdmissionReview object?

sftim commented 1 year ago

more documentation about what the admission controller receives from the API would also be helpful. Is it the customary AdmissionReview object?

We typically don't document internal details like that. Maybe that's something we could document within eg https://k8s.dev/docs/

Kubernetes is free to change its internal implementation and end users shouldn't need to know how that's done.

sftim commented 1 year ago

/sig api-machinery

Improvements are welcome

This is an alpha feature; we should aim to make the docs better before this graduates to beta. /priority backlog

sftim commented 1 year ago

/language en

If we wanted to, we could add a new tutorial that shows how to do admission-time validation. First with a webhook and then with CEL.

jimmyraywv commented 1 year ago

@sftim The issue is that we need to know what data is sent in the api server request to the ValidatingAdmissionPolicy, so that we can understand what capabilities this solution has.

If you look at the docs for Webhook Mode, you can see that the API server request is documented there: https://kubernetes.io/docs/reference/access-authn-authz/webhook/

The AdmissionReview object is also documented in the Dynamic Admission Control docs: https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#webhook-request-and-response

tengqm commented 1 year ago

PR #38522 is adding a reference to the admission config API where AdmissionReview, AdmissionRequest are documented. Is that what you want, @jimmyraywv ?

jimmyraywv commented 1 year ago

@tengqm I think that will work. However for UserInfo v1 authentication.k8s.io, for the groups []string, are those k8s RBAC groups or are those AuthN IDP groups, like group claims from OIDC?

What I am driving towards is that will this feature allow us to write policies with CEL that blur the lines between traditional validating admission webhooks, and the AuthZ Webhook Mode?

tengqm commented 1 year ago

@tengqm I think that will work. However for UserInfo v1 authentication.k8s.io, for the groups []string, are those k8s RBAC groups or are those AuthN IDP groups, like group claims from OIDC?

AFAICT, k8s doesn't have an abstraction for users or an abstraction for groups. All those information are extracted from the authentication input.

What I am driving towards is that will this feature allow us to write policies with CEL that blur the lines between traditional validating admission webhooks, and the AuthZ Webhook Mode?

I believe CEL was not designed for authn or authz's purpose. It is a mechanism for you to leverage, to impose constraints that are difficult or impossible to express using OpenAPI schemas. You can mix these admission plugins in any way to meet your requirements.

sftim commented 1 year ago

AFAICT, k8s doesn't have an abstraction for users or an abstraction for groups. All those information are extracted from the authentication input.

You can extend Kubernetes to add those things - this isn't just theoretical, there's a User API in OpenShift 4.8 for example.

Core Kubernetes doesn't have User or Group APIs. If your container platform does have extension APIs for users and groups, this alpha feature lets you use CEL to validate changes at admission time.

You asked about the (also alpha) SelfSubjectReview API.

https://kubernetes.io/docs/reference/kubernetes-api/authorization-resources/self-subject-review-v1alpha1/#SelfSubjectReviewStatus is filled in after you create a SelfSubjectReview. Here's what you might submit, as YAML, if you wanted to find out who the Kubernetes cluster thinks you are:

apiVersion: authentication.k8s.io/v1alpha1
kind: SelfSubjectReview

At HTTP level, the body of response might look like:

{
  "apiVersion": "authentication.k8s.io/v1alpha1",
  "kind": "SelfSubjectReview",
  "status": {
    "userInfo": {
      "username": "sftim",
      "uid": "71d99f8d-5b1b-4625-9149-037dc46fdc83",
      "groups": [
        "en-reviewers",
        "en-approvers",
        "system:authenticated"
      ],
      "extra": {
        "emoji": [
          "true"
        ]
      }
    }
  }
}

but a ValidatingAdmissionPolicy constrains what you send there, not what you get back.

In a couple of days, we'll have a blog article about this. I'm afraid it's not yet live. Please check back on the 20th of December.

We don't have anything to document here that I can see. Is there a page that needs to be more clear?

sftim commented 1 year ago

The issue is that we need to know what data is sent in the API server request to the ValidatingAdmissionPolicy, so that we can understand what capabilities this solution has.

As a model of its operation, the API server doesn't send any data to itself to implement and enforce a ValidatingAdmissionPolicy. Instead, the ValidatingAdmissionPolicy has access to:

A ValidatingAdmissionPolicy doesn't have access to any context about the client, the current time, the phase of the moon, etc; if you want to take those into account, you'd need to use a validating admission webhook or some even more custom way to extend Kubernetes.

The docs don't explicitly say this; they explain what is possible and leave you to work the rest out by example. In a tutorial page, if we had one, we could make that point much more explicitly.

sftim commented 1 year ago

A new reference page would be good too: we could specify what version of CEL you can use, and the (short) list of available variables.

If we add one, that reference should cover CEL expressions for CRD validation and for ValidatingAdmissionPolicies, in one place. Would that help you @samos123?

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

k8s-ci-robot commented 1 year ago

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to [this](https://github.com/kubernetes/website/issues/38504#issuecomment-1552863140): >The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. > >This bot triages issues according to the following rules: >- After 90d of inactivity, `lifecycle/stale` is applied >- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied >- After 30d of inactivity since `lifecycle/rotten` was applied, the issue is closed > >You can: >- Reopen this issue with `/reopen` >- Mark this issue as fresh with `/remove-lifecycle rotten` >- Offer to help out with [Issue Triage][1] > >Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community). > >/close not-planned > >[1]: https://www.kubernetes.dev/docs/guide/issue-triage/ Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
sftim commented 1 year ago

/reopen

I think this is worth doing.

/sig api-machinery /remove-lifecycle rotten /priority backlog

k8s-ci-robot commented 1 year ago

@sftim: Reopened this issue.

In response to [this](https://github.com/kubernetes/website/issues/38504#issuecomment-1552876953): >/reopen > >I think this is worth doing. > >/sig api-machinery >/remove-lifecycle rotten >/priority backlog Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
jpbetz commented 1 year ago

@cici37 Would you be willing to find someone to propose an outline?

k8s-triage-robot commented 8 months ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot commented 7 months ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot commented 6 months ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

k8s-ci-robot commented 6 months ago

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to [this](https://github.com/kubernetes/website/issues/38504#issuecomment-2010528289): >The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. > >This bot triages issues according to the following rules: >- After 90d of inactivity, `lifecycle/stale` is applied >- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied >- After 30d of inactivity since `lifecycle/rotten` was applied, the issue is closed > >You can: >- Reopen this issue with `/reopen` >- Mark this issue as fresh with `/remove-lifecycle rotten` >- Offer to help out with [Issue Triage][1] > >Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community). > >/close not-planned > >[1]: https://www.kubernetes.dev/docs/guide/issue-triage/ Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
cici37 commented 6 months ago

/assign @mjlshen /reopen Hi @mjlshen , here is the old issue of suggestions on existing doc style. It would be great if it would squeeze into your doc PR :)

/triage accepted

k8s-ci-robot commented 6 months ago

@cici37: GitHub didn't allow me to assign the following users: mjlshen.

Note that only kubernetes members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. For more information please see the contributor guide

In response to [this](https://github.com/kubernetes/website/issues/38504#issuecomment-2010763686): >/assign @mjlshen >/reopen >Hi @mjlshen , here is the old issue of suggestions on existing doc style. It would be great if it would squeeze into your doc PR :) > >/triage accepted Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
k8s-ci-robot commented 6 months ago

@cici37: Reopened this issue.

In response to [this](https://github.com/kubernetes/website/issues/38504#issuecomment-2010763686): >/assign @mjlshen >/reopen >Hi @mjlshen , here is the old issue of suggestions on existing doc style. It would be great if it would squeeze into your doc PR :) > >/triage accepted Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
a-mccarthy commented 5 months ago

/remove-lifecycle rotten