Bladeren bron

feat: retrieve nginx paths and enhance log cache #1158

Jacky 1 maand geleden
bovenliggende
commit
0d84b5b872
4 gewijzigde bestanden met toevoegingen van 448 en 14 verwijderingen
  1. 21 3
      internal/nginx/config_args.go
  2. 22 8
      internal/nginx_log/log_cache.go
  3. 51 3
      internal/nginx_log/nginx_log.go
  4. 354 0
      internal/nginx_log/nginx_log_test.go

+ 21 - 3
internal/nginx/config_args.go

@@ -74,6 +74,19 @@ func resolvePath(path string) string {
 	return path
 }
 
+// GetPrefix returns the prefix of the nginx executable
+func GetPrefix() string {
+	out := getNginxV()
+	r, _ := regexp.Compile(`--prefix=(\S+)`)
+	match := r.FindStringSubmatch(out)
+	if len(match) < 1 {
+		logger.Error("nginx.GetPrefix len(match) < 1")
+		return "/usr/local/nginx"
+	}
+	return resolvePath(match[1])
+}
+
+// GetConfPath returns the path to the nginx configuration file
 func GetConfPath(dir ...string) (confPath string) {
 	if settings.NginxSettings.ConfigDir == "" {
 		out := getNginxV()
@@ -97,6 +110,7 @@ func GetConfPath(dir ...string) (confPath string) {
 	return joined
 }
 
+// GetConfEntryPath returns the path to the nginx configuration file
 func GetConfEntryPath() (path string) {
 	if settings.NginxSettings.ConfigPath == "" {
 		out := getNginxV()
@@ -114,6 +128,7 @@ func GetConfEntryPath() (path string) {
 	return resolvePath(path)
 }
 
+// GetPIDPath returns the path to the nginx PID file
 func GetPIDPath() (path string) {
 	if settings.NginxSettings.PIDPath == "" {
 		out := getNginxV()
@@ -131,9 +146,10 @@ func GetPIDPath() (path string) {
 	return resolvePath(path)
 }
 
+// GetSbinPath returns the path to the nginx executable
 func GetSbinPath() (path string) {
 	out := getNginxV()
-	r, _ := regexp.Compile("--sbin-path=(\\S+)")
+	r, _ := regexp.Compile(`--sbin-path=(\S+)`)
 	match := r.FindStringSubmatch(out)
 	if len(match) < 1 {
 		logger.Error("nginx.GetPIDPath len(match) < 1")
@@ -144,10 +160,11 @@ func GetSbinPath() (path string) {
 	return resolvePath(path)
 }
 
+// GetAccessLogPath returns the path to the nginx access log file
 func GetAccessLogPath() (path string) {
 	if settings.NginxSettings.AccessLogPath == "" {
 		out := getNginxV()
-		r, _ := regexp.Compile("--http-log-path=(\\S+)")
+		r, _ := regexp.Compile(`--http-log-path=(\S+)`)
 		match := r.FindStringSubmatch(out)
 		if len(match) < 1 {
 			logger.Error("nginx.GetAccessLogPath len(match) < 1")
@@ -161,10 +178,11 @@ func GetAccessLogPath() (path string) {
 	return resolvePath(path)
 }
 
+// GetErrorLogPath returns the path to the nginx error log file
 func GetErrorLogPath() string {
 	if settings.NginxSettings.ErrorLogPath == "" {
 		out := getNginxV()
-		r, _ := regexp.Compile("--error-log-path=(\\S+)")
+		r, _ := regexp.Compile(`--error-log-path=(\S+)`)
 		match := r.FindStringSubmatch(out)
 		if len(match) < 1 {
 			logger.Error("nginx.GetErrorLogPath len(match) < 1")

+ 22 - 8
internal/nginx_log/log_cache.go

@@ -6,9 +6,10 @@ import (
 
 // NginxLogCache represents a cached log entry from nginx configuration
 type NginxLogCache struct {
-	Path string `json:"path"` // Path to the log file
-	Type string `json:"type"` // Type of log: "access" or "error"
-	Name string `json:"name"` // Name of the log file
+	Path       string `json:"path"`        // Path to the log file
+	Type       string `json:"type"`        // Type of log: "access" or "error"
+	Name       string `json:"name"`        // Name of the log file
+	ConfigFile string `json:"config_file"` // Path to the configuration file that contains this log directive
 }
 
 var (
@@ -17,15 +18,28 @@ var (
 	cacheMutex sync.RWMutex
 )
 
-// AddLogPath adds a log path to the log cache
-func AddLogPath(path, logType, name string) {
+// AddLogPath adds a log path to the log cache with the source config file
+func AddLogPath(path, logType, name, configFile string) {
 	cacheMutex.Lock()
 	defer cacheMutex.Unlock()
 
 	logCache[path] = &NginxLogCache{
-		Path: path,
-		Type: logType,
-		Name: name,
+		Path:       path,
+		Type:       logType,
+		Name:       name,
+		ConfigFile: configFile,
+	}
+}
+
+// RemoveLogPathsFromConfig removes all log paths that come from a specific config file
+func RemoveLogPathsFromConfig(configFile string) {
+	cacheMutex.Lock()
+	defer cacheMutex.Unlock()
+
+	for path, cache := range logCache {
+		if cache.ConfigFile == configFile {
+			delete(logCache, path)
+		}
 	}
 }
 

+ 51 - 3
internal/nginx_log/nginx_log.go

@@ -5,6 +5,7 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
+	"strings"
 
 	"github.com/0xJacky/Nginx-UI/internal/cache"
 	"github.com/0xJacky/Nginx-UI/internal/helper"
@@ -24,24 +25,39 @@ func init() {
 
 // scanForLogDirectives scans and parses configuration files for log directives
 func scanForLogDirectives(configPath string, content []byte) error {
+	// First, remove all log paths that came from this config file
+	// This ensures that removed log directives are properly cleaned up
+	RemoveLogPathsFromConfig(configPath)
+
 	// Find log directives using regex
 	matches := logDirectiveRegex.FindAllSubmatch(content, -1)
 
+	prefix := nginx.GetPrefix()
 	// Parse log paths
 	for _, match := range matches {
 		if len(match) >= 3 {
+			// Check if this match is from a commented line
+			if isCommentedMatch(content, match) {
+				continue // Skip commented directives
+			}
+
 			directiveType := string(match[1]) // "access_log" or "error_log"
 			logPath := string(match[2])       // Path to log file
 
+			// Handle relative paths by joining with nginx prefix
+			if !filepath.IsAbs(logPath) {
+				logPath = filepath.Join(prefix, logPath)
+			}
+
 			// Validate log path
-			if IsLogPathUnderWhiteList(logPath) && isValidLogPath(logPath) {
+			if isValidLogPath(logPath) {
 				logType := "access"
 				if directiveType == "error_log" {
 					logType = "error"
 				}
 
-				// Add to cache
-				AddLogPath(logPath, logType, filepath.Base(logPath))
+				// Add to cache with config file path
+				AddLogPath(logPath, logType, filepath.Base(logPath), configPath)
 			}
 		}
 	}
@@ -49,6 +65,35 @@ func scanForLogDirectives(configPath string, content []byte) error {
 	return nil
 }
 
+// isCommentedMatch checks if a regex match is from a commented line
+func isCommentedMatch(content []byte, match [][]byte) bool {
+	// Find the position of the match in the content
+	matchStr := string(match[0])
+	matchIndex := strings.Index(string(content), matchStr)
+	if matchIndex == -1 {
+		return false
+	}
+
+	// Find the start of the line containing this match
+	lineStart := matchIndex
+	for lineStart > 0 && content[lineStart-1] != '\n' {
+		lineStart--
+	}
+
+	// Check if the line starts with # (possibly with leading whitespace)
+	for i := lineStart; i < matchIndex; i++ {
+		char := content[i]
+		if char == '#' {
+			return true // This is a commented line
+		}
+		if char != ' ' && char != '\t' {
+			return false // Found non-whitespace before the directive, not a comment
+		}
+	}
+
+	return false
+}
+
 // GetAllLogs returns all log paths
 func GetAllLogs(filters ...func(*NginxLogCache) bool) []*NginxLogCache {
 	return GetAllLogPaths(filters...)
@@ -116,6 +161,9 @@ func IsLogPathUnderWhiteList(path string) bool {
 	if errorLogPath != "" {
 		logDirWhiteList = append(logDirWhiteList, filepath.Dir(errorLogPath))
 	}
+	if nginx.GetPrefix() != "" {
+		logDirWhiteList = append(logDirWhiteList, nginx.GetPrefix())
+	}
 
 	// No cache, check it
 	if !ok {

+ 354 - 0
internal/nginx_log/nginx_log_test.go

@@ -0,0 +1,354 @@
+package nginx_log
+
+import (
+	"testing"
+)
+
+// TestScanForLogDirectivesRemoval tests that removed log directives are properly cleaned up
+func TestScanForLogDirectivesRemoval(t *testing.T) {
+	// Clear cache before test
+	ClearLogCache()
+
+	configPath := "/etc/nginx/sites-available/test.conf"
+
+	// First scan with two log directives
+	content1 := []byte(`
+server {
+    listen 80;
+    server_name example.com;
+    
+    access_log /var/log/nginx/access.log;
+    error_log /var/log/nginx/error.log;
+}
+`)
+
+	err := scanForLogDirectives(configPath, content1)
+	if err != nil {
+		t.Fatalf("First scan failed: %v", err)
+	}
+
+	// Check that both logs are cached
+	logs := GetAllLogPaths()
+	if len(logs) != 2 {
+		t.Fatalf("Expected 2 logs after first scan, got %d", len(logs))
+	}
+
+	// Verify the config file is tracked
+	accessFound := false
+	errorFound := false
+	for _, log := range logs {
+		if log.ConfigFile != configPath {
+			t.Errorf("Expected config file %s, got %s", configPath, log.ConfigFile)
+		}
+		if log.Type == "access" {
+			accessFound = true
+		}
+		if log.Type == "error" {
+			errorFound = true
+		}
+	}
+
+	if !accessFound || !errorFound {
+		t.Error("Expected both access and error logs to be found")
+	}
+
+	// Second scan with only one log directive (error_log removed)
+	content2 := []byte(`
+server {
+    listen 80;
+    server_name example.com;
+    
+    access_log /var/log/nginx/access.log;
+}
+`)
+
+	err = scanForLogDirectives(configPath, content2)
+	if err != nil {
+		t.Fatalf("Second scan failed: %v", err)
+	}
+
+	// Check that only access log remains
+	logs = GetAllLogPaths()
+	if len(logs) != 1 {
+		t.Fatalf("Expected 1 log after second scan, got %d", len(logs))
+	}
+
+	if logs[0].Type != "access" {
+		t.Errorf("Expected remaining log to be access log, got %s", logs[0].Type)
+	}
+
+	// Third scan with no log directives
+	content3 := []byte(`
+server {
+    listen 80;
+    server_name example.com;
+}
+`)
+
+	err = scanForLogDirectives(configPath, content3)
+	if err != nil {
+		t.Fatalf("Third scan failed: %v", err)
+	}
+
+	// Check that no logs remain
+	logs = GetAllLogPaths()
+	if len(logs) != 0 {
+		t.Fatalf("Expected 0 logs after third scan, got %d", len(logs))
+	}
+}
+
+// TestScanForLogDirectivesMultipleConfigs tests that logs from different config files are handled independently
+func TestScanForLogDirectivesMultipleConfigs(t *testing.T) {
+	// Clear cache before test
+	ClearLogCache()
+
+	configPath1 := "/etc/nginx/sites-available/site1.conf"
+	configPath2 := "/etc/nginx/sites-available/site2.conf"
+
+	// Scan first config
+	content1 := []byte(`
+server {
+    listen 80;
+    server_name site1.com;
+    access_log /var/log/nginx/site1_access.log;
+}
+`)
+
+	err := scanForLogDirectives(configPath1, content1)
+	if err != nil {
+		t.Fatalf("First config scan failed: %v", err)
+	}
+
+	// Scan second config
+	content2 := []byte(`
+server {
+    listen 80;
+    server_name site2.com;
+    access_log /var/log/nginx/site2_access.log;
+}
+`)
+
+	err = scanForLogDirectives(configPath2, content2)
+	if err != nil {
+		t.Fatalf("Second config scan failed: %v", err)
+	}
+
+	// Should have 2 logs total
+	logs := GetAllLogPaths()
+	if len(logs) != 2 {
+		t.Fatalf("Expected 2 logs from 2 configs, got %d", len(logs))
+	}
+
+	// Remove log from first config, should only affect that config
+	emptyContent := []byte(`
+server {
+    listen 80;
+    server_name site1.com;
+}
+`)
+
+	err = scanForLogDirectives(configPath1, emptyContent)
+	if err != nil {
+		t.Fatalf("Empty config scan failed: %v", err)
+	}
+
+	// Should have 1 log remaining (from config2)
+	logs = GetAllLogPaths()
+	if len(logs) != 1 {
+		t.Fatalf("Expected 1 log after removing from config1, got %d", len(logs))
+	}
+
+	if logs[0].ConfigFile != configPath2 {
+		t.Errorf("Expected remaining log to be from config2 (%s), got %s", configPath2, logs[0].ConfigFile)
+	}
+}
+
+// TestScanForLogDirectivesIgnoreComments tests that commented log directives are ignored
+func TestScanForLogDirectivesIgnoreComments(t *testing.T) {
+	// Clear cache before test
+	ClearLogCache()
+
+	configPath := "/etc/nginx/sites-available/test.conf"
+
+	// Content with both active and commented log directives
+	content := []byte(`
+server {
+    listen 80;
+    server_name example.com;
+    
+    # This is a commented access log - should be ignored
+    # access_log /var/log/nginx/commented_access.log;
+    
+    # Multi-line comment block
+    #error_log /var/log/nginx/commented_error.log;
+    
+    # Active log directives (not commented)
+    access_log /var/log/nginx/active_access.log;
+    error_log /var/log/nginx/active_error.log;
+    
+    # Another commented directive with indentation
+        # access_log /var/log/nginx/indented_comment.log;
+    
+    # Inline comment after directive should still work
+    access_log /var/log/nginx/inline_comment.log; # this is active with comment
+}
+`)
+
+	err := scanForLogDirectives(configPath, content)
+	if err != nil {
+		t.Fatalf("Scan failed: %v", err)
+	}
+
+	// Should only find 3 active log directives (not the commented ones)
+	logs := GetAllLogPaths()
+	expectedCount := 3
+	if len(logs) != expectedCount {
+		t.Fatalf("Expected %d logs, got %d. Logs found: %+v", expectedCount, len(logs), logs)
+	}
+
+	// Verify the correct paths were found
+	expectedPaths := map[string]bool{
+		"/var/log/nginx/active_access.log":  false,
+		"/var/log/nginx/active_error.log":   false,
+		"/var/log/nginx/inline_comment.log": false,
+	}
+
+	for _, log := range logs {
+		if _, exists := expectedPaths[log.Path]; !exists {
+			t.Errorf("Unexpected log path found: %s", log.Path)
+		} else {
+			expectedPaths[log.Path] = true
+		}
+	}
+
+	// Check that all expected paths were found
+	for path, found := range expectedPaths {
+		if !found {
+			t.Errorf("Expected log path not found: %s", path)
+		}
+	}
+
+	// Verify no commented paths were included
+	commentedPaths := []string{
+		"/var/log/nginx/commented_access.log",
+		"/var/log/nginx/commented_error.log",
+		"/var/log/nginx/indented_comment.log",
+	}
+
+	for _, log := range logs {
+		for _, commentedPath := range commentedPaths {
+			if log.Path == commentedPath {
+				t.Errorf("Commented log path should not be included: %s", commentedPath)
+			}
+		}
+	}
+}
+
+// TestLogDirectiveRegex tests the regex pattern and comment filtering logic
+func TestLogDirectiveRegex(t *testing.T) {
+	testCases := []struct {
+		name           string
+		content        string
+		expectedActive int // number of active (non-commented) matches expected
+	}{
+		{
+			name:           "Active directives",
+			content:        "access_log /var/log/nginx/access.log;\nerror_log /var/log/nginx/error.log;",
+			expectedActive: 2,
+		},
+		{
+			name:           "Commented directives",
+			content:        "# access_log /var/log/nginx/access.log;\n#error_log /var/log/nginx/error.log;",
+			expectedActive: 0,
+		},
+		{
+			name:           "Mixed active and commented",
+			content:        "access_log /var/log/nginx/access.log;\n# error_log /var/log/nginx/error.log;",
+			expectedActive: 1,
+		},
+		{
+			name:           "Indented comments",
+			content:        "    # access_log /var/log/nginx/access.log;\n    error_log /var/log/nginx/error.log;",
+			expectedActive: 1,
+		},
+		{
+			name:           "Inline comments after directive",
+			content:        "access_log /var/log/nginx/access.log; # this is a comment",
+			expectedActive: 1,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			// Find all matches using the regex
+			matches := logDirectiveRegex.FindAllSubmatch([]byte(tc.content), -1)
+
+			// Count how many are not commented
+			activeCount := 0
+			for _, match := range matches {
+				if !isCommentedMatch([]byte(tc.content), match) {
+					activeCount++
+				}
+			}
+
+			if activeCount != tc.expectedActive {
+				t.Errorf("Test '%s': expected %d active matches, got %d. Content: %s",
+					tc.name, tc.expectedActive, activeCount, tc.content)
+			}
+		})
+	}
+}
+
+// TestIsCommentedMatch tests the isCommentedMatch function directly
+func TestIsCommentedMatch(t *testing.T) {
+	testCases := []struct {
+		name        string
+		content     string
+		matchStr    string
+		isCommented bool
+	}{
+		{
+			name:        "Not commented",
+			content:     "access_log /var/log/nginx/access.log;",
+			matchStr:    "access_log /var/log/nginx/access.log;",
+			isCommented: false,
+		},
+		{
+			name:        "Commented with #",
+			content:     "# access_log /var/log/nginx/access.log;",
+			matchStr:    "access_log /var/log/nginx/access.log;",
+			isCommented: true,
+		},
+		{
+			name:        "Commented with spaces and #",
+			content:     "    # access_log /var/log/nginx/access.log;",
+			matchStr:    "access_log /var/log/nginx/access.log;",
+			isCommented: true,
+		},
+		{
+			name:        "Not commented with spaces",
+			content:     "    access_log /var/log/nginx/access.log;",
+			matchStr:    "access_log /var/log/nginx/access.log;",
+			isCommented: false,
+		},
+		{
+			name:        "Inline comment after directive",
+			content:     "access_log /var/log/nginx/access.log; # comment",
+			matchStr:    "access_log /var/log/nginx/access.log;",
+			isCommented: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			// Create a fake match to test with
+			match := [][]byte{[]byte(tc.matchStr)}
+			result := isCommentedMatch([]byte(tc.content), match)
+
+			if result != tc.isCommented {
+				t.Errorf("Test '%s': expected isCommented=%v, got %v. Content: %q, Match: %q",
+					tc.name, tc.isCommented, result, tc.content, tc.matchStr)
+			}
+		})
+	}
+}