Skip to content

Commit cd42f7d

Browse files
aphralGdhurley
aphralG
and
dhurley
authored
Add default syslog server (#1317)
--------- Co-authored-by: Donal Hurley <d.hurley@f5.com>
1 parent 61f1fc8 commit cd42f7d

File tree

15 files changed

+163
-131
lines changed

15 files changed

+163
-131
lines changed

‎internal/collector/otel_collector_plugin.go‎

Lines changed: 31 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -613,7 +613,7 @@ func (oc *Collector) updateNginxAppProtectTcplogReceivers(
613613
oc.config.Collector.Receivers.TcplogReceivers = make(map[string]*config.TcplogReceiver)
614614
}
615615

616-
napSysLogServer := oc.findAvailableSyslogServers(ctx, nginxConfigContext.NAPSysLogServers)
616+
napSysLogServer := oc.findAvailableSyslogServer(ctx, nginxConfigContext.NAPSysLogServer)
617617

618618
if napSysLogServer != "" {
619619
if !oc.doesTcplogReceiverAlreadyExist(napSysLogServer) {
@@ -746,40 +746,29 @@ func (oc *Collector) updateResourceAttributes(
746746
return actionUpdated
747747
}
748748

749-
func (oc *Collector) findAvailableSyslogServers(ctx context.Context, napSyslogServers []string) string {
750-
napSyslogServersMap := make(map[string]bool)
751-
for _, server := range napSyslogServers {
752-
napSyslogServersMap[server] = true
753-
}
754-
755-
if oc.previousNAPSysLogServer != "" {
756-
if _, ok := napSyslogServersMap[oc.previousNAPSysLogServer]; ok {
757-
return oc.previousNAPSysLogServer
758-
}
749+
func (oc *Collector) findAvailableSyslogServer(ctx context.Context, napSyslogServer string) string {
750+
if oc.previousNAPSysLogServer != "" &&
751+
normaliseAddress(oc.previousNAPSysLogServer) == normaliseAddress(napSyslogServer) {
752+
return napSyslogServer
759753
}
760754

761-
for _, napSyslogServer := range napSyslogServers {
762-
listenConfig := &net.ListenConfig{}
763-
ln, err := listenConfig.Listen(ctx, "tcp", napSyslogServer)
764-
if err != nil {
765-
slog.DebugContext(ctx, "NAP syslog server is not reachable", "address", napSyslogServer,
766-
"error", err)
767-
768-
continue
769-
}
770-
closeError := ln.Close()
771-
if closeError != nil {
772-
slog.DebugContext(ctx, "Failed to close syslog server", "address", napSyslogServer, "error", closeError)
773-
}
774-
775-
slog.DebugContext(ctx, "Found valid NAP syslog server", "address", napSyslogServer)
755+
listenConfig := &net.ListenConfig{}
756+
ln, err := listenConfig.Listen(ctx, "tcp", napSyslogServer)
757+
if err != nil {
758+
slog.DebugContext(ctx, "NAP syslog server is not reachable", "address", napSyslogServer,
759+
"error", err)
776760

777-
oc.previousNAPSysLogServer = napSyslogServer
761+
return ""
762+
}
778763

779-
return napSyslogServer
764+
closeError := ln.Close()
765+
if closeError != nil {
766+
slog.DebugContext(ctx, "Failed to close syslog server", "address", napSyslogServer, "error", closeError)
780767
}
781768

782-
return ""
769+
oc.previousNAPSysLogServer = napSyslogServer
770+
771+
return napSyslogServer
783772
}
784773

785774
func isOSSReceiverChanged(nginxReceiver config.NginxReceiver, nginxConfigContext *model.NginxConfigContext) bool {
@@ -865,3 +854,16 @@ func setProxyWithBasicAuth(ctx context.Context, proxy *config.Proxy, parsedProxy
865854
proxyURL := parsedProxyURL.String()
866855
setProxyEnvs(ctx, proxyURL, "Setting Proxy with basic auth")
867856
}
857+
858+
func normaliseAddress(address string) string {
859+
host, port, err := net.SplitHostPort(address)
860+
if err != nil {
861+
return address
862+
}
863+
864+
if host == "localhost" {
865+
host = "127.0.0.1"
866+
}
867+
868+
return net.JoinHostPort(host, port)
869+
}

‎internal/collector/otel_collector_plugin_test.go‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -753,7 +753,7 @@ func TestCollector_updateNginxAppProtectTcplogReceivers(t *testing.T) {
753753
require.NoError(t, err)
754754

755755
nginxConfigContext := &model.NginxConfigContext{
756-
NAPSysLogServers: []string{"localhost:15632"},
756+
NAPSysLogServer: "localhost:15632",
757757
}
758758

759759
assert.Empty(t, conf.Collector.Receivers.TcplogReceivers)
@@ -786,7 +786,7 @@ func TestCollector_updateNginxAppProtectTcplogReceivers(t *testing.T) {
786786
t.Run("Test 4: NewCollector tcplogReceiver added and deleted another", func(tt *testing.T) {
787787
tcplogReceiverDeleted := collector.updateNginxAppProtectTcplogReceivers(ctx,
788788
&model.NginxConfigContext{
789-
NAPSysLogServers: []string{"localhost:1555"},
789+
NAPSysLogServer: "localhost:1555",
790790
},
791791
)
792792

@@ -939,49 +939,49 @@ func TestCollector_findAvailableSyslogServers(t *testing.T) {
939939
name string
940940
expectedSyslogServer string
941941
previousNAPSysLogServer string
942-
syslogServers []string
942+
syslogServers string
943943
portInUse bool
944944
}{
945945
{
946946
name: "Test 1: port available",
947947
expectedSyslogServer: "localhost:15632",
948948
previousNAPSysLogServer: "",
949-
syslogServers: []string{"localhost:15632"},
949+
syslogServers: "localhost:15632",
950950
portInUse: false,
951951
},
952952
{
953953
name: "Test 2: port in use",
954954
expectedSyslogServer: "",
955955
previousNAPSysLogServer: "",
956-
syslogServers: []string{"localhost:15632"},
956+
syslogServers: "localhost:15632",
957957
portInUse: true,
958958
},
959959
{
960960
name: "Test 3: syslog server already configured",
961961
expectedSyslogServer: "localhost:15632",
962962
previousNAPSysLogServer: "localhost:15632",
963-
syslogServers: []string{"localhost:15632"},
963+
syslogServers: "localhost:15632",
964964
portInUse: false,
965965
},
966966
{
967967
name: "Test 4: new syslog server",
968968
expectedSyslogServer: "localhost:15632",
969969
previousNAPSysLogServer: "localhost:1122",
970-
syslogServers: []string{"localhost:15632"},
970+
syslogServers: "localhost:15632",
971971
portInUse: false,
972972
},
973973
{
974-
name: "Test 5: port in use find next server",
974+
name: "Test 6: port hasn't changed",
975975
expectedSyslogServer: "localhost:1122",
976-
previousNAPSysLogServer: "",
977-
syslogServers: []string{"localhost:15632", "localhost:1122"},
976+
previousNAPSysLogServer: "localhost:1122",
977+
syslogServers: "localhost:1122",
978978
portInUse: true,
979979
},
980980
{
981-
name: "Test 6: port hasn't changed",
981+
name: "Test 7: port hasn't changed, but is now localhost",
982982
expectedSyslogServer: "localhost:1122",
983-
previousNAPSysLogServer: "localhost:1122",
984-
syslogServers: []string{"localhost:1122"},
983+
previousNAPSysLogServer: "127.0.0.1:1122",
984+
syslogServers: "localhost:1122",
985985
portInUse: true,
986986
},
987987
}
@@ -994,12 +994,12 @@ func TestCollector_findAvailableSyslogServers(t *testing.T) {
994994

995995
if test.portInUse {
996996
listenConfig := &net.ListenConfig{}
997-
ln, listenError := listenConfig.Listen(ctx, "tcp", "localhost:15632")
997+
ln, listenError := listenConfig.Listen(ctx, "tcp", test.syslogServers)
998998
require.NoError(t, listenError)
999999
defer ln.Close()
10001000
}
10011001

1002-
actual := collector.findAvailableSyslogServers(ctx, test.syslogServers)
1002+
actual := collector.findAvailableSyslogServer(ctx, test.syslogServers)
10031003
assert.Equal(tt, test.expectedSyslogServer, actual)
10041004
})
10051005
}

‎internal/config/config.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ func ResolveConfig() (*Config, error) {
157157
Features: viperInstance.GetStringSlice(FeaturesKey),
158158
Labels: resolveLabels(),
159159
LibDir: viperInstance.GetString(LibDirPathKey),
160+
SyslogServer: resolveSyslogServer(),
160161
}
161162

162163
defaultCollector(collector, config)
@@ -462,6 +463,12 @@ func registerFlags() {
462463
"A comma-separated list of features enabled for the agent.",
463464
)
464465

466+
fs.String(
467+
SyslogServerPort,
468+
DefSyslogServerPort,
469+
"The port Agent will start the syslog server on for logs collection",
470+
)
471+
465472
registerCommonFlags(fs)
466473
registerCommandFlags(fs)
467474
registerAuxiliaryCommandFlags(fs)
@@ -947,6 +954,12 @@ func resolveLog() *Log {
947954
}
948955
}
949956

957+
func resolveSyslogServer() *SyslogServer {
958+
return &SyslogServer{
959+
Port: viperInstance.GetString(SyslogServerPort),
960+
}
961+
}
962+
950963
func resolveLabels() map[string]interface{} {
951964
input := viperInstance.GetStringMapString(LabelsRootKey)
952965

‎internal/config/config_test.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1165,6 +1165,9 @@ func createConfig() *Config {
11651165
Level: "debug",
11661166
Path: "./test-path",
11671167
},
1168+
SyslogServer: &SyslogServer{
1169+
Port: "1512",
1170+
},
11681171
Client: &Client{
11691172
HTTP: &HTTP{
11701173
Timeout: 15 * time.Second,

‎internal/config/defaults.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ const (
2323
DefNginxReloadBackoffMaxInterval = 3 * time.Second
2424
DefNginxReloadBackoffMaxElapsedTime = 10 * time.Second
2525

26+
DefSyslogServerPort = "1514"
27+
2628
DefCommandServerHostKey = ""
2729
DefCommandServerPortKey = 0
2830
DefCommandServerTypeKey = "grpc"

‎internal/config/flags.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,8 @@ var (
136136
NginxExcludeLogsKey = pre(DataPlaneConfigRootKey, "nginx") + "exclude_logs"
137137
NginxApiTlsCa = pre(DataPlaneConfigRootKey, "nginx") + "api_tls_ca"
138138

139+
SyslogServerPort = pre("syslog_server") + "port"
140+
139141
FileWatcherMonitoringFrequencyKey = pre(FileWatcherKey) + "monitoring_frequency"
140142
NginxExcludeFilesKey = pre(FileWatcherKey) + "exclude_files"
141143
)

‎internal/config/testdata/nginx-agent.conf‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,11 @@ features:
2323
- metrics
2424
- api-action
2525
- logs-nap
26-
26+
27+
28+
syslog_server:
29+
port: 1512
30+
2731
data_plane_config:
2832
nginx:
2933
reload_monitoring_period: 30s

‎internal/config/types.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ type (
4343
Client *Client `yaml:"client" mapstructure:"client"`
4444
Collector *Collector `yaml:"collector" mapstructure:"collector"`
4545
Watchers *Watchers `yaml:"watchers" mapstructure:"watchers"`
46+
SyslogServer *SyslogServer `yaml:"syslog_server" mapstructure:"syslog_server"`
4647
Labels map[string]any `yaml:"labels" mapstructure:"labels"`
4748
Version string `yaml:"-"`
4849
Path string `yaml:"-"`
@@ -61,6 +62,9 @@ type (
6162
Nginx *NginxDataPlaneConfig `yaml:"nginx" mapstructure:"nginx"`
6263
}
6364

65+
SyslogServer struct {
66+
Port string `yaml:"port" mapstructure:"port"`
67+
}
6468
NginxDataPlaneConfig struct {
6569
ReloadBackoff *BackOff `yaml:"reload_backoff" mapstructure:"reload_backoff"`
6670
APITls TLSConfig `yaml:"api_tls" mapstructure:"api_tls"`

‎internal/datasource/config/nginx_config_parser.go‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,6 @@ func (ncp *NginxConfigParser) createNginxConfigContext(
184184
payload *crossplane.Payload,
185185
configPath string,
186186
) (*model.NginxConfigContext, error) {
187-
napSyslogServersFound := make(map[string]bool)
188187
napEnabled := false
189188

190189
nginxConfigContext := &model.NginxConfigContext{
@@ -200,7 +199,7 @@ func (ncp *NginxConfigParser) createNginxConfigContext(
200199
Listen: "",
201200
Location: "",
202201
},
203-
NAPSysLogServers: make([]string, 0),
202+
NAPSysLogServer: "",
204203
}
205204

206205
rootDir := filepath.Dir(instance.GetInstanceRuntime().GetConfigPath())
@@ -256,8 +255,8 @@ func (ncp *NginxConfigParser) createNginxConfigContext(
256255
if len(directive.Args) > 1 {
257256
napEnabled = true
258257
sysLogServer := ncp.findLocalSysLogServers(directive.Args[1])
259-
if sysLogServer != "" && !napSyslogServersFound[sysLogServer] {
260-
napSyslogServersFound[sysLogServer] = true
258+
if sysLogServer != "" {
259+
nginxConfigContext.NAPSysLogServer = sysLogServer
261260
slog.DebugContext(ctx, "Found NAP syslog server", "address", sysLogServer)
262261
}
263262
}
@@ -284,17 +283,6 @@ func (ncp *NginxConfigParser) createNginxConfigContext(
284283
nginxConfigContext.PlusAPIs = append(nginxConfigContext.PlusAPIs, plusAPIs...)
285284
}
286285

287-
if len(napSyslogServersFound) > 0 {
288-
var napSyslogServer []string
289-
for server := range napSyslogServersFound {
290-
napSyslogServer = append(napSyslogServer, server)
291-
}
292-
nginxConfigContext.NAPSysLogServers = napSyslogServer
293-
} else if napEnabled {
294-
slog.WarnContext(ctx, "Could not find available local NGINX App Protect syslog server. "+
295-
"Security violations will not be collected.")
296-
}
297-
298286
fileMeta, err := files.FileMeta(conf.File)
299287
if err != nil {
300288
slog.WarnContext(ctx, "Unable to get file metadata", "file_name", conf.File, "error", err)
@@ -303,8 +291,16 @@ func (ncp *NginxConfigParser) createNginxConfigContext(
303291
}
304292
}
305293

306-
nginxConfigContext.PlusAPIs = ncp.sortPlusAPIs(ctx, nginxConfigContext.PlusAPIs)
294+
if napEnabled && nginxConfigContext.NAPSysLogServer == "" {
295+
slog.WarnContext(ctx, fmt.Sprintf("Could not find available local NGINX App Protect syslog"+
296+
" server configured on port %s. Security violations will not be collected.",
297+
ncp.agentConfig.SyslogServer.Port))
298+
} else if napEnabled && nginxConfigContext.NAPSysLogServer != "" {
299+
slog.InfoContext(ctx, fmt.Sprintf("Found available local NGINX App Protect syslog"+
300+
"server configured on port %s", ncp.agentConfig.SyslogServer.Port))
301+
}
307302

303+
nginxConfigContext.PlusAPIs = ncp.sortPlusAPIs(ctx, nginxConfigContext.PlusAPIs)
308304
nginxConfigContext.StubStatus = ncp.FindStubStatusAPI(ctx, nginxConfigContext)
309305
nginxConfigContext.PlusAPI = ncp.FindPlusAPI(ctx, nginxConfigContext)
310306

@@ -315,11 +311,15 @@ func (ncp *NginxConfigParser) findLocalSysLogServers(sysLogServer string) string
315311
re := regexp.MustCompile(`syslog:server=([\S]+)`)
316312
matches := re.FindStringSubmatch(sysLogServer)
317313
if len(matches) > 1 {
318-
host, _, err := net.SplitHostPort(matches[1])
314+
host, port, err := net.SplitHostPort(matches[1])
319315
if err != nil {
320316
return ""
321317
}
322318

319+
if port != ncp.agentConfig.SyslogServer.Port {
320+
return ""
321+
}
322+
323323
ip := net.ParseIP(host)
324324
if ip.IsLoopback() || strings.EqualFold(host, "localhost") {
325325
return matches[1]

0 commit comments

Comments
 (0)