update dynamic audit kep to reduce scope #2738
Conversation
k8s-ci-robot
added
size/M
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: If they are not already assigned, you can assign the PR to them by writing 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 |
k8s-ci-robot
added
the
needs-ok-to-test
k8s-ci-robot
added
sig/architecture
|
/ok-to-test |
k8s-ci-robot
removed
the
needs-ok-to-test
|
|
||
| // Policy is a scoped down policy for audit webhooks | ||
| type Policy struct { | ||
| // The Level that all requests are recorded at. |
|
|
||
| // OmitStages is a list of stages for which no events are created. | ||
| // +optional | ||
| OmitStages []Stage |
There was a problem hiding this comment.
Can we please make this a whitelist? I know it deviates from the static version, but IMO not making that a whitelist was a mistake (IncludeStages). Must be non-empty (or default to all?).
| - level: <level> | ||
| level: <level> | ||
| omitStages: | ||
| - stage: <stage> |
There was a problem hiding this comment.
nit: this shouldn't have stage:, just - <stage>
| drop any pieces that do not conform to its policy. A new sink interface will be required for these changes called `EnforcedSink`, | ||
| this will largely follow suite with the existing sink but take a fully formed event and the authorizer attributes as its | ||
| parameters. It will then utilize the `LevelAndStages` method in the policy | ||
| This addition will move policy enforcement from the main handler to the backends. The policy object is a scoped down |
There was a problem hiding this comment.
nit: Rather than "... is a scoped down version ...", I would say "... is a minimal top-level policy ..."
| this will largely follow suite with the existing sink but take a fully formed event and the authorizer attributes as its | ||
| parameters. It will then utilize the `LevelAndStages` method in the policy | ||
| This addition will move policy enforcement from the main handler to the backends. The policy object is a scoped down | ||
| version of the V1 Policy object, further filtering will be done in a proxy. This has been done because how we handle policy |
| may change in the future. From the `withDynamicAudit` handler, the full event will be generated and then passed to the backends. | ||
| Each backend will copy the event and then be required to drop any pieces that do not conform to its policy. A new sink interface | ||
| will be required for these changes called `EnforcedSink`, this will largely follow suite with the existing sink but take a | ||
| fully formed event and the authorizer attributes as its parameters. It will then utilize the `LevelAndStages` method in the policy |
There was a problem hiding this comment.
I'm a bit confused by this. What is EnforcedSink? Is that client (apiserver) side, or server (webhook) side? Is it just another audit backend plugin, or something else?
| called `withDynamicAudit`. Another conditional clause will be added where the handlers are | ||
| [provisioned](https://github.com/kubernetes/apiserver/blob/master/pkg/server/config.go#L536) allowing for the proper feature gating. | ||
|
|
||
| #### Policy Enforcement |
There was a problem hiding this comment.
Do you want to add a note somewhere in here that this approach lets us prototype the flexible policy design as a CRD with a proxy backend?
|
REMINDER: KEPs are moving to k/enhancements on November 30. Please attempt to merge this KEP before then to signal consensus. Any questions regarding this move should be directed to that thread and not asked on GitHub. |
|
KEPs have moved to k/enhancements. Any questions regarding this move should be directed to that thread and not asked on GitHub. |
|
@justaugustus: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
4 participants
Scopes down the dynamic audit configuration API in case we need to change the policy in the future