feat: Fix HAProxy ACL ordering + Subdomains field

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) <noreply@anthropic.com>
2026-07-09 20:29:30 +00:00
parent 8874bc6281
commit 548be903e7
5 changed files with 123 additions and 100 deletions
--- 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)
--- 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 <domain>.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,
+		CertsDir:   certsDir,
 	})

 	if err := api.haproxy.WriteConfig(haproxyCfg); err != nil {
--- 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"
+	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
-	ExtraDomains []string `json:"extraDomains,omitempty"` // additional domains served by this instance
+	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
+	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 <domain>.pem.
-			certPath := opts.WildcardCert
+			certPath := opts.CertsDir
 			if certPath == "" {
 				certPath = "/etc/haproxy/certs/"
 			}
--- 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)
+	// 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)
 	}
-		if !strings.Contains(out, "req_ssl_sni -m str "+domain) {
-			t.Errorf("expected exact ACL for %s, got:\n%s", domain, 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)
 	}
-	// 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)
+}
+
+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")
 	}
 }

@@ -206,15 +232,15 @@ func TestGenerateWithOpts_HTTPRoutes_L7Frontend(t *testing.T) {

 	out := m.GenerateWithOpts(nil, nil, GenerateOpts{
 		HTTPRoutes: httpRoutes,
-		WildcardCert: "/etc/haproxy/certs/wildcard.pem",
+		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
--- 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)