Skip to content

Commit a44d8b1

Browse files
Saumya40-codesyurishkuro
Saumya40-codes
and
yurishkuro
authored
[refractor] Remove dependency on tlscfg.Options (#6478)
## 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 <[email protected]> Signed-off-by: Yuri Shkuro <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]>
1 parent e7ed1e1 commit a44d8b1

File tree

23 files changed

+260
-203
lines changed

23 files changed

+260
-203
lines changed

‎cmd/agent/app/reporter/grpc/builder_test.go

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919
yaml "gopkg.in/yaml.v3"
2020

2121
"github.com/jaegertracing/jaeger/internal/metricstest"
22-
"github.com/jaegertracing/jaeger/pkg/config/tlscfg"
2322
"github.com/jaegertracing/jaeger/pkg/discovery"
2423
"github.com/jaegertracing/jaeger/pkg/metrics"
2524
"github.com/jaegertracing/jaeger/proto-gen/api_v2"
@@ -201,7 +200,7 @@ func TestProxyClientTLS(t *testing.T) {
201200
tests := []struct {
202201
name string
203202
clientTLS *configtls.ClientConfig
204-
serverTLS tlscfg.Options
203+
serverTLS configtls.ServerConfig
205204
expectError bool
206205
}{
207206
{
@@ -215,10 +214,11 @@ func TestProxyClientTLS(t *testing.T) {
215214
},
216215
{
217216
name: "should fail with TLS client to untrusted TLS server",
218-
serverTLS: tlscfg.Options{
219-
Enabled: true,
220-
CertPath: testCertKeyLocation + "/example-server-cert.pem",
221-
KeyPath: testCertKeyLocation + "/example-server-key.pem",
217+
serverTLS: configtls.ServerConfig{
218+
Config: configtls.Config{
219+
CertFile: testCertKeyLocation + "/example-server-cert.pem",
220+
KeyFile: testCertKeyLocation + "/example-server-key.pem",
221+
},
222222
},
223223
clientTLS: &configtls.ClientConfig{
224224
ServerName: "example.com",
@@ -227,10 +227,11 @@ func TestProxyClientTLS(t *testing.T) {
227227
},
228228
{
229229
name: "should fail with TLS client to trusted TLS server with incorrect hostname",
230-
serverTLS: tlscfg.Options{
231-
Enabled: true,
232-
CertPath: testCertKeyLocation + "/example-server-cert.pem",
233-
KeyPath: testCertKeyLocation + "/example-server-key.pem",
230+
serverTLS: configtls.ServerConfig{
231+
Config: configtls.Config{
232+
CertFile: testCertKeyLocation + "/example-server-cert.pem",
233+
KeyFile: testCertKeyLocation + "/example-server-key.pem",
234+
},
234235
},
235236
clientTLS: &configtls.ClientConfig{
236237
Config: configtls.Config{
@@ -241,10 +242,11 @@ func TestProxyClientTLS(t *testing.T) {
241242
},
242243
{
243244
name: "should pass with TLS client to trusted TLS server with correct hostname",
244-
serverTLS: tlscfg.Options{
245-
Enabled: true,
246-
CertPath: testCertKeyLocation + "/example-server-cert.pem",
247-
KeyPath: testCertKeyLocation + "/example-server-key.pem",
245+
serverTLS: configtls.ServerConfig{
246+
Config: configtls.Config{
247+
CertFile: testCertKeyLocation + "/example-server-cert.pem",
248+
KeyFile: testCertKeyLocation + "/example-server-key.pem",
249+
},
248250
},
249251
clientTLS: &configtls.ClientConfig{
250252
Config: configtls.Config{
@@ -256,11 +258,12 @@ func TestProxyClientTLS(t *testing.T) {
256258
},
257259
{
258260
name: "should fail with TLS client without cert to trusted TLS server requiring cert",
259-
serverTLS: tlscfg.Options{
260-
Enabled: true,
261-
CertPath: testCertKeyLocation + "/example-server-cert.pem",
262-
KeyPath: testCertKeyLocation + "/example-server-key.pem",
263-
ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem",
261+
serverTLS: configtls.ServerConfig{
262+
ClientCAFile: testCertKeyLocation + "/example-CA-cert.pem",
263+
Config: configtls.Config{
264+
CertFile: testCertKeyLocation + "/example-server-cert.pem",
265+
KeyFile: testCertKeyLocation + "/example-server-key.pem",
266+
},
264267
},
265268
clientTLS: &configtls.ClientConfig{
266269
Config: configtls.Config{
@@ -272,11 +275,12 @@ func TestProxyClientTLS(t *testing.T) {
272275
},
273276
{
274277
name: "should fail with TLS client without cert to trusted TLS server requiring cert from a different CA",
275-
serverTLS: tlscfg.Options{
276-
Enabled: true,
277-
CertPath: testCertKeyLocation + "/example-server-cert.pem",
278-
KeyPath: testCertKeyLocation + "/example-server-key.pem",
279-
ClientCAPath: testCertKeyLocation + "/wrong-CA-cert.pem", // NB: wrong CA
278+
serverTLS: configtls.ServerConfig{
279+
ClientCAFile: testCertKeyLocation + "/wrong-CA-cert.pem", // NB: wrong CA
280+
Config: configtls.Config{
281+
CertFile: testCertKeyLocation + "/example-server-cert.pem",
282+
KeyFile: testCertKeyLocation + "/example-server-key.pem",
283+
},
280284
},
281285
clientTLS: &configtls.ClientConfig{
282286
Config: configtls.Config{
@@ -290,11 +294,12 @@ func TestProxyClientTLS(t *testing.T) {
290294
},
291295
{
292296
name: "should pass with TLS client with cert to trusted TLS server requiring cert",
293-
serverTLS: tlscfg.Options{
294-
Enabled: true,
295-
CertPath: testCertKeyLocation + "/example-server-cert.pem",
296-
KeyPath: testCertKeyLocation + "/example-server-key.pem",
297-
ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem",
297+
serverTLS: configtls.ServerConfig{
298+
ClientCAFile: testCertKeyLocation + "/example-CA-cert.pem",
299+
Config: configtls.Config{
300+
CertFile: testCertKeyLocation + "/example-server-cert.pem",
301+
KeyFile: testCertKeyLocation + "/example-server-key.pem",
302+
},
298303
},
299304
clientTLS: &configtls.ClientConfig{
300305
Config: configtls.Config{
@@ -314,11 +319,13 @@ func TestProxyClientTLS(t *testing.T) {
314319
ctx, cancel := context.WithCancel(context.Background())
315320
defer cancel()
316321
var opts []grpc.ServerOption
317-
if test.serverTLS.Enabled {
318-
tlsCfg, err := test.serverTLS.ToOtelServerConfig().LoadTLSConfig(ctx)
322+
323+
if test.serverTLS.CertFile != "" && test.serverTLS.KeyFile != "" {
324+
tlsCfg, err := test.serverTLS.LoadTLSConfig(ctx)
319325
require.NoError(t, err)
320326
opts = []grpc.ServerOption{grpc.Creds(credentials.NewTLS(tlsCfg))}
321327
}
328+
322329
spanHandler := &mockSpanHandler{}
323330
s, addr := initializeGRPCTestServer(t, func(s *grpc.Server) {
324331
api_v2.RegisterCollectorServiceServer(s, spanHandler)

‎cmd/agent/app/reporter/grpc/flags.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,12 @@ func (b *ConnBuilder) InitFromViper(v *viper.Viper) (*ConnBuilder, error) {
4040
b.CollectorHostPorts = strings.Split(hostPorts, ",")
4141
}
4242
b.MaxRetry = v.GetUint(retryFlag)
43-
tls, err := tlsFlagsConfig.InitFromViper(v)
43+
tlsCfg, err := tlsFlagsConfig.InitFromViper(v)
4444
if err != nil {
4545
return b, fmt.Errorf("failed to process TLS options: %w", err)
4646
}
47-
if tls.Enabled {
48-
tlsConf := tls.ToOtelClientConfig()
49-
b.TLS = &tlsConf
47+
if !tlsCfg.Insecure {
48+
b.TLS = &tlsCfg
5049
}
5150
b.DiscoveryMinPeers = v.GetInt(discoveryMinPeers)
5251
return b, nil

‎cmd/collector/app/flags/flags.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -193,11 +193,11 @@ func addGRPCFlags(flagSet *flag.FlagSet, cfg serverFlagsConfig, defaultHostPort
193193
}
194194

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

210210
func initGRPCFromViper(v *viper.Viper, opts *configgrpc.ServerConfig, cfg serverFlagsConfig) error {
211-
tlsOpts, err := cfg.tls.InitFromViper(v)
211+
tlsGRPCCfg, err := cfg.tls.InitFromViper(v)
212212
if err != nil {
213213
return fmt.Errorf("failed to parse GRPC TLS options: %w", err)
214214
}
215-
opts.TLSSetting = tlsOpts.ToOtelServerConfig()
215+
opts.TLSSetting = tlsGRPCCfg
216216
opts.NetAddr.Endpoint = ports.FormatHostPort(v.GetString(cfg.prefix + "." + flagSuffixHostPort))
217217
opts.MaxRecvMsgSizeMiB = v.GetInt(cfg.prefix+"."+flagSuffixGRPCMaxReceiveMessageLength) / (1024 * 1024)
218218
opts.Keepalive = &configgrpc.KeepaliveServerConfig{

‎cmd/collector/app/server/grpc_test.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/stretchr/testify/require"
1313
"go.opentelemetry.io/collector/config/configgrpc"
1414
"go.opentelemetry.io/collector/config/confignet"
15+
"go.opentelemetry.io/collector/config/configtls"
1516
"go.uber.org/zap"
1617
"go.uber.org/zap/zapcore"
1718
"go.uber.org/zap/zaptest/observer"
@@ -21,7 +22,6 @@ import (
2122

2223
"github.com/jaegertracing/jaeger/cmd/collector/app/handler"
2324
"github.com/jaegertracing/jaeger/internal/grpctest"
24-
"github.com/jaegertracing/jaeger/pkg/config/tlscfg"
2525
"github.com/jaegertracing/jaeger/pkg/tenancy"
2626
"github.com/jaegertracing/jaeger/proto-gen/api_v2"
2727
)
@@ -99,11 +99,12 @@ func TestSpanCollector(t *testing.T) {
9999

100100
func TestCollectorStartWithTLS(t *testing.T) {
101101
logger, _ := zap.NewDevelopment()
102-
opts := tlscfg.Options{
103-
Enabled: true,
104-
CertPath: testCertKeyLocation + "/example-server-cert.pem",
105-
KeyPath: testCertKeyLocation + "/example-server-key.pem",
106-
ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem",
102+
tlsCfg := &configtls.ServerConfig{
103+
ClientCAFile: testCertKeyLocation + "/example-CA-cert.pem",
104+
Config: configtls.Config{
105+
CertFile: testCertKeyLocation + "/example-server-cert.pem",
106+
KeyFile: testCertKeyLocation + "/example-server-key.pem",
107+
},
107108
}
108109
params := &GRPCServerParams{
109110
Handler: handler.NewGRPCHandler(logger, &mockSpanProcessor{}, &tenancy.Manager{}),
@@ -113,7 +114,7 @@ func TestCollectorStartWithTLS(t *testing.T) {
113114
NetAddr: confignet.AddrConfig{
114115
Transport: confignet.TransportTypeTCP,
115116
},
116-
TLSSetting: opts.ToOtelServerConfig(),
117+
TLSSetting: tlsCfg,
117118
},
118119
}
119120
server, err := StartGRPCServer(params)

0 commit comments

Comments
 (0)