Skip to content
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
c054bdd
feat(configtls): add validation to fail fast on missing certificates
areebahmeddd Jun 21, 2025
b483d59
run go fmt .
areebahmeddd Jun 21, 2025
eb83a67
Merge branch 'main' into feat/configtls-validation
areebahmeddd Jun 26, 2025
47dd9a0
allow mixing cert/key file and PEM
areebahmeddd Jun 29, 2025
bf96908
Merge branch 'main' into feat/configtls-validation
areebahmeddd Jun 29, 2025
52ec582
Merge branch 'main' into feat/configtls-validation
areebahmeddd Jul 2, 2025
e68361e
Merge branch 'main' into feat/configtls-validation
areebahmeddd Jul 7, 2025
88fba0a
refactor validation method
areebahmeddd Jul 7, 2025
deae819
update yaml
areebahmeddd Jul 7, 2025
30ddfb3
Merge branch 'main' into feat/configtls-validation
areebahmeddd Jul 8, 2025
d8ea4e4
improve TLS configuration validation for server connections
areebahmeddd Jul 8, 2025
e0da3bb
some final changes
areebahmeddd Jul 8, 2025
11af954
clean yaml
areebahmeddd Jul 8, 2025
a1da43b
consistent call
areebahmeddd Jul 9, 2025
7f87014
simplify validation logic
areebahmeddd Jul 9, 2025
89117ee
update names
areebahmeddd Jul 9, 2025
913636d
missed some test case
areebahmeddd Jul 9, 2025
7a9a6cc
Merge branch 'main' into feat/configtls-validation
areebahmeddd Jul 10, 2025
7e511b3
final changes
areebahmeddd Jul 10, 2025
806a4ab
Merge branch 'main' into feat/configtls-validation
areebahmeddd Jul 10, 2025
e4ccdf3
run make gotidy
areebahmeddd Jul 10, 2025
1e0fa34
Merge branch 'main' into feat/configtls-validation
areebahmeddd Jul 11, 2025
0148292
run make gotidy && make crosslink
areebahmeddd Jul 11, 2025
b0c4603
Merge branch 'main' into feat/configtls-validation
areebahmeddd Jul 17, 2025
576a79a
Merge branch 'main' into feat/configtls-validation
areebahmeddd Jul 17, 2025
043b68f
run make gotidy
areebahmeddd Jul 17, 2025
99facf8
Merge branch 'main' into feat/configtls-validation
areebahmeddd Jul 17, 2025
c50d6a1
possible fix to failing test
areebahmeddd Jul 17, 2025
0702389
fix spell check ci
areebahmeddd Jul 18, 2025
99e541b
Merge branch 'main' into feat/configtls-validation
areebahmeddd Jul 18, 2025
e4d6864
Merge branch 'main' into feat/configtls-validation
areebahmeddd Jul 22, 2025
dd40a07
fix tests
areebahmeddd Jul 22, 2025
6931b66
remove comments
areebahmeddd Jul 22, 2025
0a3bba9
remove mismatch in serverconfig
areebahmeddd Jul 23, 2025
bbcb91d
Merge branch 'main' into feat/configtls-validation
areebahmeddd Jul 24, 2025
bea985c
Merge branch 'main' into feat/configtls-validation
areebahmeddd Aug 1, 2025
288838a
run make gotidy
areebahmeddd Aug 1, 2025
338a5f8
Merge branch 'main' into feat/configtls-validation
areebahmeddd Aug 3, 2025
84a90df
Merge branch 'main' into feat/configtls-validation
areebahmeddd Aug 4, 2025
38efd0a
Merge branch 'main' into feat/configtls-validation
areebahmeddd Aug 14, 2025
51f2e8f
Merge branch 'main' into feat/configtls-validation
areebahmeddd Aug 17, 2025
85a12b5
run make gotidy and crosslink
areebahmeddd Aug 17, 2025
3c62a62
Update config/configtls/go.mod
codeboten Aug 19, 2025
692192d
Merge branch 'main' into feat/configtls-validation
codeboten Aug 19, 2025
afdc69a
Merge branch 'main' into feat/configtls-validation
codeboten Sep 4, 2025
a56a517
Update config/configtls/go.mod
codeboten Sep 4, 2025
831931f
Merge branch 'main' into feat/configtls-validation
codeboten Sep 8, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions .chloggen/configtls-cert-validation.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
component: configtls

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Add early validation for TLS server configurations to fail fast when certificates are missing instead of failing at runtime.

# One or more tracking issues or pull requests related to the change
issues: [13130, 13245]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:

# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: [user]
27 changes: 27 additions & 0 deletions config/configtls/configtls.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,21 @@ func (c Config) Validate() error {
return errors.New("provide either a CA file or the PEM-encoded string, but not both")
}

// Ensure certificate is not set using both file and PEM
if c.hasCertFile() && c.hasCertPem() {
return errors.New("provide either certificate file or PEM, but not both")
}

// Ensure key is not set using both file and PEM
if c.hasKeyFile() && c.hasKeyPem() {
return errors.New("provide either key file or PEM, but not both")
}

// Require both certificate and key if either is provided
if c.hasCert() != c.hasKey() {
return errors.New("TLS configuration must include both certificate and key (CertFile/CertPem and KeyFile/KeyPem)")
}
Comment thread
areebahmeddd marked this conversation as resolved.

minTLS, err := convertVersion(c.MinVersion, defaultMinTLSVersion)
if err != nil {
return fmt.Errorf("invalid TLS min_version: %w", err)
Expand All @@ -211,6 +226,18 @@ func (c Config) Validate() error {
return nil
}

func (c ClientConfig) Validate() error {
return nil
}
Comment thread
areebahmeddd marked this conversation as resolved.
Outdated

func (c ServerConfig) Validate() error {
// For servers, both certificate and key are always required
if !c.hasCert() && !c.hasKey() {
return errors.New("TLS configuration must include both certificate and key for server connections")
}
Comment thread
areebahmeddd marked this conversation as resolved.
Outdated
Comment thread
areebahmeddd marked this conversation as resolved.
Outdated
return nil
Comment thread
areebahmeddd marked this conversation as resolved.
}

// loadTLSConfig loads TLS certificates and returns a tls.Config.
// This will set the RootCAs and Certificates of a tls.Config.
func (c Config) loadTLSConfig() (*tls.Config, error) {
Expand Down
202 changes: 202 additions & 0 deletions config/configtls/configtls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/stretchr/testify/require"

"go.opentelemetry.io/collector/config/configopaque"
"go.opentelemetry.io/collector/confmap/xconfmap"
Comment thread
areebahmeddd marked this conversation as resolved.
)

func TestNewDefaultConfig(t *testing.T) {
Expand Down Expand Up @@ -668,6 +669,16 @@ func TestConfigValidate(t *testing.T) {
{name: `TLS Config ["0.4", ""] to give [Error]`, tlsConfig: Config{MinVersion: "0.4", MaxVersion: ""}, errorTxt: `invalid TLS min_version: unsupported TLS version: "0.4"`},
{name: `TLS Config ["1.2", "1.1"] to give [Error]`, tlsConfig: Config{MinVersion: "1.2", MaxVersion: "1.1"}, errorTxt: `invalid TLS configuration: min_version cannot be greater than max_version`},
{name: `TLS Config with both CA File and PEM`, tlsConfig: Config{CAFile: "test", CAPem: "test"}, errorTxt: `provide either a CA file or the PEM-encoded string, but not both`},
{name: `TLS Config with cert file but no key`, tlsConfig: Config{CertFile: "cert.pem"}, errorTxt: `TLS configuration must include both certificate and key (CertFile/CertPem and KeyFile/KeyPem)`},
{name: `TLS Config with key file but no cert`, tlsConfig: Config{KeyFile: "key.pem"}, errorTxt: `TLS configuration must include both certificate and key (CertFile/CertPem and KeyFile/KeyPem)`},
{name: `TLS Config with cert PEM but no key`, tlsConfig: Config{CertPem: "cert-pem"}, errorTxt: `TLS configuration must include both certificate and key (CertFile/CertPem and KeyFile/KeyPem)`},
{name: `TLS Config with key PEM but no cert`, tlsConfig: Config{KeyPem: "key-pem"}, errorTxt: `TLS configuration must include both certificate and key (CertFile/CertPem and KeyFile/KeyPem)`},
{name: `TLS Config with both cert file and cert PEM`, tlsConfig: Config{CertFile: "cert.pem", CertPem: "cert-pem", KeyFile: "key.pem"}, errorTxt: `provide either certificate file or PEM, but not both`},
{name: `TLS Config with both key file and key PEM`, tlsConfig: Config{CertFile: "cert.pem", KeyFile: "key.pem", KeyPem: "key-pem"}, errorTxt: `provide either key file or PEM, but not both`},
{name: `TLS Config with cert file and key PEM`, tlsConfig: Config{CertFile: "cert.pem", KeyPem: "key-pem"}},
{name: `TLS Config with cert PEM and key file`, tlsConfig: Config{CertPem: "cert-pem", KeyFile: "key.pem"}},
{name: `TLS Config with valid cert and key files`, tlsConfig: Config{CertFile: "cert.pem", KeyFile: "key.pem"}},
{name: `TLS Config with valid cert and key PEM`, tlsConfig: Config{CertPem: "cert-pem", KeyPem: "key-pem"}},
}

for _, test := range tests {
Expand Down Expand Up @@ -923,3 +934,194 @@ func TestCurvePreferences(t *testing.T) {
}
}
}

func TestServerConfigValidate(t *testing.T) {
tests := []struct {
name string
serverConfig ServerConfig
errorTxt string
}{
{
name: "server config without certificates",
serverConfig: ServerConfig{},
errorTxt: "TLS configuration must include both certificate and key for server connections",
},
Comment thread
areebahmeddd marked this conversation as resolved.
{
name: "valid server config with cert and key files",
serverConfig: ServerConfig{
Config: Config{
CertFile: "cert.pem",
KeyFile: "key.pem",
},
},
},
{
name: "valid server config with cert and key PEM",
serverConfig: ServerConfig{
Config: Config{
CertPem: "cert-pem",
KeyPem: "key-pem",
},
},
},
{
name: "valid server config with mixed cert file and key PEM",
serverConfig: ServerConfig{
Config: Config{
CertFile: "cert.pem",
KeyPem: "key-pem",
},
},
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
err := test.serverConfig.Validate()

if test.errorTxt == "" {
assert.NoError(t, err)
} else {
assert.EqualError(t, err, test.errorTxt)
}
})
}
}

func TestServerConfigValidateWithConfmap(t *testing.T) {
Comment thread
areebahmeddd marked this conversation as resolved.
Outdated
tests := []struct {
name string
serverConfig ServerConfig
errorTxt string
}{
{
name: "server config with cert file but no key via confmap validation",
serverConfig: ServerConfig{
Config: Config{
CertFile: "cert.pem",
},
},
errorTxt: "config: TLS configuration must include both certificate and key (CertFile/CertPem and KeyFile/KeyPem)",
},
{
name: "server config with key file but no cert via confmap validation",
serverConfig: ServerConfig{
Config: Config{
KeyFile: "key.pem",
},
},
errorTxt: "config: TLS configuration must include both certificate and key (CertFile/CertPem and KeyFile/KeyPem)",
},
Comment thread
areebahmeddd marked this conversation as resolved.
{
name: "server config with both cert file and cert PEM via confmap validation",
serverConfig: ServerConfig{
Config: Config{
CertFile: "cert.pem",
CertPem: "cert-pem",
KeyFile: "key.pem",
},
},
errorTxt: "config: provide either certificate file or PEM, but not both",
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
err := xconfmap.Validate(test.serverConfig)

if test.errorTxt == "" {
assert.NoError(t, err)
} else {
assert.EqualError(t, err, test.errorTxt)
}
})
}
}

func TestClientConfigValidate(t *testing.T) {
tests := []struct {
name string
clientConfig ClientConfig
errorTxt string
}{
{
name: "valid empty client config",
clientConfig: ClientConfig{},
},
{
name: "valid client config with insecure connection",
clientConfig: ClientConfig{
Insecure: true,
},
},
{
name: "valid client config with cert and key files",
clientConfig: ClientConfig{
Config: Config{
CertFile: "cert.pem",
KeyFile: "key.pem",
},
},
},
{
name: "valid client config with mixed cert file and key PEM",
clientConfig: ClientConfig{
Config: Config{
CertFile: "cert.pem",
KeyPem: "key-pem",
},
},
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
err := test.clientConfig.Validate()

if test.errorTxt == "" {
assert.NoError(t, err)
} else {
assert.EqualError(t, err, test.errorTxt)
}
})
}
}

func TestClientConfigValidateWithConfmap(t *testing.T) {
tests := []struct {
name string
clientConfig ClientConfig
errorTxt string
}{
{
name: "client config with cert file but no key via confmap validation",
clientConfig: ClientConfig{
Config: Config{
CertFile: "cert.pem",
},
},
errorTxt: "config: TLS configuration must include both certificate and key (CertFile/CertPem and KeyFile/KeyPem)",
},
{
name: "client config with invalid TLS version via confmap validation",
clientConfig: ClientConfig{
Config: Config{
MinVersion: "invalid",
},
},
errorTxt: `config: invalid TLS min_version: unsupported TLS version: "invalid"`,
},
Comment thread
areebahmeddd marked this conversation as resolved.
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
err := xconfmap.Validate(test.clientConfig)

if test.errorTxt == "" {
assert.NoError(t, err)
} else {
assert.EqualError(t, err, test.errorTxt)
}
})
}
}
Loading