Merge pull request #109 from flatrun/fix/certbot-non-interactive

fix(ssl): Add --non-interactive flag to certbot commands

Author: nfebe

VERSION

@@ -1 +1 @@
-0.1.55
+0.1.56

internal/api/ssl_deletion_test.go

@@ -54,7 +54,7 @@ func TestDeleteCertificate_BlockedWhenInUse(t *testing.T) {
 	}
 
 	nginxMgr := nginx.NewManager(&cfg.Nginx, tmpDir, "")
-	sslMgr := ssl.NewManager(&cfg.Certbot, tmpDir)
+	sslMgr := ssl.NewManager(&cfg.Certbot, tmpDir, nil)
 
 	orchestrator := proxy.NewOrchestratorWithManagers(nginxMgr, sslMgr)
 

internal/docker/discovery.go

@@ -28,6 +28,7 @@ type composeFile struct {
 type composeService struct {
 	Image    string        `yaml:"image"`
 	Ports    []interface{} `yaml:"ports"`
+	Expose   []interface{} `yaml:"expose"`
 	Networks []string      `yaml:"networks"`
 	Volumes  []string      `yaml:"volumes"`
 }
@@ -196,6 +197,12 @@ func (d *Discovery) parseComposeServices(composePath string) ([]models.Service,
 				service.Ports = append(service.Ports, portStr)
 			}
 		}
+		for _, p := range svc.Expose {
+			portStr := d.parsePort(p)
+			if portStr != "" {
+				service.Ports = append(service.Ports, portStr)
+			}
+		}
 
 		services = append(services, service)
 	}
@@ -538,7 +545,25 @@ func (d *Discovery) UpdateComposeFile(name string, content string) error {
 		_ = os.WriteFile(backup, data, 0644)
 	}
 
-	return os.WriteFile(composePath, []byte(content), 0644)
+	if err := os.WriteFile(composePath, []byte(content), 0644); err != nil {
+		return err
+	}
+
+	metadataPath := filepath.Join(dirPath, "service.yml")
+	if _, err := os.Stat(metadataPath); err == nil {
+		if newMeta := d.generateMetadataFromCompose(composePath, name); newMeta != nil {
+			existing, err := d.loadMetadata(metadataPath)
+			if err == nil {
+				existing.Networking.ContainerPort = newMeta.Networking.ContainerPort
+				if newMeta.Networking.Service != "" {
+					existing.Networking.Service = newMeta.Networking.Service
+				}
+				d.SaveMetadata(name, existing)
+			}
+		}
+	}
+
+	return nil
 }
 
 func (d *Discovery) SaveMetadata(name string, metadata *models.ServiceMetadata) error {
@@ -602,6 +627,12 @@ func (d *Discovery) generateMetadataFromCompose(composePath, name string) *model
 					metadata.Networking.ContainerPort = port
 				}
 			}
+		} else if len(svc.Expose) > 0 {
+			if portStr := d.parsePort(svc.Expose[0]); portStr != "" {
+				if port := d.extractContainerPort(portStr); port > 0 {
+					metadata.Networking.ContainerPort = port
+				}
+			}
 		}
 	}
 
@@ -618,7 +649,7 @@ func (d *Discovery) pickPrimaryService(services map[string]composeService) (stri
 		}
 	}
 	for name, svc := range services {
-		if len(svc.Ports) > 0 {
+		if len(svc.Ports) > 0 || len(svc.Expose) > 0 {
 			return name, svc
 		}
 	}

internal/docker/discovery_test.go

@@ -6,6 +6,9 @@ import (
 	"path/filepath"
 	"sort"
 	"testing"
+
+	"github.com/flatrun/agent/pkg/models"
+	"gopkg.in/yaml.v3"
 )
 
 func TestExtractBindMountPath(t *testing.T) {
@@ -408,6 +411,138 @@ func TestGenerateMetadataFromCompose_ServiceName(t *testing.T) {
 	}
 }
 
+func TestGenerateMetadataFromCompose_Expose(t *testing.T) {
+	tests := []struct {
+		name     string
+		compose  string
+		wantPort int
+	}{
+		{
+			name: "expose sets container port",
+			compose: `services:
+  app:
+    image: myapp:latest
+    expose:
+      - "80"
+`,
+			wantPort: 80,
+		},
+		{
+			name: "ports takes precedence over expose",
+			compose: `services:
+  app:
+    image: myapp:latest
+    ports:
+      - "8080:3000"
+    expose:
+      - "80"
+`,
+			wantPort: 3000,
+		},
+		{
+			name: "expose picks primary service in multi-service",
+			compose: `services:
+  app:
+    image: myapp:latest
+    expose:
+      - "8080"
+  db:
+    image: postgres:15
+`,
+			wantPort: 8080,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			tmpDir, err := os.MkdirTemp("", "expose-test-*")
+			if err != nil {
+				t.Fatalf("Failed to create temp dir: %v", err)
+			}
+			defer os.RemoveAll(tmpDir)
+
+			composePath := filepath.Join(tmpDir, "docker-compose.yml")
+			if err := os.WriteFile(composePath, []byte(tt.compose), 0644); err != nil {
+				t.Fatalf("Failed to write compose file: %v", err)
+			}
+
+			d := NewDiscovery(tmpDir)
+			metadata := d.generateMetadataFromCompose(composePath, "test")
+			if metadata == nil {
+				t.Fatal("generateMetadataFromCompose returned nil")
+			}
+
+			if metadata.Networking.ContainerPort != tt.wantPort {
+				t.Errorf("ContainerPort = %d, want %d", metadata.Networking.ContainerPort, tt.wantPort)
+			}
+		})
+	}
+}
+
+func TestUpdateComposeFile_SyncsMetadata(t *testing.T) {
+	tmpDir, err := os.MkdirTemp("", "sync-test-*")
+	if err != nil {
+		t.Fatalf("Failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	deployDir := filepath.Join(tmpDir, "myapp")
+	if err := os.MkdirAll(deployDir, 0755); err != nil {
+		t.Fatalf("Failed to create deploy dir: %v", err)
+	}
+
+	compose := `services:
+  app:
+    image: myapp:latest
+    expose:
+      - "3000"
+`
+	composePath := filepath.Join(deployDir, "docker-compose.yml")
+	if err := os.WriteFile(composePath, []byte(compose), 0644); err != nil {
+		t.Fatalf("Failed to write compose: %v", err)
+	}
+
+	d := NewDiscovery(tmpDir)
+	metadata := &models.ServiceMetadata{
+		Name: "myapp",
+		Networking: models.NetworkingConfig{
+			ContainerPort: 3000,
+			Expose:        true,
+		},
+	}
+	if err := d.SaveMetadata("myapp", metadata); err != nil {
+		t.Fatalf("Failed to save metadata: %v", err)
+	}
+
+	updatedCompose := `services:
+  app:
+    image: myapp:latest
+    expose:
+      - "8080"
+`
+	if err := d.UpdateComposeFile("myapp", updatedCompose); err != nil {
+		t.Fatalf("UpdateComposeFile failed: %v", err)
+	}
+
+	metadataPath := filepath.Join(deployDir, "service.yml")
+	data, err := os.ReadFile(metadataPath)
+	if err != nil {
+		t.Fatalf("Failed to read service.yml: %v", err)
+	}
+
+	var updated models.ServiceMetadata
+	if err := yaml.Unmarshal(data, &updated); err != nil {
+		t.Fatalf("Failed to parse service.yml: %v", err)
+	}
+
+	if updated.Networking.ContainerPort != 8080 {
+		t.Errorf("ContainerPort = %d, want 8080", updated.Networking.ContainerPort)
+	}
+	if !updated.Networking.Expose {
+		t.Error("Expose should be preserved as true")
+	}
+}
+
 func TestExtractBindMounts(t *testing.T) {
 	tests := []struct {
 		name     string

internal/proxy/orchestrator.go

@@ -18,7 +18,7 @@ type Orchestrator struct {
 func NewOrchestrator(cfg *config.Config) *Orchestrator {
 	return &Orchestrator{
 		nginx: nginx.NewManager(&cfg.Nginx, cfg.DeploymentsPath, cfg.Certbot.WebrootPath),
-		ssl:   ssl.NewManager(&cfg.Certbot, cfg.DeploymentsPath),
+		ssl:   ssl.NewManager(&cfg.Certbot, cfg.DeploymentsPath, nil),
 	}
 }
 

internal/setup/handlers.go

@@ -3,6 +3,7 @@ package setup
 import (
 	"net"
 	"net/http"
+	"net/mail"
 	"time"
 
 	"github.com/flatrun/agent/internal/auth"
@@ -94,9 +95,10 @@ func (h *Handlers) VerifyDNS(c *gin.Context) {
 
 func (h *Handlers) ConfigureSettings(c *gin.Context) {
 	var req struct {
-		Domain      string   `json:"domain"`
-		AutoSSL     *bool    `json:"auto_ssl"`
-		CORSOrigins []string `json:"cors_origins"`
+		Domain       string   `json:"domain"`
+		AutoSSL      *bool    `json:"auto_ssl"`
+		CertbotEmail string   `json:"certbot_email"`
+		CORSOrigins  []string `json:"cors_origins"`
 	}
 
 	if err := c.ShouldBindJSON(&req); err != nil {
@@ -111,6 +113,13 @@ func (h *Handlers) ConfigureSettings(c *gin.Context) {
 	if req.AutoSSL != nil {
 		cfg.Domain.AutoSSL = *req.AutoSSL
 	}
+	if req.CertbotEmail != "" {
+		if _, err := mail.ParseAddress(req.CertbotEmail); err != nil {
+			c.JSON(http.StatusBadRequest, gin.H{"error": "invalid email format"})
+			return
+		}
+		cfg.Certbot.Email = req.CertbotEmail
+	}
 	if len(req.CORSOrigins) > 0 {
 		originMap := make(map[string]bool)
 		for _, e := range cfg.API.AllowedOrigins {
@@ -130,9 +139,10 @@ func (h *Handlers) ConfigureSettings(c *gin.Context) {
 	}
 
 	c.JSON(http.StatusOK, gin.H{
-		"message":  "Settings configured",
-		"domain":   cfg.Domain.DefaultDomain,
-		"auto_ssl": cfg.Domain.AutoSSL,
+		"message":       "Settings configured",
+		"domain":        cfg.Domain.DefaultDomain,
+		"auto_ssl":      cfg.Domain.AutoSSL,
+		"certbot_email": cfg.Certbot.Email,
 	})
 }
 

internal/ssl/manager.go

@@ -4,6 +4,7 @@ import (
 	"crypto/x509"
 	"encoding/pem"
 	"fmt"
+	"log"
 	"os"
 	"path/filepath"
 	"strings"
@@ -15,15 +16,26 @@ import (
 	"github.com/flatrun/agent/pkg/models"
 )
 
+type ServiceExecutor interface {
+	Execute(cfg *config.ServiceExecConfig, args []string) ([]byte, error)
+}
+
+type dockerServiceExecutor struct{}
+
+func (e *dockerServiceExecutor) Execute(cfg *config.ServiceExecConfig, args []string) ([]byte, error) {
+	return docker.ExecuteService(cfg, args)
+}
+
 type Manager struct {
 	config           *config.CertbotConfig
 	certsPath        string
 	webRoot          string
 	containerWebRoot string
+	executor         ServiceExecutor
 	mu               sync.RWMutex
 }
 
-func NewManager(cfg *config.CertbotConfig, deploymentsPath string) *Manager {
+func NewManager(cfg *config.CertbotConfig, deploymentsPath string, executor ServiceExecutor) *Manager {
 	certsPath := cfg.CertsPath
 	if certsPath == "" {
 		certsPath = filepath.Join(deploymentsPath, "nginx", "certs", "live")
@@ -39,11 +51,16 @@ func NewManager(cfg *config.CertbotConfig, deploymentsPath string) *Manager {
 		containerWebRoot = "/var/www/certbot"
 	}
 
+	if executor == nil {
+		executor = &dockerServiceExecutor{}
+	}
+
 	return &Manager{
 		config:           cfg,
 		certsPath:        certsPath,
 		webRoot:          webRoot,
 		containerWebRoot: containerWebRoot,
+		executor:         executor,
 	}
 }
 
@@ -81,7 +98,12 @@ func (m *Manager) RequestCertificate(domain string) (*CertificateResult, error)
 	defer m.mu.Unlock()
 
 	if m.config.Email == "" {
-		return nil, fmt.Errorf("certbot email not configured")
+		log.Printf("warning: skipping SSL for %s — certbot email not configured (set it in Settings)", domain)
+		return &CertificateResult{
+			Domain:  domain,
+			Success: false,
+			Message: "certbot email not configured — configure it in Settings to enable SSL",
+		}, nil
 	}
 
 	if err := os.MkdirAll(m.webRoot, 0755); err != nil {
@@ -90,6 +112,7 @@ func (m *Manager) RequestCertificate(domain string) (*CertificateResult, error)
 
 	certbotArgs := []string{
 		"certonly",
+		"--non-interactive",
 		"--webroot",
 		"--webroot-path", m.containerWebRoot,
 		"--email", m.config.Email,
@@ -135,14 +158,14 @@ func (m *Manager) getServiceExecConfig() *config.ServiceExecConfig {
 
 func (m *Manager) executeCertbot(args []string) ([]byte, error) {
 	cfg := m.getServiceExecConfig()
-	return docker.ExecuteService(cfg, args)
+	return m.executor.Execute(cfg, args)
 }
 
 func (m *Manager) RenewCertificates() (*RenewalResult, error) {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
-	output, err := m.executeCertbot([]string{"renew"})
+	output, err := m.executeCertbot([]string{"renew", "--non-interactive"})
 	if err != nil {
 		return nil, fmt.Errorf("renewal failed: %s - %w", string(output), err)
 	}