Browse Source

feat(nginx): add support for dynamically loaded modules and clear modules cache #1136

Jacky 1 month ago
parent
commit
e0cec2fedb
4 changed files with 149 additions and 5 deletions
  1. 1 1
      .devcontainer/Dockerfile
  2. 10 0
      .github/workflows/build.yml
  3. 47 4
      internal/nginx/modules.go
  4. 91 0
      internal/nginx/modules_test.go

+ 1 - 1
.devcontainer/Dockerfile

@@ -5,7 +5,7 @@ RUN apt-get update && \
   apt-get install -y --no-install-recommends curl gnupg2 ca-certificates lsb-release ubuntu-keyring jq cloc software-properties-common && \
   \
   # Add PPA repository for nginx-extras
-  add-apt-repository -y ppa:ondrej/nginx-mainline && \
+  add-apt-repository -y ppa:ondrej/nginx && \
   \
   # Update package information and install Nginx-extras
   apt-get update && \

+ 10 - 0
.github/workflows/build.yml

@@ -274,6 +274,16 @@ jobs:
           files: |
             ${{ env.DIST }}.tar.gz
             ${{ env.DIST }}.tar.gz.digest
+      
+      - name: Set up nodejs
+        uses: actions/setup-node@v4
+        with:
+          node-version: current
+    
+      - name: Install dependencies
+        run: |
+          corepack enable
+          corepack prepare pnpm@latest --activate
 
       - name: Upload to R2
         if: github.event_name != 'pull_request' && github.ref == 'refs/heads/dev'

+ 47 - 4
internal/nginx/modules.go

@@ -42,6 +42,11 @@ func clearModulesCache() {
 	lastPIDSize = 0
 }
 
+// ClearModulesCache clears the modules cache (public version for external use)
+func ClearModulesCache() {
+	clearModulesCache()
+}
+
 // isPIDFileChanged checks if the PID file has changed since the last check
 func isPIDFileChanged() bool {
 	pidPath := GetPIDPath()
@@ -82,6 +87,43 @@ func updatePIDFileInfo() {
 	}
 }
 
+// addLoadedDynamicModules discovers modules loaded via load_module statements
+// that might not be present in the configure arguments (e.g., externally installed modules)
+func addLoadedDynamicModules() {
+	// Get nginx -T output to find load_module statements
+	out := getNginxT()
+	if out == "" {
+		return
+	}
+
+	// Use the shared regex function to find loaded dynamic modules
+	loadModuleRe := GetLoadModuleRegex()
+	matches := loadModuleRe.FindAllStringSubmatch(out, -1)
+
+	modulesCacheLock.Lock()
+	defer modulesCacheLock.Unlock()
+
+	for _, match := range matches {
+		if len(match) > 1 {
+			// Extract the module name from load_module statement and normalize it
+			loadModuleName := match[1]
+			normalizedName := normalizeModuleNameFromLoadModule(loadModuleName)
+
+			// Check if this module is already in our cache
+			if _, exists := modulesCache.Get(normalizedName); !exists {
+				// This is a module that's loaded but not in configure args
+				// Add it as a dynamic module that's loaded
+				modulesCache.Set(normalizedName, &Module{
+					Name:    normalizedName,
+					Params:  "",
+					Dynamic: true, // Loaded via load_module, so it's dynamic
+					Loaded:  true, // We found it in load_module statements, so it's loaded
+				})
+			}
+		}
+	}
+}
+
 // updateDynamicModulesStatus checks which dynamic modules are actually loaded in the running Nginx
 func updateDynamicModulesStatus() {
 	modulesCacheLock.Lock()
@@ -279,6 +321,9 @@ func GetModules() *orderedmap.OrderedMap[string, *Module] {
 
 	modulesCacheLock.Unlock()
 
+	// Also check for modules loaded via load_module statements that might not be in configure args
+	addLoadedDynamicModules()
+
 	// Update dynamic modules status by checking if they're actually loaded
 	updateDynamicModulesStatus()
 
@@ -290,10 +335,8 @@ func GetModules() *orderedmap.OrderedMap[string, *Module] {
 
 // IsModuleLoaded checks if a module is loaded in Nginx
 func IsModuleLoaded(module string) bool {
-	// Ensure modules are in the cache
-	if modulesCache.Len() == 0 {
-		GetModules()
-	}
+	// Get fresh modules to ensure we have the latest state
+	GetModules()
 
 	modulesCacheLock.RLock()
 	defer modulesCacheLock.RUnlock()

+ 91 - 0
internal/nginx/modules_test.go

@@ -284,6 +284,97 @@ func TestRealWorldModuleMapping(t *testing.T) {
 	}
 }
 
+func TestAddLoadedDynamicModules(t *testing.T) {
+	// Test scenario: modules loaded via load_module but not in configure args
+	// This simulates the real-world case where external modules are installed
+	// and loaded dynamically without being compiled into nginx
+
+	// We can't directly test addLoadedDynamicModules since it depends on getNginxT()
+	// But we can test the logic by simulating the behavior
+
+	testLoadModuleOutput := `
+# Configuration file /etc/nginx/modules-enabled/50-mod-stream.conf:
+load_module modules/ngx_stream_module.so;
+# Configuration file /etc/nginx/modules-enabled/70-mod-stream-geoip2.conf:
+load_module modules/ngx_stream_geoip2_module.so;
+load_module "modules/ngx_http_geoip2_module.so";
+`
+
+	// Test the regex and normalization logic
+	loadModuleRe := GetLoadModuleRegex()
+	matches := loadModuleRe.FindAllStringSubmatch(testLoadModuleOutput, -1)
+
+	expectedModules := map[string]bool{
+		"stream":        false,
+		"stream_geoip2": false,
+		"http_geoip2":   false,
+	}
+
+	t.Logf("Found %d load_module matches", len(matches))
+
+	for _, match := range matches {
+		if len(match) > 1 {
+			loadModuleName := match[1]
+			normalizedName := normalizeModuleNameFromLoadModule(loadModuleName)
+
+			t.Logf("Load module: %s -> normalized: %s", loadModuleName, normalizedName)
+
+			if _, expected := expectedModules[normalizedName]; expected {
+				expectedModules[normalizedName] = true
+			} else {
+				t.Errorf("Unexpected module found: %s (from %s)", normalizedName, loadModuleName)
+			}
+		}
+	}
+
+	// Check that all expected modules were found
+	for moduleName, found := range expectedModules {
+		if !found {
+			t.Errorf("Expected module %s was not found", moduleName)
+		}
+	}
+}
+
+func TestExternalModuleDiscovery(t *testing.T) {
+	// Test the complete normalization pipeline for external modules
+	testCases := []struct {
+		name           string
+		loadModuleName string
+		expectedResult string
+	}{
+		{
+			name:           "stream_geoip2 module",
+			loadModuleName: "ngx_stream_geoip2_module",
+			expectedResult: "stream_geoip2",
+		},
+		{
+			name:           "http_geoip2 module",
+			loadModuleName: "ngx_http_geoip2_module",
+			expectedResult: "http_geoip2",
+		},
+		{
+			name:           "custom third-party module",
+			loadModuleName: "ngx_http_custom_module",
+			expectedResult: "http_custom",
+		},
+		{
+			name:           "simple module name",
+			loadModuleName: "ngx_custom_module",
+			expectedResult: "custom",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			result := normalizeModuleNameFromLoadModule(tc.loadModuleName)
+			if result != tc.expectedResult {
+				t.Errorf("normalizeModuleNameFromLoadModule(%s) = %s, expected %s",
+					tc.loadModuleName, result, tc.expectedResult)
+			}
+		})
+	}
+}
+
 func TestGetModuleMapping(t *testing.T) {
 	// This test verifies that GetModuleMapping function works without errors
 	// Since it depends on nginx being available, we'll just test that it doesn't panic