From 77652321a73891dac49692578b47b3bc43c89896 Mon Sep 17 00:00:00 2001 From: Jakub Winkler Date: Tue, 14 Jul 2026 07:56:33 +0200 Subject: [PATCH] fix: Varnish VCL no longer crashes with multiple projects buildVCLConfig emitted one backend per project but the template only referenced DefaultBackend, so Varnish 7.x aborted on the first unused backend and crash-looped (502). The probe also used HEAD / expecting 301 while Magento returns 302 (503). Emit a single 'magento' backend (Nginx routes per project by Host) and probe GET /health_check.php expecting 200. Fixes both the embedded template (internal/varnish/templates) and the lib copy. Supersedes #127 (which patched only the unused lib template). Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 6 ++ VERSION | 2 +- internal/varnish/templates/default.vcl.tmpl | 9 +-- internal/varnish/vcl.go | 74 ++++++--------------- internal/varnish/vcl_test.go | 73 +++++++++----------- lib/templates/varnish/default.vcl.tmpl | 9 +-- 6 files changed, 67 insertions(+), 106 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6db5382..2030fd5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to MageBox will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.18.3] - 2026-07-14 + +### Fixed + +- **Varnish VCL Crash with Multiple Projects** - The shared Varnish container generated one backend per registered project, but the template only ever referenced the default backend (`set req.backend_hint = {{.DefaultBackend}}`). Varnish 7.x treats a defined-but-unused backend as a fatal compile error (`Unused backend …`), so with two or more projects the `magebox-varnish` container failed to compile its VCL and crash-looped, returning 502. The health probe also used `HEAD /` expecting `301`, but Magento returns `302`, marking the backend `sick` (503). All generated backends were identical anyway, since Nginx already routes to the correct project by `Host` header. The VCL now emits a single `magento` backend and probes `GET /health_check.php` expecting `200`. + ## [1.18.2] - 2026-06-23 ### Fixed diff --git a/VERSION b/VERSION index b57fc72..b9fb27a 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.18.2 +1.18.3 diff --git a/internal/varnish/templates/default.vcl.tmpl b/internal/varnish/templates/default.vcl.tmpl index a5e5cd0..98edb68 100644 --- a/internal/varnish/templates/default.vcl.tmpl +++ b/internal/varnish/templates/default.vcl.tmpl @@ -14,14 +14,14 @@ backend {{.Name}} { .between_bytes_timeout = 600s; .probe = { .request = - "HEAD / HTTP/1.1" + "GET {{if .ProbeURL}}{{.ProbeURL}}{{else}}/health_check.php{{end}} HTTP/1.1" "Host: localhost" "Connection: close"; .timeout = 2s; .interval = {{if .ProbeInterval}}{{.ProbeInterval}}{{else}}5s{{end}}; .window = 5; .threshold = 2; - .expected_response = 301; + .expected_response = 200; } } {{end}} @@ -38,10 +38,7 @@ sub vcl_init { } sub vcl_recv { - # Set backend based on Host header - {{range .Backends}} - # Backend: {{.Name}} - {{end}} + # Single shared backend; Nginx routes to the right project by Host header. set req.backend_hint = {{.DefaultBackend}}; # Normalize the host header diff --git a/internal/varnish/vcl.go b/internal/varnish/vcl.go index c7e8b09..b7106af 100644 --- a/internal/varnish/vcl.go +++ b/internal/varnish/vcl.go @@ -89,46 +89,28 @@ func (g *VCLGenerator) Generate(configs []*config.Config) error { return nil } -// buildVCLConfig builds the VCL configuration from project configs -func (g *VCLGenerator) buildVCLConfig(configs []*config.Config) VCLConfig { - vclCfg := VCLConfig{ - Backends: make([]BackendConfig, 0), - GracePeriod: "300s", - PurgeACL: []string{"localhost", "127.0.0.1", "::1", "host.docker.internal"}, +// buildVCLConfig builds the VCL configuration. +// +// All MageBox projects share one Varnish container and one Nginx backend on :8080; Nginx +// already routes to the right project by Host header. Earlier this emitted one backend per +// registered project, but every backend was identical and the template only ever references +// DefaultBackend — so with 2+ projects Varnish 7.x aborted compilation on the first unused +// backend ("Unused backend …") and crash-looped. We therefore emit a single backend. +func (g *VCLGenerator) buildVCLConfig(_ []*config.Config) VCLConfig { + backend := BackendConfig{ + Name: "magento", + Host: getHostIP(), + Port: 8080, // Nginx backend listens on 8080 (Varnish connects here) + ProbeURL: "/health_check.php", + ProbeInterval: "5s", + } + + return VCLConfig{ + Backends: []BackendConfig{backend}, + DefaultBackend: backend.Name, + GracePeriod: "300s", + PurgeACL: []string{"localhost", "127.0.0.1", "::1", "host.docker.internal"}, } - - // Backend host - detect host IP for Docker to reach nginx - backendHost := getHostIP() - backendPort := 8080 // Nginx backend listens on 8080 (Varnish connects here) - - for _, cfg := range configs { - // Each project gets a backend pointing to Nginx - backend := BackendConfig{ - Name: sanitizeName(cfg.Name), - Host: backendHost, - Port: backendPort, - ProbeURL: "/health_check.php", - ProbeInterval: "5s", - } - vclCfg.Backends = append(vclCfg.Backends, backend) - - // First project is default backend - if vclCfg.DefaultBackend == "" { - vclCfg.DefaultBackend = backend.Name - } - } - - // If no projects, create a default backend - if len(vclCfg.Backends) == 0 { - vclCfg.Backends = append(vclCfg.Backends, BackendConfig{ - Name: "default", - Host: backendHost, - Port: backendPort, - }) - vclCfg.DefaultBackend = "default" - } - - return vclCfg } // renderVCL renders the VCL template @@ -162,20 +144,6 @@ func (g *VCLGenerator) VCLFilePath() string { return filepath.Join(g.vclDir, "default.vcl") } -// sanitizeName converts a project name to a valid VCL identifier -func sanitizeName(name string) string { - // Replace non-alphanumeric characters with underscores - result := make([]byte, 0, len(name)) - for _, c := range []byte(name) { - if (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') { - result = append(result, c) - } else { - result = append(result, '_') - } - } - return string(result) -} - // Controller manages Varnish service type Controller struct { platform *platform.Platform diff --git a/internal/varnish/vcl_test.go b/internal/varnish/vcl_test.go index 7961913..a887a32 100644 --- a/internal/varnish/vcl_test.go +++ b/internal/varnish/vcl_test.go @@ -80,16 +80,20 @@ func TestVCLGenerator_Generate(t *testing.T) { contentStr := string(content) - // Verify essential VCL elements + // Verify essential VCL elements. The backend is always the single shared "magento" + // backend regardless of project name (Nginx routes per project by Host). checks := []string{ "vcl 4.1", - "backend mystore", + "backend magento", "acl purge", "sub vcl_recv", "sub vcl_hash", "sub vcl_backend_response", "sub vcl_deliver", "X-Magento-Tags", + // Health probe must hit /health_check.php and expect 200 (Magento returns 302 on HEAD /). + "GET /health_check.php HTTP/1.1", + ".expected_response = 200", } for _, check := range checks { @@ -131,12 +135,20 @@ func TestVCLGenerator_GenerateMultipleProjects(t *testing.T) { contentStr := string(content) - // Both backends should be present - if !strings.Contains(contentStr, "backend store1") { - t.Error("VCL should contain backend store1") + // Regression: with 2+ projects we must emit exactly ONE backend, not one per project. + // Varnish 7.x treats a defined-but-unused backend as a fatal compile error, so the + // previous per-project backends crash-looped the shared container. + if n := strings.Count(contentStr, "\nbackend "); n != 1 { + t.Errorf("expected exactly 1 backend definition, got %d", n) } - if !strings.Contains(contentStr, "backend store2") { - t.Error("VCL should contain backend store2") + if !strings.Contains(contentStr, "backend magento") { + t.Error("VCL should contain the single shared backend magento") + } + if strings.Contains(contentStr, "backend store1") || strings.Contains(contentStr, "backend store2") { + t.Error("VCL must not contain per-project backends (would be unused -> Varnish compile error)") + } + if !strings.Contains(contentStr, "set req.backend_hint = magento") { + t.Error("backend_hint should reference the single magento backend") } } @@ -156,32 +168,9 @@ func TestVCLGenerator_GenerateEmptyConfigs(t *testing.T) { contentStr := string(content) - // Should have default backend - if !strings.Contains(contentStr, "backend default") { - t.Error("VCL should contain default backend when no configs provided") - } -} - -func TestSanitizeName(t *testing.T) { - tests := []struct { - input string - expected string - }{ - {"mystore", "mystore"}, - {"my-store", "my_store"}, - {"my_store", "my_store"}, - {"my.store", "my_store"}, - {"MyStore123", "MyStore123"}, - {"store@123", "store_123"}, - {"store with spaces", "store_with_spaces"}, - } - - for _, tt := range tests { - t.Run(tt.input, func(t *testing.T) { - if got := sanitizeName(tt.input); got != tt.expected { - t.Errorf("sanitizeName(%v) = %v, want %v", tt.input, got, tt.expected) - } - }) + // Should have the single shared backend even with no projects. + if !strings.Contains(contentStr, "backend magento") { + t.Error("VCL should contain the magento backend when no configs provided") } } @@ -195,14 +184,18 @@ func TestVCLGenerator_buildVCLConfig(t *testing.T) { vclCfg := g.buildVCLConfig(configs) - // Should have 2 backends - if len(vclCfg.Backends) != 2 { - t.Errorf("Expected 2 backends, got %d", len(vclCfg.Backends)) + // Always exactly one shared backend, regardless of how many projects exist. + if len(vclCfg.Backends) != 1 { + t.Errorf("Expected 1 backend, got %d", len(vclCfg.Backends)) } - - // First backend should be default - if vclCfg.DefaultBackend != "store1" { - t.Errorf("DefaultBackend = %v, want store1", vclCfg.DefaultBackend) + if vclCfg.DefaultBackend != "magento" { + t.Errorf("DefaultBackend = %v, want magento", vclCfg.DefaultBackend) + } + if vclCfg.Backends[0].Name != "magento" { + t.Errorf("backend name = %v, want magento", vclCfg.Backends[0].Name) + } + if vclCfg.Backends[0].ProbeURL != "/health_check.php" { + t.Errorf("ProbeURL = %v, want /health_check.php", vclCfg.Backends[0].ProbeURL) } // Should have purge ACL diff --git a/lib/templates/varnish/default.vcl.tmpl b/lib/templates/varnish/default.vcl.tmpl index a5e5cd0..98edb68 100644 --- a/lib/templates/varnish/default.vcl.tmpl +++ b/lib/templates/varnish/default.vcl.tmpl @@ -14,14 +14,14 @@ backend {{.Name}} { .between_bytes_timeout = 600s; .probe = { .request = - "HEAD / HTTP/1.1" + "GET {{if .ProbeURL}}{{.ProbeURL}}{{else}}/health_check.php{{end}} HTTP/1.1" "Host: localhost" "Connection: close"; .timeout = 2s; .interval = {{if .ProbeInterval}}{{.ProbeInterval}}{{else}}5s{{end}}; .window = 5; .threshold = 2; - .expected_response = 301; + .expected_response = 200; } } {{end}} @@ -38,10 +38,7 @@ sub vcl_init { } sub vcl_recv { - # Set backend based on Host header - {{range .Backends}} - # Backend: {{.Name}} - {{end}} + # Single shared backend; Nginx routes to the right project by Host header. set req.backend_hint = {{.DefaultBackend}}; # Normalize the host header