{"action":"created","issue":{"url":"https://api.github.com/repos/kubernetes/kubernetes/issues/55729","repository_url":"https://api.github.com/repos/kubernetes/kubernetes","labels_url":"https://api.github.com/repos/kubernetes/kubernetes/issues/55729/labels{/name}","comments_url":"https://api.github.com/repos/kubernetes/kubernetes/issues/55729/comments","events_url":"https://api.github.com/repos/kubernetes/kubernetes/issues/55729/events","html_url":"https://github.com/kubernetes/kubernetes/issues/55729","id":273936063,"number":55729,"title":"API conventions for webhook (imperative) communication","user":{"login":"lavalamp","id":647318,"avatar_url":"https://avatars0.githubusercontent.com/u/647318?v=4","gravatar_id":"","url":"https://api.github.com/users/lavalamp","html_url":"https://github.com/lavalamp","followers_url":"https://api.github.com/users/lavalamp/followers","following_url":"https://api.github.com/users/lavalamp/following{/other_user}","gists_url":"https://api.github.com/users/lavalamp/gists{/gist_id}","starred_url":"https://api.github.com/users/lavalamp/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lavalamp/subscriptions","organizations_url":"https://api.github.com/users/lavalamp/orgs","repos_url":"https://api.github.com/users/lavalamp/repos","events_url":"https://api.github.com/users/lavalamp/events{/privacy}","received_events_url":"https://api.github.com/users/lavalamp/received_events","type":"User","site_admin":false},"labels":[{"id":173493835,"url":"https://api.github.com/repos/kubernetes/kubernetes/labels/sig/api-machinery","name":"sig/api-machinery","color":"d2b48c","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":10,"created_at":"2017-11-14T20:43:30Z","updated_at":"2017-11-14T21:42:50Z","closed_at":null,"author_association":"OWNER","body":"I think we haven't taken a principled stance on how imperative API calls ought to work in Kubernetes. Most of our API surface area is REST calls and the current API system is optimized for those. We have a few outliers which are special snowflakes (logs, exec, attach, portforward, proxy). But we are recently adding more and more webhooks to the system, and expanding the target audience of who will be implementing the webhooks; and I think therefore it's time to make sure the API patterns we use will make sense to the people who need to implement the webhooks.\r\n\r\nCurrently, webhooks in the system use the REST styled spec/status objects. But this doesn't make a lot of sense:\r\n1. Why \"spec\" and \"status\" instead of \"request\" and \"response\"? People will at a minimum have to look up in the API documentation to figure out what they should do. It may feel \"natural\" to folks who have been working on Kubernetes for the past year or two, but I think that's status quo or familiarity bias, and it won't make sense to anyone fresh to the system.\r\n2. These objects are not intended for long-term persistence; they are not really objects. They are ephemeral constructs that only exist during one small part of the overall request path.\r\n3. The input and output of our existing webhooks are very different data types. It is not very natural to phrase such things as different fields in the same object type. (Imagine writing these as a go function, having the fields of e.g. AdmissionReview as the input parameters and also having the fields of AdmissionReview as the output parameters: `func Review(inSpec spec, _ status) (_spec, outStatus status)`; I think few reviewers on the project would accept that.)\r\n\r\nArguments I have heard in favor of the current one-object system:\r\n * There's just one type for webhook authors to look up.\r\n   * My response: FooRequest and FooResponse are not really more burdensome to look up.\r\n * Consistency with our current webhooks\r\n   * My response: consistency with a bad idea belongs in the \"con\" section. @cheftako says \"I'd rather explain why this one is different than explain why we did it a way that we consider wrong.\"\r\n\r\nHow do you know if a given thing should follow webhook rules or normal Kubernetes API rules?\r\n* It supports exactly one verb (likely POST)\r\n* It is probably used by control plane infrastructure during the course of servicing a top level (e.g., initiated by a user via `kubectl`) API request.\r\n\r\nSo, rough proposal:\r\n* Webhooks / imperative calls shall have a specific metadata type.\r\n* Webhook calls will have separate FooRequest and FooResponse types.\r\n* Neither the Request nor the Response shall contain \"spec\" or \"status\" fields.\r\n\r\nHookMeta should look like this, not sure if we need any other fields right now:\r\n\r\n```go\r\n// HookMeta contains metadata that is used to track webhook (imperative) requests.\r\n// Webhooks must echo back everything here in their response.\r\n// This is different from ObjectMeta and ListMeta because webhook (imperative) calls\r\n// do not represent objects, nor do they represent lists of objects: they represent commands.\r\ntype HookMeta struct {\r\n  // UID is a time-and-space unique identifier to this particular invocation of the webhook.\r\n  // It is useful for matching up logs / audit logs between caller and callee, and for proving\r\n  // that the callee is responding to the correct response (doesn't have any weird\r\n  // caching bugs, for example).\r\n  UID UID\r\n}\r\n```\r\n\r\nAnd an example webhook API might be:\r\n\r\n```go\r\ntype AdmissionReviewRequest struct {\r\n  TypeMeta `json:\",inline\"`\r\n  HookMeta `json:\"metadata\"`\r\n\r\n  Kind GVR\r\n  Resource GVR\r\n  Namespace string\r\n  Name string\r\n  ...\r\n}\r\n\r\ntype AdmissionReviewResponse struct {\r\n  TypeMeta `json:\",inline\"`\r\n  HookMeta `json:\"metadata\"`\r\n  \r\n  Allowed bool\r\n  Status *metav1.Status\r\n  \r\n  // mutating webhooks may set...\r\n  PatchType string\r\n  Patch []byte\r\n}\r\n```\r\n\r\nFinally, what should we do about existing webhooks which do not use this format? I think, for any that are alpha, we should require they change to this format for beta. For ones seeking to go beta -> v1, we should consider the costs to making existing implementations change, with a bias towards making the API change for v1. For webhooks that are already in v1 (SubjectAccessReview?), we should not change until we're ready to go to v2."},"comment":{"url":"https://api.github.com/repos/kubernetes/kubernetes/issues/comments/344408734","html_url":"https://github.com/kubernetes/kubernetes/issues/55729#issuecomment-344408734","issue_url":"https://api.github.com/repos/kubernetes/kubernetes/issues/55729","id":344408734,"user":{"login":"lavalamp","id":647318,"avatar_url":"https://avatars0.githubusercontent.com/u/647318?v=4","gravatar_id":"","url":"https://api.github.com/users/lavalamp","html_url":"https://github.com/lavalamp","followers_url":"https://api.github.com/users/lavalamp/followers","following_url":"https://api.github.com/users/lavalamp/following{/other_user}","gists_url":"https://api.github.com/users/lavalamp/gists{/gist_id}","starred_url":"https://api.github.com/users/lavalamp/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lavalamp/subscriptions","organizations_url":"https://api.github.com/users/lavalamp/orgs","repos_url":"https://api.github.com/users/lavalamp/repos","events_url":"https://api.github.com/users/lavalamp/events{/privacy}","received_events_url":"https://api.github.com/users/lavalamp/received_events","type":"User","site_admin":false},"created_at":"2017-11-14T21:42:50Z","updated_at":"2017-11-14T21:42:50Z","author_association":"OWNER","body":"> Do I need to link the XKCD with 15 standards?\r\n\r\n1 -> 2 is a lot more sensible than 14 -> 15 =D"}}
{"actor":{"display_login":"lavalamp"}}