From a68482290dabaf68cbff27e40ae1525062bd1f86 Mon Sep 17 00:00:00 2001 From: Rodrigo Villablanca Date: Sat, 11 Apr 2020 22:27:51 -0400 Subject: [PATCH 1/4] Handling error returned by request.Request.ParseForm() --- pkg/kubelet/server/stats/handler.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/kubelet/server/stats/handler.go b/pkg/kubelet/server/stats/handler.go index c6dd5a0c569..ca2f7525e6f 100644 --- a/pkg/kubelet/server/stats/handler.go +++ b/pkg/kubelet/server/stats/handler.go @@ -28,7 +28,7 @@ import ( cadvisorapi "github.com/google/cadvisor/info/v1" "k8s.io/klog" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" statsapi "k8s.io/kubernetes/pkg/kubelet/apis/stats/v1alpha1" @@ -215,13 +215,17 @@ func (h *handler) handleStats(request *restful.Request, response *restful.Respon // If "only_cpu_and_memory" GET param is true then only cpu and memory is returned in response. func (h *handler) handleSummary(request *restful.Request, response *restful.Response) { onlyCPUAndMemory := false - request.Request.ParseForm() + var err error + err = request.Request.ParseForm() + if err != nil { + handleError(response, "/stats/summary", err) + return + } if onlyCluAndMemoryParam, found := request.Request.Form["only_cpu_and_memory"]; found && len(onlyCluAndMemoryParam) == 1 && onlyCluAndMemoryParam[0] == "true" { onlyCPUAndMemory = true } var summary *statsapi.Summary - var err error if onlyCPUAndMemory { summary, err = h.summaryProvider.GetCPUAndMemoryStats() } else { From 0972c9ba4d5a1db6d71e50d5727e0f8dcb9e1dba Mon Sep 17 00:00:00 2001 From: Rodrigo Villablanca Date: Sun, 12 Apr 2020 11:50:43 -0400 Subject: [PATCH 2/4] Improvements --- pkg/kubelet/server/stats/handler.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/kubelet/server/stats/handler.go b/pkg/kubelet/server/stats/handler.go index ca2f7525e6f..ae98e3a9f39 100644 --- a/pkg/kubelet/server/stats/handler.go +++ b/pkg/kubelet/server/stats/handler.go @@ -28,7 +28,7 @@ import ( cadvisorapi "github.com/google/cadvisor/info/v1" "k8s.io/klog" - v1 "k8s.io/api/core/v1" + "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" statsapi "k8s.io/kubernetes/pkg/kubelet/apis/stats/v1alpha1" @@ -215,8 +215,7 @@ func (h *handler) handleStats(request *restful.Request, response *restful.Respon // If "only_cpu_and_memory" GET param is true then only cpu and memory is returned in response. func (h *handler) handleSummary(request *restful.Request, response *restful.Response) { onlyCPUAndMemory := false - var err error - err = request.Request.ParseForm() + err := request.Request.ParseForm() if err != nil { handleError(response, "/stats/summary", err) return From ae603b8ef143a5f3d567711a869f5f9df908a948 Mon Sep 17 00:00:00 2001 From: Rodrigo Villablanca Date: Sun, 12 Apr 2020 18:37:22 -0400 Subject: [PATCH 3/4] Add some testing --- pkg/kubelet/server/server_test.go | 14 ++++++++++++++ pkg/kubelet/server/stats/handler.go | 3 ++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/pkg/kubelet/server/server_test.go b/pkg/kubelet/server/server_test.go index 96dce2c86a9..d65828eac12 100644 --- a/pkg/kubelet/server/server_test.go +++ b/pkg/kubelet/server/server_test.go @@ -1579,6 +1579,20 @@ func TestDebuggingDisabledHandlers(t *testing.T) { } +func TestFailedParseParamsSummaryHandler(t *testing.T) { + fw := newServerTest() + defer fw.testHTTPServer.Close() + + resp, err := http.Post(fw.testHTTPServer.URL+"/stats/summary", "invalid/content/type", nil) + assert.NoError(t, err) + defer resp.Body.Close() + v, err := ioutil.ReadAll(resp.Body) + assert.NoError(t, err) + assert.Equal(t, http.StatusInternalServerError, resp.StatusCode) + assert.Contains(t, string(v), "parse form failed") + +} + func TestTrimURLPath(t *testing.T) { tests := []struct { path, expected string diff --git a/pkg/kubelet/server/stats/handler.go b/pkg/kubelet/server/stats/handler.go index ae98e3a9f39..3dc23b45fe1 100644 --- a/pkg/kubelet/server/stats/handler.go +++ b/pkg/kubelet/server/stats/handler.go @@ -26,6 +26,7 @@ import ( restful "github.com/emicklei/go-restful" cadvisorapi "github.com/google/cadvisor/info/v1" + "github.com/pkg/errors" "k8s.io/klog" "k8s.io/api/core/v1" @@ -217,7 +218,7 @@ func (h *handler) handleSummary(request *restful.Request, response *restful.Resp onlyCPUAndMemory := false err := request.Request.ParseForm() if err != nil { - handleError(response, "/stats/summary", err) + handleError(response, "/stats/summary", errors.Wrapf(err, "parse form failed")) return } if onlyCluAndMemoryParam, found := request.Request.Form["only_cpu_and_memory"]; found && From 1014d307a55ee4aaf51b187fe653090ae40a2645 Mon Sep 17 00:00:00 2001 From: Rodrigo Villablanca Date: Wed, 15 Apr 2020 09:24:54 -0400 Subject: [PATCH 4/4] After run ./hack/update-bazel.sh --- pkg/kubelet/server/stats/BUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/kubelet/server/stats/BUILD b/pkg/kubelet/server/stats/BUILD index 878382309f3..49851893f53 100644 --- a/pkg/kubelet/server/stats/BUILD +++ b/pkg/kubelet/server/stats/BUILD @@ -29,6 +29,7 @@ go_library( "//staging/src/k8s.io/component-base/metrics:go_default_library", "//vendor/github.com/emicklei/go-restful:go_default_library", "//vendor/github.com/google/cadvisor/info/v1:go_default_library", + "//vendor/github.com/pkg/errors:go_default_library", "//vendor/k8s.io/klog:go_default_library", ], )