Skip to content

Commit

Permalink
[exporter/syslog]: fix setting network connection (#31202)
Browse files Browse the repository at this point in the history
**Description:** fix setting network connection, do not load TLS
configuration for UDP

**Link to tracking Issue:** #31130 

**Testing:** unit test, manual tests with syslog server

**Documentation:** added information that TLS config is applied only
when TCP is used

Signed-off-by: Katarzyna Kujawa <[email protected]>
  • Loading branch information
kasia-kujawa authored Feb 27, 2024
1 parent 357e717 commit f825274
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 9 deletions.
27 changes: 27 additions & 0 deletions .chloggen/fix-31130.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Use this changelog template to create an entry for release notes.

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

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

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: fix setting network connection, do not load TLS configuration for UDP

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [31130]

# (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:

# If your change doesn't affect end users or the exported elements of any package,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
# 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: []
2 changes: 1 addition & 1 deletion exporter/syslogexporter/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ This means that syslog messages received via the Syslog receiver and exported vi
- `rfc5424` - Expects the syslog messages to be rfc5424 compliant
- `rfc3164` - Expects the syslog messages to be rfc3164 compliant
- `enable_octet_counting` (default = `false`) - Whether or not to enable rfc6587 octet counting
- `tls` - configuration for TLS/mTLS
- `tls` - configuration for TLS/mTLS (applied only when `network` is set to `tcp`)
- `insecure` (default = `false`) whether to enable client transport security, by default, TLS is enabled.
- `cert_file` - Path to the TLS cert to use for TLS required connections. Should only be used if `insecure` is set to `false`.
- `key_file` - Path to the TLS key to use for TLS required connections. Should only be used if `insecure` is set to `false`.
Expand Down
16 changes: 10 additions & 6 deletions exporter/syslogexporter/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,21 @@ type syslogexporter struct {
}

func initExporter(cfg *Config, createSettings exporter.CreateSettings) (*syslogexporter, error) {
tlsConfig, err := cfg.TLSSetting.LoadTLSConfig()
if err != nil {
return nil, err
}

cfg.Network = strings.ToLower(cfg.Network)

var loadedTLSConfig *tls.Config
if cfg.Network == "tcp" {
var err error
loadedTLSConfig, err = cfg.TLSSetting.LoadTLSConfig()
if err != nil {
return nil, err
}
}

s := &syslogexporter{
config: cfg,
logger: createSettings.Logger,
tlsConfig: tlsConfig,
tlsConfig: loadedTLSConfig,
formatter: createFormatter(cfg.Protocol, cfg.EnableOctetCounting),
}

Expand Down
54 changes: 54 additions & 0 deletions exporter/syslogexporter/exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package syslogexporter

import (
"context"
"crypto/tls"
"errors"
"io"
"net"
Expand All @@ -15,6 +16,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/configtls"
"go.opentelemetry.io/collector/consumer/consumererror"
"go.opentelemetry.io/collector/exporter"
"go.opentelemetry.io/collector/pdata/pcommon"
Expand Down Expand Up @@ -183,3 +185,55 @@ func TestSyslogExportFail(t *testing.T) {
assert.ErrorContains(t, consumerErr, "dial tcp 127.0.0.1:112: connect")
assert.Equal(t, droppedLog, originalForm)
}

func TestTLSConfig(t *testing.T) {

tests := []struct {
name string
network string
tlsSettings configtls.TLSClientSetting
tlsConfig *tls.Config
}{
{name: "TCP with TLS configuration",
network: "tcp",
tlsSettings: configtls.TLSClientSetting{},
tlsConfig: &tls.Config{},
},
{name: "TCP insecure",
network: "tcp",
tlsSettings: configtls.TLSClientSetting{Insecure: true},
tlsConfig: nil,
},
{name: "UDP with TLS configuration",
network: "udp",
tlsSettings: configtls.TLSClientSetting{},
tlsConfig: nil,
},
{name: "UDP insecure",
network: "udp",
tlsSettings: configtls.TLSClientSetting{Insecure: true},
tlsConfig: nil,
},
}

for _, testInstance := range tests {
t.Run(testInstance.name, func(t *testing.T) {

exporter, err := initExporter(
&Config{Endpoint: "test.com",
Network: testInstance.network,
Port: 514,
Protocol: "rfc5424",
TLSSetting: testInstance.tlsSettings},
createExporterCreateSettings())

assert.NoError(t, err)
if testInstance.tlsConfig != nil {
assert.NotNil(t, exporter.tlsConfig)
} else {
assert.Nil(t, exporter.tlsConfig)
}

})
}
}
4 changes: 2 additions & 2 deletions exporter/syslogexporter/sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ func (s *sender) dial() error {
s.conn = nil
}
var err error
if s.tlsConfig != nil {
s.conn, err = tls.Dial("tcp", s.addr, s.tlsConfig)
if s.tlsConfig != nil && s.network == "tcp" {
s.conn, err = tls.Dial(s.network, s.addr, s.tlsConfig)
} else {
s.conn, err = net.Dial(s.network, s.addr)
}
Expand Down

0 comments on commit f825274

Please sign in to comment.