From 35a221018bfb3c6b927b0066dda407214211be83 Mon Sep 17 00:00:00 2001 From: Sam Lai <70988+slai@users.noreply.github.com> Date: Wed, 23 Dec 2020 15:09:42 +0000 Subject: [PATCH] [http_listener_v2] Stop() succeeds even if fails to start (#8502) * [http_listener_v2] Stop() succeeds even if fails to start In cases where the http_listener_v2 plugin config is invalid, when the agent attempts to cleanup by stopping all the inputs, the Stop method here panics as it tries to call listener.Stop() when no listener has been set. This also masks the error message returned from the Start method. ``` > telegraf --test 2020-10-27T12:21:45Z I! Starting Telegraf 1.16.0 2020-10-27T12:21:45Z I! Using config file: /etc/telegraf/telegraf.conf ... panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x1245130] goroutine 45 [running]: github.com/influxdata/telegraf/plugins/inputs/http_listener_v2.(*HTTPListenerV2).Stop(0xc00043e000) /go/src/github.com/influxdata/telegraf/plugins/inputs/http_listener_v2/http_listener_v2.go:178 +0x30 github.com/influxdata/telegraf/agent.stopServiceInputs(0xc00045e480, 0x5, 0x8) /go/src/github.com/influxdata/telegraf/agent/agent.go:445 +0x82 github.com/influxdata/telegraf/agent.(*Agent).testRunInputs(0xc000288080, 0x32be8c0, 0xc0000f1f00, 0x0, 0xc00000f480, 0x0, 0x0) /go/src/github.com/influxdata/telegraf/agent/agent.go:434 +0x1b7 github.com/influxdata/telegraf/agent.(*Agent).test.func4(0xc000057b70, 0xc000288080, 0x32be8c0, 0xc0000f1f00, 0x0, 0xc00000f480) /go/src/github.com/influxdata/telegraf/agent/agent.go:977 +0x8b created by github.com/influxdata/telegraf/agent.(*Agent).test /go/src/github.com/influxdata/telegraf/agent/agent.go:975 +0x352 ``` This fixes this issue by checking if the listener has been set before calling listener.Stop. ``` > ./telegraf --config test.conf --test 2020-10-27T12:43:25Z I! Starting Telegraf 2020-10-27T12:43:25Z E! [agent] Starting input inputs.http_listener_v2: listen tcp: address address_without_port: missing port in address ``` * retry CI --- .../http_listener_v2/http_listener_v2.go | 4 +++- .../http_listener_v2/http_listener_v2_test.go | 21 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/plugins/inputs/http_listener_v2/http_listener_v2.go b/plugins/inputs/http_listener_v2/http_listener_v2.go index 1023c0d10..41ce35df5 100644 --- a/plugins/inputs/http_listener_v2/http_listener_v2.go +++ b/plugins/inputs/http_listener_v2/http_listener_v2.go @@ -175,7 +175,9 @@ func (h *HTTPListenerV2) Start(acc telegraf.Accumulator) error { // Stop cleans up all resources func (h *HTTPListenerV2) Stop() { - h.listener.Close() + if h.listener != nil { + h.listener.Close() + } h.wg.Wait() } diff --git a/plugins/inputs/http_listener_v2/http_listener_v2_test.go b/plugins/inputs/http_listener_v2/http_listener_v2_test.go index 4457fcacd..1f3b629d0 100644 --- a/plugins/inputs/http_listener_v2/http_listener_v2_test.go +++ b/plugins/inputs/http_listener_v2/http_listener_v2_test.go @@ -103,6 +103,27 @@ func createURL(listener *HTTPListenerV2, scheme string, path string, rawquery st return u.String() } +func TestInvalidListenerConfig(t *testing.T) { + parser, _ := parsers.NewInfluxParser() + + listener := &HTTPListenerV2{ + Log: testutil.Logger{}, + ServiceAddress: "address_without_port", + Path: "/write", + Methods: []string{"POST"}, + Parser: parser, + TimeFunc: time.Now, + MaxBodySize: internal.Size{Size: 70000}, + DataSource: "body", + } + + acc := &testutil.Accumulator{} + require.Error(t, listener.Start(acc)) + + // Stop is called when any ServiceInput fails to start; it must succeed regardless of state + listener.Stop() +} + func TestWriteHTTPSNoClientAuth(t *testing.T) { listener := newTestHTTPSListenerV2() listener.TLSAllowedCACerts = nil