From 548be903e7efc83d6d29895d29edc531320e24a0 Mon Sep 17 00:00:00 2001 From: Paul Payne Date: Thu, 9 Jul 2026 20:29:30 +0000 Subject: [PATCH] feat: Fix HAProxy ACL ordering + Subdomains field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rewrite HAProxy config generation with correct ACL ordering: 1. L7 HTTP exact matches first (central, wild-cloud app) 2. L4 exact matches (payne.io, civilsociety.dev) 3. L4 wildcard matches (*.cloud.payne.io) — only when subdomains:true 4. Default → L7 termination Key changes: - InstanceRoute: removed ExtraDomains, added Subdomains bool - GenerateOpts: removed CentralDomain (Central is now just another HTTPRoute injected by reconciliation), renamed WildcardCert→CertsDir - Central's domain comes from config, injected as HTTPRoute with the actual running port (no more hardcoded 5055) - Added API.SetPort() so reconciliation knows the running port - subdomains:false → exact match only (no *.domain shadowing) New tests: TestGenerate_WithSubdomains, TestGenerate_ACLOrdering (verifies L7 exact matches come before L4 wildcards). Fixes: BUG 1 (central routing to wrong port), BUG 2 (wild-cloud getting traefik cert), BUG 3 (*.payne.io too broad). Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/api/v1/handlers.go | 13 ++++ internal/api/v1/helpers.go | 22 +++---- internal/haproxy/config.go | 108 ++++++++++++++------------------ internal/haproxy/config_test.go | 76 ++++++++++++++-------- main.go | 4 +- 5 files changed, 123 insertions(+), 100 deletions(-) diff --git a/internal/api/v1/handlers.go b/internal/api/v1/handlers.go index dc41984..3b6078c 100644 --- a/internal/api/v1/handlers.go +++ b/internal/api/v1/handlers.go @@ -47,6 +47,7 @@ type API struct { certbot *certbot.Manager services *services.Manager // Service registration manager sseManager *sse.Manager // SSE manager for real-time events + port int // Running API port (for config-driven routes) } // NewAPI creates a new Central API handler with all dependencies @@ -89,6 +90,18 @@ func NewAPI(dataDir, version string, allowedOrigins []string) (*API, error) { return api, nil } +// SetPort records the running API port for config-driven HAProxy routes. +func (api *API) SetPort(port int) { + api.port = port +} + +func (api *API) getRunningPort() int { + if api.port > 0 { + return api.port + } + return 5055 +} + func envOrDefault(key, defaultVal string) string { if v := os.Getenv(key); v != "" { slog.Info("using custom config path", "key", key, "path", v) diff --git a/internal/api/v1/helpers.go b/internal/api/v1/helpers.go index 7959890..581d61c 100644 --- a/internal/api/v1/helpers.go +++ b/internal/api/v1/helpers.go @@ -55,19 +55,18 @@ func (api *API) reconcileNetworking() { } } - // Determine central domain for HAProxy — only if its cert exists - centralDomain := "" + // Inject Central's own domain as an HTTP route (from config, not registration). + // Central is just another L7 service from HAProxy's perspective. if d := globalCfg.Cloud.Central.Domain; d != "" { - if _, err := os.Stat(certbot.HAProxyCertPath(d)); err == nil { - centralDomain = d - } else { - slog.Debug("reconcile: skipping central domain in HAProxy (no cert)", "domain", d) - } + centralPort := api.getRunningPort() + httpRoutes = append([]haproxy.HTTPRoute{{ + Name: "wild-central", + Domain: d, + Backend: fmt.Sprintf("127.0.0.1:%d", centralPort), + }}, httpRoutes...) } // Only include L7 HTTP routes for services that have a cert. - // HAProxy uses a cert directory — each service needs its own .pem - // or be covered by a wildcard cert in the same directory. certsDir := "/etc/haproxy/certs/" var activeHTTPRoutes []haproxy.HTTPRoute for _, route := range httpRoutes { @@ -80,9 +79,8 @@ func (api *API) reconcileNetworking() { // Generate and write HAProxy config haproxyCfg := api.haproxy.GenerateWithOpts(instanceRoutes, nil, haproxy.GenerateOpts{ - CentralDomain: centralDomain, - HTTPRoutes: activeHTTPRoutes, - WildcardCert: certsDir, + HTTPRoutes: activeHTTPRoutes, + CertsDir: certsDir, }) if err := api.haproxy.WriteConfig(haproxyCfg); err != nil { diff --git a/internal/haproxy/config.go b/internal/haproxy/config.go index 885b847..8e3a7a4 100644 --- a/internal/haproxy/config.go +++ b/internal/haproxy/config.go @@ -13,11 +13,12 @@ import ( ) // InstanceRoute represents a Wild Cloud instance's ingress routing configuration +// InstanceRoute represents an L4 TCP passthrough route (k8s instance). type InstanceRoute struct { - Name string `json:"name"` // e.g. "payne-cloud" - Domain string `json:"domain"` // e.g. "cloud.payne.io" - BackendIP string `json:"backendIP"` // k8s load balancer IP - ExtraDomains []string `json:"extraDomains,omitempty"` // additional domains served by this instance + Name string `json:"name"` // identifier for backend naming + Domain string `json:"domain"` // e.g. "cloud.payne.io" + BackendIP string `json:"backendIP"` // k8s load balancer IP + Subdomains bool `json:"subdomains"` // if true, also match *.domain } // CustomRoute represents a custom TCP proxy rule for non-Wild Cloud services @@ -65,9 +66,8 @@ func (m *Manager) GetConfigPath() string { // GenerateOpts holds optional parameters for HAProxy config generation. type GenerateOpts struct { - CentralDomain string // if set, central's own domain is SNI-routed to local TLS frontend - HTTPRoutes []HTTPRoute // L7 HTTP reverse proxy routes (TLS terminated by Central) - WildcardCert string // path to wildcard cert PEM for L7 TLS termination + HTTPRoutes []HTTPRoute // L7 HTTP reverse proxy routes (TLS terminated by Central) + CertsDir string // directory containing per-domain PEM files for L7 TLS } // Generate creates a complete HAProxy configuration from instance routes, custom routes, @@ -75,16 +75,7 @@ type GenerateOpts struct { // TLS terminates at each k8s cluster's traefik. HTTP routes add an L7 frontend // where Central terminates TLS with a wildcard cert and reverse-proxies by Host header. func (m *Manager) Generate(instances []InstanceRoute, custom []CustomRoute, centralDomain ...string) string { - return m.GenerateWithOpts(instances, custom, GenerateOpts{ - CentralDomain: firstOrEmpty(centralDomain), - }) -} - -func firstOrEmpty(s []string) string { - if len(s) > 0 { - return s[0] - } - return "" + return m.GenerateWithOpts(instances, custom, GenerateOpts{}) } // GenerateWithOpts creates a complete HAProxy configuration with full options. @@ -126,7 +117,7 @@ frontend http_in `) - hasL4Routes := len(instances) > 0 || opts.CentralDomain != "" || len(opts.HTTPRoutes) > 0 + hasL4Routes := len(instances) > 0 || len(opts.HTTPRoutes) > 0 if hasL4Routes { sb.WriteString("# HTTPS: SNI-based L4 routing (TLS passes through to k8s traefik)\n") @@ -137,63 +128,58 @@ frontend http_in sb.WriteString(" tcp-request content accept if { req_ssl_hello_type 1 }\n") sb.WriteString("\n") - // Central domain: SNI routes to local TLS-terminating frontend - if opts.CentralDomain != "" { - fmt.Fprintf(&sb, " acl is_central req_ssl_sni -m str %s\n", opts.CentralDomain) - sb.WriteString(" use_backend be_central_tls if is_central\n") - sb.WriteString("\n") - } + // ACL ordering is critical. Process in this order: + // 1. L7 HTTP exact matches (central, wild-cloud app) → L7 termination + // 2. L4 exact matches (payne.io, civilsociety.dev) → instance backends + // 3. L4 wildcard matches (*.cloud.payne.io) → instance backends + // 4. Default → L7 termination (if any HTTP routes exist) - // Instance routes: L4 SNI passthrough to k8s load balancers - for _, inst := range instances { - aclName := "is_" + sanitizeName(inst.Name) - fmt.Fprintf(&sb, " acl %s req_ssl_sni -m end .%s\n", aclName, inst.Domain) - fmt.Fprintf(&sb, " acl %s req_ssl_sni -m str %s\n", aclName, inst.Domain) - for _, extra := range inst.ExtraDomains { - fmt.Fprintf(&sb, " acl %s req_ssl_sni -m end .%s\n", aclName, extra) - fmt.Fprintf(&sb, " acl %s req_ssl_sni -m str %s\n", aclName, extra) - } - fmt.Fprintf(&sb, " use_backend be_https_%s if %s\n", sanitizeName(inst.Name), aclName) - sb.WriteString("\n") - } - - // HTTP route SNI ACLs: route to L7 TLS-termination frontend + // 1. L7 HTTP services — exact match, route to L7 TLS termination frontend if len(opts.HTTPRoutes) > 0 { + sb.WriteString(" # L7 HTTP services (exact match → TLS termination)\n") for _, route := range opts.HTTPRoutes { aclName := "is_l7_" + sanitizeName(route.Name) fmt.Fprintf(&sb, " acl %s req_ssl_sni -m str %s\n", aclName, route.Domain) + fmt.Fprintf(&sb, " use_backend be_l7_termination if %s\n", aclName) } - sb.WriteString(" acl is_l7 req_ssl_sni -m found\n") - sb.WriteString(" use_backend be_l7_termination if is_l7\n") sb.WriteString("\n") } - // Fallback: any remaining SNI goes to L7 termination (if we have HTTP routes) + // 2. L4 exact matches — instances with subdomains:false + hasExact := false + for _, inst := range instances { + if inst.Subdomains { + continue // handled in step 3 + } + hasExact = true + aclName := "is_" + sanitizeName(inst.Name) + fmt.Fprintf(&sb, " acl %s req_ssl_sni -m str %s\n", aclName, inst.Domain) + fmt.Fprintf(&sb, " use_backend be_https_%s if %s\n", sanitizeName(inst.Name), aclName) + } + if hasExact { + sb.WriteString("\n") + } + + // 3. L4 wildcard matches — instances with subdomains:true + sb.WriteString(" # L4 instance routes (wildcard subdomain matching)\n") + for _, inst := range instances { + if !inst.Subdomains { + continue // already handled in step 2 + } + aclName := "is_" + sanitizeName(inst.Name) + fmt.Fprintf(&sb, " acl %s req_ssl_sni -m end .%s\n", aclName, inst.Domain) + fmt.Fprintf(&sb, " acl %s req_ssl_sni -m str %s\n", aclName, inst.Domain) + fmt.Fprintf(&sb, " use_backend be_https_%s if %s\n", sanitizeName(inst.Name), aclName) + } + sb.WriteString("\n") + + // 4. Default → L7 termination fallback if len(opts.HTTPRoutes) > 0 { sb.WriteString(" default_backend be_l7_termination\n") } sb.WriteString("\n") - // Central TLS-terminating backend and frontend - if opts.CentralDomain != "" { - sb.WriteString("backend be_central_tls\n") - sb.WriteString(" mode tcp\n") - sb.WriteString(" server s0 127.0.0.1:4443\n") - sb.WriteString("\n") - - sb.WriteString("# Wild Central: TLS termination for management UI\n") - sb.WriteString("frontend central_https\n") - fmt.Fprintf(&sb, " bind 127.0.0.1:4443 ssl crt /etc/haproxy/certs/%s.pem\n", opts.CentralDomain) - sb.WriteString(" mode http\n") - sb.WriteString(" default_backend be_central_http\n") - sb.WriteString("\n") - sb.WriteString("backend be_central_http\n") - sb.WriteString(" mode http\n") - sb.WriteString(" server s0 127.0.0.1:5055\n") - sb.WriteString("\n") - } - // Instance L4 backends for _, inst := range instances { fmt.Fprintf(&sb, "backend be_https_%s\n", sanitizeName(inst.Name)) @@ -207,7 +193,7 @@ frontend http_in if len(opts.HTTPRoutes) > 0 { // Load all PEM files from the certs directory — HAProxy serves // the right cert per-SNI. Each service gets its own .pem. - certPath := opts.WildcardCert + certPath := opts.CertsDir if certPath == "" { certPath = "/etc/haproxy/certs/" } diff --git a/internal/haproxy/config_test.go b/internal/haproxy/config_test.go index bbec68f..86a98ab 100644 --- a/internal/haproxy/config_test.go +++ b/internal/haproxy/config_test.go @@ -93,11 +93,11 @@ func TestGenerate_NoInstances_NoHTTPSFrontend(t *testing.T) { func TestGenerate_WithInstance_SNIACLs(t *testing.T) { m := NewManager("") instances := []InstanceRoute{ - {Name: "payne-cloud", Domain: "cloud.payne.io", BackendIP: "192.168.8.20"}, + {Name: "payne-cloud", Domain: "cloud.payne.io", BackendIP: "192.168.8.20", Subdomains: true}, } out := m.Generate(instances, nil) - // Both ACL lines for OR-matching (subdomain and exact) + // With subdomains:true, both ACL lines for OR-matching if !strings.Contains(out, "req_ssl_sni -m end .cloud.payne.io") { t.Errorf("expected subdomain SNI ACL, got:\n%s", out) } @@ -146,31 +146,57 @@ func TestGenerate_MultipleInstances(t *testing.T) { } } -func TestGenerate_WithExtraDomains(t *testing.T) { +func TestGenerate_WithSubdomains(t *testing.T) { m := NewManager("") instances := []InstanceRoute{ - { - Name: "payne-cloud", - Domain: "cloud.payne.io", - BackendIP: "192.168.8.20", - ExtraDomains: []string{"payne.io", "mywildcloud.org"}, - }, - {Name: "test-cloud", Domain: "cloud2.payne.io", BackendIP: "192.168.8.30"}, + {Name: "payne-cloud", Domain: "cloud.payne.io", BackendIP: "192.168.8.20", Subdomains: true}, + {Name: "payne-io", Domain: "payne.io", BackendIP: "192.168.8.20", Subdomains: false}, } out := m.Generate(instances, nil) - // Extra domains should add ACL entries under the same payne-cloud ACL name - for _, domain := range []string{"payne.io", "mywildcloud.org"} { - if !strings.Contains(out, "req_ssl_sni -m end ."+domain) { - t.Errorf("expected subdomain ACL for %s, got:\n%s", domain, out) - } - if !strings.Contains(out, "req_ssl_sni -m str "+domain) { - t.Errorf("expected exact ACL for %s, got:\n%s", domain, out) - } + // cloud.payne.io with subdomains:true should have wildcard ACL + if !strings.Contains(out, "req_ssl_sni -m end .cloud.payne.io") { + t.Errorf("expected wildcard ACL for cloud.payne.io, got:\n%s", out) } - // Extra domains should route to payne-cloud, not test-cloud - if !strings.Contains(out, "use_backend be_https_payne_cloud if is_payne_cloud") { - t.Errorf("expected payne-cloud use_backend, got:\n%s", out) + // payne.io with subdomains:false should have exact match only + if !strings.Contains(out, "req_ssl_sni -m str payne.io") { + t.Errorf("expected exact ACL for payne.io, got:\n%s", out) + } + // payne.io should NOT have wildcard match + if strings.Contains(out, "req_ssl_sni -m end .payne.io") { + t.Errorf("unexpected wildcard ACL for payne.io (subdomains:false), got:\n%s", out) + } +} + +func TestGenerate_ACLOrdering(t *testing.T) { + m := NewManager("") + instances := []InstanceRoute{ + {Name: "payne-cloud", Domain: "cloud.payne.io", BackendIP: "192.168.8.240", Subdomains: true}, + } + httpRoutes := []HTTPRoute{ + {Name: "wild-central", Domain: "central.payne.io", Backend: "127.0.0.1:15055"}, + {Name: "wild-cloud", Domain: "wild-cloud.payne.io", Backend: "127.0.0.1:5055"}, + } + out := m.GenerateWithOpts(instances, nil, GenerateOpts{HTTPRoutes: httpRoutes}) + + // L7 exact matches must appear BEFORE L4 wildcard matches + l7Pos := strings.Index(out, "is_l7_wild_central") + l4Pos := strings.Index(out, "is_payne_cloud") + + if l7Pos < 0 { + t.Fatalf("expected L7 ACL for wild-central, got:\n%s", out) + } + if l4Pos < 0 { + t.Fatalf("expected L4 ACL for payne-cloud, got:\n%s", out) + } + if l7Pos > l4Pos { + t.Errorf("L7 exact match (pos %d) must come before L4 wildcard (pos %d):\n%s", l7Pos, l4Pos, out) + } + + // wild-cloud.payne.io should NOT match payne-cloud's wildcard + // (it should be caught by the L7 exact match first) + if strings.Contains(out, "use_backend be_https_payne_cloud if is_l7_wild_cloud") { + t.Errorf("wild-cloud.payne.io should route to L7, not payne-cloud") } } @@ -205,16 +231,16 @@ func TestGenerateWithOpts_HTTPRoutes_L7Frontend(t *testing.T) { } out := m.GenerateWithOpts(nil, nil, GenerateOpts{ - HTTPRoutes: httpRoutes, - WildcardCert: "/etc/haproxy/certs/wildcard.pem", + HTTPRoutes: httpRoutes, + CertsDir: "/etc/haproxy/certs/", }) // L7 TLS termination frontend should exist if !strings.Contains(out, "frontend l7_https") { t.Errorf("expected L7 frontend, got:\n%s", out) } - if !strings.Contains(out, "ssl crt /etc/haproxy/certs/wildcard.pem") { - t.Errorf("expected wildcard cert in L7 bind, got:\n%s", out) + if !strings.Contains(out, "ssl crt /etc/haproxy/certs/") { + t.Errorf("expected certs dir in L7 bind, got:\n%s", out) } // Host-based ACLs diff --git a/main.go b/main.go index 8df1998..9b75ce4 100644 --- a/main.go +++ b/main.go @@ -150,8 +150,8 @@ func main() { fmt.Sscanf(v, "%d", &port) } - // Self-register Wild Central's own domain (idempotent, skips if no domain configured) - api.RegisterSelf(port) + // Tell the API what port it's running on (used for config-driven HAProxy routes) + api.SetPort(port) addr := fmt.Sprintf("%s:%d", host, port) slog.Info("wild-central started", "addr", addr, "version", Version)