diff --git a/cmd/query/app/apiv3/gateway_test.go b/cmd/query/app/apiv3/gateway_test.go index 6bbe0ce4933..98bc7828c57 100644 --- a/cmd/query/app/apiv3/gateway_test.go +++ b/cmd/query/app/apiv3/gateway_test.go @@ -22,7 +22,6 @@ import ( "github.com/jaegertracing/jaeger/cmd/query/app/internal/api_v3" "github.com/jaegertracing/jaeger/model" _ "github.com/jaegertracing/jaeger/pkg/gogocodec" // force gogo codec registration - "github.com/jaegertracing/jaeger/pkg/tenancy" "github.com/jaegertracing/jaeger/storage/spanstore" spanstoremocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks" ) @@ -42,15 +41,12 @@ type testGateway struct { reader *spanstoremocks.Reader url string router *mux.Router - // used to set a tenancy header when executing requests - setupRequest func(*http.Request) } func (gw *testGateway) execRequest(t *testing.T, url string) ([]byte, int) { req, err := http.NewRequest(http.MethodGet, gw.url+url, nil) require.NoError(t, err) req.Header.Set("Content-Type", "application/json") - gw.setupRequest(req) response, err := http.DefaultClient.Do(req) require.NoError(t, err) body, err := io.ReadAll(response.Body) @@ -101,11 +97,8 @@ func makeTestTrace() (*model.Trace, model.TraceID) { func runGatewayTests( t *testing.T, basePath string, - tenancyOptions tenancy.Options, - setupRequest func(*http.Request), ) { - gw := setupHTTPGateway(t, basePath, tenancyOptions) - gw.setupRequest = setupRequest + gw := setupHTTPGateway(t, basePath) t.Run("GetServices", gw.runGatewayGetServices) t.Run("GetOperations", gw.runGatewayGetOperations) t.Run("GetTrace", gw.runGatewayGetTrace) diff --git a/cmd/query/app/apiv3/http_gateway.go b/cmd/query/app/apiv3/http_gateway.go index d520f6a03d5..6092c8304cd 100644 --- a/cmd/query/app/apiv3/http_gateway.go +++ b/cmd/query/app/apiv3/http_gateway.go @@ -23,7 +23,6 @@ import ( "github.com/jaegertracing/jaeger/cmd/query/app/internal/api_v3" "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" "github.com/jaegertracing/jaeger/model" - "github.com/jaegertracing/jaeger/pkg/tenancy" "github.com/jaegertracing/jaeger/storage/spanstore" ) @@ -46,7 +45,6 @@ const ( // HTTPGateway exposes APIv3 HTTP endpoints. type HTTPGateway struct { QueryService *querysvc.QueryService - TenancyMgr *tenancy.Manager Logger *zap.Logger Tracer trace.TracerProvider } @@ -62,16 +60,13 @@ func (h *HTTPGateway) RegisterRoutes(router *mux.Router) { // addRoute adds a new endpoint to the router with given path and handler function. // This code is mostly copied from ../http_handler. -func (h *HTTPGateway) addRoute( +func (*HTTPGateway) addRoute( router *mux.Router, f func(http.ResponseWriter, *http.Request), route string, _ ...any, /* args */ ) *mux.Route { var handler http.Handler = http.HandlerFunc(f) - if h.TenancyMgr.Enabled { - handler = tenancy.ExtractTenantHTTPHandler(h.TenancyMgr, handler) - } handler = otelhttp.WithRouteTag(route, handler) return router.HandleFunc(route, handler.ServeHTTP) } diff --git a/cmd/query/app/apiv3/http_gateway_test.go b/cmd/query/app/apiv3/http_gateway_test.go index aea953c1d50..6bb0b68b8a7 100644 --- a/cmd/query/app/apiv3/http_gateway_test.go +++ b/cmd/query/app/apiv3/http_gateway_test.go @@ -6,7 +6,6 @@ package apiv3 import ( "errors" "fmt" - "io" "net/http" "net/http/httptest" "net/url" @@ -22,7 +21,6 @@ import ( "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" "github.com/jaegertracing/jaeger/model" "github.com/jaegertracing/jaeger/pkg/jtracer" - "github.com/jaegertracing/jaeger/pkg/tenancy" "github.com/jaegertracing/jaeger/pkg/testutils" dependencyStoreMocks "github.com/jaegertracing/jaeger/storage/dependencystore/mocks" "github.com/jaegertracing/jaeger/storage/spanstore" @@ -32,7 +30,6 @@ import ( func setupHTTPGatewayNoServer( _ *testing.T, basePath string, - tenancyOptions tenancy.Options, ) *testGateway { gw := &testGateway{ reader: &spanstoremocks.Reader{}, @@ -45,7 +42,6 @@ func setupHTTPGatewayNoServer( hgw := &HTTPGateway{ QueryService: q, - TenancyMgr: tenancy.NewManager(&tenancyOptions), Logger: zap.NewNop(), Tracer: jtracer.NoOp().OTEL, } @@ -61,9 +57,8 @@ func setupHTTPGatewayNoServer( func setupHTTPGateway( t *testing.T, basePath string, - tenancyOptions tenancy.Options, ) *testGateway { - gw := setupHTTPGatewayNoServer(t, basePath, tenancyOptions) + gw := setupHTTPGatewayNoServer(t, basePath) httpServer := httptest.NewServer(gw.router) t.Cleanup(func() { httpServer.Close() }) @@ -76,22 +71,7 @@ func setupHTTPGateway( } func TestHTTPGateway(t *testing.T) { - for _, ten := range []bool{false, true} { - t.Run(fmt.Sprintf("tenancy=%v", ten), func(t *testing.T) { - tenancyOptions := tenancy.Options{ - Enabled: ten, - } - tm := tenancy.NewManager(&tenancyOptions) - runGatewayTests(t, "/", - tenancyOptions, - func(req *http.Request) { - if ten { - // Add a tenancy header on outbound requests - req.Header.Add(tm.Header, "dummy") - } - }) - }) - } + runGatewayTests(t, "/") } func TestHTTPGatewayTryHandleError(t *testing.T) { @@ -126,7 +106,7 @@ func TestHTTPGatewayOTLPError(t *testing.T) { } func TestHTTPGatewayGetTraceErrors(t *testing.T) { - gw := setupHTTPGatewayNoServer(t, "", tenancy.Options{}) + gw := setupHTTPGatewayNoServer(t, "") // malformed trace id r, err := http.NewRequest(http.MethodGet, "/api/v3/traces/xyz", nil) @@ -233,7 +213,7 @@ func TestHTTPGatewayFindTracesErrors(t *testing.T) { require.NoError(t, err) w := httptest.NewRecorder() - gw := setupHTTPGatewayNoServer(t, "", tenancy.Options{}) + gw := setupHTTPGatewayNoServer(t, "") gw.router.ServeHTTP(w, r) assert.Contains(t, w.Body.String(), tc.expErr) }) @@ -245,7 +225,7 @@ func TestHTTPGatewayFindTracesErrors(t *testing.T) { require.NoError(t, err) w := httptest.NewRecorder() - gw := setupHTTPGatewayNoServer(t, "", tenancy.Options{}) + gw := setupHTTPGatewayNoServer(t, "") gw.reader. On("FindTraces", matchContext, qp). Return(nil, errors.New(simErr)).Once() @@ -256,7 +236,7 @@ func TestHTTPGatewayFindTracesErrors(t *testing.T) { } func TestHTTPGatewayGetServicesErrors(t *testing.T) { - gw := setupHTTPGatewayNoServer(t, "", tenancy.Options{}) + gw := setupHTTPGatewayNoServer(t, "") const simErr = "simulated error" gw.reader. @@ -271,7 +251,7 @@ func TestHTTPGatewayGetServicesErrors(t *testing.T) { } func TestHTTPGatewayGetOperationsErrors(t *testing.T) { - gw := setupHTTPGatewayNoServer(t, "", tenancy.Options{}) + gw := setupHTTPGatewayNoServer(t, "") qp := spanstore.OperationQueryParameters{ServiceName: "foo", SpanKind: "server"} const simErr = "simulated error" @@ -285,41 +265,3 @@ func TestHTTPGatewayGetOperationsErrors(t *testing.T) { gw.router.ServeHTTP(w, r) assert.Contains(t, w.Body.String(), simErr) } - -func TestHTTPGatewayTenancyRejection(t *testing.T) { - basePath := "/" - tenancyOptions := tenancy.Options{Enabled: true} - gw := setupHTTPGateway(t, basePath, tenancyOptions) - - traceID := model.NewTraceID(150, 160) - gw.reader.On("GetTrace", matchContext, matchTraceID).Return( - &model.Trace{ - Spans: []*model.Span{ - { - TraceID: traceID, - SpanID: model.NewSpanID(180), - OperationName: "foobar", - }, - }, - }, nil).Once() - - req, err := http.NewRequest(http.MethodGet, gw.url+"/api/v3/traces/123", nil) - require.NoError(t, err) - req.Header.Set("Content-Type", "application/json") - // We don't set tenant header - response, err := http.DefaultClient.Do(req) - require.NoError(t, err) - body, err := io.ReadAll(response.Body) - require.NoError(t, err) - require.NoError(t, response.Body.Close()) - require.Equal(t, http.StatusUnauthorized, response.StatusCode, "response=%s", string(body)) - - // Try again with tenant header set - tm := tenancy.NewManager(&tenancyOptions) - req.Header.Set(tm.Header, "acme") - response, err = http.DefaultClient.Do(req) - require.NoError(t, err) - require.NoError(t, response.Body.Close()) - require.Equal(t, http.StatusOK, response.StatusCode) - // Skip unmarshal of response; it is enough that it succeeded -} diff --git a/cmd/query/app/http_handler.go b/cmd/query/app/http_handler.go index c4a8fb7b5eb..cbceee76c46 100644 --- a/cmd/query/app/http_handler.go +++ b/cmd/query/app/http_handler.go @@ -27,7 +27,6 @@ import ( uiconv "github.com/jaegertracing/jaeger/model/converter/json" ui "github.com/jaegertracing/jaeger/model/json" "github.com/jaegertracing/jaeger/pkg/jtracer" - "github.com/jaegertracing/jaeger/pkg/tenancy" "github.com/jaegertracing/jaeger/plugin/metrics/disabled" "github.com/jaegertracing/jaeger/proto-gen/api_v2/metrics" "github.com/jaegertracing/jaeger/storage/metricsstore" @@ -76,7 +75,6 @@ type APIHandler struct { queryService *querysvc.QueryService metricsQueryService querysvc.MetricsQueryService queryParser queryParser - tenancyMgr *tenancy.Manager basePath string apiPrefix string logger *zap.Logger @@ -84,14 +82,13 @@ type APIHandler struct { } // NewAPIHandler returns an APIHandler -func NewAPIHandler(queryService *querysvc.QueryService, tm *tenancy.Manager, options ...HandlerOption) *APIHandler { +func NewAPIHandler(queryService *querysvc.QueryService, options ...HandlerOption) *APIHandler { aH := &APIHandler{ queryService: queryService, queryParser: queryParser{ traceQueryLookbackDuration: defaultTraceQueryLookbackDuration, timeNow: time.Now, }, - tenancyMgr: tm, } for _, option := range options { @@ -135,9 +132,6 @@ func (aH *APIHandler) handleFunc( ) *mux.Route { route := aH.formatRoute(routeFmt, args...) var handler http.Handler = http.HandlerFunc(f) - if aH.tenancyMgr.Enabled { - handler = tenancy.ExtractTenantHTTPHandler(aH.tenancyMgr, handler) - } handler = traceResponseHandler(handler) handler = otelhttp.WithRouteTag(route, handler) return router.HandleFunc(route, handler.ServeHTTP) diff --git a/cmd/query/app/http_handler_test.go b/cmd/query/app/http_handler_test.go index 1422839d6ce..fabb461d3b0 100644 --- a/cmd/query/app/http_handler_test.go +++ b/cmd/query/app/http_handler_test.go @@ -35,7 +35,6 @@ import ( "github.com/jaegertracing/jaeger/model/adjuster" ui "github.com/jaegertracing/jaeger/model/json" "github.com/jaegertracing/jaeger/pkg/jtracer" - "github.com/jaegertracing/jaeger/pkg/tenancy" "github.com/jaegertracing/jaeger/plugin/metrics/disabled" "github.com/jaegertracing/jaeger/proto-gen/api_v2/metrics" depsmocks "github.com/jaegertracing/jaeger/storage/dependencystore/mocks" @@ -95,7 +94,6 @@ type structuredTraceResponse struct { func initializeTestServerWithHandler(t *testing.T, queryOptions querysvc.QueryServiceOptions, options ...HandlerOption) *testServer { return initializeTestServerWithOptions( t, - &tenancy.Manager{}, queryOptions, append( []HandlerOption{ @@ -112,7 +110,6 @@ func initializeTestServerWithHandler(t *testing.T, queryOptions querysvc.QuerySe func initializeTestServerWithOptions( t *testing.T, - tenancyMgr *tenancy.Manager, queryOptions querysvc.QueryServiceOptions, options ...HandlerOption, ) *testServer { @@ -121,10 +118,10 @@ func initializeTestServerWithOptions( dependencyStorage := &depsmocks.Reader{} qs := querysvc.NewQueryService(readStorage, dependencyStorage, queryOptions) r := NewRouter() - handler := NewAPIHandler(qs, tenancyMgr, options...) + handler := NewAPIHandler(qs, options...) handler.RegisterRoutes(r) ts := &testServer{ - server: httptest.NewServer(tenancy.ExtractTenantHTTPHandler(tenancyMgr, r)), + server: httptest.NewServer(r), spanReader: readStorage, dependencyReader: dependencyStorage, handler: handler, @@ -147,7 +144,7 @@ type testServer struct { } func withTestServer(t *testing.T, doTest func(s *testServer), queryOptions querysvc.QueryServiceOptions, options ...HandlerOption) { - ts := initializeTestServerWithOptions(t, &tenancy.Manager{}, queryOptions, options...) + ts := initializeTestServerWithOptions(t, queryOptions, options...) doTest(ts) } @@ -168,7 +165,7 @@ func TestLogOnServerError(t *testing.T) { readStorage := &spanstoremocks.Reader{} dependencyStorage := &depsmocks.Reader{} qs := querysvc.NewQueryService(readStorage, dependencyStorage, querysvc.QueryServiceOptions{}) - h := NewAPIHandler(qs, &tenancy.Manager{}, HandlerOptions.Logger(logger)) + h := NewAPIHandler(qs, HandlerOptions.Logger(logger)) e := errors.New("test error") h.handleError(&httptest.ResponseRecorder{}, e, http.StatusInternalServerError) require.Len(t, logs.All(), 1) @@ -385,7 +382,7 @@ func TestSearchByTraceIDSuccess(t *testing.T) { func TestSearchByTraceIDSuccessWithArchive(t *testing.T) { archiveReadMock := &spanstoremocks.Reader{} - ts := initializeTestServerWithOptions(t, &tenancy.Manager{}, querysvc.QueryServiceOptions{ + ts := initializeTestServerWithOptions(t, querysvc.QueryServiceOptions{ ArchiveSpanReader: archiveReadMock, }) ts.spanReader.On("GetTrace", mock.AnythingOfType("*context.valueCtx"), mock.AnythingOfType("model.TraceID")). @@ -426,7 +423,6 @@ func TestSearchByTraceIDFailure(t *testing.T) { func TestSearchModelConversionFailure(t *testing.T) { ts := initializeTestServerWithOptions( t, - &tenancy.Manager{}, querysvc.QueryServiceOptions{ Adjuster: adjuster.Func(func(trace *model.Trace) (*model.Trace, error) { return trace, errAdjustment @@ -883,94 +879,3 @@ func execJSON(req *http.Request, additionalHeaders map[string]string, out any) e func parsedError(code int, err string) string { return fmt.Sprintf(`%d error from server: {"data":null,"total":0,"limit":0,"offset":0,"errors":[{"code":%d,"msg":"%s"}]}`+"\n", code, code, err) } - -func TestSearchTenancyHTTP(t *testing.T) { - tenancyOptions := tenancy.Options{ - Enabled: true, - } - ts := initializeTestServerWithOptions(t, - tenancy.NewManager(&tenancyOptions), - querysvc.QueryServiceOptions{}) - ts.spanReader.On("GetTrace", mock.AnythingOfType("*context.valueCtx"), mock.AnythingOfType("model.TraceID")). - Return(mockTrace, nil).Twice() - - var response structuredResponse - err := getJSON(ts.server.URL+`/api/traces?traceID=1&traceID=2`, &response) - require.Error(t, err) - assert.Equal(t, "401 error from server: missing tenant header", err.Error()) - assert.Empty(t, response.Errors) - assert.Nil(t, response.Data) - - err = getJSONCustomHeaders( - ts.server.URL+`/api/traces?traceID=1&traceID=2`, - map[string]string{"x-tenant": "acme"}, - &response) - require.NoError(t, err) - assert.Empty(t, response.Errors) - assert.Len(t, response.Data, 2) -} - -func TestSearchTenancyRejectionHTTP(t *testing.T) { - tenancyOptions := tenancy.Options{ - Enabled: true, - } - ts := initializeTestServerWithOptions(t, tenancy.NewManager(&tenancyOptions), querysvc.QueryServiceOptions{}) - ts.spanReader.On("GetTrace", mock.AnythingOfType("*context.valueCtx"), mock.AnythingOfType("model.TraceID")). - Return(mockTrace, nil).Twice() - - req, err := http.NewRequest(http.MethodGet, ts.server.URL+`/api/traces?traceID=1&traceID=2`, nil) - require.NoError(t, err) - req.Header.Add("Accept", "application/json") - // We don't set tenant header - resp, err := httpClient.Do(req) - require.NoError(t, err) - defer resp.Body.Close() - assert.Equal(t, http.StatusUnauthorized, resp.StatusCode) - - tm := tenancy.NewManager(&tenancyOptions) - req.Header.Set(tm.Header, "acme") - resp, err = http.DefaultClient.Do(req) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusOK, resp.StatusCode) - // Skip unmarshal of response; it is enough that it succeeded -} - -func TestSearchTenancyFlowTenantHTTP(t *testing.T) { - tenancyOptions := tenancy.Options{ - Enabled: true, - } - ts := initializeTestServerWithOptions(t, tenancy.NewManager(&tenancyOptions), querysvc.QueryServiceOptions{}) - ts.spanReader.On("GetTrace", mock.MatchedBy(func(v any) bool { - ctx, ok := v.(context.Context) - if !ok || tenancy.GetTenant(ctx) != "acme" { - return false - } - return true - }), mock.AnythingOfType("model.TraceID")).Return(mockTrace, nil).Twice() - ts.spanReader.On("GetTrace", mock.MatchedBy(func(v any) bool { - ctx, ok := v.(context.Context) - if !ok || tenancy.GetTenant(ctx) != "megacorp" { - return false - } - return true - }), mock.AnythingOfType("model.TraceID")).Return(nil, errStorage).Once() - - var responseAcme structuredResponse - err := getJSONCustomHeaders( - ts.server.URL+`/api/traces?traceID=1&traceID=2`, - map[string]string{"x-tenant": "acme"}, - &responseAcme) - require.NoError(t, err) - assert.Empty(t, responseAcme.Errors) - assert.Len(t, responseAcme.Data, 2) - - var responseMegacorp structuredResponse - err = getJSONCustomHeaders( - ts.server.URL+`/api/traces?traceID=1&traceID=2`, - map[string]string{"x-tenant": "megacorp"}, - &responseMegacorp) - assert.Contains(t, err.Error(), "storage error") - assert.Empty(t, responseMegacorp.Errors) - assert.Nil(t, responseMegacorp.Data) -} diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index 787560a20f6..83159f5b38b 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -212,7 +212,6 @@ func createHTTPRouter( querySvc *querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, queryOpts *QueryOptions, - tm *tenancy.Manager, telset telemetery.Setting, ) *mux.Router { apiHandlerOptions := []HandlerOption{ @@ -223,7 +222,6 @@ func createHTTPRouter( apiHandler := NewAPIHandler( querySvc, - tm, apiHandlerOptions...) r := NewRouter() if queryOpts.BasePath != "/" { @@ -232,7 +230,6 @@ func createHTTPRouter( (&apiv3.HTTPGateway{ QueryService: querySvc, - TenancyMgr: tm, Logger: telset.Logger, Tracer: telset.TracerProvider, }).RegisterRoutes(r) @@ -249,8 +246,11 @@ func createHTTPServerLegacy( tm *tenancy.Manager, telset telemetery.Setting, ) (*httpServer, error) { - r := createHTTPRouter(querySvc, metricsQuerySvc, queryOpts, tm, telset) + r := createHTTPRouter(querySvc, metricsQuerySvc, queryOpts, telset) var handler http.Handler = r + if tm.Enabled { + handler = tenancy.ExtractTenantHTTPHandler(tm, handler) + } handler = responseHeadersHandler(handler, queryOpts.HTTP.ResponseHeaders) if queryOpts.BearerTokenPropagation { handler = bearertoken.PropagationHandler(telset.Logger, handler) @@ -274,8 +274,11 @@ func createHTTPServerOTEL( tm *tenancy.Manager, telset telemetery.Setting, ) (*httpServer, error) { - r := createHTTPRouter(querySvc, metricsQuerySvc, queryOpts, tm, telset) + r := createHTTPRouter(querySvc, metricsQuerySvc, queryOpts, telset) var handler http.Handler = r + if tm.Enabled { + handler = tenancy.ExtractTenantHTTPHandler(tm, handler) + } if queryOpts.BearerTokenPropagation { handler = bearertoken.PropagationHandler(telset.Logger, handler) }