fix: Prevent duplicate domains

[nfebe]

No description provided.

[sourceant]

Code Review Summary

This pull request introduces a significant new feature: a comprehensive initial setup flow for the agent. This feature streamlines first-time configuration, including Docker environment validation, instance IP detection, domain/SSL setup, CORS configuration, and initial user creation. Beyond the new setup, the PR substantially enhances the robustness of the Nginx proxy management by implementing atomic configuration updates with rollback capabilities and intelligent handling of duplicate domain paths. Key dependencies have also been updated, improving overall stability.

🚀 Key Improvements

• ✨ Initial Setup Flow: A structured and guided initial setup experience for the agent, including critical environment validations and configuration steps.
• 🔒 Enhanced Nginx Configuration Reliability: Nginx config changes are now atomic, with automatic rollback to the previous valid configuration if a new one fails validation.
• 🛡️ Improved Security Configuration: Dynamic CORS management and mandatory username/password policies for initial admin user creation contribute to a more secure default state.
• ✅ Duplicate Domain/Path Handling: The proxy configuration now correctly deduplicates location blocks for the same domain and path prefix, preventing potential Nginx errors.
• ⬆️ Dependency Updates: go.mod and go.sum reflect important updates to github.com/docker/docker and golang.org/x/crypto, among others, ensuring the project uses more recent and potentially more secure/performant libraries.

💡 Minor Suggestions

• Review internal/proxy/orchestrator_test.go to refactor testableOrchestrator to test the actual Orchestrator more directly with mocked dependencies, reducing logic duplication in tests.
• Consider adding more comprehensive documentation for the new setup package's public API and internal design decisions.

[sourceant]

🔍 Code Review

💡 1. **internal/nginx/manager.go** (Lines 230-236) - BUG

The modification to TestConfig to explicitly check for [emerg] or [error] in Nginx output, while allowing warnings, is a robust improvement. This prevents Nginx from being reloaded with potentially problematic configurations while still allowing non-critical warnings (like ssl_stapling issues) to pass.

Suggested Code:

		outputStr := string(output)		if isNginxConfigValid(outputStr) {			log.Printf("nginx config test passed with warnings: %s", outputStr)			return nil		}		return fmt.Errorf("nginx config test failed: %s - %w", outputStr, err)

Current Code:

		return fmt.Errorf("nginx config test failed: %s - %w", string(output), err)

💡 2. **internal/nginx/manager.go** (Lines 443-447) - BUG

The addition of seenPaths to deduplicate location blocks within groupDomainsByHost is a critical fix. Without this, multiple domain configurations for the same domain and path prefix could lead to redundant location blocks in Nginx, which might cause unpredictable routing or Nginx configuration errors. The added logging for skipped duplicates is also helpful for debugging.

Suggested Code:

		seenPaths := make(map[string]bool)
		hasSSL := false

Current Code:

		hasSSL := false

💡 3. **internal/proxy/orchestrator.go** (Lines 108-115) - REFACTOR

This change significantly improves the robustness of proxy setup. By backing up the previous Nginx configuration and restoring it if the new configuration fails the TestConfig, the system ensures atomicity and prevents leaving Nginx in an invalid or broken state. This greatly enhances stability and fault tolerance.

Suggested Code:

		if hadPreviousConfig {
			if restoreErr := o.nginx.WriteVirtualHost(deployment.Name, previousConfig); restoreErr != nil {
				log.Printf("warning: failed to restore previous vhost config: %v", restoreErr)
			}
		} else {
			_ = o.nginx.DeleteVirtualHost(deployment.Name)
		}

Current Code:

		_ = o.nginx.DeleteVirtualHost(deployment.Name)

Verdict: APPROVE

Posted as a comment because posting a review failed.