From 13eca1ae515c76c9c30ca5684a7f038841859137 Mon Sep 17 00:00:00 2001 From: Jason Simmons <1382389+jasimmons@users.noreply.github.com> Date: Mon, 2 Dec 2019 09:17:21 -0500 Subject: [PATCH] Propagate headers to lifecycle handlers Propagate HTTP headers to HTTP-based lifecycle handlers. This normalizes the underlying HTTPGetAction of lifecycle handlers with that of probes, which do already propagate HTTP headers. --- .../kuberuntime/fake_kuberuntime_manager.go | 6 +-- .../kuberuntime/kuberuntime_container_test.go | 7 +-- .../kuberuntime/kuberuntime_manager.go | 2 +- pkg/kubelet/lifecycle/handlers.go | 27 ++++++++-- pkg/kubelet/lifecycle/handlers_test.go | 51 +++++++++++++++++-- pkg/kubelet/types/types.go | 7 ++- 6 files changed, 79 insertions(+), 21 deletions(-) diff --git a/pkg/kubelet/kuberuntime/fake_kuberuntime_manager.go b/pkg/kubelet/kuberuntime/fake_kuberuntime_manager.go index 2552ebe8d3cf..10ab1e96dd87 100644 --- a/pkg/kubelet/kuberuntime/fake_kuberuntime_manager.go +++ b/pkg/kubelet/kuberuntime/fake_kuberuntime_manager.go @@ -40,12 +40,12 @@ const ( ) type fakeHTTP struct { - url string + req *http.Request err error } -func (f *fakeHTTP) Get(url string) (*http.Response, error) { - f.url = url +func (f *fakeHTTP) Do(req *http.Request) (*http.Response, error) { + f.req = req return nil, f.err } diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_test.go b/pkg/kubelet/kuberuntime/kuberuntime_container_test.go index a30ab2f1e6fd..812b90cf46ad 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container_test.go @@ -17,6 +17,7 @@ limitations under the License. package kuberuntime import ( + "net/http" "path/filepath" "strings" "testing" @@ -279,11 +280,11 @@ func TestLifeCycleHook(t *testing.T) { // Configured and working HTTP hook t.Run("PreStop-HTTPGet", func(t *testing.T) { - defer func() { fakeHTTP.url = "" }() + defer func() { fakeHTTP.req, _ = http.NewRequest(http.MethodGet, "", nil) }() testPod.Spec.Containers[0].Lifecycle = httpLifeCycle m.killContainer(testPod, cID, "foo", "testKill", &gracePeriod) - if !strings.Contains(fakeHTTP.url, httpLifeCycle.PreStop.HTTPGet.Host) { + if !strings.Contains(fakeHTTP.req.URL.String(), httpLifeCycle.PreStop.HTTPGet.Host) { t.Errorf("HTTP Prestop hook was not invoked") } }) @@ -297,7 +298,7 @@ func TestLifeCycleHook(t *testing.T) { m.killContainer(testPod, cID, "foo", "testKill", &gracePeriodLocal) - if strings.Contains(fakeHTTP.url, httpLifeCycle.PreStop.HTTPGet.Host) { + if strings.Contains(fakeHTTP.req.URL.String(), httpLifeCycle.PreStop.HTTPGet.Host) { t.Errorf("HTTP Should not execute when gracePeriod is 0") } }) diff --git a/pkg/kubelet/kuberuntime/kuberuntime_manager.go b/pkg/kubelet/kuberuntime/kuberuntime_manager.go index 1ba50d84e784..d80b82b149bb 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_manager.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_manager.go @@ -159,7 +159,7 @@ func NewKubeGenericRuntimeManager( podStateProvider podStateProvider, osInterface kubecontainer.OSInterface, runtimeHelper kubecontainer.RuntimeHelper, - httpClient types.HTTPGetter, + httpClient types.HttpDoer, imageBackOff *flowcontrol.Backoff, serializeImagePulls bool, imagePullQPS float32, diff --git a/pkg/kubelet/lifecycle/handlers.go b/pkg/kubelet/lifecycle/handlers.go index 3ed7360a2d78..bd64b0fdf180 100644 --- a/pkg/kubelet/lifecycle/handlers.go +++ b/pkg/kubelet/lifecycle/handlers.go @@ -39,7 +39,7 @@ const ( ) type HandlerRunner struct { - httpGetter kubetypes.HTTPGetter + httpDoer kubetypes.HttpDoer commandRunner kubecontainer.ContainerCommandRunner containerManager podStatusProvider } @@ -48,9 +48,9 @@ type podStatusProvider interface { GetPodStatus(uid types.UID, name, namespace string) (*kubecontainer.PodStatus, error) } -func NewHandlerRunner(httpGetter kubetypes.HTTPGetter, commandRunner kubecontainer.ContainerCommandRunner, containerManager podStatusProvider) kubecontainer.HandlerRunner { +func NewHandlerRunner(httpDoer kubetypes.HttpDoer, commandRunner kubecontainer.ContainerCommandRunner, containerManager podStatusProvider) kubecontainer.HandlerRunner { return &HandlerRunner{ - httpGetter: httpGetter, + httpDoer: httpDoer, commandRunner: commandRunner, containerManager: containerManager, } @@ -108,7 +108,7 @@ func resolvePort(portReference intstr.IntOrString, container *v1.Container) (int // formatURL formats a URL from args. func formatURL(scheme string, host string, port int, path string) *url.URL { u, err := url.Parse(path) - // Something is bustsed with the path, but it's too late to reject it. Pass it along as is. + // Something is busted with the path, but it's too late to reject it. Pass it along as is. if err != nil { u = &url.URL{ Path: path, @@ -119,6 +119,16 @@ func formatURL(scheme string, host string, port int, path string) *url.URL { return u } +// buildHeader takes a list of HTTPHeader string pairs +// and returns a populated string->[]string http.Header map. +func buildHeader(headerList []v1.HTTPHeader) http.Header { + headers := make(http.Header) + for _, header := range headerList { + headers[header.Name] = append(headers[header.Name], header.Value) + } + return headers +} + func (hr *HandlerRunner) runHTTPHandler(pod *v1.Pod, container *v1.Container, handler *v1.Handler) (string, error) { host := handler.HTTPGet.Host if len(host) == 0 { @@ -144,7 +154,14 @@ func (hr *HandlerRunner) runHTTPHandler(pod *v1.Pod, container *v1.Container, ha } path := handler.HTTPGet.Path url := formatURL("http", host, port, path) - resp, err := hr.httpGetter.Get(url.String()) + + req, err := http.NewRequest(http.MethodGet, url.String(), nil) + if err != nil { + return "", err + } + req.Header = buildHeader(handler.HTTPGet.HTTPHeaders) + resp, err := hr.httpDoer.Do(req) + return getHttpRespBody(resp), err } diff --git a/pkg/kubelet/lifecycle/handlers_test.go b/pkg/kubelet/lifecycle/handlers_test.go index 900bd3936885..800ea6c71e3c 100644 --- a/pkg/kubelet/lifecycle/handlers_test.go +++ b/pkg/kubelet/lifecycle/handlers_test.go @@ -122,13 +122,15 @@ func TestRunHandlerExec(t *testing.T) { } type fakeHTTP struct { - url string - err error - resp *http.Response + url string + headers http.Header + err error + resp *http.Response } -func (f *fakeHTTP) Get(url string) (*http.Response, error) { - f.url = url +func (f *fakeHTTP) Do(req *http.Request) (*http.Response, error) { + f.url = req.URL.String() + f.headers = req.Header.Clone() return f.resp, f.err } @@ -165,6 +167,45 @@ func TestRunHandlerHttp(t *testing.T) { } } +func TestRunHandlerHttpWithHeaders(t *testing.T) { + fakeHttp := fakeHTTP{} + handlerRunner := NewHandlerRunner(&fakeHttp, &fakeContainerCommandRunner{}, nil) + + containerID := kubecontainer.ContainerID{Type: "test", ID: "abc1234"} + containerName := "containerFoo" + + container := v1.Container{ + Name: containerName, + Lifecycle: &v1.Lifecycle{ + PostStart: &v1.Handler{ + HTTPGet: &v1.HTTPGetAction{ + Host: "foo", + Port: intstr.FromInt(8080), + Path: "/bar", + HTTPHeaders: []v1.HTTPHeader{ + {Name: "foo", Value: "bar"}, + }, + }, + }, + }, + } + pod := v1.Pod{} + pod.ObjectMeta.Name = "podFoo" + pod.ObjectMeta.Namespace = "nsFoo" + pod.Spec.Containers = []v1.Container{container} + _, err := handlerRunner.Run(containerID, &pod, &container, container.Lifecycle.PostStart) + + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if fakeHttp.url != "http://foo:8080/bar" { + t.Errorf("unexpected url: %s", fakeHttp.url) + } + if fakeHttp.headers["foo"][0] != "bar" { + t.Errorf("missing http header: %s", fakeHttp.headers) + } +} + func TestRunHandlerNil(t *testing.T) { handlerRunner := NewHandlerRunner(&fakeHTTP{}, &fakeContainerCommandRunner{}, nil) containerID := kubecontainer.ContainerID{Type: "test", ID: "abc1234"} diff --git a/pkg/kubelet/types/types.go b/pkg/kubelet/types/types.go index 9c8f165ffe68..01122e5898c1 100644 --- a/pkg/kubelet/types/types.go +++ b/pkg/kubelet/types/types.go @@ -26,10 +26,9 @@ import ( // TODO: Reconcile custom types in kubelet/types and this subpackage -// HTTPGetter is an interface representing the ability to perform HTTP GET requests. -type HTTPGetter interface { - // Get issues a GET to the specified URL. - Get(url string) (*http.Response, error) +// HttpDoer encapsulates http.Do functionality +type HttpDoer interface { + Do(req *http.Request) (*http.Response, error) } // Timestamp wraps around time.Time and offers utilities to format and parse