Update audit.md #46655
Update audit.md #46655
Conversation
Adjust phrasing of the metadata audit level description to provide consistency of terms between levels. Previously the metadata level was the only one referencing "request metadata" rather than "event metadata".
|
Welcome @mallardduck! |
✅ Pull request preview available for checkingBuilt without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
|
The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
|
/remove-lifecycle rotten |
|
|
||
| - `None` - don't log events that match this rule. | ||
| - `Metadata` - log request metadata (requesting user, timestamp, resource, | ||
| - `Metadata` - log event metadata (requesting user, timestamp, resource, |
There was a problem hiding this comment.
this seems wrong. The metadata is about requests, not about events. The events are the carrier of the metadata, not the source.
There was a problem hiding this comment.
Similarly, line 68 is not accurate.
There was a problem hiding this comment.
why not accurate? What's wrong with that line?
There was a problem hiding this comment.
Maybe this would clarify it for line 69: log events with request metadata only (....)
There was a problem hiding this comment.
Interesting, so if the metadata is about requests then 70/71 are wrong too. Since it uses the same phrasing and that was the root of my confusion.
In terms of all the options except None - don't they all provide the same type of metadata, or does the subject of that metadata actually change based on the option? If they provide metadata on the same subject, then the phrasing on all 3 active options should be uniform to be more clear.
Today line 69 reads as the only one referencing the request metadata versus event metadata.
There was a problem hiding this comment.
@sttts - I like that suggestion, so keeping with the goal of clearer consistency that would mean the others would read as:
- Metadata - log events with request metadata [...]
- Request - log events with request metadata and body but not response body. [...]
- RequestResponse - log events with request metadata, request body and response body. [...]
There was a problem hiding this comment.
Of course this might be hairsplitting. Technically this is what we are talking about here:
The fields metav1.TypeMeta, Level, AuditID and Stage are event metadata.
All the other fields lik RequestURI, Verb, .... are request metadata, with exception of RequestObject and ResponseObject which are not logged in Metadata mode.
There was a problem hiding this comment.
Right. The audit module generates audit events, based on request/response body and/or metadata. When we say "log events with request metadata only", we are talking about things that are generated. When we say "log request metadata", we are talking about the data source. It is the wording here that causes confusion, IIUC.
8000There was a problem hiding this comment.
I've updated this PR per the feedback here. It seemed that the suggestion @sttts provided and I applied to all 3 was going to be acceptable to clarify the wording?
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tengqm 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 |
|
/label tide/merge-method-squash |
|
A lot better now. Thank you! /lgtm |
|
LGTM label has been added. Git tree hash: 27a746e2c4abe869c42d526ebe5bebe124d1b957
|
* Update audit.md Adjust phrasing of the metadata audit level description to provide consistency of terms between levels. Previously the metadata level was the only one referencing "request metadata" rather than "event metadata". * Adjust wording per feedback thread
* Update audit.md Adjust phrasing of the metadata audit level description to provide consistency of terms between levels. Previously the metadata level was the only one referencing "request metadata" rather than "event metadata". * Adjust wording per feedback thread
* Update audit.md Adjust phrasing of the metadata audit level description to provide consistency of terms between levels. Previously the metadata level was the only one referencing "request metadata" rather than "event metadata". * Adjust wording per feedback thread
Adjust phrasing of the metadata audit level description to provide consistency of terms between levels.
Previously the metadata level was the only one referencing "request metadata" rather than "event metadata".
If the metadata logged does not change between these levels, then it seems apt to use a consistent term.