diff --git a/cmd/collector/app/builder_flags.go b/cmd/collector/app/builder_flags.go index 492307afde4..fc22d42db2c 100644 --- a/cmd/collector/app/builder_flags.go +++ b/cmd/collector/app/builder_flags.go @@ -50,6 +50,10 @@ var tlsHTTPFlagsConfig = tlscfg.ServerFlagsConfig{ Prefix: "collector.http", } +var tlsZipkinFlagsConfig = tlscfg.ServerFlagsConfig{ + Prefix: "collector.zipkin", +} + // CollectorOptions holds configuration for collector type CollectorOptions struct { // DynQueueSizeMemory determines how much memory to use for the queue @@ -66,6 +70,8 @@ type CollectorOptions struct { TLSGRPC tlscfg.Options // TLSHTTP configures secure transport for HTTP endpoint to collect spans TLSHTTP tlscfg.Options + // TLSZipkin configures secure transport for Zipkin endpoint to collect spans + TLSZipkin tlscfg.Options // CollectorTags is the string representing collector tags to append to each and every span CollectorTags map[string]string // CollectorZipkinHTTPHostPort is the host:port address that the Zipkin collector service listens in on for http requests @@ -101,6 +107,7 @@ func AddFlags(flags *flag.FlagSet) { tlsGRPCFlagsConfig.AddFlags(flags) tlsHTTPFlagsConfig.AddFlags(flags) + tlsZipkinFlagsConfig.AddFlags(flags) } // InitFromViper initializes CollectorOptions with properties from viper @@ -127,6 +134,11 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper) (*CollectorOptions, } else { return cOpts, fmt.Errorf("failed to parse HTTP TLS options: %w", err) } + if tlsZipkin, err := tlsZipkinFlagsConfig.InitFromViper(v); err == nil { + cOpts.TLSZipkin = tlsZipkin + } else { + return cOpts, fmt.Errorf("failed to parse Zipkin TLS options: %w", err) + } return cOpts, nil } diff --git a/cmd/collector/app/builder_flags_test.go b/cmd/collector/app/builder_flags_test.go index e2572fb1afd..3a1d7675007 100644 --- a/cmd/collector/app/builder_flags_test.go +++ b/cmd/collector/app/builder_flags_test.go @@ -80,6 +80,19 @@ func TestCollectorOptionsWithFailedGRPCFlags(t *testing.T) { assert.Contains(t, err.Error(), "failed to parse gRPC TLS options") } +func TestCollectorOptionsWithFailedZipkinFlags(t *testing.T) { + c := &CollectorOptions{} + v, command := config.Viperize(AddFlags) + err := command.ParseFlags([]string{ + "--collector.zipkin.tls.enabled=false", + "--collector.zipkin.tls.cert=blah", // invalid unless tls.enabled + }) + require.NoError(t, err) + _, err = c.InitFromViper(v) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to parse Zipkin TLS options") +} + func TestCollectorOptionsWithFlags_CheckMaxReceiveMessageLength(t *testing.T) { c := &CollectorOptions{} v, command := config.Viperize(AddFlags) diff --git a/cmd/collector/app/collector.go b/cmd/collector/app/collector.go index 3e7a14350c6..0f92a9965e7 100644 --- a/cmd/collector/app/collector.go +++ b/cmd/collector/app/collector.go @@ -46,11 +46,12 @@ type Collector struct { spanHandlers *SpanHandlers // state, read only - hServer *http.Server - zkServer *http.Server - grpcServer *grpc.Server - tlsGRPCCertWatcherCloser io.Closer - tlsHTTPCertWatcherCloser io.Closer + hServer *http.Server + zkServer *http.Server + grpcServer *grpc.Server + tlsGRPCCertWatcherCloser io.Closer + tlsHTTPCertWatcherCloser io.Closer + tlsZipkinCertWatcherCloser io.Closer } // CollectorParams to construct a new Jaeger Collector. @@ -125,9 +126,11 @@ func (c *Collector) Start(builderOpts *CollectorOptions) error { c.tlsGRPCCertWatcherCloser = &builderOpts.TLSGRPC c.tlsHTTPCertWatcherCloser = &builderOpts.TLSHTTP + c.tlsZipkinCertWatcherCloser = &builderOpts.TLSZipkin zkServer, err := server.StartZipkinServer(&server.ZipkinServerParams{ HostPort: builderOpts.CollectorZipkinHTTPHostPort, Handler: c.spanHandlers.ZipkinSpansHandler, + TLSConfig: builderOpts.TLSZipkin, HealthCheck: c.hCheck, AllowedHeaders: builderOpts.CollectorZipkinAllowedHeaders, AllowedOrigins: builderOpts.CollectorZipkinAllowedOrigins, @@ -189,6 +192,7 @@ func (c *Collector) Close() error { // watchers actually never return errors from Close _ = c.tlsGRPCCertWatcherCloser.Close() _ = c.tlsHTTPCertWatcherCloser.Close() + _ = c.tlsZipkinCertWatcherCloser.Close() return nil } diff --git a/cmd/collector/app/server/zipkin.go b/cmd/collector/app/server/zipkin.go index 64ebc3c338b..49432a76c65 100644 --- a/cmd/collector/app/server/zipkin.go +++ b/cmd/collector/app/server/zipkin.go @@ -27,6 +27,7 @@ import ( "github.com/jaegertracing/jaeger/cmd/collector/app/handler" "github.com/jaegertracing/jaeger/cmd/collector/app/zipkin" + "github.com/jaegertracing/jaeger/pkg/config/tlscfg" "github.com/jaegertracing/jaeger/pkg/healthcheck" "github.com/jaegertracing/jaeger/pkg/httpmetrics" "github.com/jaegertracing/jaeger/pkg/recoveryhandler" @@ -34,6 +35,7 @@ import ( // ZipkinServerParams to construct a new Jaeger Collector Zipkin Server type ZipkinServerParams struct { + TLSConfig tlscfg.Options HostPort string Handler handler.ZipkinSpansHandler AllowedOrigins string @@ -62,6 +64,13 @@ func StartZipkinServer(params *ZipkinServerParams) (*http.Server, error) { Addr: params.HostPort, ErrorLog: errorLog, } + if params.TLSConfig.Enabled { + tlsCfg, err := params.TLSConfig.Config(params.Logger) // This checks if the certificates are correctly provided + if err != nil { + return nil, err + } + server.TLSConfig = tlsCfg + } serveZipkin(server, listener, params) return server, nil @@ -84,7 +93,13 @@ func serveZipkin(server *http.Server, listener net.Listener, params *ZipkinServe recoveryHandler := recoveryhandler.NewRecoveryHandler(params.Logger, true) server.Handler = cors.Handler(httpmetrics.Wrap(recoveryHandler(r), params.MetricsFactory)) go func(listener net.Listener, server *http.Server) { - if err := server.Serve(listener); err != nil { + var err error + if params.TLSConfig.Enabled { + err = server.ServeTLS(listener, "", "") + } else { + err = server.Serve(listener) + } + if err != nil { if err != http.ErrServerClosed { params.Logger.Error("Could not launch Zipkin server", zap.Error(err)) } diff --git a/cmd/collector/app/server/zipkin_test.go b/cmd/collector/app/server/zipkin_test.go index b7a501dee2d..7eb58c1e354 100644 --- a/cmd/collector/app/server/zipkin_test.go +++ b/cmd/collector/app/server/zipkin_test.go @@ -15,17 +15,23 @@ package server import ( + "crypto/tls" + "fmt" + "net" "net/http" "net/http/httptest" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/uber/jaeger-lib/metrics/metricstest" "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/collector/app/handler" + "github.com/jaegertracing/jaeger/pkg/config/tlscfg" "github.com/jaegertracing/jaeger/pkg/healthcheck" + "github.com/jaegertracing/jaeger/ports" ) // test wrong port number @@ -57,3 +63,187 @@ func TestSpanCollectorZipkin(t *testing.T) { assert.NoError(t, err) assert.NotNil(t, response) } + +func TestSpanCollectorZipkinTLS(t *testing.T) { + testCases := []struct { + name string + serverTLS tlscfg.Options + clientTLS tlscfg.Options + expectTLSClientErr bool + expectZipkinClientErr bool + expectServerFail bool + }{ + { + name: "should fail with TLS client to untrusted TLS server", + serverTLS: tlscfg.Options{ + Enabled: true, + CertPath: testCertKeyLocation + "/example-server-cert.pem", + KeyPath: testCertKeyLocation + "/example-server-key.pem", + }, + clientTLS: tlscfg.Options{ + Enabled: true, + ServerName: "example.com", + }, + expectTLSClientErr: true, + expectZipkinClientErr: true, + expectServerFail: false, + }, + { + name: "should fail with TLS client to trusted TLS server with incorrect hostname", + serverTLS: tlscfg.Options{ + Enabled: true, + CertPath: testCertKeyLocation + "/example-server-cert.pem", + KeyPath: testCertKeyLocation + "/example-server-key.pem", + }, + clientTLS: tlscfg.Options{ + Enabled: true, + CAPath: testCertKeyLocation + "/example-CA-cert.pem", + ServerName: "nonEmpty", + }, + expectTLSClientErr: true, + expectZipkinClientErr: true, + expectServerFail: false, + }, + { + name: "should pass with TLS client to trusted TLS server with correct hostname", + serverTLS: tlscfg.Options{ + Enabled: true, + CertPath: testCertKeyLocation + "/example-server-cert.pem", + KeyPath: testCertKeyLocation + "/example-server-key.pem", + }, + clientTLS: tlscfg.Options{ + Enabled: true, + CAPath: testCertKeyLocation + "/example-CA-cert.pem", + ServerName: "example.com", + }, + expectTLSClientErr: false, + expectZipkinClientErr: false, + expectServerFail: false, + }, + { + name: "should fail with TLS client without cert to trusted TLS server requiring cert", + serverTLS: tlscfg.Options{ + Enabled: true, + CertPath: testCertKeyLocation + "/example-server-cert.pem", + KeyPath: testCertKeyLocation + "/example-server-key.pem", + ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem", + }, + clientTLS: tlscfg.Options{ + Enabled: true, + CAPath: testCertKeyLocation + "/example-CA-cert.pem", + ServerName: "example.com", + }, + expectTLSClientErr: false, + expectServerFail: false, + expectZipkinClientErr: true, + }, + { + name: "should pass with TLS client with cert to trusted TLS server requiring cert", + serverTLS: tlscfg.Options{ + Enabled: true, + CertPath: testCertKeyLocation + "/example-server-cert.pem", + KeyPath: testCertKeyLocation + "/example-server-key.pem", + ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem", + }, + clientTLS: tlscfg.Options{ + Enabled: true, + CAPath: testCertKeyLocation + "/example-CA-cert.pem", + ServerName: "example.com", + CertPath: testCertKeyLocation + "/example-client-cert.pem", + KeyPath: testCertKeyLocation + "/example-client-key.pem", + }, + expectTLSClientErr: false, + expectServerFail: false, + expectZipkinClientErr: false, + }, + { + name: "should fail with TLS client without cert to trusted TLS server requiring cert from a different CA", + serverTLS: tlscfg.Options{ + Enabled: true, + CertPath: testCertKeyLocation + "/example-server-cert.pem", + KeyPath: testCertKeyLocation + "/example-server-key.pem", + ClientCAPath: testCertKeyLocation + "/wrong-CA-cert.pem", // NB: wrong CA + }, + clientTLS: tlscfg.Options{ + Enabled: true, + CAPath: testCertKeyLocation + "/example-CA-cert.pem", + ServerName: "example.com", + CertPath: testCertKeyLocation + "/example-client-cert.pem", + KeyPath: testCertKeyLocation + "/example-client-key.pem", + }, + expectTLSClientErr: false, + expectServerFail: false, + expectZipkinClientErr: true, + }, + { + name: "should fail with TLS client with cert to trusted TLS server with incorrect TLS min", + serverTLS: tlscfg.Options{ + Enabled: true, + CertPath: testCertKeyLocation + "/example-server-cert.pem", + KeyPath: testCertKeyLocation + "/example-server-key.pem", + ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem", + MinVersion: "1.5", + }, + clientTLS: tlscfg.Options{ + Enabled: true, + CAPath: testCertKeyLocation + "/example-CA-cert.pem", + ServerName: "example.com", + CertPath: testCertKeyLocation + "/example-client-cert.pem", + KeyPath: testCertKeyLocation + "/example-client-key.pem", + }, + expectTLSClientErr: true, + expectServerFail: true, + expectZipkinClientErr: false, + }, + } + + for _, test := range testCases { + t.Run(test.name, func(t *testing.T) { + logger, _ := zap.NewDevelopment() + params := &ZipkinServerParams{ + HostPort: fmt.Sprintf(":%d", ports.CollectorZipkin), + Handler: handler.NewZipkinSpanHandler(logger, nil, nil), + MetricsFactory: metricstest.NewFactory(time.Hour), + HealthCheck: healthcheck.New(), + Logger: logger, + TLSConfig: test.serverTLS, + } + + server, err := StartZipkinServer(params) + + if test.expectServerFail { + require.Error(t, err) + return + } + require.NoError(t, err) + defer server.Close() + + clientTLSCfg, err0 := test.clientTLS.Config(zap.NewNop()) + require.NoError(t, err0) + dialer := &net.Dialer{Timeout: 2 * time.Second} + conn, clientError := tls.DialWithDialer(dialer, "tcp", fmt.Sprintf("localhost:%d", ports.CollectorZipkin), clientTLSCfg) + + if test.expectTLSClientErr { + require.Error(t, clientError) + } else { + require.NoError(t, clientError) + require.Nil(t, conn.Close()) + } + + client := &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: clientTLSCfg, + }, + } + + response, requestError := client.Post(fmt.Sprintf("https://localhost:%d", ports.CollectorZipkin), "", nil) + + if test.expectZipkinClientErr { + require.Error(t, requestError) + } else { + require.NoError(t, requestError) + require.NotNil(t, response) + } + }) + } +} diff --git a/go.mod b/go.mod index 275779c25ac..01518c016e4 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,6 @@ require ( github.com/bsm/sarama-cluster v2.1.13+incompatible github.com/crossdock/crossdock-go v0.0.0-20160816171116-049aabb0122b github.com/dgraph-io/badger/v3 v3.2103.2 - github.com/dgraph-io/ristretto v0.1.0 // indirect github.com/fsnotify/fsnotify v1.5.4 github.com/go-openapi/errors v0.20.2 github.com/go-openapi/loads v0.21.1 @@ -45,6 +44,7 @@ require ( github.com/uber/jaeger-lib v2.4.1+incompatible github.com/xdg-go/scram v1.1.1 go.opentelemetry.io/collector/model v0.49.0 + go.opentelemetry.io/collector/pdata v0.49.0 go.uber.org/atomic v1.9.0 go.uber.org/automaxprocs v1.5.1 go.uber.org/zap v1.21.0 @@ -55,8 +55,6 @@ require ( gopkg.in/yaml.v2 v2.4.0 ) -require go.opentelemetry.io/collector/pdata v0.49.0 - require ( github.com/HdrHistogram/hdrhistogram-go v1.0.1 // indirect github.com/VividCortex/gohistogram v1.0.0 // indirect @@ -67,6 +65,7 @@ require ( github.com/cespare/xxhash/v2 v2.1.2 // indirect github.com/cpuguy83/go-md2man/v2 v2.0.1 // indirect github.com/davecgh/go-spew v1.1.1 // indirect + github.com/dgraph-io/ristretto v0.1.0 // indirect github.com/dustin/go-humanize v1.0.0 // indirect github.com/eapache/go-resiliency v1.2.0 // indirect github.com/eapache/go-xerial-snappy v0.0.0-20180814174437-776d5712da21 // indirect diff --git a/ports/ports.go b/ports/ports.go index 6ab2e997f81..b2bd3317813 100644 --- a/ports/ports.go +++ b/ports/ports.go @@ -37,6 +37,8 @@ const ( CollectorHTTP = 14268 // CollectorAdminHTTP is the default admin HTTP port (health check, metrics, etc.) CollectorAdminHTTP = 14269 + // CollectorZipkin is the port for Zipkin server for sending spans + CollectorZipkin = 9411 // QueryGRPC is the default port of GRPC requests for Query trace retrieval QueryGRPC = 16685