Skip to content

Commit

Permalink
[refractor] Remove dependency on tlscfg.Options (#6478)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?
- Resolves #6468 

## Description of the changes
- Removes the dependency of various `jaeger` packages on tlscfg.Options
- Options is now `private` struct in tlscfg
- In place of `tlscfg.Options` corresponding `configtls` by OTEL is used

## How was this change tested?
- Running `go test -v` in all the packages which were changed/dependent
on tlscfg.Options

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [x] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `npm run lint` and `npm run test`

---------

Signed-off-by: Saumya Shah <saumyabshah90@gmail.com>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Co-authored-by: Yuri Shkuro <github@ysh.us>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
  • Loading branch information
3 people authored Jan 5, 2025
1 parent e7ed1e1 commit a44d8b1
Show file tree
Hide file tree
Showing 23 changed files with 260 additions and 203 deletions.
69 changes: 38 additions & 31 deletions cmd/agent/app/reporter/grpc/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
yaml "gopkg.in/yaml.v3"

"github.com/jaegertracing/jaeger/internal/metricstest"
"github.com/jaegertracing/jaeger/pkg/config/tlscfg"
"github.com/jaegertracing/jaeger/pkg/discovery"
"github.com/jaegertracing/jaeger/pkg/metrics"
"github.com/jaegertracing/jaeger/proto-gen/api_v2"
Expand Down Expand Up @@ -201,7 +200,7 @@ func TestProxyClientTLS(t *testing.T) {
tests := []struct {
name string
clientTLS *configtls.ClientConfig
serverTLS tlscfg.Options
serverTLS configtls.ServerConfig
expectError bool
}{
{
Expand All @@ -215,10 +214,11 @@ func TestProxyClientTLS(t *testing.T) {
},
{
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",
serverTLS: configtls.ServerConfig{
Config: configtls.Config{
CertFile: testCertKeyLocation + "/example-server-cert.pem",
KeyFile: testCertKeyLocation + "/example-server-key.pem",
},
},
clientTLS: &configtls.ClientConfig{
ServerName: "example.com",
Expand All @@ -227,10 +227,11 @@ func TestProxyClientTLS(t *testing.T) {
},
{
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",
serverTLS: configtls.ServerConfig{
Config: configtls.Config{
CertFile: testCertKeyLocation + "/example-server-cert.pem",
KeyFile: testCertKeyLocation + "/example-server-key.pem",
},
},
clientTLS: &configtls.ClientConfig{
Config: configtls.Config{
Expand All @@ -241,10 +242,11 @@ func TestProxyClientTLS(t *testing.T) {
},
{
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",
serverTLS: configtls.ServerConfig{
Config: configtls.Config{
CertFile: testCertKeyLocation + "/example-server-cert.pem",
KeyFile: testCertKeyLocation + "/example-server-key.pem",
},
},
clientTLS: &configtls.ClientConfig{
Config: configtls.Config{
Expand All @@ -256,11 +258,12 @@ func TestProxyClientTLS(t *testing.T) {
},
{
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",
serverTLS: configtls.ServerConfig{
ClientCAFile: testCertKeyLocation + "/example-CA-cert.pem",
Config: configtls.Config{
CertFile: testCertKeyLocation + "/example-server-cert.pem",
KeyFile: testCertKeyLocation + "/example-server-key.pem",
},
},
clientTLS: &configtls.ClientConfig{
Config: configtls.Config{
Expand All @@ -272,11 +275,12 @@ func TestProxyClientTLS(t *testing.T) {
},
{
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
serverTLS: configtls.ServerConfig{
ClientCAFile: testCertKeyLocation + "/wrong-CA-cert.pem", // NB: wrong CA
Config: configtls.Config{
CertFile: testCertKeyLocation + "/example-server-cert.pem",
KeyFile: testCertKeyLocation + "/example-server-key.pem",
},
},
clientTLS: &configtls.ClientConfig{
Config: configtls.Config{
Expand All @@ -290,11 +294,12 @@ func TestProxyClientTLS(t *testing.T) {
},
{
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",
serverTLS: configtls.ServerConfig{
ClientCAFile: testCertKeyLocation + "/example-CA-cert.pem",
Config: configtls.Config{
CertFile: testCertKeyLocation + "/example-server-cert.pem",
KeyFile: testCertKeyLocation + "/example-server-key.pem",
},
},
clientTLS: &configtls.ClientConfig{
Config: configtls.Config{
Expand All @@ -314,11 +319,13 @@ func TestProxyClientTLS(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
var opts []grpc.ServerOption
if test.serverTLS.Enabled {
tlsCfg, err := test.serverTLS.ToOtelServerConfig().LoadTLSConfig(ctx)

if test.serverTLS.CertFile != "" && test.serverTLS.KeyFile != "" {
tlsCfg, err := test.serverTLS.LoadTLSConfig(ctx)
require.NoError(t, err)
opts = []grpc.ServerOption{grpc.Creds(credentials.NewTLS(tlsCfg))}
}

spanHandler := &mockSpanHandler{}
s, addr := initializeGRPCTestServer(t, func(s *grpc.Server) {
api_v2.RegisterCollectorServiceServer(s, spanHandler)
Expand Down
7 changes: 3 additions & 4 deletions cmd/agent/app/reporter/grpc/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,12 @@ func (b *ConnBuilder) InitFromViper(v *viper.Viper) (*ConnBuilder, error) {
b.CollectorHostPorts = strings.Split(hostPorts, ",")
}
b.MaxRetry = v.GetUint(retryFlag)
tls, err := tlsFlagsConfig.InitFromViper(v)
tlsCfg, err := tlsFlagsConfig.InitFromViper(v)
if err != nil {
return b, fmt.Errorf("failed to process TLS options: %w", err)
}
if tls.Enabled {
tlsConf := tls.ToOtelClientConfig()
b.TLS = &tlsConf
if !tlsCfg.Insecure {
b.TLS = &tlsCfg
}
b.DiscoveryMinPeers = v.GetInt(discoveryMinPeers)
return b, nil
Expand Down
8 changes: 4 additions & 4 deletions cmd/collector/app/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,11 +193,11 @@ func addGRPCFlags(flagSet *flag.FlagSet, cfg serverFlagsConfig, defaultHostPort
}

func initHTTPFromViper(v *viper.Viper, opts *confighttp.ServerConfig, cfg serverFlagsConfig) error {
tlsOpts, err := cfg.tls.InitFromViper(v)
tlsHTTPCfg, err := cfg.tls.InitFromViper(v)
if err != nil {
return fmt.Errorf("failed to parse HTTP TLS options: %w", err)
}
opts.TLSSetting = tlsOpts.ToOtelServerConfig()
opts.TLSSetting = tlsHTTPCfg
opts.Endpoint = ports.FormatHostPort(v.GetString(cfg.prefix + "." + flagSuffixHostPort))
opts.IdleTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPIdleTimeout)
opts.ReadTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPReadTimeout)
Expand All @@ -208,11 +208,11 @@ func initHTTPFromViper(v *viper.Viper, opts *confighttp.ServerConfig, cfg server
}

func initGRPCFromViper(v *viper.Viper, opts *configgrpc.ServerConfig, cfg serverFlagsConfig) error {
tlsOpts, err := cfg.tls.InitFromViper(v)
tlsGRPCCfg, err := cfg.tls.InitFromViper(v)
if err != nil {
return fmt.Errorf("failed to parse GRPC TLS options: %w", err)
}
opts.TLSSetting = tlsOpts.ToOtelServerConfig()
opts.TLSSetting = tlsGRPCCfg
opts.NetAddr.Endpoint = ports.FormatHostPort(v.GetString(cfg.prefix + "." + flagSuffixHostPort))
opts.MaxRecvMsgSizeMiB = v.GetInt(cfg.prefix+"."+flagSuffixGRPCMaxReceiveMessageLength) / (1024 * 1024)
opts.Keepalive = &configgrpc.KeepaliveServerConfig{
Expand Down
15 changes: 8 additions & 7 deletions cmd/collector/app/server/grpc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/config/configgrpc"
"go.opentelemetry.io/collector/config/confignet"
"go.opentelemetry.io/collector/config/configtls"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
"go.uber.org/zap/zaptest/observer"
Expand All @@ -21,7 +22,6 @@ import (

"github.com/jaegertracing/jaeger/cmd/collector/app/handler"
"github.com/jaegertracing/jaeger/internal/grpctest"
"github.com/jaegertracing/jaeger/pkg/config/tlscfg"
"github.com/jaegertracing/jaeger/pkg/tenancy"
"github.com/jaegertracing/jaeger/proto-gen/api_v2"
)
Expand Down Expand Up @@ -99,11 +99,12 @@ func TestSpanCollector(t *testing.T) {

func TestCollectorStartWithTLS(t *testing.T) {
logger, _ := zap.NewDevelopment()
opts := tlscfg.Options{
Enabled: true,
CertPath: testCertKeyLocation + "/example-server-cert.pem",
KeyPath: testCertKeyLocation + "/example-server-key.pem",
ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem",
tlsCfg := &configtls.ServerConfig{
ClientCAFile: testCertKeyLocation + "/example-CA-cert.pem",
Config: configtls.Config{
CertFile: testCertKeyLocation + "/example-server-cert.pem",
KeyFile: testCertKeyLocation + "/example-server-key.pem",
},
}
params := &GRPCServerParams{
Handler: handler.NewGRPCHandler(logger, &mockSpanProcessor{}, &tenancy.Manager{}),
Expand All @@ -113,7 +114,7 @@ func TestCollectorStartWithTLS(t *testing.T) {
NetAddr: confignet.AddrConfig{
Transport: confignet.TransportTypeTCP,
},
TLSSetting: opts.ToOtelServerConfig(),
TLSSetting: tlsCfg,
},
}
server, err := StartGRPCServer(params)
Expand Down
Loading

0 comments on commit a44d8b1

Please sign in to comment.