Skip to content

Commit 1a09227

Browse files
chahatsagarmainchahatsagarmain
authored
[kafka] OTEL helper instead of tlscfg package (#6270)
## Which problem is this PR solving? - Part of #4316 ## Description of the changes - ## How was this change tested? - ## Checklist - [ ] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [ ] I have signed all commits - [ ] I have added unit tests for the new functionality - [ ] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: chahatsagarmain <[email protected]>
1 parent 2c99855 commit 1a09227

File tree

8 files changed

+27
-40
lines changed

8 files changed

+27
-40
lines changed

‎cmd/ingester/app/flags_test.go

+3-4
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"github.com/stretchr/testify/require"
1313

1414
"github.com/jaegertracing/jaeger/pkg/config"
15-
"github.com/jaegertracing/jaeger/pkg/config/tlscfg"
1615
"github.com/jaegertracing/jaeger/pkg/kafka/auth"
1716
"github.com/jaegertracing/jaeger/pkg/testutils"
1817
"github.com/jaegertracing/jaeger/plugin/storage/kafka"
@@ -64,15 +63,15 @@ func TestTLSFlags(t *testing.T) {
6463
},
6564
{
6665
flags: []string{"--kafka.consumer.authentication=kerberos", "--kafka.consumer.tls.enabled=true"},
67-
expected: auth.AuthenticationConfig{Authentication: "kerberos", Kerberos: kerb, TLS: tlscfg.Options{Enabled: true}, PlainText: plain},
66+
expected: auth.AuthenticationConfig{Authentication: "kerberos", Kerberos: kerb, PlainText: plain},
6867
},
6968
{
7069
flags: []string{"--kafka.consumer.authentication=tls"},
71-
expected: auth.AuthenticationConfig{Authentication: "tls", Kerberos: kerb, TLS: tlscfg.Options{Enabled: true}, PlainText: plain},
70+
expected: auth.AuthenticationConfig{Authentication: "tls", Kerberos: kerb, PlainText: plain},
7271
},
7372
{
7473
flags: []string{"--kafka.consumer.authentication=tls", "--kafka.consumer.tls.enabled=false"},
75-
expected: auth.AuthenticationConfig{Authentication: "tls", Kerberos: kerb, TLS: tlscfg.Options{Enabled: true}, PlainText: plain},
74+
expected: auth.AuthenticationConfig{Authentication: "tls", Kerberos: kerb, PlainText: plain},
7675
},
7776
}
7877

‎cmd/ingester/main.go

-3
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,6 @@ func main() {
6868
consumer.Start()
6969

7070
svc.RunAndThen(func() {
71-
if err := options.TLS.Close(); err != nil {
72-
logger.Error("Failed to close TLS certificates watcher", zap.Error(err))
73-
}
7471
if err = consumer.Close(); err != nil {
7572
logger.Error("Failed to close consumer", zap.Error(err))
7673
}

‎pkg/kafka/auth/config.go

+10-11
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
"github.com/Shopify/sarama"
1111
"github.com/spf13/viper"
12+
"go.opentelemetry.io/collector/config/configtls"
1213
"go.uber.org/zap"
1314

1415
"github.com/jaegertracing/jaeger/pkg/config/tlscfg"
@@ -30,10 +31,10 @@ var authTypes = []string{
3031

3132
// AuthenticationConfig describes the configuration properties needed authenticate with kafka cluster
3233
type AuthenticationConfig struct {
33-
Authentication string `mapstructure:"type"`
34-
Kerberos KerberosConfig `mapstructure:"kerberos"`
35-
TLS tlscfg.Options `mapstructure:"tls"`
36-
PlainText PlainTextConfig `mapstructure:"plaintext"`
34+
Authentication string
35+
Kerberos KerberosConfig
36+
TLS configtls.ClientConfig
37+
PlainText PlainTextConfig
3738
}
3839

3940
// SetConfiguration set configure authentication into sarama config structure
@@ -42,9 +43,8 @@ func (config *AuthenticationConfig) SetConfiguration(saramaConfig *sarama.Config
4243
if strings.Trim(authentication, " ") == "" {
4344
authentication = none
4445
}
45-
if config.Authentication == tls || config.TLS.Enabled {
46-
err := setTLSConfiguration(&config.TLS, saramaConfig, logger)
47-
if err != nil {
46+
if config.Authentication == tls {
47+
if err := setTLSConfiguration(&config.TLS, saramaConfig, logger); err != nil {
4848
return err
4949
}
5050
}
@@ -79,15 +79,14 @@ func (config *AuthenticationConfig) InitFromViper(configPrefix string, v *viper.
7979
Prefix: configPrefix,
8080
}
8181

82-
var err error
83-
config.TLS, err = tlsClientConfig.InitFromViper(v)
82+
tlsOpts, err := tlsClientConfig.InitFromViper(v)
8483
if err != nil {
8584
return fmt.Errorf("failed to process Kafka TLS options: %w", err)
8685
}
8786
if config.Authentication == tls {
88-
config.TLS.Enabled = true
87+
tlsOpts.Enabled = true
88+
config.TLS = tlsOpts.ToOtelClientConfig()
8989
}
90-
9190
config.PlainText.Username = v.GetString(configPrefix + plainTextPrefix + suffixPlainTextUsername)
9291
config.PlainText.Password = v.GetString(configPrefix + plainTextPrefix + suffixPlainTextPassword)
9392
config.PlainText.Mechanism = v.GetString(configPrefix + plainTextPrefix + suffixPlainTextMechanism)

‎pkg/kafka/auth/config_test.go

+3-6
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@ import (
1111
"github.com/spf13/viper"
1212
"github.com/stretchr/testify/assert"
1313
"github.com/stretchr/testify/require"
14+
"go.opentelemetry.io/collector/config/configtls"
1415
"go.uber.org/zap"
1516
"go.uber.org/zap/zaptest"
1617

1718
"github.com/jaegertracing/jaeger/pkg/config"
18-
"github.com/jaegertracing/jaeger/pkg/config/tlscfg"
1919
)
2020

2121
func addFlags(flags *flag.FlagSet) {
@@ -64,9 +64,7 @@ func Test_InitFromViper(t *testing.T) {
6464
KeyTabPath: "/path/to/keytab",
6565
DisablePAFXFast: true,
6666
},
67-
TLS: tlscfg.Options{
68-
Enabled: true,
69-
},
67+
TLS: configtls.ClientConfig{},
7068
PlainText: PlainTextConfig{
7169
Username: "user",
7270
Password: "password",
@@ -139,7 +137,7 @@ func TestSetConfiguration(t *testing.T) {
139137
{
140138
name: "TLS authentication with invalid cipher suite",
141139
authType: "tls",
142-
expectedError: "error loading tls config: failed to get cipher suite ids from cipher suite names: cipher suite fail not supported or doesn't exist",
140+
expectedError: "error loading tls config: failed to load TLS config: invalid TLS cipher suite: \"fail\"",
143141
},
144142
}
145143

@@ -149,7 +147,6 @@ func TestSetConfiguration(t *testing.T) {
149147
"--kafka.auth.authentication=" + tt.authType,
150148
})
151149
authConfig := &AuthenticationConfig{}
152-
defer authConfig.TLS.Close()
153150
err := authConfig.InitFromViper(configPrefix, v)
154151
require.NoError(t, err)
155152

‎pkg/kafka/auth/tls.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,17 @@
44
package auth
55

66
import (
7+
"context"
78
"fmt"
89

910
"github.com/Shopify/sarama"
11+
"go.opentelemetry.io/collector/config/configtls"
1012
"go.uber.org/zap"
11-
12-
"github.com/jaegertracing/jaeger/pkg/config/tlscfg"
1313
)
1414

15-
func setTLSConfiguration(config *tlscfg.Options, saramaConfig *sarama.Config, logger *zap.Logger) error {
16-
if config.Enabled {
17-
tlsConfig, err := config.Config(logger)
15+
func setTLSConfiguration(config *configtls.ClientConfig, saramaConfig *sarama.Config, _ *zap.Logger) error {
16+
if !config.Insecure {
17+
tlsConfig, err := config.LoadTLSConfig(context.Background())
1818
if err != nil {
1919
return fmt.Errorf("error loading tls config: %w", err)
2020
}

‎pkg/kafka/auth/tls_test.go

+2-6
Original file line numberDiff line numberDiff line change
@@ -9,20 +9,16 @@ import (
99
"github.com/Shopify/sarama"
1010
"github.com/stretchr/testify/assert"
1111
"github.com/stretchr/testify/require"
12+
"go.opentelemetry.io/collector/config/configtls"
1213
"go.uber.org/zap/zaptest"
13-
14-
"github.com/jaegertracing/jaeger/pkg/config/tlscfg"
1514
)
1615

1716
func TestSetTLSConfiguration(t *testing.T) {
1817
logger := zaptest.NewLogger(t)
1918
saramaConfig := sarama.NewConfig()
20-
tlsConfig := &tlscfg.Options{
21-
Enabled: true,
22-
}
19+
tlsConfig := &configtls.ClientConfig{}
2320
err := setTLSConfiguration(tlsConfig, saramaConfig, logger)
2421
require.NoError(t, err)
2522
assert.True(t, saramaConfig.Net.TLS.Enable)
2623
assert.NotNil(t, saramaConfig.Net.TLS.Config)
27-
defer tlsConfig.Close()
2824
}

‎plugin/storage/kafka/factory.go

-1
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,5 @@ func (f *Factory) Close() error {
105105
if f.producer != nil {
106106
errs = append(errs, f.producer.Close())
107107
}
108-
errs = append(errs, f.options.Config.TLS.Close())
109108
return errors.Join(errs...)
110109
}

‎plugin/storage/kafka/options_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@ import (
1111
"github.com/Shopify/sarama"
1212
"github.com/stretchr/testify/assert"
1313
"github.com/stretchr/testify/require"
14+
"go.opentelemetry.io/collector/config/configtls"
1415

1516
"github.com/jaegertracing/jaeger/pkg/config"
16-
"github.com/jaegertracing/jaeger/pkg/config/tlscfg"
1717
"github.com/jaegertracing/jaeger/pkg/kafka/auth"
1818
)
1919

@@ -181,15 +181,15 @@ func TestTLSFlags(t *testing.T) {
181181
},
182182
{
183183
flags: []string{"--kafka.producer.authentication=kerberos", "--kafka.producer.tls.enabled=true"},
184-
expected: auth.AuthenticationConfig{Authentication: "kerberos", Kerberos: kerb, TLS: tlscfg.Options{Enabled: true}, PlainText: plain},
184+
expected: auth.AuthenticationConfig{Authentication: "kerberos", Kerberos: kerb, TLS: configtls.ClientConfig{}, PlainText: plain},
185185
},
186186
{
187187
flags: []string{"--kafka.producer.authentication=tls"},
188-
expected: auth.AuthenticationConfig{Authentication: "tls", Kerberos: kerb, TLS: tlscfg.Options{Enabled: true}, PlainText: plain},
188+
expected: auth.AuthenticationConfig{Authentication: "tls", Kerberos: kerb, TLS: configtls.ClientConfig{}, PlainText: plain},
189189
},
190190
{
191191
flags: []string{"--kafka.producer.authentication=tls", "--kafka.producer.tls.enabled=false"},
192-
expected: auth.AuthenticationConfig{Authentication: "tls", Kerberos: kerb, TLS: tlscfg.Options{Enabled: true}, PlainText: plain},
192+
expected: auth.AuthenticationConfig{Authentication: "tls", Kerberos: kerb, TLS: configtls.ClientConfig{}, PlainText: plain},
193193
},
194194
}
195195

0 commit comments

Comments
 (0)