fix(proxy): Prevent duplicate domains and restore vhost on failure

internal/api/domains_test.go

@@ -381,4 +381,74 @@ func TestAddDomain(t *testing.T) {
 			t.Error("expected domain ID to be generated")
 		}
 	})
+
+	t.Run("rejects duplicate domain", func(t *testing.T) {
+		createTestDeployment(t, tmpDir, "duplicate-domain", &models.ServiceMetadata{
+			Name: "duplicate-domain",
+			Type: "web",
+			Domains: []models.DomainConfig{
+				{
+					ID:            "existing-1",
+					Service:       "web",
+					ContainerPort: 80,
+					Domain:        "existing.example.com",
+				},
+			},
+		})
+
+		duplicateDomain := models.DomainConfig{
+			Service:       "web",
+			ContainerPort: 8080,
+			Domain:        "existing.example.com",
+		}
+		body, _ := json.Marshal(duplicateDomain)
+
+		w := httptest.NewRecorder()
+		c, _ := gin.CreateTestContext(w)
+		c.Params = gin.Params{{Key: "name", Value: "duplicate-domain"}}
+		c.Request = httptest.NewRequest("POST", "/", bytes.NewReader(body))
+		c.Request.Header.Set("Content-Type", "application/json")
+
+		server.addDomain(c)
+
+		if w.Code != http.StatusConflict {
+			t.Errorf("expected status 409 Conflict, got %d: %s", w.Code, w.Body.String())
+		}
+	})
+
+	t.Run("allows same domain with different path prefix", func(t *testing.T) {
+		createTestDeployment(t, tmpDir, "different-path", &models.ServiceMetadata{
+			Name: "different-path",
+			Type: "web",
+			Domains: []models.DomainConfig{
+				{
+					ID:            "existing-1",
+					Service:       "api",
+					ContainerPort: 8080,
+					Domain:        "app.example.com",
+					PathPrefix:    "/api",
+				},
+			},
+		})
+
+		newDomain := models.DomainConfig{
+			Service:       "web",
+			ContainerPort: 80,
+			Domain:        "app.example.com",
+			PathPrefix:    "/web",
+		}
+		body, _ := json.Marshal(newDomain)
+
+		w := httptest.NewRecorder()
+		c, _ := gin.CreateTestContext(w)
+		c.Params = gin.Params{{Key: "name", Value: "different-path"}}
+		c.Request = httptest.NewRequest("POST", "/", bytes.NewReader(body))
+		c.Request.Header.Set("Content-Type", "application/json")
+
+		server.addDomain(c)
+
+		if w.Code != http.StatusCreated {
+			t.Errorf("expected status 201 Created, got %d: %s", w.Code, w.Body.String())
+		}
+	})
 }

internal/api/server.go

@@ -3470,6 +3470,15 @@ func (s *Server) addDomain(c *gin.Context) {
 		deployment.Metadata.Domains = []models.DomainConfig{existingDomain}
 	}
 
+	for _, existing := range deployment.Metadata.Domains {
+		if existing.Domain == domain.Domain && existing.PathPrefix == domain.PathPrefix {
+			c.JSON(http.StatusConflict, gin.H{
+				"error": fmt.Sprintf("Domain %s%s already exists", domain.Domain, domain.PathPrefix),
+			})
+			return
+		}
+	}
+
 	deployment.Metadata.Domains = append(deployment.Metadata.Domains, domain)
 
 	if err := s.manager.SaveMetadata(name, deployment.Metadata); err != nil {

internal/nginx/manager.go

@@ -3,6 +3,7 @@ package nginx
 import (
 	"bytes"
 	"fmt"
+	"log"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -120,6 +121,14 @@ func (m *Manager) GetVirtualHost(deploymentName string) (string, error) {
 	return string(data), nil
 }
 
+func (m *Manager) WriteVirtualHost(deploymentName string, content string) error {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+
+	configFile := filepath.Join(m.configPath, deploymentName+".conf")
+	return os.WriteFile(configFile, []byte(content), 0644)
+}
+
 func (m *Manager) UpdateVirtualHost(deployment *models.Deployment) error {
 	return m.CreateVirtualHost(deployment)
 }
@@ -229,12 +238,23 @@ func (m *Manager) TestConfig() error {
 	cmd := exec.Command("docker", "exec", m.config.ContainerName, "nginx", "-t")
 	output, err := cmd.CombinedOutput()
 	if err != nil {
-		return fmt.Errorf("nginx config test failed: %s - %w", string(output), err)
+		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)
 	}
 
 	return nil
 }
 
+func isNginxConfigValid(output string) bool {
+	hasError := strings.Contains(output, "[emerg]") || strings.Contains(output, "[error]")
+	hasSuccess := strings.Contains(output, "syntax is ok") || strings.Contains(output, "test is successful")
+	return !hasError && hasSuccess
+}
+
 func (m *Manager) waitForContainerReady(maxRetries int) error {
 	for i := 0; i < maxRetries; i++ {
 		cmd := exec.Command("docker", "inspect", "-f", "{{.State.Status}}", m.config.ContainerName)
@@ -433,6 +453,7 @@ func (m *Manager) groupDomainsByHost(domains []models.DomainConfig, deploymentNa
 		})
 
 		var locations []locationData
+		seenPaths := make(map[string]bool)
 		hasSSL := false
 		sslDomain := host
 		var serverAliases []string
@@ -443,6 +464,12 @@ func (m *Manager) groupDomainsByHost(domains []models.DomainConfig, deploymentNa
 				path = "/"
 			}
 
+			if seenPaths[path] {
+				log.Printf("warning: skipping duplicate location %q for host %q", path, host)
+				continue
+			}
+			seenPaths[path] = true
+
 			service := d.Service
 			if service == "" {
 				service = deploymentName

internal/nginx/manager_test.go

@@ -1719,3 +1719,111 @@ func TestUpdateDeploymentRateLimits(t *testing.T) {
 		t.Error("rate_limits.conf should still contain app2")
 	}
 }
+
+func TestWriteVirtualHost(t *testing.T) {
+	tmpDir, err := os.MkdirTemp("", "nginx-test-*")
+	if err != nil {
+		t.Fatalf("failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	m := NewManager(&config.NginxConfig{
+		ConfigPath: tmpDir,
+	}, "/deployments", "")
+
+	content := "# test nginx config\nserver { listen 80; }"
+	if err := m.WriteVirtualHost("test-app", content); err != nil {
+		t.Fatalf("WriteVirtualHost failed: %v", err)
+	}
+
+	configFile := filepath.Join(tmpDir, "test-app.conf")
+	data, err := os.ReadFile(configFile)
+	if err != nil {
+		t.Fatalf("failed to read config file: %v", err)
+	}
+
+	if string(data) != content {
+		t.Errorf("expected content %q, got %q", content, string(data))
+	}
+
+	readContent, err := m.GetVirtualHost("test-app")
+	if err != nil {
+		t.Fatalf("GetVirtualHost failed: %v", err)
+	}
+
+	if readContent != content {
+		t.Errorf("GetVirtualHost returned %q, expected %q", readContent, content)
+	}
+}
+
+func TestIsNginxConfigValid(t *testing.T) {
+	tests := []struct {
+		name   string
+		output string
+		want   bool
+	}{
+		{
+			name:   "valid config no warnings",
+			output: "nginx: the configuration file /etc/nginx/nginx.conf syntax is ok\nnginx: configuration file /etc/nginx/nginx.conf test is successful",
+			want:   true,
+		},
+		{
+			name:   "valid config with ssl_stapling warning",
+			output: "2026/02/03 17:33:27 [warn] 2572#2572: \"ssl_stapling\" ignored\nnginx: the configuration file /etc/nginx/nginx.conf syntax is ok\nnginx: configuration file /etc/nginx/nginx.conf test is successful",
+			want:   true,
+		},
+		{
+			name:   "invalid config with emerg error",
+			output: "nginx: [emerg] unknown directive \"invalid\" in /etc/nginx/conf.d/test.conf:1\nnginx: configuration file /etc/nginx/nginx.conf test failed",
+			want:   false,
+		},
+		{
+			name:   "invalid config with error",
+			output: "nginx: [error] cannot load certificate\nnginx: configuration file /etc/nginx/nginx.conf test failed",
+			want:   false,
+		},
+		{
+			name:   "no success indicator",
+			output: "[warn] some warning",
+			want:   false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := isNginxConfigValid(tt.output)
+			if got != tt.want {
+				t.Errorf("isNginxConfigValid() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestGroupDomainsByHost_DeduplicatesLocations(t *testing.T) {
+	m := NewManager(&config.NginxConfig{}, "/deployments", "")
+
+	domains := []models.DomainConfig{
+		{ID: "1", Domain: "example.com", PathPrefix: "", ContainerPort: 80, Service: "web"},
+		{ID: "2", Domain: "example.com", PathPrefix: "", ContainerPort: 8080, Service: "api"},
+		{ID: "3", Domain: "example.com", PathPrefix: "/api", ContainerPort: 3000, Service: "backend"},
+	}
+
+	servers := m.groupDomainsByHost(domains, "test-app")
+
+	if len(servers) != 1 {
+		t.Fatalf("expected 1 server, got %d", len(servers))
+	}
+
+	if len(servers[0].Locations) != 2 {
+		t.Errorf("expected 2 unique locations, got %d", len(servers[0].Locations))
+	}
+
+	pathCounts := make(map[string]int)
+	for _, loc := range servers[0].Locations {
+		pathCounts[loc.Path]++
+	}
+
+	if pathCounts["/"] != 1 {
+		t.Errorf("expected exactly 1 location for '/', got %d", pathCounts["/"])
+	}
+}

internal/proxy/interfaces.go

@@ -10,6 +10,8 @@ type NginxManager interface {
 	UpdateVirtualHost(deployment *models.Deployment) error
 	DeleteVirtualHost(deploymentName string) error
 	VirtualHostExists(deploymentName string) bool
+	GetVirtualHost(deploymentName string) (string, error)
+	WriteVirtualHost(deploymentName string, content string) error
 	TestConfig() error
 	Reload() error
 }

internal/proxy/orchestrator.go

@@ -88,13 +88,30 @@ func (o *Orchestrator) setupMultiDomainDeployment(deployment *models.Deployment,
 	}
 	deployment.Metadata.Domains = domains
 
+	var previousConfig string
+	hadPreviousConfig := o.nginx.VirtualHostExists(deployment.Name)
+	if hadPreviousConfig {
+		var err error
+		previousConfig, err = o.nginx.GetVirtualHost(deployment.Name)
+		if err != nil {
+			log.Printf("warning: failed to backup previous vhost config: %v", err)
+			hadPreviousConfig = false
+		}
+	}
+
 	if err := o.nginx.CreateVirtualHost(deployment); err != nil {
 		return nil, fmt.Errorf("failed to create virtual host: %w", err)
 	}
 	result.VirtualHostCreated = true
 
 	if err := o.nginx.TestConfig(); err != nil {
-		_ = o.nginx.DeleteVirtualHost(deployment.Name)
+		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)
+		}
 		return nil, fmt.Errorf("nginx config validation failed: %w", err)
 	}
 

internal/proxy/orchestrator_test.go

@@ -12,9 +12,12 @@ type mockNginxManager struct {
 	createVirtualHostCalls   []string
 	updateVirtualHostCalls   []string
 	deleteVirtualHostCalls   []string
+	getVirtualHostCalls      []string
+	writeVirtualHostCalls    []string
 	testConfigCalls          int
 	reloadCalls              int
 	virtualHostExistsReturns map[string]bool
+	virtualHostContents      map[string]string
 	createVirtualHostErr     error
 	testConfigErr            error
 	testConfigErrCount       int
@@ -46,6 +49,23 @@ func (m *mockNginxManager) VirtualHostExists(deploymentName string) bool {
 	return m.virtualHostExistsReturns[deploymentName]
 }
 
+func (m *mockNginxManager) GetVirtualHost(deploymentName string) (string, error) {
+	m.getVirtualHostCalls = append(m.getVirtualHostCalls, deploymentName)
+	if m.virtualHostContents == nil {
+		return "", nil
+	}
+	return m.virtualHostContents[deploymentName], nil
+}
+
+func (m *mockNginxManager) WriteVirtualHost(deploymentName string, content string) error {
+	m.writeVirtualHostCalls = append(m.writeVirtualHostCalls, deploymentName)
+	if m.virtualHostContents == nil {
+		m.virtualHostContents = make(map[string]string)
+	}
+	m.virtualHostContents[deploymentName] = content
+	return nil
+}
+
 func (m *mockNginxManager) TestConfig() error {
 	m.testConfigCalls++
 	if m.testConfigErrCount > 0 && m.testConfigCalls <= m.testConfigErrCount {