API server returns 500 when issued a Get with invalid label selector #39730

Open
gmarek opened this Issue Jan 11, 2017 · 3 comments

Projects

None yet

3 participants

@gmarek
Member
gmarek commented Jan 11, 2017 edited

I'm seeing logs like on 1.5 cluster:

E0111 09:00:06.466257       5 errors.go:63] apiserver received an error that is not an unversioned.Status: unable to parse requirement: invalid label key "<label key>": name part must be no more than 63 characters; name part must match the regex ([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9] (e.g. 'MyName' or 'my.name' or '123-abc')
I0111 09:00:06.466418       5 handlers.go:162] GET /api/v1/namespaces/jenkins/pods?labelSelector=<label>: (613.07�s) 500

We probably should return 400 (or maybe 412?), eventually 418. But certainly not 500.

cc @deads2k @lavalamp @wojtek-t @smarterclayton @kubernetes/sig-api-machinery-misc

@gmarek gmarek added this to the v1.5 milestone Jan 11, 2017
@lavalamp lavalamp was assigned by gmarek Jan 11, 2017
@shyamjvs
Member
shyamjvs commented Jan 11, 2017 edited

@gmarek I tried digging this up a little bit and figured out that this is not an issue with the apiserver on the master, but instead with the heapster-apiserver (the one which negotiates request/response serialization with the heapster client on one side and is bound to the master metrics API on the other). The following seems to be the issue:

  • The errToAPIStatus function inside the heapster apiserver does status := http.StatusInternalServerError if the error type is not statusError. This is why the response is being logged with 500 (even though it is (most probably) a 400 from the master apiserver).
  • The logStackOnRecover function should do better than just flagging http.StatusInternalServerError as the error. Maybe set code to http.StatusBadRequest if we know that the request was not matching the negotiated pattern.

cc @piosz @kubernetes/sig-instrumentation-misc

@lavalamp
Member

hm, there should already be an equivalent of that errToAPIStatus function in the generic apiserver library somewhere.

@lavalamp lavalamp removed their assignment Jan 11, 2017
@gmarek
Member
gmarek commented Jan 12, 2017

@lavalamp - who should work on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment