Skip to content

Commit 3f99356

Browse files
dhurleydhurley
authored
Fix how NAP syslog servers are discovered (#1189)
1 parent 6c9f617 commit 3f99356

File tree

7 files changed

+194
-149
lines changed

7 files changed

+194
-149
lines changed

‎internal/collector/otel_collector_plugin.go‎

Lines changed: 57 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"errors"
1010
"fmt"
1111
"log/slog"
12+
"net"
1213
"os"
1314
"strings"
1415
"sync"
@@ -46,11 +47,12 @@ const (
4647
type (
4748
// Collector The OTel collector plugin start an embedded OTel collector for metrics collection in the OTel format.
4849
Collector struct {
49-
service types.CollectorInterface
50-
cancel context.CancelFunc
51-
config *config.Config
52-
mu *sync.Mutex
53-
stopped bool
50+
service types.CollectorInterface
51+
config *config.Config
52+
mu *sync.Mutex
53+
cancel context.CancelFunc
54+
previousNAPSysLogServer string
55+
stopped bool
5456
}
5557
)
5658

@@ -86,10 +88,11 @@ func NewCollector(conf *config.Config) (*Collector, error) {
8688
}
8789

8890
return &Collector{
89-
config: conf,
90-
service: oTelCollector,
91-
stopped: true,
92-
mu: &sync.Mutex{},
91+
config: conf,
92+
service: oTelCollector,
93+
stopped: true,
94+
mu: &sync.Mutex{},
95+
previousNAPSysLogServer: "",
9396
}, nil
9497
}
9598

@@ -550,10 +553,12 @@ func (oc *Collector) updateNginxAppProtectTcplogReceivers(nginxConfigContext *mo
550553
oc.config.Collector.Receivers.TcplogReceivers = make(map[string]*config.TcplogReceiver)
551554
}
552555

553-
if nginxConfigContext.NAPSysLogServer != "" {
554-
if !oc.doesTcplogReceiverAlreadyExist(nginxConfigContext.NAPSysLogServer) {
556+
napSysLogServer := oc.findAvailableSyslogServers(nginxConfigContext.NAPSysLogServers)
557+
558+
if napSysLogServer != "" {
559+
if !oc.doesTcplogReceiverAlreadyExist(napSysLogServer) {
555560
oc.config.Collector.Receivers.TcplogReceivers["nginx_app_protect"] = &config.TcplogReceiver{
556-
ListenAddress: nginxConfigContext.NAPSysLogServer,
561+
ListenAddress: napSysLogServer,
557562
Operators: []config.Operator{
558563
// regex captures the priority number from the log line
559564
{
@@ -606,13 +611,13 @@ func (oc *Collector) updateNginxAppProtectTcplogReceivers(nginxConfigContext *mo
606611
}
607612
}
608613

609-
tcplogReceiverDeleted := oc.areNapReceiversDeleted(nginxConfigContext)
614+
tcplogReceiverDeleted := oc.areNapReceiversDeleted(napSysLogServer)
610615

611616
return newTcplogReceiverAdded || tcplogReceiverDeleted
612617
}
613618

614-
func (oc *Collector) areNapReceiversDeleted(nginxConfigContext *model.NginxConfigContext) bool {
615-
listenAddressesToBeDeleted := oc.configDeletedNapReceivers(nginxConfigContext)
619+
func (oc *Collector) areNapReceiversDeleted(napSysLogServer string) bool {
620+
listenAddressesToBeDeleted := oc.configDeletedNapReceivers(napSysLogServer)
616621
if len(listenAddressesToBeDeleted) != 0 {
617622
delete(oc.config.Collector.Receivers.TcplogReceivers, "nginx_app_protect")
618623
return true
@@ -621,17 +626,17 @@ func (oc *Collector) areNapReceiversDeleted(nginxConfigContext *model.NginxConfi
621626
return false
622627
}
623628

624-
func (oc *Collector) configDeletedNapReceivers(nginxConfigContext *model.NginxConfigContext) map[string]bool {
629+
func (oc *Collector) configDeletedNapReceivers(napSysLogServer string) map[string]bool {
625630
elements := make(map[string]bool)
626631

627632
for _, tcplogReceiver := range oc.config.Collector.Receivers.TcplogReceivers {
628633
elements[tcplogReceiver.ListenAddress] = true
629634
}
630635

631-
if nginxConfigContext.NAPSysLogServer != "" {
636+
if napSysLogServer != "" {
632637
addressesToDelete := make(map[string]bool)
633-
if !elements[nginxConfigContext.NAPSysLogServer] {
634-
addressesToDelete[nginxConfigContext.NAPSysLogServer] = true
638+
if !elements[napSysLogServer] {
639+
addressesToDelete[napSysLogServer] = true
635640
}
636641

637642
return addressesToDelete
@@ -675,6 +680,39 @@ func (oc *Collector) updateResourceAttributes(
675680
return actionUpdated
676681
}
677682

683+
func (oc *Collector) findAvailableSyslogServers(napSyslogServers []string) string {
684+
napSyslogServersMap := make(map[string]bool)
685+
for _, server := range napSyslogServers {
686+
napSyslogServersMap[server] = true
687+
}
688+
689+
if oc.previousNAPSysLogServer != "" {
690+
if _, ok := napSyslogServersMap[oc.previousNAPSysLogServer]; ok {
691+
return oc.previousNAPSysLogServer
692+
}
693+
}
694+
695+
for _, napSyslogServer := range napSyslogServers {
696+
ln, err := net.Listen("tcp", napSyslogServer)
697+
if err != nil {
698+
slog.Debug("NAP syslog server is not reachable", "address", napSyslogServer,
699+
"error", err)
700+
701+
continue
702+
}
703+
closeError := ln.Close()
704+
if closeError != nil {
705+
slog.Debug("Failed to close syslog server", "address", napSyslogServer, "error", closeError)
706+
}
707+
708+
slog.Debug("Found valid NAP syslog server", "address", napSyslogServer)
709+
710+
return napSyslogServer
711+
}
712+
713+
return ""
714+
}
715+
678716
func isOSSReceiverChanged(nginxReceiver config.NginxReceiver, nginxConfigContext *model.NginxConfigContext) bool {
679717
return nginxReceiver.StubStatus.URL != nginxConfigContext.StubStatus.URL ||
680718
len(nginxReceiver.AccessLogs) != len(nginxConfigContext.AccessLogs)

‎internal/collector/otel_collector_plugin_test.go‎

Lines changed: 74 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"bytes"
99
"context"
1010
"errors"
11+
"net"
1112
"path/filepath"
1213
"testing"
1314

@@ -738,7 +739,7 @@ func TestCollector_updateNginxAppProtectTcplogReceivers(t *testing.T) {
738739
require.NoError(t, err)
739740

740741
nginxConfigContext := &model.NginxConfigContext{
741-
NAPSysLogServer: "localhost:151",
742+
NAPSysLogServers: []string{"localhost:15632"},
742743
}
743744

744745
assert.Empty(t, conf.Collector.Receivers.TcplogReceivers)
@@ -748,7 +749,7 @@ func TestCollector_updateNginxAppProtectTcplogReceivers(t *testing.T) {
748749

749750
assert.True(tt, tcplogReceiverAdded)
750751
assert.Len(tt, conf.Collector.Receivers.TcplogReceivers, 1)
751-
assert.Equal(tt, "localhost:151", conf.Collector.Receivers.TcplogReceivers["nginx_app_protect"].ListenAddress)
752+
assert.Equal(tt, "localhost:15632", conf.Collector.Receivers.TcplogReceivers["nginx_app_protect"].ListenAddress)
752753
assert.Len(tt, conf.Collector.Receivers.TcplogReceivers["nginx_app_protect"].Operators, 6)
753754
})
754755

@@ -758,7 +759,7 @@ func TestCollector_updateNginxAppProtectTcplogReceivers(t *testing.T) {
758759
tcplogReceiverAdded := collector.updateNginxAppProtectTcplogReceivers(nginxConfigContext)
759760
assert.False(t, tcplogReceiverAdded)
760761
assert.Len(t, conf.Collector.Receivers.TcplogReceivers, 1)
761-
assert.Equal(t, "localhost:151", conf.Collector.Receivers.TcplogReceivers["nginx_app_protect"].ListenAddress)
762+
assert.Equal(t, "localhost:15632", conf.Collector.Receivers.TcplogReceivers["nginx_app_protect"].ListenAddress)
762763
assert.Len(t, conf.Collector.Receivers.TcplogReceivers["nginx_app_protect"].Operators, 6)
763764
})
764765

@@ -771,17 +772,85 @@ func TestCollector_updateNginxAppProtectTcplogReceivers(t *testing.T) {
771772
t.Run("Test 4: NewCollector tcplogReceiver added and deleted another", func(tt *testing.T) {
772773
tcplogReceiverDeleted := collector.updateNginxAppProtectTcplogReceivers(
773774
&model.NginxConfigContext{
774-
NAPSysLogServer: "localhost:152",
775+
NAPSysLogServers: []string{"localhost:1555"},
775776
},
776777
)
777778

778779
assert.True(t, tcplogReceiverDeleted)
779780
assert.Len(t, conf.Collector.Receivers.TcplogReceivers, 1)
780-
assert.Equal(t, "localhost:152", conf.Collector.Receivers.TcplogReceivers["nginx_app_protect"].ListenAddress)
781+
assert.Equal(t, "localhost:1555", conf.Collector.Receivers.TcplogReceivers["nginx_app_protect"].ListenAddress)
781782
assert.Len(t, conf.Collector.Receivers.TcplogReceivers["nginx_app_protect"].Operators, 6)
782783
})
783784
}
784785

786+
func TestCollector_findAvailableSyslogServers(t *testing.T) {
787+
conf := types.OTelConfig(t)
788+
conf.Collector.Log.Path = ""
789+
conf.Collector.Processors.Batch = nil
790+
conf.Collector.Processors.Attribute = nil
791+
conf.Collector.Processors.Resource = nil
792+
conf.Collector.Processors.LogsGzip = nil
793+
collector, err := NewCollector(conf)
794+
require.NoError(t, err)
795+
796+
tests := []struct {
797+
name string
798+
expectedSyslogServer string
799+
previousNAPSysLogServer string
800+
syslogServers []string
801+
portInUse bool
802+
}{
803+
{
804+
name: "Test 1: port available",
805+
expectedSyslogServer: "localhost:15632",
806+
previousNAPSysLogServer: "",
807+
syslogServers: []string{"localhost:15632"},
808+
portInUse: false,
809+
},
810+
{
811+
name: "Test 2: port in use",
812+
expectedSyslogServer: "",
813+
previousNAPSysLogServer: "",
814+
syslogServers: []string{"localhost:15632"},
815+
portInUse: true,
816+
},
817+
{
818+
name: "Test 3: syslog server already configured",
819+
expectedSyslogServer: "localhost:15632",
820+
previousNAPSysLogServer: "localhost:15632",
821+
syslogServers: []string{"localhost:15632"},
822+
portInUse: false,
823+
},
824+
{
825+
name: "Test 4: new syslog server",
826+
expectedSyslogServer: "localhost:15632",
827+
previousNAPSysLogServer: "localhost:1122",
828+
syslogServers: []string{"localhost:15632"},
829+
portInUse: false,
830+
},
831+
{
832+
name: "Test 5: port in use find next server",
833+
expectedSyslogServer: "localhost:1122",
834+
previousNAPSysLogServer: "",
835+
syslogServers: []string{"localhost:15632", "localhost:1122"},
836+
portInUse: true,
837+
},
838+
}
839+
840+
for _, test := range tests {
841+
t.Run(test.name, func(tt *testing.T) {
842+
if test.portInUse {
843+
ln, listenError := net.Listen("tcp", "localhost:15632")
844+
require.NoError(t, listenError)
845+
defer ln.Close()
846+
}
847+
848+
actual := collector.findAvailableSyslogServers(test.syslogServers)
849+
assert.Equal(tt, test.expectedSyslogServer, actual)
850+
})
851+
}
852+
}
853+
785854
func createFakeCollector() *typesfakes.FakeCollectorInterface {
786855
fakeCollector := &typesfakes.FakeCollectorInterface{}
787856
fakeCollector.RunStub = func(ctx context.Context) error { return nil }

‎internal/datasource/config/nginx_config_parser.go‎

Lines changed: 7 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,7 @@ const (
4949

5050
type (
5151
NginxConfigParser struct {
52-
agentConfig *config.Config
53-
previousNAPSysLogServer string
52+
agentConfig *config.Config
5453
}
5554
)
5655

@@ -68,8 +67,7 @@ type (
6867

6968
func NewNginxConfigParser(agentConfig *config.Config) *NginxConfigParser {
7069
return &NginxConfigParser{
71-
agentConfig: agentConfig,
72-
previousNAPSysLogServer: "",
70+
agentConfig: agentConfig,
7371
}
7472
}
7573

@@ -125,6 +123,7 @@ func (ncp *NginxConfigParser) createNginxConfigContext(
125123
Listen: "",
126124
Location: "",
127125
},
126+
NAPSysLogServers: make([]string, 0),
128127
}
129128

130129
rootDir := filepath.Dir(instance.GetInstanceRuntime().GetConfigPath())
@@ -205,11 +204,11 @@ func (ncp *NginxConfigParser) createNginxConfigContext(
205204
}
206205

207206
if len(napSyslogServersFound) > 0 {
208-
syslogServer := ncp.findAvailableSyslogServers(ctx, napSyslogServersFound)
209-
if syslogServer != "" {
210-
nginxConfigContext.NAPSysLogServer = syslogServer
211-
ncp.previousNAPSysLogServer = syslogServer
207+
var napSyslogServer []string
208+
for server := range napSyslogServersFound {
209+
napSyslogServer = append(napSyslogServer, server)
212210
}
211+
nginxConfigContext.NAPSysLogServers = napSyslogServer
213212
} else if napEnabled {
214213
slog.WarnContext(ctx, "Could not find available local NGINX App Protect syslog server. "+
215214
"Security violations will not be collected.")
@@ -226,31 +225,6 @@ func (ncp *NginxConfigParser) createNginxConfigContext(
226225
return nginxConfigContext, nil
227226
}
228227

229-
func (ncp *NginxConfigParser) findAvailableSyslogServers(ctx context.Context, napSyslogServers map[string]bool) string {
230-
if ncp.previousNAPSysLogServer != "" {
231-
if _, ok := napSyslogServers[ncp.previousNAPSysLogServer]; ok {
232-
return ncp.previousNAPSysLogServer
233-
}
234-
}
235-
236-
for napSyslogServer := range napSyslogServers {
237-
ln, err := net.Listen("tcp", napSyslogServer)
238-
if err != nil {
239-
slog.DebugContext(ctx, "NAP syslog server is not reachable", "address", napSyslogServer,
240-
"error", err)
241-
242-
continue
243-
}
244-
ln.Close()
245-
246-
slog.DebugContext(ctx, "Found valid NAP syslog server", "address", napSyslogServer)
247-
248-
return napSyslogServer
249-
}
250-
251-
return ""
252-
}
253-
254228
func (ncp *NginxConfigParser) findLocalSysLogServers(sysLogServer string) string {
255229
re := regexp.MustCompile(`syslog:server=([\S]+)`)
256230
matches := re.FindStringSubmatch(sysLogServer)

0 commit comments

Comments
 (0)