Fix include directive parsing to handle relative paths (#1174)

Author: dhurley

internal/datasource/config/nginx_config_parser.go

@@ -142,7 +142,7 @@ func (ncp *NginxConfigParser) createNginxConfigContext(
 			func(ctx context.Context, parent, directive *crossplane.Directive) error {
 				switch directive.Directive {
 				case "include":
-					include := ncp.parseIncludeDirective(directive)
+					include := ncp.parseIncludeDirective(directive, &conf)
 
 					nginxConfigContext.Includes = append(nginxConfigContext.Includes, include)
 				case "log_format":
@@ -267,12 +267,15 @@ func (ncp *NginxConfigParser) findLocalSysLogServers(sysLogServer string) string
 	return ""
 }
 
-func (ncp *NginxConfigParser) parseIncludeDirective(directive *crossplane.Directive) string {
+func (ncp *NginxConfigParser) parseIncludeDirective(
+	directive *crossplane.Directive,
+	configFile *crossplane.Config,
+) string {
 	var include string
 	if filepath.IsAbs(directive.Args[0]) {
 		include = directive.Args[0]
 	} else {
-		include = filepath.Join(filepath.Dir(directive.File), directive.Args[0])
+		include = filepath.Join(filepath.Dir(configFile.File), directive.Args[0])
 	}
 
 	return include

internal/datasource/config/nginx_config_parser_test.go

@@ -507,7 +507,7 @@ func TestNginxConfigParser_Parse(t *testing.T) {
 			allowedDirectories: []string{dir},
 		},
 		{
-			name:     "Test 10: Check with multiple syslog servers",
+			name:     "Test 10: Available NAP syslog server",
 			instance: protos.NginxPlusInstance([]string{}),
 			content: testconfig.NginxConfigWithMultipleSysLogs(errorLog.Name(), accessLog.Name(),
 				"192.168.12.34:1517", "my.domain.com:1517", "127.0.0.1:1515"),
@@ -521,7 +521,7 @@ func TestNginxConfigParser_Parse(t *testing.T) {
 			expectedLog:        "Found valid NAP syslog server",
 		},
 		{
-			name:     "Test 10: Check with multiple syslog servers",
+			name:     "Test 11: Unavailable NAP syslog server",
 			instance: protos.NginxPlusInstance([]string{}),
 			content: testconfig.NginxConfigWithMultipleSysLogs(errorLog.Name(), accessLog.Name(),
 				"192.168.12.34:1517", "my.domain.com:1517", "not.allowed:1515"),
@@ -1521,6 +1521,62 @@ func TestNginxConfigParser_checkDuplicate(t *testing.T) {
 	}
 }
 
+func TestNginxConfigParser_parseIncludeDirective(t *testing.T) {
+	parser := NewNginxConfigParser(types.AgentConfig())
+
+	tests := []struct {
+		name     string
+		confFile string
+		expected string
+		args     []string
+	}{
+		{
+			name:     "Test 1: relative path",
+			args:     []string{"test.conf"},
+			confFile: "/etc/nginx/nginx.conf",
+			expected: "/etc/nginx/test.conf",
+		},
+		{
+			name:     "Test 2: absolute path",
+			args:     []string{"/usr/local/nginx/conf/vhost.conf"},
+			confFile: "/etc/nginx/nginx.conf",
+			expected: "/usr/local/nginx/conf/vhost.conf",
+		},
+		{
+			name:     "Test 3: wildcard",
+			args:     []string{"/etc/nginx/conf.d/*.conf"},
+			confFile: "/etc/nginx/nginx.conf",
+			expected: "/etc/nginx/conf.d/*.conf",
+		},
+		{
+			name:     "Test 4: relative path with subdirectory",
+			args:     []string{"conf.d/default.conf"},
+			confFile: "/etc/nginx/nginx.conf",
+			expected: "/etc/nginx/conf.d/default.conf",
+		},
+		{
+			name:     "Test 5: parent directory reference",
+			args:     []string{"../sites-enabled/*.conf"},
+			confFile: "/etc/nginx/conf.d/nginx.conf",
+			expected: "/etc/nginx/sites-enabled/*.conf",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			include := parser.parseIncludeDirective(
+				&crossplane.Directive{
+					Args: tc.args,
+				},
+				&crossplane.Config{
+					File: tc.confFile,
+				},
+			)
+			assert.Equal(t, tc.expected, include)
+		})
+	}
+}
+
 func protoListEqual(protoListA, protoListB []*mpi.File) bool {
 	for i := range protoListA {
 		res := proto.Equal(protoListA[i], protoListB[i])

internal/watcher/file/file_watcher_service.go

@@ -138,8 +138,12 @@ func (fws *FileWatcherService) addWatchers(ctx context.Context) {
 		}
 
 		if !slices.Contains(fws.watcher.WatchList(), directory) {
-			fws.addWatcher(ctx, directory)
-			fws.filesChanged.Store(true)
+			err := fws.addWatcher(ctx, directory)
+			if err != nil {
+				slog.DebugContext(ctx, "Failed to add file watcher", "directory", directory, "error", err)
+			} else {
+				fws.filesChanged.Store(true)
+			}
 		}
 	}
 }
@@ -208,7 +212,7 @@ func (fws *FileWatcherService) checkForUpdates(ctx context.Context, ch chan<- Fi
 	}
 }
 
-func (fws *FileWatcherService) addWatcher(ctx context.Context, directory string) {
+func (fws *FileWatcherService) addWatcher(ctx context.Context, directory string) error {
 	slog.DebugContext(ctx, "Checking if file watcher needs to be added", "directory", directory)
 
 	if _, err := os.Stat(directory); errors.Is(err, os.ErrNotExist) {
@@ -220,9 +224,7 @@ func (fws *FileWatcherService) addWatcher(ctx context.Context, directory string)
 
 	slog.DebugContext(ctx, "Adding watcher", "directory", directory)
 
-	if err := fws.watcher.Add(directory); err != nil {
-		slog.WarnContext(ctx, "Failed to add file watcher", "directory", directory, "error", err)
-	}
+	return fws.watcher.Add(directory)
 }
 
 func (fws *FileWatcherService) removeWatcher(ctx context.Context, path string) {

internal/watcher/file/file_watcher_service_test.go

@@ -62,7 +62,7 @@ func TestFileWatcherService_addWatcher(t *testing.T) {
 	require.NoError(t, err)
 	defer os.Remove(testDirectory)
 
-	fileWatcherService.addWatcher(ctx, testDirectory)
+	require.NoError(t, fileWatcherService.addWatcher(ctx, testDirectory))
 
 	directoriesBeingWatched := fileWatcherService.watcher.WatchList()
 	assert.Len(t, directoriesBeingWatched, 1)
@@ -79,7 +79,7 @@ func TestFileWatcherService_addWatcher_Error(t *testing.T) {
 	tempDir := t.TempDir()
 	testDirectory := path.Join(tempDir, "test_dir")
 
-	fileWatcherService.addWatcher(ctx, testDirectory)
+	require.Error(t, fileWatcherService.addWatcher(ctx, testDirectory))
 
 	directoriesBeingWatched := fileWatcherService.watcher.WatchList()
 	assert.Empty(t, directoriesBeingWatched)