Add default syslog server #1317
Conversation
| napSyslogServersMap := make(map[string]bool) | ||
| for _, server := range napSyslogServers { | ||
| napSyslogServersMap[server] = true | ||
| func (oc *Collector) findAvailableSyslogServers(ctx context.Context, napSyslogServer string) string { |
There was a problem hiding this comment.
| func (oc *Collector) findAvailableSyslogServers(ctx context.Context, napSyslogServer string) string { | |
| func (oc *Collector) findAvailableSyslogServer(ctx context.Context, napSyslogServer string) string { |
internal/config/config.go
Outdated
| fs.String( | ||
| SyslogServerPort, | ||
| DefSyslogServerPort, | ||
| "The port Agent will start the syslog server for Nginx App Protect Security violations on", |
There was a problem hiding this comment.
| "The port Agent will start the syslog server for Nginx App Protect Security violations on", | |
| "The port Agent will start the syslog server on for logs collection", |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1317 +/- ##
==========================================
+ Coverage 84.88% 86.23% +1.35%
==========================================
Files 100 102 +2
Lines 14520 12520 -2000
==========================================
- Hits 12325 10797 -1528
+ Misses 1734 1248 -486
- Partials 461 475 +14
... and 93 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
| } | ||
|
|
||
| oc.previousNAPSysLogServer = napSyslogServer | ||
| slog.InfoContext(ctx, "Found valid NAP syslog server", "address", napSyslogServer) |
There was a problem hiding this comment.
can we remove this info message and have it in the config parser instead?
There was a problem hiding this comment.
There is a log message already in the parser that it found one. I had this on as the config parser could find a NAP server but when the otel plugin goes to use it the port is unavailable but I can remove it if its causing problems
| } else if napEnabled { | ||
| slog.WarnContext(ctx, "Could not find available local NGINX App Protect syslog server. "+ | ||
| "Security violations will not be collected.") | ||
| if napEnabled && nginxConfigContext.NAPSysLogServer == "" { |
There was a problem hiding this comment.
can you move this if statement outside the for loop so that its not logged multiple times?
also can you can an else statement to log on info message saying that you found a syslog server configured for NAP?
Proposed changes
Added syslog server port option in agent config
Default value is
1514, agent will parse the NGINX config for Nap syslog server directives, when one is found agent will check if the host is valid and then check if the port matches the configured syslog server port in the config (or default if not set)If the agent fails to find a valid syslog server it will log the following warning, which contains the configured port
Changed NapSyslogServers in NginxConfigContext to a single value as we no longer allow multiple syslog servers.
Checklist
Before creating a PR, run through this checklist and mark each as complete.
CONTRIBUTINGdocumentmake install-toolsand have attached any dependency changes to this pull requestREADME.md)