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.

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

audit.Event.RequestObject underspecified for patch requests #60110

Closed
tallclair opened this issue Feb 21, 2018 · 4 comments
Closed

audit.Event.RequestObject underspecified for patch requests #60110

tallclair opened this issue Feb 21, 2018 · 4 comments
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.

Comments

@tallclair
Copy link
Member

The documentation on the RequestObject field of the audit.Event reads:

API object from the request, in JSON format. The RequestObject is recorded as-is in the request
(possibly re-encoded as JSON), prior to version conversion, defaulting, admission or
merging. It is an external versioned object type, and may not be a valid object on its own.
Omitted for non-resource requests. Only logged at Request Level and higher.

This doesn't really fit the patch request model, which specifies an array of field operations rather than an actual object. E.g.

  "requestObject": [
    {
      "op": "replace",
      "path": "/spec/parallelism",
      "value": 0
    },
    {
      "op": "remove",
      "path": "/spec/template/spec/containers/0/terminationMessagePolicy"
    },
    {
      "op": "remove",
      "path": "/spec/template/spec/schedulerName"
    },
    {
      "op": "remove",
      "path": "/spec/template/spec/affinity"
    },
    {
      "op": "remove",
      "path": "/spec/backoffLimit"
    }
  ],

We should determine what the expected behavior is for a patch request. If this is working as intended, we should at least document this case.

/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. sig/auth Categorizes an issue or PR as relevant to SIG Auth. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. 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

@tpepper
Copy link
Member

tpepper commented Feb 21, 2018

@tallclair even though you're suggesting not to, if you decide to push for this in 1.10 as per #58083 please remember to mark this one for the milestone. I see you tentatively did on #60108 but not here.

@crassirostris
Copy link

crassirostris commented Feb 25, 2018

I think it's hard to argue that this kind of requests are totally valid. I guess we have to just mention that in the documentation

@crassirostris
Copy link

Docs PR is merged, closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

No branches or pull requests

5 participants