Skip to content

Commit f36c84d

Browse files
chahatsagarmainchahatsagarmain
committed
resolved comments
Signed-off-by: chahatsagarmain <[email protected]>
1 parent db9367e commit f36c84d

File tree

8 files changed

+78
-109
lines changed

8 files changed

+78
-109
lines changed

‎cmd/collector/app/collector.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,6 @@ func (c *Collector) Start(options *flags.CollectorOptions) error {
123123
if options.Zipkin.Endpoint == "" {
124124
c.logger.Info("Not listening for Zipkin HTTP traffic, port not configured")
125125
} else {
126-
fmt.Printf("%v zipkin\n", options.Zipkin.Endpoint)
127126
zipkinReceiver, err := handler.StartZipkinReceiver(options, c.logger, c.spanProcessor, c.tenancyMgr)
128127
if err != nil {
129128
return fmt.Errorf("could not start Zipkin receiver: %w", err)

‎cmd/collector/app/collector_test.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ package app
66
import (
77
"context"
88
"expvar"
9-
"fmt"
109
"io"
1110
"sync/atomic"
1211
"testing"
@@ -124,8 +123,6 @@ func TestNewCollector(t *testing.T) {
124123
})
125124

126125
collectorOpts := optionsForEphemeralPorts()
127-
fmt.Printf("Collector Options: %+v\n", collectorOpts.OTLP.GRPC)
128-
fmt.Print(collectorOpts.OTLP.HTTP.Endpoint)
129126
require.NoError(t, c.Start(collectorOpts))
130127
assert.NotNil(t, c.SpanHandlers())
131128
require.NoError(t, c.Close())

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ func addGRPCFlags(flagSet *flag.FlagSet, cfg serverFlagsConfig, defaultHostPort
190190
cfg.tls.AddFlags(flagSet)
191191
}
192192

193-
func initHTTPFromViper(v *viper.Viper, _ *zap.Logger, opts *confighttp.ServerConfig, corsOpts *corscfg.Options, cfg serverFlagsConfig) error {
193+
func initHTTPFromViper(v *viper.Viper, _ *zap.Logger, opts *confighttp.ServerConfig, corsOpts *confighttp.CORSConfig, cfg serverFlagsConfig) error {
194194
tlsOpts, err := cfg.tls.InitFromViper(v)
195195
if err != nil {
196196
return fmt.Errorf("failed to parse HTTP TLS options: %w", err)
@@ -201,7 +201,7 @@ func initHTTPFromViper(v *viper.Viper, _ *zap.Logger, opts *confighttp.ServerCon
201201
opts.ReadTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPReadTimeout)
202202
opts.ReadHeaderTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPReadHeaderTimeout)
203203
if corsOpts != nil {
204-
opts.CORS = corsOpts.ToOTELCorsConfig()
204+
opts.CORS = corsOpts
205205
}
206206

207207
return nil
@@ -245,7 +245,7 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper, logger *zap.Logger)
245245
cOpts.OTLP.Enabled = v.GetBool(flagCollectorOTLPEnabled)
246246
corsOpts := corsOTLPFlags.InitFromViper(v)
247247

248-
if err := initHTTPFromViper(v, logger, &cOpts.OTLP.HTTP, &corsOpts, otlpServerFlagsCfg.HTTP); err != nil {
248+
if err := initHTTPFromViper(v, logger, &cOpts.OTLP.HTTP, corsOpts, otlpServerFlagsCfg.HTTP); err != nil {
249249
return cOpts, fmt.Errorf("failed to parse OTLP/HTTP server options: %w", err)
250250
}
251251
if err := initGRPCFromViper(v, logger, &cOpts.OTLP.GRPC, otlpServerFlagsCfg.GRPC); err != nil {
@@ -255,7 +255,7 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper, logger *zap.Logger)
255255
cOpts.Zipkin.KeepAlive = v.GetBool(flagZipkinKeepAliveEnabled)
256256
corsOpts = corsZipkinFlags.InitFromViper(v)
257257

258-
if err := initHTTPFromViper(v, logger, &cOpts.Zipkin.ServerConfig, &corsOpts, zipkinServerFlagsCfg); err != nil {
258+
if err := initHTTPFromViper(v, logger, &cOpts.Zipkin.ServerConfig, corsOpts, zipkinServerFlagsCfg); err != nil {
259259
return cOpts, fmt.Errorf("failed to parse Zipkin server options: %w", err)
260260
}
261261

‎cmd/collector/app/handler/zipkin_receiver_tls_test.go‎

Lines changed: 67 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@ import (
1313
"time"
1414

1515
"github.com/stretchr/testify/require"
16+
"go.opentelemetry.io/collector/config/configtls"
1617

1718
"github.com/jaegertracing/jaeger/cmd/collector/app/flags"
18-
"github.com/jaegertracing/jaeger/pkg/config/tlscfg"
1919
"github.com/jaegertracing/jaeger/pkg/tenancy"
2020
"github.com/jaegertracing/jaeger/pkg/testutils"
2121
"github.com/jaegertracing/jaeger/ports"
@@ -25,21 +25,21 @@ func TestSpanCollectorZipkinTLS(t *testing.T) {
2525
const testCertKeyLocation = "../../../../pkg/config/tlscfg/testdata"
2626
testCases := []struct {
2727
name string
28-
serverTLS tlscfg.Options
29-
clientTLS tlscfg.Options
28+
serverTLS configtls.ServerConfig
29+
clientTLS configtls.ClientConfig
3030
expectTLSClientErr bool
3131
expectZipkinClientErr bool
3232
expectServerFail bool
3333
}{
3434
{
3535
name: "should fail with TLS client to untrusted TLS server",
36-
serverTLS: tlscfg.Options{
37-
Enabled: true,
38-
CertPath: testCertKeyLocation + "/example-server-cert.pem",
39-
KeyPath: testCertKeyLocation + "/example-server-key.pem",
36+
serverTLS: configtls.ServerConfig{
37+
Config: configtls.Config{
38+
CertFile: testCertKeyLocation + "/example-server-cert.pem",
39+
KeyFile: testCertKeyLocation + "/example-server-key.pem",
40+
},
4041
},
41-
clientTLS: tlscfg.Options{
42-
Enabled: true,
42+
clientTLS: configtls.ClientConfig{
4343
ServerName: "example.com",
4444
},
4545
expectTLSClientErr: true,
@@ -48,14 +48,16 @@ func TestSpanCollectorZipkinTLS(t *testing.T) {
4848
},
4949
{
5050
name: "should fail with TLS client to trusted TLS server with incorrect hostname",
51-
serverTLS: tlscfg.Options{
52-
Enabled: true,
53-
CertPath: testCertKeyLocation + "/example-server-cert.pem",
54-
KeyPath: testCertKeyLocation + "/example-server-key.pem",
51+
serverTLS: configtls.ServerConfig{
52+
Config: configtls.Config{
53+
CertFile: testCertKeyLocation + "/example-server-cert.pem",
54+
KeyFile: testCertKeyLocation + "/example-server-key.pem",
55+
},
5556
},
56-
clientTLS: tlscfg.Options{
57-
Enabled: true,
58-
CAPath: testCertKeyLocation + "/example-CA-cert.pem",
57+
clientTLS: configtls.ClientConfig{
58+
Config: configtls.Config{
59+
CAFile: testCertKeyLocation + "/example-CA-cert.pem",
60+
},
5961
ServerName: "nonEmpty",
6062
},
6163
expectTLSClientErr: true,
@@ -64,14 +66,16 @@ func TestSpanCollectorZipkinTLS(t *testing.T) {
6466
},
6567
{
6668
name: "should pass with TLS client to trusted TLS server with correct hostname",
67-
serverTLS: tlscfg.Options{
68-
Enabled: true,
69-
CertPath: testCertKeyLocation + "/example-server-cert.pem",
70-
KeyPath: testCertKeyLocation + "/example-server-key.pem",
69+
serverTLS: configtls.ServerConfig{
70+
Config: configtls.Config{
71+
CertFile: testCertKeyLocation + "/example-server-cert.pem",
72+
KeyFile: testCertKeyLocation + "/example-server-key.pem",
73+
},
7174
},
72-
clientTLS: tlscfg.Options{
73-
Enabled: true,
74-
CAPath: testCertKeyLocation + "/example-CA-cert.pem",
75+
clientTLS: configtls.ClientConfig{
76+
Config: configtls.Config{
77+
CAFile: testCertKeyLocation + "/example-CA-cert.pem",
78+
},
7579
ServerName: "example.com",
7680
},
7781
expectTLSClientErr: false,
@@ -80,78 +84,64 @@ func TestSpanCollectorZipkinTLS(t *testing.T) {
8084
},
8185
{
8286
name: "should fail with TLS client without cert to trusted TLS server requiring cert",
83-
serverTLS: tlscfg.Options{
84-
Enabled: true,
85-
CertPath: testCertKeyLocation + "/example-server-cert.pem",
86-
KeyPath: testCertKeyLocation + "/example-server-key.pem",
87-
ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem",
87+
serverTLS: configtls.ServerConfig{
88+
ClientCAFile: testCertKeyLocation + "/example-CA-cert.pem",
89+
Config: configtls.Config{
90+
CertFile: testCertKeyLocation + "/example-server-cert.pem",
91+
KeyFile: testCertKeyLocation + "/example-server-key.pem",
92+
},
8893
},
89-
clientTLS: tlscfg.Options{
90-
Enabled: true,
91-
CAPath: testCertKeyLocation + "/example-CA-cert.pem",
94+
clientTLS: configtls.ClientConfig{
95+
Config: configtls.Config{
96+
CAFile: testCertKeyLocation + "/example-CA-cert.pem",
97+
},
9298
ServerName: "example.com",
9399
},
94100
expectTLSClientErr: false,
95-
expectServerFail: false,
96101
expectZipkinClientErr: true,
102+
expectServerFail: false,
97103
},
98104
{
99105
name: "should pass with TLS client with cert to trusted TLS server requiring cert",
100-
serverTLS: tlscfg.Options{
101-
Enabled: true,
102-
CertPath: testCertKeyLocation + "/example-server-cert.pem",
103-
KeyPath: testCertKeyLocation + "/example-server-key.pem",
104-
ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem",
106+
serverTLS: configtls.ServerConfig{
107+
ClientCAFile: testCertKeyLocation + "/example-CA-cert.pem",
108+
Config: configtls.Config{
109+
CertFile: testCertKeyLocation + "/example-server-cert.pem",
110+
KeyFile: testCertKeyLocation + "/example-server-key.pem",
111+
},
105112
},
106-
clientTLS: tlscfg.Options{
107-
Enabled: true,
108-
CAPath: testCertKeyLocation + "/example-CA-cert.pem",
113+
clientTLS: configtls.ClientConfig{
114+
Config: configtls.Config{
115+
CAFile: testCertKeyLocation + "/example-CA-cert.pem",
116+
CertFile: testCertKeyLocation + "/example-client-cert.pem",
117+
KeyFile: testCertKeyLocation + "/example-client-key.pem",
118+
},
109119
ServerName: "example.com",
110-
CertPath: testCertKeyLocation + "/example-client-cert.pem",
111-
KeyPath: testCertKeyLocation + "/example-client-key.pem",
112120
},
113121
expectTLSClientErr: false,
114-
expectServerFail: false,
115122
expectZipkinClientErr: false,
123+
expectServerFail: false,
116124
},
117125
{
118-
name: "should fail with TLS client without cert to trusted TLS server requiring cert from a different CA",
119-
serverTLS: tlscfg.Options{
120-
Enabled: true,
121-
CertPath: testCertKeyLocation + "/example-server-cert.pem",
122-
KeyPath: testCertKeyLocation + "/example-server-key.pem",
123-
ClientCAPath: testCertKeyLocation + "/wrong-CA-cert.pem", // NB: wrong CA
126+
name: "should fail with TLS client without cert to trusted TLS server requiring cert from different CA",
127+
serverTLS: configtls.ServerConfig{
128+
ClientCAFile: testCertKeyLocation + "/wrong-CA-cert.pem",
129+
Config: configtls.Config{
130+
CertFile: testCertKeyLocation + "/example-server-cert.pem",
131+
KeyFile: testCertKeyLocation + "/example-server-key.pem",
132+
},
124133
},
125-
clientTLS: tlscfg.Options{
126-
Enabled: true,
127-
CAPath: testCertKeyLocation + "/example-CA-cert.pem",
134+
clientTLS: configtls.ClientConfig{
135+
Config: configtls.Config{
136+
CAFile: testCertKeyLocation + "/example-CA-cert.pem",
137+
CertFile: testCertKeyLocation + "/example-client-cert.pem",
138+
KeyFile: testCertKeyLocation + "/example-client-key.pem",
139+
},
128140
ServerName: "example.com",
129-
CertPath: testCertKeyLocation + "/example-client-cert.pem",
130-
KeyPath: testCertKeyLocation + "/example-client-key.pem",
131141
},
132142
expectTLSClientErr: false,
133-
expectServerFail: false,
134143
expectZipkinClientErr: true,
135-
},
136-
{
137-
name: "should fail with TLS client with cert to trusted TLS server with incorrect TLS min",
138-
serverTLS: tlscfg.Options{
139-
Enabled: true,
140-
CertPath: testCertKeyLocation + "/example-server-cert.pem",
141-
KeyPath: testCertKeyLocation + "/example-server-key.pem",
142-
ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem",
143-
MinVersion: "1.5",
144-
},
145-
clientTLS: tlscfg.Options{
146-
Enabled: true,
147-
CAPath: testCertKeyLocation + "/example-CA-cert.pem",
148-
ServerName: "example.com",
149-
CertPath: testCertKeyLocation + "/example-client-cert.pem",
150-
KeyPath: testCertKeyLocation + "/example-client-key.pem",
151-
},
152-
expectTLSClientErr: true,
153-
expectServerFail: true,
154-
expectZipkinClientErr: false,
144+
expectServerFail: false,
155145
},
156146
}
157147

@@ -163,7 +153,7 @@ func TestSpanCollectorZipkinTLS(t *testing.T) {
163153

164154
opts := &flags.CollectorOptions{}
165155
opts.Zipkin.Endpoint = ports.PortToHostPort(ports.CollectorZipkin)
166-
opts.Zipkin.TLSSetting = test.serverTLS.ToOtelServerConfig()
156+
opts.Zipkin.TLSSetting = &test.serverTLS
167157

168158
server, err := StartZipkinReceiver(opts, logger, spanProcessor, tm)
169159
if test.expectServerFail {
@@ -175,7 +165,7 @@ func TestSpanCollectorZipkinTLS(t *testing.T) {
175165
require.NoError(t, server.Shutdown(context.Background()))
176166
}()
177167

178-
clientTLSCfg, err0 := test.clientTLS.ToOtelClientConfig().LoadTLSConfig(context.Background())
168+
clientTLSCfg, err0 := test.clientTLS.LoadTLSConfig(context.Background())
179169
require.NoError(t, err0)
180170
dialer := &net.Dialer{Timeout: 2 * time.Second}
181171
conn, clientError := tls.DialWithDialer(dialer, "tcp", fmt.Sprintf("localhost:%d", ports.CollectorZipkin), clientTLSCfg)

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ type GRPCServerParams struct {
4242
func StartGRPCServer(params *GRPCServerParams) (*grpc.Server, error) {
4343
var server *grpc.Server
4444
var grpcOpts []configgrpc.ToServerOption
45-
// explicitly setting tranport type
4645
params.NetAddr.Transport = confignet.TransportTypeTCP
4746
server, err := params.ToServer(context.Background(), nil, component.TelemetrySettings{
4847
Logger: params.Logger,

‎pkg/config/corscfg/flags.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"strings"
99

1010
"github.com/spf13/viper"
11+
"go.opentelemetry.io/collector/config/confighttp"
1112
)
1213

1314
const (
@@ -25,14 +26,14 @@ func (c Flags) AddFlags(flags *flag.FlagSet) {
2526
flags.String(c.Prefix+corsAllowedOrigins, "", "Comma-separated CORS allowed origins. See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin")
2627
}
2728

28-
func (c Flags) InitFromViper(v *viper.Viper) Options {
29-
var p Options
29+
func (c Flags) InitFromViper(v *viper.Viper) *confighttp.CORSConfig {
30+
var p confighttp.CORSConfig
3031

3132
allowedHeaders := v.GetString(c.Prefix + corsAllowedHeaders)
3233
allowedOrigins := v.GetString(c.Prefix + corsAllowedOrigins)
3334

3435
p.AllowedOrigins = strings.Split(strings.ReplaceAll(allowedOrigins, " ", ""), ",")
3536
p.AllowedHeaders = strings.Split(strings.ReplaceAll(allowedHeaders, " ", ""), ",")
3637

37-
return p
38+
return &p
3839
}

‎pkg/config/corscfg/flags_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
"github.com/stretchr/testify/assert"
1111
"github.com/stretchr/testify/require"
12+
"go.opentelemetry.io/collector/config/confighttp"
1213

1314
"github.com/jaegertracing/jaeger/pkg/config"
1415
"github.com/jaegertracing/jaeger/pkg/testutils"
@@ -31,10 +32,10 @@ func TestCORSFlags(t *testing.T) {
3132
corsOpts := flagCfg.InitFromViper(v)
3233
fmt.Println(corsOpts)
3334

34-
assert.Equal(t, Options{
35+
assert.Equal(t, confighttp.CORSConfig{
3536
AllowedHeaders: []string{"Content-Type", "Accept", "X-Requested-With"},
3637
AllowedOrigins: []string{"http://example.domain.com", "http://*.domain.com"},
37-
}, corsOpts)
38+
}, *corsOpts)
3839
})
3940
}
4041

‎pkg/config/corscfg/options.go‎

Lines changed: 0 additions & 18 deletions
This file was deleted.

0 commit comments

Comments
 (0)