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

KEP-555: omit managed fields from audit log #2982

Merged
merged 1 commit into from
Sep 28, 2021
Merged

KEP-555: omit managed fields from audit log #2982

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
62 changes: 62 additions & 0 deletions keps/sig-api-machinery/555-server-side-apply/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
- [Proposed Change](#proposed-change)
- [Alternatives](#alternatives)
- [Implementation History](#implementation-history)
- [API Audit](#api-audit)
- [Production Readiness Review Questionnaire](#production-readiness-review-questionnaire)
- [Feature Enablement and Rollback](#feature-enablement-and-rollback)
- [Rollout, Upgrade and Rollback Planning](#rollout-upgrade-and-rollback-planning)
Expand Down Expand Up @@ -286,6 +287,67 @@ The conversion between the two and creating the diff was complex and would have

- 12/2019 [#86083](https://github.com/kubernetes/kubernetes/pull/86083) implementing a poc for the described approach


#### API Audit

The `ManagedFields` fields of an object in the API audit log may not be very useful. We want to provide a mechanism,
so the cluster operator can opt in so that the managed fields can be omitted from the audit log.

We propose the following changes to the `audit.k8s.io/Policy` API that provides the cluster operator with a more
granular way to control the omission of managed fields in audit log:
```go
type Policy struct {
// +optional
OmitManagedFields bool `json:"omitManagedFields,omitempty"`
}

type PolicyRule struct {
// +optional
OmitManagedFields *bool `json:"omitManagedFields,omitempty"`
}
```
The above API changes will be introduced in `v1`, `v1beta1` and `v1alpha1` of `audit.k8s.io`
Copy link
Contributor

Choose a reason for hiding this comment

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

we never roundtrip these, do we? So technically we could do this in v1 only.

Copy link
Member

Choose a reason for hiding this comment

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

But there's no harm in doing it in all, no?

Copy link
Contributor

Choose a reason for hiding this comment

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

only that we encourage use of v1 by not implementing new stuff in old versions.


A new field `OmitManagedFields` is added to both `Policy` and `PolicyRule` making the following possible:
- `Policy.OmitManagedFields` sets the default policy for omitting managed fields globally.
- the default value is `false`, managed fields are not omitted, this retains the current behavior.
Copy link
Member

Choose a reason for hiding this comment

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

Any plans for changing the default value in a future release since the fields may not be very useful? making it opt-out instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not that i am aware of, primarily we didn't want to quietly drop managed fields from audit log. The level RequestResponse stipulates that the response object in its "entirety" is being audited, so if we are to drop any fields from the audit log then I think it should be an opt-in by the operator.

- a value of `true` will omit managed fields from being written to the API audit log unless `PolicyRule` overrides.
- `PolicyRule:OmitManagedFields` can be used to override the global default for a particular set of request(s),
it has three possible values:
- `nil` (default value): the cluster operator did not specify any value,
the global default specified in `Policy.OmitManagedFields` is in effect.
- `true`: the cluster operator opted in to omit managed fields for a given set of request(s), and it overrides the global default.
- `false`: the cluster operator opted in to not omit managed fields for a given set of request(s), and it overrides the global default.

This ensures the following:
- with an existing `Policy` object, the new version of the apiserver will maintain current behavior which
is to include managed fields in audit log
- the cluster operator must opt in to enable omission of managed fields

Let's look at a few examples:
```yaml
# omit managed fields for all request and all response bodies
apiVersion: audit.k8s.io/v1
kind: Policy
omitManagedFields: true
rules:
- level: RequestResponse
```

```yaml
# omit managed fields for all request and all response bodies
# except for Pod for which we want to include managed fields in audit log
apiVersion: audit.k8s.io/v1
kind: Policy
omitManagedFields: true
rules:
- level: RequestResponse
omitManagedFields: false
resources: ["pods"]

- level: RequestResponse
```

## Production Readiness Review Questionnaire

<!--
Expand Down
3 changes: 2 additions & 1 deletion keps/sig-api-machinery/555-server-side-apply/kep.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ title: Apply
kep-number: 555
authors:
- "@lavalamp"
- "@tkashem"
owning-sig: sig-api-machinery
participating-sigs:
- sig-api-machinery
Expand All @@ -15,7 +16,7 @@ prr-approvers:
- "@deads2k"
editor: TBD
creation-date: 2018-03-28
last-updated: 2021-02-21
last-updated: 2021-09-21
status: implementable
see-also:
- n/a
Expand Down