Added tcp+tls support for syslog server #2
Conversation
| if err != nil { | ||
| fmt.Errorf("Failed reading the tls certificates %w", err) | ||
| } | ||
| syslogW = &syslogWriter{framer: defaultFramer, formatter: defaultFormatter, tlsConfig: syslogtlsConfig} |
There was a problem hiding this comment.
Construction of syslogWriter can be done via method chaining so that line 106 and 110 can be common. wdyt?
syslogW = syslogWriter.New().WithFramer(framer).WithFormatter(formatter).WithTlsConfig(tlsconfig)
| } | ||
|
|
||
| func readTlsConfig(certFile, keyFile, CAFile, serverName string, insecureSkipVerify bool) (*tls.Config, error) { |
There was a problem hiding this comment.
this function can be a method of sysCfg. All function args will be part of object and no need to pass them explicitly.
| var certs []tls.Certificate | ||
| if certFile != "" { | ||
| cert, err := tls.LoadX509KeyPair(certFile, keyFile) |
There was a problem hiding this comment.
Add a TODO comment for auto-reload of client certs
| InsecureSkipVerify: insecureSkipVerify, | ||
| RootCAs: rootCAs, | ||
| ServerName: serverName, |
There was a problem hiding this comment.
Can this serverName be made an optional field?
| @@ -29,9 +31,28 @@ func (w *syslogWriter) basicDialer() (serverConn, error) { | |||
| return sc, err | |||
| } | |||
|
|
|||
| func (w *syslogWriter) tlsDialer() (serverConn, error) { | |||
| c, err := tls.Dial("tcp", fmt.Sprintf("%s:%d", w.sysCfg.SyslogConfig.RemoteHost, w.sysCfg.SyslogConfig.Port), w.tlsConfig) | |||
There was a problem hiding this comment.
basicDialer uses net.Dial(w.sysCfg.SyslogConfig.Protocol. And w.sysCfg.SyslogConfig.Protocol can be any protocol with TLS as well. Better if we merge basicDialer and tlsDialer into one with respective handling
| // connect updates the syslogWriter with a new serverConn | ||
| func (w *syslogWriter) connect() (serverConn, error) { | ||
| conn, err := w.basicDialer() | ||
| var ( | ||
| conn serverConn |
There was a problem hiding this comment.
var conn serverConn
var err error
| conn serverConn | ||
| err error | ||
| ) | ||
| if w.sysCfg.SyslogConfig.Protocol == "tcp+tls" { |
There was a problem hiding this comment.
switch with the default case is better here. If going forward if cases like "TCP", "UDP", "TCP+TLS" are to be supported
Describe Your Changes
Added support to send logs to the syslog server over tcp+tls.
Checklist
The following checks are mandatory: