multiple syslog server support

Author: aphralG

internal/collector/otel_collector_plugin.go

@@ -543,17 +543,12 @@ func (oc *Collector) updateExistingNginxOSSReceiver(
 
 func (oc *Collector) updateTcplogReceivers(nginxConfigContext *model.NginxConfigContext) bool {
 	newTcplogReceiverAdded := false
-	if nginxConfigContext.NAPSysLogServers != nil {
-	napLoop:
-		for _, napSysLogServer := range nginxConfigContext.NAPSysLogServers {
-			if oc.doesTcplogReceiverAlreadyExist(napSysLogServer) {
-				continue napLoop
-			}
-
+	if nginxConfigContext.NAPSysLogServer != "" {
+		if !oc.doesTcplogReceiverAlreadyExist(nginxConfigContext.NAPSysLogServer) {
 			oc.config.Collector.Receivers.TcplogReceivers = append(
 				oc.config.Collector.Receivers.TcplogReceivers,
 				config.TcplogReceiver{
-					ListenAddress: napSysLogServer,
+					ListenAddress: nginxConfigContext.NAPSysLogServer,
 					Operators: []config.Operator{
 						{
 							Type: "add",
@@ -621,12 +616,10 @@ func (oc *Collector) configDeletedNapReceivers(nginxConfigContext *model.NginxCo
 		elements[tcplogReceiver.ListenAddress] = true
 	}
 
-	if nginxConfigContext.NAPSysLogServers != nil {
+	if nginxConfigContext.NAPSysLogServer != "" {
 		addressesToDelete := make(map[string]bool)
-		for _, napAddress := range nginxConfigContext.NAPSysLogServers {
-			if !elements[napAddress] {
-				addressesToDelete[napAddress] = true
-			}
+		if !elements[nginxConfigContext.NAPSysLogServer] {
+			addressesToDelete[nginxConfigContext.NAPSysLogServer] = true
 		}
 
 		return addressesToDelete

internal/collector/otel_collector_plugin_test.go

@@ -734,9 +734,7 @@ func TestCollector_updateTcplogReceivers(t *testing.T) {
 	require.NoError(t, err)
 
 	nginxConfigContext := &model.NginxConfigContext{
-		NAPSysLogServers: []string{
-			"localhost:151",
-		},
+		NAPSysLogServer: "localhost:151",
 	}
 
 	assert.Empty(t, conf.Collector.Receivers.TcplogReceivers)
@@ -767,9 +765,8 @@ func TestCollector_updateTcplogReceivers(t *testing.T) {
 	})
 
 	t.Run("Test 4: New tcplogReceiver added and deleted another", func(tt *testing.T) {
-		tcplogReceiverDeleted := collector.updateTcplogReceivers(&model.NginxConfigContext{NAPSysLogServers: []string{
-			"localhost:152",
-		}})
+		tcplogReceiverDeleted := collector.
+			updateTcplogReceivers(&model.NginxConfigContext{NAPSysLogServer: "localhost:152"})
 		assert.True(t, tcplogReceiverDeleted)
 		assert.Len(t, conf.Collector.Receivers.TcplogReceivers, 1)
 		assert.Equal(t, "localhost:152", conf.Collector.Receivers.TcplogReceivers[0].ListenAddress)

internal/datasource/config/nginx_config_parser.go

@@ -173,18 +173,16 @@ func (ncp *NginxConfigParser) createNginxConfigContext(
 					}
 				case "app_protect_security_log":
 					if len(directive.Args) > 1 {
-						syslogArg := directive.Args[1]
-						re := regexp.MustCompile(`syslog:server=([\S]+)`)
-						matches := re.FindStringSubmatch(syslogArg)
-						if len(matches) > 1 {
-							syslogServer := matches[1]
-							if !napSyslogServersFound[syslogServer] {
-								nginxConfigContext.NAPSysLogServers = append(
-									nginxConfigContext.NAPSysLogServers,
-									syslogServer,
-								)
-								napSyslogServersFound[syslogServer] = true
-								slog.DebugContext(ctx, "Found NAP syslog server", "address", syslogServer)
+						slog.Info("args", "", directive.Args)
+						sysLogServers := ncp.findValidSysLogServers(directive.Args)
+						if len(sysLogServers) == 0 {
+							slog.WarnContext(ctx, "Could not find valid Nap Syslog server")
+						}
+						for i := range sysLogServers {
+							sysLogServer := sysLogServers[i]
+							if !napSyslogServersFound[sysLogServer] {
+								napSyslogServersFound[sysLogServer] = true
+								slog.DebugContext(ctx, "Found NAP syslog server", "address", sysLogServer)
 							}
 						}
 					}
@@ -207,6 +205,13 @@ func (ncp *NginxConfigParser) createNginxConfigContext(
 			nginxConfigContext.PlusAPI = plusAPI
 		}
 
+		if len(napSyslogServersFound) > 0 {
+			syslogServer := ncp.parseSyslogDirective(ctx, napSyslogServersFound)
+			if syslogServer != "" {
+				nginxConfigContext.NAPSysLogServer = syslogServer
+			}
+		}
+
 		fileMeta, err := files.FileMeta(conf.File)
 		if err != nil {
 			slog.WarnContext(ctx, "Unable to get file metadata", "file_name", conf.File, "error", err)
@@ -218,6 +223,40 @@ func (ncp *NginxConfigParser) createNginxConfigContext(
 	return nginxConfigContext, nil
 }
 
+func (ncp *NginxConfigParser) parseSyslogDirective(ctx context.Context, napSyslogServers map[string]bool) string {
+	for napSyslogServer := range napSyslogServers {
+		ln, err := net.Listen("tcp", napSyslogServer)
+		if err != nil {
+			slog.WarnContext(ctx, "NAP syslog server is not reachable", "address", napSyslogServer,
+				"error", err)
+
+			continue
+		}
+		ln.Close()
+		slog.DebugContext(ctx, "Found valid NAP syslog server", "address", napSyslogServer)
+
+		return napSyslogServer
+	}
+	slog.WarnContext(ctx, "Could not find usable NAP syslog server")
+
+	return ""
+}
+
+func (ncp *NginxConfigParser) findValidSysLogServers(sysLogServers []string) []string {
+	re := regexp.MustCompile(`syslog:server=([\S]+)`)
+	var servers []string
+	for i := range sysLogServers {
+		matches := re.FindStringSubmatch(sysLogServers[i])
+		if len(matches) > 1 {
+			if strings.HasPrefix(matches[1], "localhost") || strings.HasPrefix(matches[1], "127.0.0.1") {
+				servers = append(servers, matches[1])
+			}
+		}
+	}
+
+	return servers
+}
+
 func (ncp *NginxConfigParser) parseIncludeDirective(directive *crossplane.Directive) string {
 	var include string
 	if filepath.IsAbs(directive.Args[0]) {

internal/datasource/config/nginx_config_parser_test.go

@@ -9,6 +9,7 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -357,7 +358,7 @@ func TestNginxConfigParser_Parse(t *testing.T) {
 				ltsvAccessLog.Name(),
 				errorLog.Name(),
 				protos.NginxOssInstance([]string{}).GetInstanceMeta().GetInstanceId(),
-				[]string{"127.0.0.1:1515"},
+				"127.0.0.1:1515",
 			),
 			expectedLog:        "",
 			allowedDirectories: []string{dir},
@@ -377,7 +378,7 @@ func TestNginxConfigParser_Parse(t *testing.T) {
 				ltsvAccessLog.Name(),
 				errorLog.Name(),
 				protos.NginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(),
-				[]string{"127.0.0.1:1515"},
+				"127.0.0.1:1515",
 			),
 			expectedLog:        "",
 			allowedDirectories: []string{dir},
@@ -392,7 +393,7 @@ func TestNginxConfigParser_Parse(t *testing.T) {
 				errorLog.Name(),
 				[]*mpi.File{&allowedFileWithMetas},
 				protos.NginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(),
-				nil,
+				"",
 			),
 			expectedLog:        "",
 			allowedDirectories: []string{dir},
@@ -402,13 +403,13 @@ func TestNginxConfigParser_Parse(t *testing.T) {
 			instance: protos.NginxPlusInstance([]string{}),
 			content:  testconfig.NginxConfWithSSLCertsWithVariables(),
 			expectedConfigContext: &model.NginxConfigContext{
-				StubStatus:       &model.APIDetails{},
-				PlusAPI:          &model.APIDetails{},
-				InstanceID:       protos.NginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(),
-				Files:            []*mpi.File{},
-				AccessLogs:       []*model.AccessLog{},
-				ErrorLogs:        []*model.ErrorLog{},
-				NAPSysLogServers: nil,
+				StubStatus:      &model.APIDetails{},
+				PlusAPI:         &model.APIDetails{},
+				InstanceID:      protos.NginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(),
+				Files:           []*mpi.File{},
+				AccessLogs:      []*model.AccessLog{},
+				ErrorLogs:       []*model.ErrorLog{},
+				NAPSysLogServer: "",
 			},
 			allowedDirectories: []string{dir},
 		},
@@ -426,7 +427,7 @@ func TestNginxConfigParser_Parse(t *testing.T) {
 				combinedAccessLog.Name(),
 				ltsvAccessLog.Name(),
 				protos.NginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(),
-				[]string{"127.0.0.1:1515"},
+				"127.0.0.1:1515",
 			),
 			expectedLog: "Currently error log outputs to stderr. Log monitoring is disabled while applying a " +
 				"config; log errors to file to enable error monitoring",
@@ -446,7 +447,7 @@ func TestNginxConfigParser_Parse(t *testing.T) {
 				combinedAccessLog.Name(),
 				ltsvAccessLog.Name(),
 				protos.NginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(),
-				[]string{"127.0.0.1:1515"},
+				"127.0.0.1:1515",
 			),
 			expectedLog: "Currently error log outputs to stdout. Log monitoring is disabled while applying a " +
 				"config; log errors to file to enable error monitoring",
@@ -465,7 +466,7 @@ func TestNginxConfigParser_Parse(t *testing.T) {
 				errorLog.Name(),
 				[]*mpi.File{&certFileWithMetas},
 				protos.NginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(),
-				nil,
+				"",
 			),
 			allowedDirectories: []string{dir},
 		},
@@ -483,7 +484,7 @@ func TestNginxConfigParser_Parse(t *testing.T) {
 				errorLog.Name(),
 				[]*mpi.File{&diffCertFileWithMetas, &certFileWithMetas},
 				protos.NginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(),
-				nil,
+				"",
 			),
 			allowedDirectories: []string{dir},
 		},
@@ -501,7 +502,7 @@ func TestNginxConfigParser_Parse(t *testing.T) {
 				errorLog.Name(),
 				[]*mpi.File{&certFileWithMetas},
 				protos.NginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(),
-				nil,
+				"",
 			),
 			allowedDirectories: []string{dir},
 		},
@@ -532,6 +533,7 @@ func TestNginxConfigParser_Parse(t *testing.T) {
 			result, parseError := nginxConfig.Parse(ctx, test.instance)
 			require.NoError(t, parseError)
 
+			t.Logf("Log: %s", logBuf.String())
 			helpers.ValidateLog(t, test.expectedLog, logBuf)
 			logBuf.Reset()
 
@@ -548,7 +550,7 @@ func TestNginxConfigParser_Parse(t *testing.T) {
 			assert.Truef(t,
 				protoListEqual(test.expectedConfigContext.Files, result.Files),
 				"Expect %s Got %s", test.expectedConfigContext.Files, result.Files)
-			assert.Equal(t, test.expectedConfigContext.NAPSysLogServers, result.NAPSysLogServers)
+			assert.Equal(t, test.expectedConfigContext.NAPSysLogServer, result.NAPSysLogServer)
 			assert.Equal(t, test.expectedConfigContext.PlusAPI, result.PlusAPI)
 			assert.ElementsMatch(t, test.expectedConfigContext.AccessLogs, result.AccessLogs)
 			assert.ElementsMatch(t, test.expectedConfigContext.ErrorLogs, result.ErrorLogs)
@@ -582,6 +584,107 @@ func TestNginxConfigParser_sslCert(t *testing.T) {
 	assert.Equal(t, certFile, sslCert.GetFileMeta().GetName())
 }
 
+func TestNginxConfigParser_SyslogServerParse(t *testing.T) {
+	ctx := context.Background()
+	dir := t.TempDir()
+
+	file := helpers.CreateFileWithErrorCheck(t, dir, "nginx-parse-config.conf")
+	defer helpers.RemoveFileWithErrorCheck(t, file.Name())
+
+	errorLog := helpers.CreateFileWithErrorCheck(t, dir, "error.log")
+	defer helpers.RemoveFileWithErrorCheck(t, errorLog.Name())
+
+	accessLog := helpers.CreateFileWithErrorCheck(t, dir, "access.log")
+	defer helpers.RemoveFileWithErrorCheck(t, accessLog.Name())
+
+	instance := protos.NginxOssInstance([]string{})
+	instance.InstanceRuntime.ConfigPath = file.Name()
+
+	tests := []struct {
+		name                  string
+		content               string
+		expectedLog           string
+		expectedSyslogServers string
+		portInUse             bool
+	}{
+		{
+			name:                  "Test 1: Valid port",
+			expectedSyslogServers: "127.0.0.1:1515",
+			content: testconfig.NginxConfigWithMultipleSysLogs(errorLog.Name(), accessLog.Name(),
+				"192.168.12.34:1517", "my.domain.com:1517", "127.0.0.1:1515"),
+			expectedLog: "Found valid NAP syslog server",
+			portInUse:   false,
+		},
+		{
+			name:                  "Test 2: No valid server",
+			expectedSyslogServers: "",
+			content: testconfig.NginxConfigWithMultipleSysLogs(errorLog.Name(), accessLog.Name(),
+				"random.domain:1515", "192.168.12.34:1517", "my.domain.com:1517"),
+			expectedLog: "Could not find valid Nap Syslog server",
+			portInUse:   false,
+		},
+		{
+			name:                  "Test 3: Port unavailable, use next valid sever",
+			expectedSyslogServers: "localhost:1516",
+			content: testconfig.NginxConfigWithMultipleSysLogs(errorLog.Name(), accessLog.Name(),
+				"192.168.12.34:1517", "127.0.0.1:1515", "localhost:1516"),
+			expectedLog: "NAP syslog server is not reachable",
+			portInUse:   true,
+		},
+		{
+			name:                  "Test 4: Port unavailable, no server available",
+			expectedSyslogServers: "",
+			content: testconfig.NginxConfigWithMultipleSysLogs(errorLog.Name(), accessLog.Name(),
+				"my.domain.com:1517", "127.0.0.1:1515", "my.domain.com:1517"),
+			expectedLog: "Could not find usable NAP syslog server",
+			portInUse:   true,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			logBuf := &bytes.Buffer{}
+			stub.StubLoggerWith(logBuf)
+
+			agentConfig := types.AgentConfig()
+			agentConfig.AllowedDirectories = []string{dir}
+			nginxConfig := NewNginxConfigParser(agentConfig)
+
+			writeErr := os.WriteFile(file.Name(), []byte(test.content), 0o600)
+			require.NoError(t, writeErr)
+
+			if test.portInUse {
+				ln, err := net.Listen("tcp", "127.0.0.1:1515")
+				require.NoError(t, err)
+				defer ln.Close()
+			}
+
+			result, parseError := nginxConfig.Parse(ctx, instance)
+			require.NoError(t, parseError)
+
+			t.Logf("Log: %s", logBuf.String())
+			helpers.ValidateLog(t, test.expectedLog, logBuf)
+			logBuf.Reset()
+
+			assert.Equal(t, test.expectedSyslogServers, result.NAPSysLogServer)
+		})
+	}
+}
+
+func TestNginxConfigParser_findValidSysLogServers(t *testing.T) {
+	servers := []string{
+		"/etc/app_protect/conf/log_default.json", "syslog:server=192.168.12.34:1517",
+		"app_protect_security_log", "/etc/app_protect/conf/log_default1.json", "syslog:server=my.domain.com:1517",
+		"app_protect_security_log", "/etc/app_protect/conf/log_default2.json", "syslog:server=127.0.0.1:1515",
+		"app_protect_security_log", "/etc/app_protect/conf/log_default3.json", "syslog:server=localhost:1516",
+	}
+	expected := []string{"127.0.0.1:1515", "localhost:1516"}
+	ncp := NewNginxConfigParser(types.AgentConfig())
+	result := ncp.findValidSysLogServers(servers)
+
+	assert.Equal(t, expected, result)
+}
+
 func TestNginxConfigParser_checkLog(t *testing.T) {
 	logBuf := &bytes.Buffer{}
 	stub.StubLoggerWith(logBuf)
@@ -1067,6 +1170,7 @@ func TestNginxConfigParser_pingAPIEndpoint_PlusAPI(t *testing.T) {
 	})
 
 	fakeServer := httptest.NewServer(handler)
+	t.Logf(fakeServer.URL)
 	defer fakeServer.Close()
 
 	nginxConfigParser := NewNginxConfigParser(types.AgentConfig())