From 83206c5e2f98262b362265d3afda1190153f6c63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juraci=20Paix=C3=A3o=20Kr=C3=B6hling?= Date: Mon, 6 Dec 2021 21:04:54 +0100 Subject: [PATCH] `confighttp`: add component.Host parameter to ToServer (#4514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * `confighttp`: add component.Host parameter to ToServer Signed-off-by: Juraci Paixão Kröhling * s/assert/require/ for some tests, s/how/now/ Signed-off-by: Juraci Paixão Kröhling --- CHANGELOG.md | 1 + config/confighttp/confighttp.go | 4 +-- config/confighttp/confighttp_test.go | 47 +++++++++++++++++++++------- receiver/otlpreceiver/otlp.go | 9 ++++-- 4 files changed, 45 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a3f11cacc63..68b2df90437 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Remove `config.NewConfigMapFrom[File|Buffer]`, add testonly version (#4502) - `configtls`: TLS 1.2 is the new default mininum version (#4503) +- `confighttp`: `ToServer` now accepts a `component.Host`, in line with gRPC's counterpart (#4514) ## 🧰 Bug fixes 🧰 diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index 423e70027a7..ad93a735034 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -187,7 +187,7 @@ func WithErrorHandler(e middleware.ErrorHandler) ToServerOption { } // ToServer creates an http.Server from settings object. -func (hss *HTTPServerSettings) ToServer(handler http.Handler, settings component.TelemetrySettings, opts ...ToServerOption) *http.Server { +func (hss *HTTPServerSettings) ToServer(_ component.Host, settings component.TelemetrySettings, handler http.Handler, opts ...ToServerOption) (*http.Server, error) { serverOpts := &toServerOptions{} for _, o := range opts { o(serverOpts) @@ -223,5 +223,5 @@ func (hss *HTTPServerSettings) ToServer(handler http.Handler, settings component return &http.Server{ Handler: handler, - } + }, nil } diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go index 6afe9d66234..57ab70a9e1c 100644 --- a/config/confighttp/confighttp_test.go +++ b/config/confighttp/confighttp_test.go @@ -380,11 +380,16 @@ func TestHttpReception(t *testing.T) { TLSSetting: tt.tlsServerCreds, } ln, err := hss.ToListener() - assert.NoError(t, err) - s := hss.ToServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - _, errWrite := fmt.Fprint(w, "test") - assert.NoError(t, errWrite) - }), componenttest.NewNopTelemetrySettings()) + require.NoError(t, err) + + s, err := hss.ToServer( + componenttest.NewNopHost(), + componenttest.NewNopTelemetrySettings(), + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, errWrite := fmt.Fprint(w, "test") + assert.NoError(t, errWrite) + })) + require.NoError(t, err) go func() { _ = s.Serve(ln) @@ -403,7 +408,8 @@ func TestHttpReception(t *testing.T) { TLSSetting: *tt.tlsClientCreds, } client, errClient := hcs.ToClient(map[config.ComponentID]component.Extension{}) - assert.NoError(t, errClient) + require.NoError(t, errClient) + resp, errResp := client.Get(hcs.Endpoint) if tt.hasError { assert.Error(t, errResp) @@ -460,10 +466,16 @@ func TestHttpCors(t *testing.T) { } ln, err := hss.ToListener() - assert.NoError(t, err) - s := hss.ToServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusOK) - }), componenttest.NewNopTelemetrySettings()) + require.NoError(t, err) + + s, err := hss.ToServer( + componenttest.NewNopHost(), + componenttest.NewNopTelemetrySettings(), + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + require.NoError(t, err) + go func() { _ = s.Serve(ln) }() @@ -499,7 +511,11 @@ func TestHttpCorsInvalidSettings(t *testing.T) { } // This effectively does not enable CORS but should also not cause an error - s := hss.ToServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}), componenttest.NewNopTelemetrySettings()) + s, err := hss.ToServer( + componenttest.NewNopHost(), + componenttest.NewNopTelemetrySettings(), + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + require.NoError(t, err) require.NotNil(t, s) require.NoError(t, s.Close()) } @@ -541,7 +557,14 @@ func ExampleHTTPServerSettings() { settings := HTTPServerSettings{ Endpoint: ":443", } - s := settings.ToServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), componenttest.NewNopTelemetrySettings()) + s, err := settings.ToServer( + componenttest.NewNopHost(), + componenttest.NewNopTelemetrySettings(), + http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + if err != nil { + panic(err) + } + l, err := settings.ToListener() if err != nil { panic(err) diff --git a/receiver/otlpreceiver/otlp.go b/receiver/otlpreceiver/otlp.go index 2e1a3b8ee1c..75c771c2746 100644 --- a/receiver/otlpreceiver/otlp.go +++ b/receiver/otlpreceiver/otlp.go @@ -128,11 +128,16 @@ func (r *otlpReceiver) startProtocolServers(host component.Host) error { } } if r.cfg.HTTP != nil { - r.serverHTTP = r.cfg.HTTP.ToServer( - r.httpMux, + r.serverHTTP, err = r.cfg.HTTP.ToServer( + host, r.settings.TelemetrySettings, + r.httpMux, confighttp.WithErrorHandler(errorHandler), ) + if err != nil { + return err + } + err = r.startHTTPServer(r.cfg.HTTP, host) if err != nil { return err