Skip to content

Commit 7ccfa49

Browse files
committed
fix logs
1 parent 4a38ea9 commit 7ccfa49

File tree

4 files changed

+101
-51
lines changed

4 files changed

+101
-51
lines changed

internal/datasource/config/nginx_config_parser.go

Lines changed: 22 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ func (ncp *NginxConfigParser) createNginxConfigContext(
109109
payload *crossplane.Payload,
110110
) (*model.NginxConfigContext, error) {
111111
napSyslogServersFound := make(map[string]bool)
112+
napEnabled := false
112113

113114
nginxConfigContext := &model.NginxConfigContext{
114115
InstanceID: instance.GetInstanceMeta().GetInstanceId(),
@@ -175,17 +176,11 @@ func (ncp *NginxConfigParser) createNginxConfigContext(
175176
}
176177
case "app_protect_security_log":
177178
if len(directive.Args) > 1 {
178-
sysLogServers := ncp.findValidSysLogServers(directive.Args)
179-
if len(sysLogServers) == 0 {
180-
slog.WarnContext(ctx, "Could not find usable NAP syslog server, "+
181-
"security violations will be unavailable")
182-
}
183-
for i := range sysLogServers {
184-
sysLogServer := sysLogServers[i]
185-
if !napSyslogServersFound[sysLogServer] {
186-
napSyslogServersFound[sysLogServer] = true
187-
slog.DebugContext(ctx, "Found NAP syslog server", "address", sysLogServer)
188-
}
179+
napEnabled = true
180+
sysLogServer := ncp.findValidSysLogServers(directive.Args[1])
181+
if sysLogServer != "" && !napSyslogServersFound[sysLogServer] {
182+
napSyslogServersFound[sysLogServer] = true
183+
slog.DebugContext(ctx, "Found NAP syslog server", "address", sysLogServer)
189184
}
190185
}
191186
}
@@ -213,6 +208,9 @@ func (ncp *NginxConfigParser) createNginxConfigContext(
213208
nginxConfigContext.NAPSysLogServer = syslogServer
214209
ncp.previousNAPSysLogServer = syslogServer
215210
}
211+
} else if napEnabled {
212+
slog.WarnContext(ctx, "Could not find usable NAP syslog server, "+
213+
"security violations will be unavailable")
216214
}
217215

218216
fileMeta, err := files.FileMeta(conf.File)
@@ -236,40 +234,37 @@ func (ncp *NginxConfigParser) parseSyslogDirective(ctx context.Context, napSyslo
236234
for napSyslogServer := range napSyslogServers {
237235
ln, err := net.Listen("tcp", napSyslogServer)
238236
if err != nil {
239-
slog.WarnContext(ctx, "NAP syslog server is not reachable", "address", napSyslogServer,
237+
slog.DebugContext(ctx, "NAP syslog server is not reachable", "address", napSyslogServer,
240238
"error", err)
241239

242240
continue
243241
}
244242
ln.Close()
243+
245244
slog.DebugContext(ctx, "Found valid NAP syslog server", "address", napSyslogServer)
246245

247246
return napSyslogServer
248247
}
249-
slog.WarnContext(ctx, "Could not find usable NAP syslog server, security violations will be unavailable")
250248

251249
return ""
252250
}
253251

254-
func (ncp *NginxConfigParser) findValidSysLogServers(sysLogServers []string) []string {
252+
func (ncp *NginxConfigParser) findValidSysLogServers(sysLogServer string) string {
255253
re := regexp.MustCompile(`syslog:server=([\S]+)`)
256-
var servers []string
257-
for i := range sysLogServers {
258-
matches := re.FindStringSubmatch(sysLogServers[i])
259-
if len(matches) > 1 {
260-
host, _, err := net.SplitHostPort(matches[1])
261-
if err != nil {
262-
continue
263-
}
254+
matches := re.FindStringSubmatch(sysLogServer)
255+
if len(matches) > 1 {
256+
host, _, err := net.SplitHostPort(matches[1])
257+
if err != nil {
258+
return ""
259+
}
264260

265-
ip := net.ParseIP(host)
266-
if ip.IsLoopback() || strings.EqualFold(host, "localhost") {
267-
servers = append(servers, matches[1])
268-
}
261+
ip := net.ParseIP(host)
262+
if ip.IsLoopback() || strings.EqualFold(host, "localhost") {
263+
return matches[1]
269264
}
270265
}
271266

272-
return servers
267+
return ""
273268
}
274269

275270
func (ncp *NginxConfigParser) parseIncludeDirective(directive *crossplane.Directive) string {

internal/datasource/config/nginx_config_parser_test.go

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,34 @@ func TestNginxConfigParser_Parse(t *testing.T) {
506506
),
507507
allowedDirectories: []string{dir},
508508
},
509+
{
510+
name: "Test 10: Check with multiple syslog servers",
511+
instance: protos.NginxPlusInstance([]string{}),
512+
content: testconfig.NginxConfigWithMultipleSysLogs(errorLog.Name(), accessLog.Name(),
513+
"192.168.12.34:1517", "my.domain.com:1517", "127.0.0.1:1515"),
514+
expectedConfigContext: modelHelpers.ConfigContextWithSysLog(
515+
accessLog.Name(),
516+
errorLog.Name(),
517+
protos.NginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(),
518+
"127.0.0.1:1515",
519+
),
520+
allowedDirectories: []string{dir},
521+
expectedLog: "Found valid NAP syslog server",
522+
},
523+
{
524+
name: "Test 10: Check with multiple syslog servers",
525+
instance: protos.NginxPlusInstance([]string{}),
526+
content: testconfig.NginxConfigWithMultipleSysLogs(errorLog.Name(), accessLog.Name(),
527+
"192.168.12.34:1517", "my.domain.com:1517", "not.allowed:1515"),
528+
expectedConfigContext: modelHelpers.ConfigContextWithSysLog(
529+
accessLog.Name(),
530+
errorLog.Name(),
531+
protos.NginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(),
532+
"",
533+
),
534+
allowedDirectories: []string{dir},
535+
expectedLog: "Could not find usable NAP syslog server, security violations will be unavailable",
536+
},
509537
}
510538

511539
for _, test := range tests {
@@ -637,7 +665,7 @@ func TestNginxConfigParser_SyslogServerParse(t *testing.T) {
637665
expectedSyslogServers: "",
638666
content: testconfig.NginxConfigWithMultipleSysLogs(errorLog.Name(), accessLog.Name(),
639667
"my.domain.com:1517", "127.0.0.1:1515", "my.domain.com:1517"),
640-
expectedLog: "Could not find usable NAP syslog server",
668+
expectedLog: "NAP syslog server is not reachable",
641669
portInUse: true,
642670
},
643671
{
@@ -684,17 +712,17 @@ func TestNginxConfigParser_SyslogServerParse(t *testing.T) {
684712

685713
func TestNginxConfigParser_findValidSysLogServers(t *testing.T) {
686714
servers := []string{
687-
"/etc/app_protect/conf/log_default.json", "syslog:server=192.168.12.34:1517",
688-
"app_protect_security_log", "/etc/app_protect/conf/log_default1.json", "syslog:server=my.domain.com:1517",
689-
"app_protect_security_log", "/etc/app_protect/conf/log_default2.json", "syslog:server=127.0.0.1:1515",
690-
"app_protect_security_log", "/etc/app_protect/conf/log_default3.json", "syslog:server=localhost:1516",
691-
"app_protect_security_log\", \"/etc/app_protect/conf/log_default3.json\", \"syslog:server=127.255.255.255:1517",
715+
"syslog:server=192.168.12.34:1517", "syslog:server=my.domain.com:1517", "syslog:server=127.0.0.1:1515",
716+
"syslog:server=localhost:1516", "syslog:server=127.255.255.255:1517",
692717
}
693-
expected := []string{"127.0.0.1:1515", "localhost:1516", "127.255.255.255:1517"}
718+
expected := []string{"", "", "127.0.0.1:1515", "localhost:1516", "127.255.255.255:1517"}
694719
ncp := NewNginxConfigParser(types.AgentConfig())
695-
result := ncp.findValidSysLogServers(servers)
696720

697-
assert.Equal(t, expected, result)
721+
for i, server := range servers {
722+
result := ncp.findValidSysLogServers(server)
723+
724+
assert.Equal(t, expected[i], result)
725+
}
698726
}
699727

700728
func TestNginxConfigParser_checkLog(t *testing.T) {

test/config/nginx/nginx-with-multiple-syslog-servers.conf

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,6 @@ http {
1919
}
2020

2121
http {
22-
log_format ltsv "time:$time_local"
23-
"\thost:$remote_addr"
24-
"\tmethod:$request_method"
25-
"\turi:$request_uri"
26-
"\tprotocol:$server_protocol"
27-
"\tstatus:$status"
28-
"\tsize:$body_bytes_sent"
29-
"\treferer:$http_referer"
30-
"\tua:$http_user_agent"
31-
"\treqtime:$request_time"
32-
"\tapptime:$upstream_response_time";
3322

3423
server {
3524
listen 9093;
@@ -42,9 +31,9 @@ http {
4231

4332
server {
4433

45-
app_protect_security_log "/etc/app_protect/conf/log_default.json" syslog:server=%s
46-
app_protect_security_log "/etc/app_protect/conf/log_default1.json" syslog:server=%s
47-
app_protect_security_log "/etc/app_protect/conf/log_default2.json" syslog:server=%s
48-
app_protect_security_log "/etc/app_protect/conf/log_default3.json" syslog:server=%s
34+
app_protect_security_log "/etc/app_protect/conf/log_default.json" syslog:server=%s;
35+
app_protect_security_log "/etc/app_protect/conf/log_default1.json" syslog:server=%s;
36+
app_protect_security_log "/etc/app_protect/conf/log_default2.json" syslog:server=%s;
37+
app_protect_security_log "/etc/app_protect/conf/log_default3.json" syslog:server=%s;
4938
}
5039
}

test/model/config.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,3 +160,41 @@ func ConfigContextWithFiles(
160160
NAPSysLogServer: syslogServers,
161161
}
162162
}
163+
164+
func ConfigContextWithSysLog(
165+
accessLogName,
166+
errorLogName string,
167+
instanceID string,
168+
syslogServers string,
169+
) *model.NginxConfigContext {
170+
return &model.NginxConfigContext{
171+
StubStatus: &model.APIDetails{
172+
URL: "",
173+
Listen: "",
174+
Location: "",
175+
},
176+
PlusAPI: &model.APIDetails{
177+
URL: "",
178+
Listen: "",
179+
Location: "",
180+
},
181+
AccessLogs: []*model.AccessLog{
182+
{
183+
Name: accessLogName,
184+
Format: "$remote_addr - $remote_user [$time_local]",
185+
Readable: true,
186+
Permissions: "0600",
187+
},
188+
},
189+
ErrorLogs: []*model.ErrorLog{
190+
{
191+
Name: errorLogName,
192+
Readable: true,
193+
LogLevel: "notice",
194+
Permissions: "0600",
195+
},
196+
},
197+
InstanceID: instanceID,
198+
NAPSysLogServer: syslogServers,
199+
}
200+
}

0 commit comments

Comments
 (0)