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

Don't duplicate status in audit events #60108

Closed
tallclair opened this issue Feb 21, 2018 · 8 comments · Fixed by #62695
Closed

Don't duplicate status in audit events #60108

tallclair opened this issue Feb 21, 2018 · 8 comments · Fixed by #62695
Labels
area/audit kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/auth Categorizes an issue or PR as relevant to SIG Auth.
Milestone
v1.11

Comments

@tallclair
Copy link
Member

We copy the ResponseObject into the ResponseStatus field of audit events. This was done to capture the response status even when the response is omitted, but results in (potentially large) duplicate data being sent. In some examples, the status message is repeated in the status details, resulting in a long message being duplicated 4 times.

I suggest that rather than copying the whole ResponseStatus, we selectively copy the bounded fields, specifically:

  • status
  • reason
  • code

Note that this is a breaking change, and if there is agreement should be done before graduating audit to stable.

/kind bug
/sig auth
/priority important-soon
@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. sig/auth Categorizes an issue or PR as relevant to SIG Auth. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Feb 21, 2018
@tallclair
Copy link
Member Author

/cc @crassirostris

@tallclair tallclair added this to the v1.10 milestone Feb 21, 2018
@crassirostris crassirostris mentioned this issue Feb 21, 2018
Closed
11 tasks
@tpepper tpepper mentioned this issue Feb 21, 2018
Closed
@jberkus jberkus mentioned this issue Feb 21, 2018
Closed
@jdumars
Copy link
Member

jdumars commented Feb 23, 2018

Could you go ahead and add "approved-for-milestone" label to this, as well as status? That will help it stay in the milestone if this is a 1.10 blocker. Thanks!

@crassirostris crassirostris mentioned this issue Feb 25, 2018
Closed
14 tasks
@crassirostris crassirostris modified the milestones: v1.10, v1.11 Feb 25, 2018
@crassirostris
Copy link

Moving it to 1.11, together with graduating audit logging API to stable

@ericchiang
Copy link
Contributor

/sig auth
/kind bug
/milestone v1.11
/priority important-soon
/status approved-for-milestone

@k8s-github-robot
Copy link

[MILESTONENOTIFIER] Milestone Issue: Up-to-date for process

@tallclair

Issue Labels
  • sig/auth: Issue will be escalated to these SIGs if needed.
  • priority/important-soon: Escalate to the issue owners and SIG owner; move out of milestone after several unsuccessful escalation attempts.
  • kind/bug: Fixes a bug discovered during the current release.
Help

@CaoShuFeng
Copy link
Contributor

No one is working on this?
Mind if I assign it?

@CaoShuFeng CaoShuFeng mentioned this issue Apr 17, 2018
Merged
CaoShuFeng added a commit to CaoShuFeng/kubernetes that referenced this issue Apr 25, 2018
@CaoShuFeng
avoid duplicate status in audit events
4d20c38 
Fixes: kubernetes#60108
@tpepper
Copy link
Member

tpepper commented May 14, 2018

@CaoShuFeng status check? This has maybe gone dormant. Is it still targeting 1.11?

@CaoShuFeng
Copy link
Contributor

@CaoShuFeng status check? This has maybe gone dormant. Is it still targeting 1.11?

Yes.

k8s-github-robot pushed a commit that referenced this issue May 15, 2018
Merge pull request #62695 from CaoShuFeng/duplicated_status
d0f4a8f 
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

avoid duplicate status in audit events

Fixes: #60108



**What this PR does / why we need it**:

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #

**Special notes for your reviewer**:
/assign @sttts @tallclair 

**Release note**:

```
Action required: When Response is a metav1.Status, it is no longer copied into the audit.Event status. Only the "status", "reason" and "code" fields are set.
```
k8s-publishing-bot pushed a commit to kubernetes/apiserver that referenced this issue May 15, 2018
@CaoShuFeng @k8s-publishing-bot
avoid duplicate status in audit events
69b26e0 
Fixes: kubernetes/kubernetes#60108

Kubernetes-commit: 4d20c38c337525f8105969a582ce421f52d09c8e
k8s-publishing-bot added a commit to kubernetes/apiserver that referenced this issue May 15, 2018
@k8s-publishing-bot
Merge pull request #62695 from CaoShuFeng/duplicated_status
0132b69 
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

avoid duplicate status in audit events

Fixes: kubernetes/kubernetes#60108

**What this PR does / why we need it**:

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #

**Special notes for your reviewer**:
/assign @sttts @tallclair

**Release note**:

```
Action required: When Response is a metav1.Status, it is no longer copied into the audit.Event status. Only the "status", "reason" and "code" fields are set.
```

Kubernetes-commit: d0f4a8fa17221f79babad9338955be38b8716e78
vikaschoudhary16 pushed a commit to vikaschoudhary16/kubernetes that referenced this issue May 18, 2018
@CaoShuFeng @vikaschoudhary16
avoid duplicate status in audit events
8bfd0c8 
Fixes: kubernetes#60108
wenjiaswe pushed a commit to wenjiaswe/kubernetes that referenced this issue Jun 19, 2018
@CaoShuFeng @wenjiaswe
avoid duplicate status in audit events
7dc7b97 
Fixes: kubernetes#60108
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/audit kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/auth Categorizes an issue or PR as relevant to SIG Auth.
Projects
None yet
Milestone
v1.11
Development

Successfully merging a pull request may close this issue.

avoid duplicate status in audit events CaoShuFeng/kubernetes
8 participants
@jdumars @crassirostris @ericchiang @tpepper @k8s-github-robot @k8s-ci-robot @CaoShuFeng @tallclair