Skip to content

Commit 142b32f

Browse files
dhurleydhurley
committed
Fix how NAP syslog servers are discovered
1 parent 6c9f617 commit 142b32f

File tree

7 files changed

+196
-149
lines changed

7 files changed

+196
-149
lines changed

‎internal/collector/otel_collector_plugin.go‎

Lines changed: 59 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,14 @@ 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+
slog.Error("Updating NAP SysLog Server list", "list", napSysLogServer)
559+
560+
if napSysLogServer != "" {
561+
if !oc.doesTcplogReceiverAlreadyExist(napSysLogServer) {
555562
oc.config.Collector.Receivers.TcplogReceivers["nginx_app_protect"] = &config.TcplogReceiver{
556-
ListenAddress: nginxConfigContext.NAPSysLogServer,
563+
ListenAddress: napSysLogServer,
557564
Operators: []config.Operator{
558565
// regex captures the priority number from the log line
559566
{
@@ -606,13 +613,13 @@ func (oc *Collector) updateNginxAppProtectTcplogReceivers(nginxConfigContext *mo
606613
}
607614
}
608615

609-
tcplogReceiverDeleted := oc.areNapReceiversDeleted(nginxConfigContext)
616+
tcplogReceiverDeleted := oc.areNapReceiversDeleted(napSysLogServer)
610617

611618
return newTcplogReceiverAdded || tcplogReceiverDeleted
612619
}
613620

614-
func (oc *Collector) areNapReceiversDeleted(nginxConfigContext *model.NginxConfigContext) bool {
615-
listenAddressesToBeDeleted := oc.configDeletedNapReceivers(nginxConfigContext)
621+
func (oc *Collector) areNapReceiversDeleted(napSysLogServer string) bool {
622+
listenAddressesToBeDeleted := oc.configDeletedNapReceivers(napSysLogServer)
616623
if len(listenAddressesToBeDeleted) != 0 {
617624
delete(oc.config.Collector.Receivers.TcplogReceivers, "nginx_app_protect")
618625
return true
@@ -621,17 +628,17 @@ func (oc *Collector) areNapReceiversDeleted(nginxConfigContext *model.NginxConfi
621628
return false
622629
}
623630

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

627634
for _, tcplogReceiver := range oc.config.Collector.Receivers.TcplogReceivers {
628635
elements[tcplogReceiver.ListenAddress] = true
629636
}
630637

631-
if nginxConfigContext.NAPSysLogServer != "" {
638+
if napSysLogServer != "" {
632639
addressesToDelete := make(map[string]bool)
633-
if !elements[nginxConfigContext.NAPSysLogServer] {
634-
addressesToDelete[nginxConfigContext.NAPSysLogServer] = true
640+
if !elements[napSysLogServer] {
641+
addressesToDelete[napSysLogServer] = true
635642
}
636643

637644
return addressesToDelete
@@ -675,6 +682,39 @@ func (oc *Collector) updateResourceAttributes(
675682
return actionUpdated
676683
}
677684

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