Make whitespace trimming in log messages optional (#506)

The Log Cache Syslog server trims leading and trailing whitespace in the
messages. The log messages should be not changed.

This change:
- adds config parameter to define if the whitespace in the log messages
should be trimmed or not
- trims or leaves the log message untouched based on the config
parameter

The default behavior is to trim the log messages, so that it is
compatible with the behavior up until now.

Author: chombium

jobs/log-cache-syslog-server/spec

@@ -35,6 +35,9 @@ properties:
   syslog_idle_timeout:
     description: "Timeout for the Syslog Server connection"
     default: "2m"
+  syslog_trim_message_whitespace:
+    description: "Defines if the leading and trailing whitespace in the Syslog log messages should be trimmed"
+    default: true
 
   syslog_client_ca_cert:
     description: The CA certificate for key/cert verification.

jobs/log-cache-syslog-server/templates/bpm.yml.erb

@@ -16,6 +16,7 @@ processes:
   env:
     SYSLOG_PORT: "<%= p('syslog_port') %>"
     SYSLOG_IDLE_TIMEOUT: "<%= p('syslog_idle_timeout') %>"
+    SYSLOG_TRIM_MESSAGE_WHITESPACE: "<%= p('syslog_trim_message_whitespace') %>"
 
     SYSLOG_TLS_CERT_PATH: "<%= "#{certDir}/syslog.crt" %>"
     SYSLOG_TLS_KEY_PATH: "<%= "#{certDir}/syslog.key" %>"

src/cmd/syslog-server/config.go

@@ -18,8 +18,9 @@ type Config struct {
 	SyslogTLSCertPath string `env:"SYSLOG_TLS_CERT_PATH, report"`
 	SyslogTLSKeyPath  string `env:"SYSLOG_TLS_KEY_PATH, report"`
 
-	SyslogIdleTimeout      time.Duration `env:"SYSLOG_IDLE_TIMEOUT, report"`
-	SyslogMaxMessageLength int           `env:"SYSLOG_MAX_MESSAGE_LENGTH, report"`
+	SyslogIdleTimeout           time.Duration `env:"SYSLOG_IDLE_TIMEOUT, report"`
+	SyslogMaxMessageLength      int           `env:"SYSLOG_MAX_MESSAGE_LENGTH, report"`
+	SyslogTrimMessageWhitespace bool          `env:"SYSLOG_TRIM_MESSAGE_WHITESPACE, report"`
 
 	SyslogClientTrustedCAFile string `env:"SYSLOG_CLIENT_TRUSTED_CA_FILE,  report"`
 
@@ -36,7 +37,8 @@ func LoadConfig() (*Config, error) {
 		MetricsServer: config.MetricsServer{
 			Port: 6061,
 		},
-		SyslogMaxMessageLength: 65 * 1024, // Diego should never send logs bigger than 64Kib
+		SyslogMaxMessageLength:      65 * 1024, // Diego should never send logs bigger than 64Kib
+		SyslogTrimMessageWhitespace: true,
 	}
 
 	if err := envstruct.Load(&c); err != nil {

src/cmd/syslog-server/main.go

@@ -75,6 +75,7 @@ func main() {
 		syslog.WithServerPort(cfg.SyslogPort),
 		syslog.WithIdleTimeout(cfg.SyslogIdleTimeout),
 		syslog.WithServerMaxMessageLength(cfg.SyslogMaxMessageLength),
+		syslog.WithServerTrimMessageWhitespace(cfg.SyslogTrimMessageWhitespace),
 	}
 	if cfg.SyslogTLSCertPath != "" || cfg.SyslogTLSKeyPath != "" {
 		serverOptions = append(serverOptions, syslog.WithServerTLS(cfg.SyslogTLSCertPath, cfg.SyslogTLSKeyPath))

src/internal/syslog/server.go

@@ -25,14 +25,15 @@ import (
 
 type Server struct {
 	sync.Mutex
-	port             int
-	l                net.Listener
-	envelopes        chan *loggregator_v2.Envelope
-	syslogClientCA   string
-	syslogCert       string
-	syslogKey        string
-	idleTimeout      time.Duration
-	maxMessageLength int
+	port                  int
+	l                     net.Listener
+	envelopes             chan *loggregator_v2.Envelope
+	syslogClientCA        string
+	syslogCert            string
+	syslogKey             string
+	idleTimeout           time.Duration
+	maxMessageLength      int
+	trimMessageWhitespace bool
 
 	ingress        metrics.Counter
 	invalidIngress metrics.Counter
@@ -52,10 +53,11 @@ func NewServer(
 	opts ...ServerOption,
 ) *Server {
 	s := &Server{
-		loggr:            loggr,
-		envelopes:        make(chan *loggregator_v2.Envelope, 100),
-		idleTimeout:      2 * time.Minute,
-		maxMessageLength: 65 * 1024, // Diego should never send logs bigger than 64Kib
+		loggr:                 loggr,
+		envelopes:             make(chan *loggregator_v2.Envelope, 100),
+		idleTimeout:           2 * time.Minute,
+		maxMessageLength:      65 * 1024, // Diego should never send logs bigger than 64Kib
+		trimMessageWhitespace: true,
 	}
 
 	for _, o := range opts {
@@ -86,6 +88,12 @@ func WithServerMaxMessageLength(l int) ServerOption {
 	}
 }
 
+func WithServerTrimMessageWhitespace(t bool) ServerOption {
+	return func(s *Server) {
+		s.trimMessageWhitespace = t
+	}
+}
+
 func WithServerTLS(cert, key string) ServerOption {
 	return func(s *Server) {
 		s.syslogCert = cert
@@ -238,9 +246,15 @@ func (s *Server) convertToEnvelope(msg *rfc5424.SyslogMessage) (*loggregator_v2.
 }
 
 func (s *Server) convertMessage(env *loggregator_v2.Envelope, msg *rfc5424.SyslogMessage) *loggregator_v2.Envelope {
+	var payload string
+	if s.trimMessageWhitespace {
+		payload = strings.TrimSpace(*msg.Message)
+	} else {
+		payload = strings.TrimSuffix(*msg.Message, "\n")
+	}
 	env.Message = &loggregator_v2.Envelope_Log{
 		Log: &loggregator_v2.Log{
-			Payload: []byte(strings.TrimSpace(*msg.Message)),
+			Payload: []byte(payload),
 			Type:    s.typeFromPriority(int(*msg.Priority)),
 		},
 	}

src/internal/syslog/server_test.go

@@ -31,6 +31,7 @@ func newTlsServerTestSetup(opts ...syslog.ServerOption) (*syslog.Server, *testhe
 		syslog.WithSyslogClientCA(testing.LogCacheTestCerts.CA()),
 		syslog.WithServerPort(0),
 		syslog.WithIdleTimeout(100 * time.Millisecond),
+		syslog.WithServerTrimMessageWhitespace(true),
 	}
 	options = append(options, opts...)
 
@@ -151,6 +152,55 @@ var _ = Describe("Syslog", func() {
 		))
 	})
 
+	It("trims whitespace in log messages when configured to do so", func() {
+		tests := []struct {
+			trimWhitespace bool
+			expectedMsg    string
+		}{
+			{expectedMsg: "just a test with with whitespace"},
+			{trimWhitespace: true, expectedMsg: "just a test with with whitespace"},
+			{trimWhitespace: false, expectedMsg: "    just a test with with whitespace    "},
+		}
+
+		for i, tc := range tests {
+			var server *syslog.Server
+			if i == 0 {
+				server, _, _ = newTlsServerTestSetup()
+			} else {
+				server, _, _ = newTlsServerTestSetup(syslog.WithServerTrimMessageWhitespace(tc.trimWhitespace))
+			}
+			defer server.Stop()
+
+			tlsConfig := buildClientTLSConfig(tls.VersionTLS12, tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384)
+			conn, err := tlsClientConnection(server.Addr(), tlsConfig)
+			Expect(err).ToNot(HaveOccurred())
+
+			const messageWithWhitespace = `174 <14>1 1970-01-01T00:00:00.012345+00:00 test-hostname test-app-id [APP/2] - [tags@47450 key="value" source_type="actual-source-type"]     just a test with with whitespace    ` + "\n"
+			_, err = fmt.Fprint(conn, messageWithWhitespace)
+			Expect(err).ToNot(HaveOccurred())
+
+			br := loggregator_v2.EgressBatchRequest{}
+			ctx := context.Background()
+			Expect(server.Stream(ctx, &br)()).Should(ContainElement(
+				&loggregator_v2.Envelope{
+					Tags: map[string]string{
+						"source_type": "actual-source-type",
+						"key":         "value",
+					},
+					InstanceId: "2",
+					Timestamp:  12345000,
+					SourceId:   "test-app-id",
+					Message: &loggregator_v2.Envelope_Log{
+						Log: &loggregator_v2.Log{
+							Payload: []byte(tc.expectedMsg),
+							Type:    loggregator_v2.Log_OUT,
+						},
+					},
+				},
+			))
+		}
+	})
+
 	It("max syslog length is configurable", func() {
 		server, spyMetrics, _ := newTlsServerTestSetup(syslog.WithServerMaxMessageLength(128))
 		defer server.Stop()