Address Feedback From PR Review

Signed-off-by: Mahad Zaryab <[email protected]>

cmd/jaeger/internal/extension/jaegerquery/server.go

@@ -159,11 +159,8 @@ func (s *server) createMetricReader(host component.Host) (metricsstore.Reader, e
 }
 
 func (s *server) makeQueryOptions() *queryApp.QueryOptions {
-	return &queryApp.QueryOptions{
-		HTTP: s.config.HTTP,
-		GRPC: s.config.GRPC,
-		// TODO handle TLS
-	}
+	// TODO handle TLS
+	return &s.config.QueryOptions
 }
 
 func (s *server) Shutdown(ctx context.Context) error {

cmd/query/app/additional_headers_handler.go

@@ -9,11 +9,11 @@ import (
 	"go.opentelemetry.io/collector/config/configopaque"
 )
 
-func additionalHeadersHandler(h http.Handler, additionalHeaders map[string]configopaque.String) http.Handler {
+func responseHeadersHandler(h http.Handler, responseHeaders map[string]configopaque.String) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		header := w.Header()
-		for key, value := range additionalHeaders {
-			header[key] = []string{value.String()}
+		for key, value := range responseHeaders {
+			header.Set(key, value.String())
 		}
 
 		h.ServeHTTP(w, r)

cmd/query/app/additional_headers_test.go

@@ -13,18 +13,18 @@ import (
 	"go.opentelemetry.io/collector/config/configopaque"
 )
 
-func TestAdditionalHeadersHandler(t *testing.T) {
-	additionalHeaders := make(map[string]configopaque.String)
-	additionalHeaders["Access-Control-Allow-Origin"] = "https://mozilla.org"
-	additionalHeaders["Access-Control-Expose-Headers"] = "X-My-Custom-Header"
-	additionalHeaders["Access-Control-Expose-Headers"] = "X-Another-Custom-Header"
-	additionalHeaders["Access-Control-Request-Headers"] = "field1, field2"
+func TestResponseHeadersHandler(t *testing.T) {
+	responseHeaders := make(map[string]configopaque.String)
+	responseHeaders["Access-Control-Allow-Origin"] = "https://mozilla.org"
+	responseHeaders["Access-Control-Expose-Headers"] = "X-My-Custom-Header"
+	responseHeaders["Access-Control-Expose-Headers"] = "X-Another-Custom-Header"
+	responseHeaders["Access-Control-Request-Headers"] = "field1, field2"
 
 	emptyHandler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		w.Write([]byte{})
 	})
 
-	handler := additionalHeadersHandler(emptyHandler, additionalHeaders)
+	handler := responseHeadersHandler(emptyHandler, responseHeaders)
 	server := httptest.NewServer(handler)
 	defer server.Close()
 
@@ -34,7 +34,7 @@ func TestAdditionalHeadersHandler(t *testing.T) {
 	resp, err := server.Client().Do(req)
 	require.NoError(t, err)
 
-	for k, v := range additionalHeaders {
+	for k, v := range responseHeaders {
 		assert.Equal(t, []string{v.String()}, resp.Header[k])
 	}
 }

cmd/query/app/flags.go

@@ -105,13 +105,13 @@ func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) (*Q
 		return qOpts, fmt.Errorf("failed to process gRPC TLS options: %w", err)
 	}
 	otelTLSCfg := tlsGrpc.ToOtelServerConfig()
-	qOpts.GRPC.TLSSetting = &otelTLSCfg
+	qOpts.GRPC.TLSSetting = otelTLSCfg
 	tlsHTTP, err := tlsHTTPFlagsConfig.InitFromViper(v)
 	if err != nil {
 		return qOpts, fmt.Errorf("failed to process HTTP TLS options: %w", err)
 	}
 	otelHTTPCfg := tlsHTTP.ToOtelServerConfig()
-	qOpts.HTTP.TLSSetting = &otelHTTPCfg
+	qOpts.HTTP.TLSSetting = otelHTTPCfg
 	qOpts.BasePath = v.GetString(queryBasePath)
 	qOpts.UIConfig.AssetsPath = v.GetString(queryStaticFiles)
 	qOpts.UIConfig.LogAccess = v.GetBool(queryLogStaticAssetsAccess)

cmd/query/app/server.go

@@ -172,7 +172,7 @@ func createHTTPServer(
 
 	apiHandler.RegisterRoutes(r)
 	var handler http.Handler = r
-	handler = additionalHeadersHandler(handler, queryOpts.HTTP.ResponseHeaders)
+	handler = responseHeadersHandler(handler, queryOpts.HTTP.ResponseHeaders)
 	if queryOpts.BearerTokenPropagation {
 		handler = bearertoken.PropagationHandler(telset.Logger, handler)
 	}

pkg/config/tlscfg/options.go

@@ -143,8 +143,12 @@ func (o *Options) ToOtelClientConfig() configtls.ClientConfig {
 }
 
 // ToOtelServerConfig provides a mapping between from Options to OTEL's TLS Server Configuration.
-func (o *Options) ToOtelServerConfig() configtls.ServerConfig {
-	cfg := configtls.ServerConfig{
+func (o *Options) ToOtelServerConfig() *configtls.ServerConfig {
+	if !o.Enabled {
+		return nil
+	}
+
+	cfg := &configtls.ServerConfig{
 		ClientCAFile: o.ClientCAPath,
 		Config: configtls.Config{
 			CAFile:         o.CAPath,

pkg/config/tlscfg/options_test.go

@@ -236,11 +236,19 @@ func TestToOtelServerConfig(t *testing.T) {
 	testCases := []struct {
 		name     string
 		options  Options
-		expected configtls.ServerConfig
+		expected *configtls.ServerConfig
 	}{
+		{
+			name: "not enabled",
+			options: Options{
+				Enabled: false,
+			},
+			expected: nil,
+		},
 		{
 			name: "default mapping",
 			options: Options{
+				Enabled:      true,
 				ClientCAPath: "path/to/client/ca.pem",
 				CAPath:       "path/to/ca.pem",
 				CertPath:     "path/to/cert.pem",
@@ -249,7 +257,7 @@ func TestToOtelServerConfig(t *testing.T) {
 				MinVersion:   "1.2",
 				MaxVersion:   "1.3",
 			},
-			expected: configtls.ServerConfig{
+			expected: &configtls.ServerConfig{
 				ClientCAFile: "path/to/client/ca.pem",
 				Config: configtls.Config{
 					CAFile:       "path/to/ca.pem",
@@ -264,6 +272,7 @@ func TestToOtelServerConfig(t *testing.T) {
 		{
 			name: "overrides set",
 			options: Options{
+				Enabled:        true,
 				ClientCAPath:   "path/to/client/ca.pem",
 				CAPath:         "path/to/ca.pem",
 				CertPath:       "path/to/cert.pem",
@@ -273,7 +282,7 @@ func TestToOtelServerConfig(t *testing.T) {
 				MaxVersion:     "1.3",
 				ReloadInterval: 24 * time.Hour,
 			},
-			expected: configtls.ServerConfig{
+			expected: &configtls.ServerConfig{
 				ClientCAFile:       "path/to/client/ca.pem",
 				ReloadClientCAFile: true,
 				Config: configtls.Config{