Skip to content

Commit c6e81e6

Browse files
axwFiery-Fenix
authored andcommitted
Move TLS config for Kafka components to top level (open-telemetry#39115)
#### Description Deprecates `auth::tls` and introduce `tls` to be consistent with other components, such as the OTLP receiver and exporter. Until we remove it altogether, `auth::tls` will continue to be honoured only if `tls` is unspecified. Why? Two reasons: - TLS is primarily used for encryption. It might also be for client authentication, but that's less common. - Consistency with the majority of other components using configtls.ClientConfig. Almost all of them put it at the top-level. Put together, I think this makes for a clearer structure. Having consistency makes it easier to understand the configuration, particularly if you're already familiar with other components. Moving out of "auth" will also make it easier to understand if you're not using TLS for auth. #### Link to tracking issue Fixes open-telemetry#37776 #### Testing Unit tests pass. Added tests covering the new deprecated config handling. #### Documentation Simplified READMEs by just pointing to the configtls docs, reducing opportunity for drift.
1 parent 3e7f826 commit c6e81e6

File tree

17 files changed

+306
-162
lines changed

17 files changed

+306
-162
lines changed
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: deprecation
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: kafkaexporter
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Deprecate `auth::tls` and introduce `tls` config
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [37776]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: [user]
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: deprecation
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: kafkametricsreceiver
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Deprecate `auth::tls` and introduce `tls` config
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [37776]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: [user]
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: deprecation
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: kafkareceiver
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Deprecate `auth::tls` and introduce `tls` config
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [37776]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: [user]
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: deprecation
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: kafkatopicsobserverextension
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Deprecate `auth::tls` and introduce `tls` config
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [37776]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: [user]

exporter/kafkaexporter/README.md

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ The following settings can be optionally configured:
4040
- `partition_traces_by_id` (default = false): configures the exporter to include the trace ID as the message key in trace messages sent to kafka. *Please note:* this setting does not have any effect on Jaeger encoding exporters since Jaeger exporters include trace ID as the message key by default.
4141
- `partition_metrics_by_resource_attributes` (default = false) configures the exporter to include the hash of sorted resource attributes as the message partitioning key in metric messages sent to kafka.
4242
- `partition_logs_by_resource_attributes` (default = false) configures the exporter to include the hash of sorted resource attributes as the message partitioning key in log messages sent to kafka.
43+
- `tls`: see [TLS Configuration Settings](https://github.com/open-telemetry/opentelemetry-collector/blob/main/config/configtls/README.md) for the full set of available options.
4344
- `auth`
4445
- `plain_text` (Deprecated in v0.123.0: use sasl with mechanism set to PLAIN instead.)
4546
- `username`: The username to use.
@@ -51,17 +52,7 @@ The following settings can be optionally configured:
5152
- `version` (default = 0): The SASL protocol version to use (0 or 1)
5253
- `aws_msk.region`: AWS Region in case of AWS_MSK_IAM or AWS_MSK_IAM_OAUTHBEARER mechanism
5354
- `aws_msk.broker_addr`: MSK Broker address in case of AWS_MSK_IAM mechanism
54-
- `tls`: see [TLS Configuration Settings](https://github.com/open-telemetry/opentelemetry-collector/blob/main/config/configtls/README.md) for the full set of available options.
55-
- `ca_file`: path to the CA cert. For a client this verifies the server certificate. Should
56-
only be used if `insecure` is set to false.
57-
- `cert_file`: path to the TLS cert to use for TLS required connections. Should
58-
only be used if `insecure` is set to false.
59-
- `key_file`: path to the TLS key to use for TLS required connections. Should
60-
only be used if `insecure` is set to false.
61-
- `insecure` (default = false): Disable verifying the server's certificate chain and host
62-
name (`InsecureSkipVerify` in the tls config)
63-
- `server_name_override`: ServerName indicates the name of the server requested by the client
64-
in order to support virtual hosting.
55+
- `tls` (Deprecated in v0.124.0: configure tls at the top level): this is an alias for tls at the top level.
6556
- `kerberos`
6657
- `service_name`: Kerberos service name
6758
- `realm`: Kerberos realm

extension/observer/kafkatopicsobserver/README.md

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ The following settings can be optionally configured:
2828
- `protocol_version` (default = 2.1.0): Kafka protocol version e.g. 2.0.0
2929
- `client_id` (default = "otel-collector"): The client ID to configure the Kafka client with.
3030
- `topics_sync_interval` (default 5s)
31+
- `tls`: see [TLS Configuration Settings](https://github.com/open-telemetry/opentelemetry-collector/blob/main/config/configtls/README.md) for the full set of available options.
3132
- `auth`
3233
- `plain_text` (Deprecated in v0.123.0: use sasl with mechanism set to PLAIN instead.)
3334
- `username`: The username to use.
@@ -38,17 +39,7 @@ The following settings can be optionally configured:
3839
- `mechanism`: The sasl mechanism to use (SCRAM-SHA-256, SCRAM-SHA-512, AWS_MSK_IAM, AWS_MSK_IAM_OAUTHBEARER or PLAIN)
3940
- `aws_msk.region`: AWS Region in case of AWS_MSK_IAM or AWS_MSK_IAM_OAUTHBEARER mechanism
4041
- `aws_msk.broker_addr`: MSK Broker address in case of AWS_MSK_IAM mechanism
41-
- `tls`
42-
- `ca_file`: path to the CA cert. For a client this verifies the server certificate. Should
43-
only be used if `insecure` is set to false.
44-
- `cert_file`: path to the TLS cert to use for TLS required connections. Should
45-
only be used if `insecure` is set to false.
46-
- `key_file`: path to the TLS key to use for TLS required connections. Should
47-
only be used if `insecure` is set to false.
48-
- `insecure` (default = false): Disable verifying the server's certificate
49-
chain and host name (`InsecureSkipVerify` in the tls config)
50-
- `server_name_override`: ServerName indicates the name of the server requested by the client
51-
in order to support virtual hosting.
42+
- `tls` (Deprecated in v0.124.0: configure tls at the top level): this is an alias for tls at the top level.
5243
- `kerberos`
5344
- `service_name`: Kerberos service name
5445
- `realm`: Kerberos realm

internal/kafka/authentication.go

Lines changed: 5 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -7,42 +7,31 @@ import (
77
"context"
88
"crypto/sha256"
99
"crypto/sha512"
10-
"crypto/tls"
11-
"fmt"
1210

1311
"github.com/IBM/sarama"
1412
"github.com/aws/aws-msk-iam-sasl-signer-go/signer"
15-
"go.opentelemetry.io/collector/config/configtls"
1613

1714
"github.com/open-telemetry/opentelemetry-collector-contrib/internal/kafka/awsmsk"
1815
"github.com/open-telemetry/opentelemetry-collector-contrib/internal/kafka/configkafka"
1916
)
2017

21-
// ConfigureSaramaAuthentication configures authentication in sarama.Config.
18+
// configureSaramaAuthentication configures authentication in sarama.Config.
2219
//
2320
// The provided config is assumed to have been validated.
24-
func ConfigureSaramaAuthentication(
21+
func configureSaramaAuthentication(
2522
ctx context.Context,
2623
config configkafka.AuthenticationConfig,
2724
saramaConfig *sarama.Config,
28-
) error {
25+
) {
2926
if config.PlainText != nil {
3027
configurePlaintext(*config.PlainText, saramaConfig)
3128
}
32-
if config.TLS != nil {
33-
if err := configureTLS(ctx, *config.TLS, saramaConfig); err != nil {
34-
return err
35-
}
36-
}
3729
if config.SASL != nil {
38-
if err := configureSASL(ctx, *config.SASL, saramaConfig); err != nil {
39-
return err
40-
}
30+
configureSASL(ctx, *config.SASL, saramaConfig)
4131
}
4232
if config.Kerberos != nil {
4333
configureKerberos(*config.Kerberos, saramaConfig)
4434
}
45-
return nil
4635
}
4736

4837
func configurePlaintext(config configkafka.PlainTextConfig, saramaConfig *sarama.Config) {
@@ -51,7 +40,7 @@ func configurePlaintext(config configkafka.PlainTextConfig, saramaConfig *sarama
5140
saramaConfig.Net.SASL.Password = config.Password
5241
}
5342

54-
func configureSASL(ctx context.Context, config configkafka.SASLConfig, saramaConfig *sarama.Config) error {
43+
func configureSASL(ctx context.Context, config configkafka.SASLConfig, saramaConfig *sarama.Config) {
5544
saramaConfig.Net.SASL.Enable = true
5645
saramaConfig.Net.SASL.User = config.Username
5746
saramaConfig.Net.SASL.Password = config.Password
@@ -74,21 +63,7 @@ func configureSASL(ctx context.Context, config configkafka.SASLConfig, saramaCon
7463
case "AWS_MSK_IAM_OAUTHBEARER":
7564
saramaConfig.Net.SASL.Mechanism = sarama.SASLTypeOAuth
7665
saramaConfig.Net.SASL.TokenProvider = &awsMSKTokenProvider{ctx: ctx, region: config.AWSMSK.Region}
77-
tlsConfig := tls.Config{}
78-
saramaConfig.Net.TLS.Enable = true
79-
saramaConfig.Net.TLS.Config = &tlsConfig
80-
}
81-
return nil
82-
}
83-
84-
func configureTLS(ctx context.Context, config configtls.ClientConfig, saramaConfig *sarama.Config) error {
85-
tlsConfig, err := config.LoadTLSConfig(ctx)
86-
if err != nil {
87-
return fmt.Errorf("error loading tls config: %w", err)
8866
}
89-
saramaConfig.Net.TLS.Enable = true
90-
saramaConfig.Net.TLS.Config = tlsConfig
91-
return nil
9267
}
9368

9469
func configureKerberos(config configkafka.KerberosConfig, saramaConfig *sarama.Config) {

internal/kafka/authentication_test.go

Lines changed: 5 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,10 @@ package kafka
55

66
import (
77
"context"
8-
"crypto/tls"
98
"testing"
109

1110
"github.com/IBM/sarama"
1211
"github.com/stretchr/testify/assert"
13-
"github.com/stretchr/testify/require"
14-
"go.opentelemetry.io/collector/config/configtls"
1512

1613
"github.com/open-telemetry/opentelemetry-collector-contrib/internal/kafka/configkafka"
1714
)
@@ -47,13 +44,6 @@ func TestAuthentication(t *testing.T) {
4744
saramaSASLPLAINConfig.Net.SASL.Password = "pass"
4845
saramaSASLPLAINConfig.Net.SASL.Mechanism = sarama.SASLTypePlaintext
4946

50-
saramaTLSCfg := &sarama.Config{}
51-
saramaTLSCfg.Net.TLS.Enable = true
52-
tlsClient := configtls.ClientConfig{}
53-
tlscfg, err := tlsClient.LoadTLSConfig(context.Background())
54-
require.NoError(t, err)
55-
saramaTLSCfg.Net.TLS.Config = tlscfg
56-
5747
saramaSASLAWSIAMOAUTHConfig := &sarama.Config{}
5848
saramaSASLAWSIAMOAUTHConfig.Net.SASL.Enable = true
5949
saramaSASLAWSIAMOAUTHConfig.Net.SASL.Mechanism = sarama.SASLTypeOAuth
@@ -62,10 +52,6 @@ func TestAuthentication(t *testing.T) {
6252
region: "region",
6353
}
6454

65-
tlsConfig := tls.Config{}
66-
saramaSASLAWSIAMOAUTHConfig.Net.TLS.Enable = true
67-
saramaSASLAWSIAMOAUTHConfig.Net.TLS.Config = &tlsConfig
68-
6955
saramaKerberosCfg := &sarama.Config{}
7056
saramaKerberosCfg.Net.SASL.Mechanism = sarama.SASLTypeGSSAPI
7157
saramaKerberosCfg.Net.SASL.Enable = true
@@ -103,17 +89,6 @@ func TestAuthentication(t *testing.T) {
10389
},
10490
saramaConfig: saramaPlaintext,
10591
},
106-
{
107-
auth: configkafka.AuthenticationConfig{TLS: &configtls.ClientConfig{}},
108-
saramaConfig: saramaTLSCfg,
109-
},
110-
{
111-
auth: configkafka.AuthenticationConfig{TLS: &configtls.ClientConfig{
112-
Config: configtls.Config{CAFile: "/doesnotexists"},
113-
}},
114-
saramaConfig: saramaTLSCfg,
115-
err: "failed to load TLS config",
116-
},
11792
{
11893
auth: configkafka.AuthenticationConfig{
11994
Kerberos: &configkafka.KerberosConfig{ServiceName: "foobar"},
@@ -168,26 +143,11 @@ func TestAuthentication(t *testing.T) {
168143
for _, test := range tests {
169144
t.Run("", func(t *testing.T) {
170145
config := &sarama.Config{}
171-
err := ConfigureSaramaAuthentication(context.Background(), test.auth, config)
172-
if test.err != "" {
173-
assert.ErrorContains(t, err, test.err)
174-
} else {
175-
// equalizes SCRAMClientGeneratorFunc to do assertion with the same reference.
176-
config.Net.SASL.SCRAMClientGeneratorFunc = test.saramaConfig.Net.SASL.SCRAMClientGeneratorFunc
177-
assert.Equal(t, test.saramaConfig, config)
178-
}
179-
})
180-
}
181-
}
146+
configureSaramaAuthentication(context.Background(), test.auth, config)
182147

183-
func TestConfigureSaramaAuthentication_TLS(t *testing.T) {
184-
auth := configkafka.AuthenticationConfig{
185-
TLS: &configtls.ClientConfig{
186-
Config: configtls.Config{
187-
CAFile: "/nonexistent",
188-
},
189-
},
148+
// equalizes SCRAMClientGeneratorFunc to do assertion with the same reference.
149+
config.Net.SASL.SCRAMClientGeneratorFunc = test.saramaConfig.Net.SASL.SCRAMClientGeneratorFunc
150+
assert.Equal(t, test.saramaConfig, config)
151+
})
190152
}
191-
err := ConfigureSaramaAuthentication(context.Background(), auth, &sarama.Config{})
192-
require.ErrorContains(t, err, "failed to load TLS config")
193153
}

0 commit comments

Comments
 (0)