From c26332ab10d92278233c117767c6f0089ef56034 Mon Sep 17 00:00:00 2001 From: Christian Haudum Date: Thu, 25 Nov 2021 21:47:05 +0100 Subject: [PATCH 1/3] Set correct `Content-Type` header in query response Because the internal query request handlers did not set the response content type explicitly to `application/json`, and the external request handler takes the response content type from the internal response to set it for the resulting HTTP response, that response was also incorrectly returning `Content-Type: text/plain`. Setting the content type in the internal response correctly results in the expected `Content-Type` header `application/json; charset=UTF-8`. Fixes #4791 Signed-off-by: Christian Haudum --- CHANGELOG.md | 1 + pkg/querier/worker_service.go | 14 +++++--------- pkg/querier/worker_service_test.go | 4 ++++ 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 64815afea8f5..b90d201b199e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## Main +* [xxxx](https://github.com/grafana/loki/pull/xxxx) **chaudum**: Set correct `Content-Type` header in query response * [4794](https://github.com/grafana/loki/pull/4794) **taisho6339**: Aggregate inotify watcher to file target manager * [4663](https://github.com/grafana/loki/pull/4663) **taisho6339**: Add SASL&mTLS authentication support for Kafka in Promtail * [4745](https://github.com/grafana/loki/pull/4745) **taisho6339**: Expose Kafka message key in labels diff --git a/pkg/querier/worker_service.go b/pkg/querier/worker_service.go index 9eeb6a8bcfb2..02154551c889 100644 --- a/pkg/querier/worker_service.go +++ b/pkg/querier/worker_service.go @@ -52,15 +52,11 @@ func InitWorkerService( authMiddleware middleware.Interface, ) (serve services.Service, err error) { - // Create a couple Middlewares used to handle panics, perform auth, and parse Form's in http request - internalMiddleware := middleware.Merge( + // Create a couple Middlewares used to handle panics, perform auth, parse forms in http request, and set content type in response + handlerMiddleware := middleware.Merge( serverutil.RecoveryHTTPMiddleware, authMiddleware, serverutil.NewPrepopulateMiddleware(), - ) - // External middleware also needs to set JSON content type headers - externalMiddleware := middleware.Merge( - internalMiddleware, serverutil.ResponseJSONMiddleware(), ) @@ -72,7 +68,7 @@ func InitWorkerService( // There are some routes which are always registered on the external router, add them now and // wrap them with the externalMiddleware for route, handler := range alwaysExternalRoutesToHandlers { - externalRouter.Path(route).Methods("GET", "POST").Handler(externalMiddleware.Wrap(handler)) + externalRouter.Path(route).Methods("GET", "POST").Handler(handlerMiddleware.Wrap(handler)) } // If the querier is running standalone without the query-frontend or query-scheduler, we must register the internal @@ -91,7 +87,7 @@ func InitWorkerService( // Register routes externally for _, route := range routes { - externalRouter.Path(route).Methods("GET", "POST").Handler(externalMiddleware.Wrap(internalRouter)) + externalRouter.Path(route).Methods("GET", "POST").Handler(handlerMiddleware.Wrap(internalRouter)) } //If no frontend or scheduler address has been configured, then there is no place for the @@ -129,7 +125,7 @@ func InitWorkerService( return "internalQuerier" })) - internalHandler = internalMiddleware.Wrap(internalHandler) + internalHandler = handlerMiddleware.Wrap(internalHandler) //Return a querier worker pointed to the internal querier HTTP handler so there is not a conflict in routes between the querier //and the query frontend diff --git a/pkg/querier/worker_service_test.go b/pkg/querier/worker_service_test.go index 5a282a5d8f02..4012a2e09bd0 100644 --- a/pkg/querier/worker_service_test.go +++ b/pkg/querier/worker_service_test.go @@ -97,6 +97,10 @@ func Test_InitQuerierService(t *testing.T) { }) t.Run("wrap external handler with response json middleware", func(t *testing.T) { + // note: this test only assures that the content type of the reponse is + // set if the handler function does not override it, which happens in the + // actual implementation, see + // https://github.com/grafana/loki/blob/34a012adcfade43291de3a7670f53679ea06aefe/pkg/lokifrontend/frontend/transport/handler.go#L136-L139 config := WorkerServiceConfig{ QueryFrontendEnabled: false, QuerySchedulerEnabled: false, From 1d5d03063cd58348150ab16bbb6ce423ab4a4eee Mon Sep 17 00:00:00 2001 From: Christian Haudum Date: Thu, 25 Nov 2021 22:18:11 +0100 Subject: [PATCH 2/3] fixup! Set correct `Content-Type` header in query response Signed-off-by: Christian Haudum --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b90d201b199e..88d51e33be8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ ## Main -* [xxxx](https://github.com/grafana/loki/pull/xxxx) **chaudum**: Set correct `Content-Type` header in query response +* [4828](https://github.com/grafana/loki/pull/4282) **chaudum**: Set correct `Content-Type` header in query response * [4794](https://github.com/grafana/loki/pull/4794) **taisho6339**: Aggregate inotify watcher to file target manager * [4663](https://github.com/grafana/loki/pull/4663) **taisho6339**: Add SASL&mTLS authentication support for Kafka in Promtail * [4745](https://github.com/grafana/loki/pull/4745) **taisho6339**: Expose Kafka message key in labels From 003542c1307e915fe58d50d9c9e929545b7d51df Mon Sep 17 00:00:00 2001 From: Christian Haudum Date: Fri, 26 Nov 2021 08:17:59 +0100 Subject: [PATCH 3/3] fixup! fixup! Set correct `Content-Type` header in query response Signed-off-by: Christian Haudum --- pkg/querier/worker_service_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/querier/worker_service_test.go b/pkg/querier/worker_service_test.go index 4012a2e09bd0..7668ccafc61c 100644 --- a/pkg/querier/worker_service_test.go +++ b/pkg/querier/worker_service_test.go @@ -97,7 +97,7 @@ func Test_InitQuerierService(t *testing.T) { }) t.Run("wrap external handler with response json middleware", func(t *testing.T) { - // note: this test only assures that the content type of the reponse is + // note: this test only assures that the content type of the response is // set if the handler function does not override it, which happens in the // actual implementation, see // https://github.com/grafana/loki/blob/34a012adcfade43291de3a7670f53679ea06aefe/pkg/lokifrontend/frontend/transport/handler.go#L136-L139