Skip to content

fix(ssl): Resolve missing SSL blocks in vhosts #76

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nfebe merged 1 commit into from
Feb 7, 2026
Merged

fix(ssl): Resolve missing SSL blocks in vhosts #76

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 90 additions & 0 deletions internal/api/domains_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,4 +451,94 @@ func TestAddDomain(t *testing.T) {
t.Errorf("expected status 201 Created, got %d: %s", w.Code, w.Body.String())
}
})

t.Run("defaults container port from legacy networking", func(t *testing.T) {
createTestDeployment(t, tmpDir, "port-default", &models.ServiceMetadata{
Name: "port-default",
Type: "web",
Networking: models.NetworkingConfig{
Expose: true,
Domain: "legacy.example.com",
ContainerPort: 9090,
},
})

newDomain := models.DomainConfig{
Domain: "new.example.com",
}
body, _ := json.Marshal(newDomain)

w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "name", Value: "port-default"}}
c.Request = httptest.NewRequest("POST", "/", bytes.NewReader(body))
c.Request.Header.Set("Content-Type", "application/json")

server.addDomain(c)

metadataPath := filepath.Join(tmpDir, "port-default", "service.yml")
data, err := os.ReadFile(metadataPath)
if err != nil {
t.Fatalf("Failed to read metadata: %v", err)
}
var metadata models.ServiceMetadata
if err := yaml.Unmarshal(data, &metadata); err != nil {
t.Fatalf("Failed to unmarshal metadata: %v", err)
}

// Legacy domain migrated + new domain = 2
if len(metadata.Domains) != 2 {
t.Fatalf("expected 2 domains, got %d", len(metadata.Domains))
}

newDomainEntry := metadata.Domains[1]
if newDomainEntry.ContainerPort != 9090 {
t.Errorf("expected new domain to inherit container port 9090, got %d", newDomainEntry.ContainerPort)
}
})

t.Run("preserves explicit container port", func(t *testing.T) {
createTestDeployment(t, tmpDir, "port-explicit", &models.ServiceMetadata{
Name: "port-explicit",
Type: "web",
Networking: models.NetworkingConfig{
Expose: true,
Domain: "legacy.example.com",
ContainerPort: 9090,
},
})

newDomain := models.DomainConfig{
Domain: "explicit.example.com",
ContainerPort: 3000,
}
body, _ := json.Marshal(newDomain)

w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "name", Value: "port-explicit"}}
c.Request = httptest.NewRequest("POST", "/", bytes.NewReader(body))
c.Request.Header.Set("Content-Type", "application/json")

server.addDomain(c)

metadataPath := filepath.Join(tmpDir, "port-explicit", "service.yml")
data, err := os.ReadFile(metadataPath)
if err != nil {
t.Fatalf("Failed to read metadata: %v", err)
}
var metadata models.ServiceMetadata
if err := yaml.Unmarshal(data, &metadata); err != nil {
t.Fatalf("Failed to unmarshal metadata: %v", err)
}

if len(metadata.Domains) != 2 {
t.Fatalf("expected 2 domains, got %d", len(metadata.Domains))
}

newDomainEntry := metadata.Domains[1]
if newDomainEntry.ContainerPort != 3000 {
t.Errorf("expected explicit port 3000 to be preserved, got %d", newDomainEntry.ContainerPort)
}
})
}
4 changes: 4 additions & 0 deletions internal/api/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3479,6 +3479,10 @@ func (s *Server) addDomain(c *gin.Context) {
}
}

if domain.ContainerPort == 0 && deployment.Metadata.Networking.ContainerPort != 0 {
domain.ContainerPort = deployment.Metadata.Networking.ContainerPort
}

deployment.Metadata.Domains = append(deployment.Metadata.Domains, domain)

if err := s.manager.SaveMetadata(name, deployment.Metadata); err != nil {
Expand Down
221 changes: 221 additions & 0 deletions internal/nginx/manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1799,6 +1799,227 @@ func TestIsNginxConfigValid(t *testing.T) {
}
}

func TestGenerateConfig_SSLToggle(t *testing.T) {
cfg := &config.NginxConfig{
ContainerWebrootPath: "/var/www/html",
}
m := NewManager(cfg, "/deployments", "")

deployment := &models.Deployment{
Name: "ssl-toggle-test",
Metadata: &models.ServiceMetadata{
Networking: models.NetworkingConfig{
Expose: true,
Domain: "toggle.example.com",
ContainerPort: 8080,
},
SSL: models.SSLConfig{Enabled: true},
},
}

sslConfig, err := m.generateConfig(deployment)
if err != nil {
t.Fatalf("generateConfig with SSL failed: %v", err)
}

if !strings.Contains(sslConfig, "listen 443 ssl;") {
t.Error("SSL-enabled config must contain 'listen 443 ssl;'")
}
if !strings.Contains(sslConfig, "ssl_certificate") {
t.Error("SSL-enabled config must contain ssl_certificate directive")
}

deployment.Metadata.SSL.Enabled = false
httpConfig, err := m.generateConfig(deployment)
if err != nil {
t.Fatalf("generateConfig without SSL failed: %v", err)
}

if strings.Contains(httpConfig, "listen 443") {
t.Error("HTTP-only config must not contain 'listen 443'")
}
if strings.Contains(httpConfig, "ssl_certificate") {
t.Error("HTTP-only config must not contain ssl_certificate directive")
}
if !strings.Contains(httpConfig, "listen 80;") {
t.Error("HTTP-only config must contain 'listen 80;'")
}
}

func TestGenerateMultiDomainConfig_SSLToggle(t *testing.T) {
cfg := &config.NginxConfig{
ContainerWebrootPath: "/var/www/html",
}
m := NewManager(cfg, "/deployments", "")

t.Run("all domains SSL enabled", func(t *testing.T) {
deployment := &models.Deployment{
Name: "multi-ssl",
Metadata: &models.ServiceMetadata{
Domains: []models.DomainConfig{
{
ID: "d1",
Service: "web",
ContainerPort: 8080,
Domain: "app.example.com",
SSL: models.SSLConfig{Enabled: true},
},
{
ID: "d2",
Service: "api",
ContainerPort: 3000,
Domain: "api.example.com",
SSL: models.SSLConfig{Enabled: true},
},
},
},
}

config, err := m.generateMultiDomainConfig(deployment)
if err != nil {
t.Fatalf("generateMultiDomainConfig failed: %v", err)
}

if !strings.Contains(config, "listen 443 ssl;") {
t.Error("all-SSL multi-domain config must contain 'listen 443 ssl;'")
}
if !strings.Contains(config, "ssl_certificate") {
t.Error("all-SSL multi-domain config must contain ssl_certificate directive")
}
})

t.Run("all domains SSL disabled", func(t *testing.T) {
deployment := &models.Deployment{
Name: "multi-http",
Metadata: &models.ServiceMetadata{
Domains: []models.DomainConfig{
{
ID: "d1",
Service: "web",
ContainerPort: 8080,
Domain: "app.example.com",
SSL: models.SSLConfig{Enabled: false},
},
{
ID: "d2",
Service: "api",
ContainerPort: 3000,
Domain: "api.example.com",
SSL: models.SSLConfig{Enabled: false},
},
},
},
}

config, err := m.generateMultiDomainConfig(deployment)
if err != nil {
t.Fatalf("generateMultiDomainConfig failed: %v", err)
}

if strings.Contains(config, "listen 443") {
t.Error("HTTP-only multi-domain config must not contain 'listen 443'")
}
if strings.Contains(config, "ssl_certificate") {
t.Error("HTTP-only multi-domain config must not contain ssl_certificate")
}
})

t.Run("mixed SSL domains", func(t *testing.T) {
deployment := &models.Deployment{
Name: "multi-mixed",
Metadata: &models.ServiceMetadata{
Domains: []models.DomainConfig{
{
ID: "d1",
Service: "web",
ContainerPort: 8080,
Domain: "secure.example.com",
SSL: models.SSLConfig{Enabled: true},
},
{
ID: "d2",
Service: "api",
ContainerPort: 3000,
Domain: "plain.example.com",
SSL: models.SSLConfig{Enabled: false},
},
},
},
}

config, err := m.generateMultiDomainConfig(deployment)
if err != nil {
t.Fatalf("generateMultiDomainConfig failed: %v", err)
}

if !strings.Contains(config, "listen 443 ssl;") {
t.Error("mixed multi-domain config must contain 'listen 443 ssl;' for the SSL domain")
}
if !strings.Contains(config, "secure.example.com") {
t.Error("mixed config must include secure.example.com")
}
if !strings.Contains(config, "plain.example.com") {
t.Error("mixed config must include plain.example.com")
}
})
}

func TestGenerateMultiDomainConfig_ContainerPort(t *testing.T) {
cfg := &config.NginxConfig{
ContainerWebrootPath: "/var/www/html",
}
m := NewManager(cfg, "/deployments", "")

t.Run("uses specified container port", func(t *testing.T) {
deployment := &models.Deployment{
Name: "port-test",
Metadata: &models.ServiceMetadata{
Domains: []models.DomainConfig{
{
ID: "d1",
Service: "web",
ContainerPort: 9090,
Domain: "app.example.com",
},
},
},
}

config, err := m.generateMultiDomainConfig(deployment)
if err != nil {
t.Fatalf("generateMultiDomainConfig failed: %v", err)
}

if !strings.Contains(config, "web:9090") {
t.Errorf("config should proxy to port 9090, got:\n%s", config)
}
})

t.Run("defaults to port 80 when container port is zero", func(t *testing.T) {
deployment := &models.Deployment{
Name: "port-default-test",
Metadata: &models.ServiceMetadata{
Domains: []models.DomainConfig{
{
ID: "d1",
Service: "web",
Domain: "app.example.com",
},
},
},
}

config, err := m.generateMultiDomainConfig(deployment)
if err != nil {
t.Fatalf("generateMultiDomainConfig failed: %v", err)
}

if !strings.Contains(config, "web:80") {
t.Errorf("config should default to port 80 when ContainerPort is 0, got:\n%s", config)
}
})
}

func TestGroupDomainsByHost_DeduplicatesLocations(t *testing.T) {
m := NewManager(&config.NginxConfig{}, "/deployments", "")

Expand Down
7 changes: 5 additions & 2 deletions internal/ssl/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,9 +327,12 @@ type MultiCertificateResult struct {
}

func (m *Manager) RequestCertificatesForDomains(domains []models.DomainConfig) (*MultiCertificateResult, error) {
// AutoCert is intentionally checked independently of SSL.Enabled:
// the orchestrator temporarily disables Enabled before certs exist,
// then re-enables it once obtained.
domainSet := make(map[string]bool)
for _, d := range domains {
if d.SSL.Enabled && d.SSL.AutoCert && d.Domain != "" {
if d.SSL.AutoCert && d.Domain != "" {
domainSet[d.Domain] = true
for _, alias := range d.Aliases {
domainSet[alias] = true
Expand Down Expand Up @@ -371,7 +374,7 @@ func (m *Manager) RequestCertificatesForDomains(domains []models.DomainConfig) (
func (m *Manager) GetDomainsNeedingCertificates(domains []models.DomainConfig) []string {
var result []string
for _, d := range domains {
if d.SSL.Enabled && d.SSL.AutoCert && d.Domain != "" {
if d.SSL.AutoCert && d.Domain != "" {
if !m.CertificateExists(d.Domain) {
result = append(result, d.Domain)
}
Expand Down
Loading
Loading