Skip to content

Commit e018522

Browse files
authored
fix(caddy): isolate routes by listen port to prevent wildcard conflicts (#63)
* fix(caddy): isolate routes by listen port to prevent wildcard conflicts Previously, all ingress routes were placed in a single Caddy server with multiple listen addresses. This caused conflicts when multiple wildcard ingresses matched the same hostname pattern on different ports (e.g., *.host.kernel.sh:443 and *.host.kernel.sh:3000), because Caddy evaluated routes without considering the request's listening port. This change creates separate Caddy servers for each unique listen port, ensuring that requests on port 3000 only see routes meant for port 3000, and requests on port 443 only see routes meant for port 443. Example before (broken): servers: ingress: listen: [":443", ":3000"] routes: - match: *.host.kernel.sh -> port 80 # Always wins - match: *.host.kernel.sh -> port 3000 # Never reached Example after (fixed): servers: ingress-443: listen: [":443"] routes: - match: *.host.kernel.sh -> port 80 ingress-3000: listen: [":3000"] routes: - match: *.host.kernel.sh -> port 3000 This enables apps to expose multiple ports (e.g., 3000, 8080) via wildcard ingresses without conflicts. * fix: disable automatic HTTPS for ports without TLS routes For non-TLS ingresses on non-standard ports, Caddy needs automatic HTTPS completely disabled, not just redirects disabled. Previously the code only disabled HTTPS completely for port 80, but any port without TLS routes should have it disabled. This fixes the integration tests that create non-TLS ingresses on random high ports.
1 parent 81c03b9 commit e018522

2 files changed

Lines changed: 189 additions & 79 deletions

File tree

lib/ingress/config.go

Lines changed: 63 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"log/slog"
88
"os"
99
"path/filepath"
10-
"slices"
1110
"sort"
1211
"strings"
1312

@@ -195,19 +194,22 @@ func (g *CaddyConfigGenerator) GenerateConfig(ctx context.Context, ingresses []I
195194
}
196195

197196
// buildConfig builds the complete Caddy configuration.
197+
// Routes are grouped by listen port to prevent conflicts when multiple wildcard
198+
// ingresses match the same hostname pattern on different ports.
198199
func (g *CaddyConfigGenerator) buildConfig(ctx context.Context, ingresses []Ingress) map[string]interface{} {
199200
log := logger.FromContext(ctx)
200201

201-
// Build routes from ingresses
202-
routes := []interface{}{}
203-
redirectRoutes := []interface{}{}
202+
// Group routes by listen port to isolate them in separate Caddy servers.
203+
// This prevents conflicts when multiple wildcard ingresses match the same
204+
// hostname pattern on different ports (e.g., *.host.kernel.sh:443 and *.host.kernel.sh:3000).
205+
routesByPort := map[int][]interface{}{}
204206
tlsHostnames := []string{}
205-
listenPorts := map[int]bool{}
207+
tlsPortsByHostname := map[string][]int{} // Track which ports need TLS for each hostname
208+
tlsEnabledPorts := map[int]bool{} // Track which ports have at least one TLS route
206209

207210
for _, ingress := range ingresses {
208211
for _, rule := range ingress.Rules {
209212
port := rule.Match.GetPort()
210-
listenPorts[port] = true
211213

212214
// Determine hostname pattern (wildcard or literal) and instance expression
213215
var hostnameMatch string
@@ -256,23 +258,23 @@ func (g *CaddyConfigGenerator) buildConfig(ctx context.Context, ingresses []Ingr
256258
"host": []string{hostnameMatch},
257259
},
258260
},
259-
"handle": []interface{}{reverseProxy},
261+
"handle": []interface{}{reverseProxy},
262+
"terminal": true,
260263
}
261264

262-
// Add terminal to stop processing after this route matches
263-
route["terminal"] = true
264-
265-
routes = append(routes, route)
265+
// Add route to port-specific group
266+
routesByPort[port] = append(routesByPort[port], route)
266267

267268
// Track TLS hostnames for automation policy
268269
// For patterns, use the wildcard for TLS (e.g., "*.example.com")
269270
if rule.TLS {
270271
tlsHostnames = append(tlsHostnames, hostnameMatch)
272+
tlsPortsByHostname[hostnameMatch] = append(tlsPortsByHostname[hostnameMatch], port)
273+
tlsEnabledPorts[port] = true
271274

272275
// Add HTTP redirect route if requested
273-
// Uses protocol matcher to only redirect HTTP, not HTTPS (which would cause redirect loop)
276+
// These go to port 80 server
274277
if rule.RedirectHTTP {
275-
listenPorts[80] = true
276278
redirectRoute := map[string]interface{}{
277279
"match": []interface{}{
278280
map[string]interface{}{
@@ -291,16 +293,14 @@ func (g *CaddyConfigGenerator) buildConfig(ctx context.Context, ingresses []Ingr
291293
},
292294
"terminal": true,
293295
}
294-
redirectRoutes = append(redirectRoutes, redirectRoute)
296+
routesByPort[80] = append(routesByPort[80], redirectRoute)
295297
}
296298
}
297299
}
298300
}
299301

300302
// Add API ingress route if configured
301303
// This routes requests to the API hostname directly to localhost (Hypeman API)
302-
// IMPORTANT: API route must be prepended to routes so it takes precedence over
303-
// wildcard patterns that might otherwise match the API hostname
304304
if g.apiIngress.IsEnabled() {
305305
log.InfoContext(ctx, "adding API ingress route", "hostname", g.apiIngress.Hostname, "port", g.apiIngress.Port)
306306

@@ -321,18 +321,17 @@ func (g *CaddyConfigGenerator) buildConfig(ctx context.Context, ingresses []Ingr
321321
"handle": []interface{}{apiReverseProxy},
322322
"terminal": true,
323323
}
324-
// Prepend API route so it takes precedence over wildcards
325-
routes = append([]interface{}{apiRoute}, routes...)
326324

327-
// Add TLS configuration for API hostname
325+
// Determine which port the API route goes to
326+
apiListenPort := 80
328327
if g.apiIngress.TLS {
329-
listenPorts[443] = true
328+
apiListenPort = 443
330329
tlsHostnames = append(tlsHostnames, g.apiIngress.Hostname)
330+
tlsPortsByHostname[g.apiIngress.Hostname] = append(tlsPortsByHostname[g.apiIngress.Hostname], 443)
331+
tlsEnabledPorts[443] = true
331332

332333
// Add HTTP to HTTPS redirect for API hostname
333-
// Prepend so it takes precedence over wildcard redirects
334334
if g.apiIngress.RedirectHTTP {
335-
listenPorts[80] = true
336335
apiRedirectRoute := map[string]interface{}{
337336
"match": []interface{}{
338337
map[string]interface{}{
@@ -351,22 +350,12 @@ func (g *CaddyConfigGenerator) buildConfig(ctx context.Context, ingresses []Ingr
351350
},
352351
"terminal": true,
353352
}
354-
redirectRoutes = append([]interface{}{apiRedirectRoute}, redirectRoutes...)
353+
// Prepend so API redirect takes precedence
354+
routesByPort[80] = append([]interface{}{apiRedirectRoute}, routesByPort[80]...)
355355
}
356-
} else {
357-
listenPorts[80] = true
358356
}
359-
}
360-
361-
// Build listen addresses (sorted for deterministic config output)
362-
ports := make([]int, 0, len(listenPorts))
363-
for port := range listenPorts {
364-
ports = append(ports, port)
365-
}
366-
sort.Ints(ports)
367-
listenAddrs := make([]string, 0, len(ports))
368-
for _, port := range ports {
369-
listenAddrs = append(listenAddrs, fmt.Sprintf("%s:%d", g.listenAddress, port))
357+
// Prepend API route so it takes precedence over wildcards
358+
routesByPort[apiListenPort] = append([]interface{}{apiRoute}, routesByPort[apiListenPort]...)
370359
}
371360

372361
// Build base config (admin API only)
@@ -377,19 +366,20 @@ func (g *CaddyConfigGenerator) buildConfig(ctx context.Context, ingresses []Ingr
377366
},
378367
}
379368

380-
// Only add HTTP server if we have listen addresses (i.e., ingresses exist)
381-
if len(listenAddrs) > 0 {
382-
// Build server configuration
383-
server := map[string]interface{}{
384-
"listen": listenAddrs,
385-
}
369+
// Build servers map - one server per unique listen port
370+
// This isolates routes by port, preventing conflicts when multiple wildcard
371+
// ingresses match the same hostname pattern on different ports.
372+
if len(routesByPort) > 0 {
373+
servers := map[string]interface{}{}
386374

387-
// Combine redirect routes (for HTTP) and main routes
388-
// Use slices.Concat to avoid modifying original slices
389-
allRoutes := slices.Concat(redirectRoutes, routes)
375+
// Get sorted list of ports for deterministic output
376+
ports := make([]int, 0, len(routesByPort))
377+
for port := range routesByPort {
378+
ports = append(ports, port)
379+
}
380+
sort.Ints(ports)
390381

391-
// Add catch-all route at the end to return 404 for unmatched hostnames
392-
// This must be last since routes are evaluated in order
382+
// Catch-all route returns 404 for unmatched hostnames
393383
catchAllRoute := map[string]interface{}{
394384
"handle": []interface{}{
395385
map[string]interface{}{
@@ -402,31 +392,40 @@ func (g *CaddyConfigGenerator) buildConfig(ctx context.Context, ingresses []Ingr
402392
},
403393
},
404394
}
405-
allRoutes = append(allRoutes, catchAllRoute)
406395

407-
server["routes"] = allRoutes
396+
for _, port := range ports {
397+
routes := routesByPort[port]
408398

409-
// Configure automatic HTTPS settings
410-
if len(tlsHostnames) > 0 {
411-
// When we have TLS hostnames, disable only redirects - we handle them explicitly
412-
server["automatic_https"] = map[string]interface{}{
413-
"disable_redirects": true,
399+
// Add catch-all at the end of each server's routes
400+
allRoutes := append(routes, catchAllRoute)
401+
402+
server := map[string]interface{}{
403+
"listen": []string{fmt.Sprintf("%s:%d", g.listenAddress, port)},
404+
"routes": allRoutes,
405+
"logs": map[string]interface{}{}, // Disable access logs
414406
}
415-
} else {
416-
// No TLS hostnames - disable automatic HTTPS completely
417-
server["automatic_https"] = map[string]interface{}{
418-
"disable": true,
407+
408+
// Configure automatic HTTPS settings based on whether this port has TLS routes
409+
if tlsEnabledPorts[port] {
410+
// This port has TLS routes - disable only automatic redirects (we handle them explicitly)
411+
server["automatic_https"] = map[string]interface{}{
412+
"disable_redirects": true,
413+
}
414+
} else {
415+
// No TLS routes on this port - disable automatic HTTPS completely
416+
server["automatic_https"] = map[string]interface{}{
417+
"disable": true,
418+
}
419419
}
420-
}
421420

422-
// Disable access logs (per-request logs) - we only want system logs
423-
server["logs"] = map[string]interface{}{}
421+
// Use descriptive server names
422+
serverName := fmt.Sprintf("ingress-%d", port)
423+
servers[serverName] = server
424+
}
424425

425426
config["apps"] = map[string]interface{}{
426427
"http": map[string]interface{}{
427-
"servers": map[string]interface{}{
428-
"ingress": server,
429-
},
428+
"servers": servers,
430429
},
431430
}
432431
}

lib/ingress/config_test.go

Lines changed: 126 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -295,21 +295,33 @@ func TestGenerateConfig_DeterministicOrder(t *testing.T) {
295295
}
296296
}
297297

298-
// Also verify the listen addresses are in sorted order (80, 443, 9000)
298+
// Verify we have separate servers for each port (port-isolated architecture)
299299
var config map[string]interface{}
300300
err := json.Unmarshal(firstOutput, &config)
301301
require.NoError(t, err)
302302

303303
apps := config["apps"].(map[string]interface{})
304304
httpApp := apps["http"].(map[string]interface{})
305305
servers := httpApp["servers"].(map[string]interface{})
306-
ingressServer := servers["ingress"].(map[string]interface{})
307-
listenAddrs := ingressServer["listen"].([]interface{})
308306

309-
require.Len(t, listenAddrs, 3)
310-
assert.Equal(t, "0.0.0.0:80", listenAddrs[0].(string))
311-
assert.Equal(t, "0.0.0.0:443", listenAddrs[1].(string))
312-
assert.Equal(t, "0.0.0.0:9000", listenAddrs[2].(string))
307+
// Should have 3 separate servers, one per port
308+
require.Len(t, servers, 3)
309+
310+
// Verify each server exists and has correct listen address
311+
server80 := servers["ingress-80"].(map[string]interface{})
312+
listen80 := server80["listen"].([]interface{})
313+
require.Len(t, listen80, 1)
314+
assert.Equal(t, "0.0.0.0:80", listen80[0].(string))
315+
316+
server443 := servers["ingress-443"].(map[string]interface{})
317+
listen443 := server443["listen"].([]interface{})
318+
require.Len(t, listen443, 1)
319+
assert.Equal(t, "0.0.0.0:443", listen443[0].(string))
320+
321+
server9000 := servers["ingress-9000"].(map[string]interface{})
322+
listen9000 := server9000["listen"].([]interface{})
323+
require.Len(t, listen9000, 1)
324+
assert.Equal(t, "0.0.0.0:9000", listen9000[0].(string))
313325
}
314326

315327
func TestGenerateConfig_DefaultPort(t *testing.T) {
@@ -731,10 +743,11 @@ func TestGenerateConfig_MixedTLSAndNonTLS(t *testing.T) {
731743
// Verify HTTP redirect is present (for TLS rule with redirect_http)
732744
assert.Contains(t, configStr, "301")
733745

734-
// Verify automatic_https has disable_redirects (not fully disabled)
735-
// because we have TLS hostnames
746+
// With port-isolated servers:
747+
// - Port 443 server has "disable_redirects": true (TLS enabled, manual redirects)
748+
// - Port 80 server has "disable": true (HTTP only, no TLS)
736749
assert.Contains(t, configStr, `"disable_redirects"`)
737-
assert.NotContains(t, configStr, `"disable": true`)
750+
assert.Contains(t, configStr, `"disable": true`) // Port 80 server disables HTTPS completely
738751
}
739752

740753
func TestHasTLSRules(t *testing.T) {
@@ -891,11 +904,109 @@ func TestGenerateConfig_TLSHostnameDeduplication(t *testing.T) {
891904
assert.Len(t, subjects, 1, "TLS subjects should be deduplicated")
892905
assert.Equal(t, "*.example.com", subjects[0].(string))
893906

894-
// Verify all three ports are in listen addresses
895-
configStr := string(data)
896-
assert.Contains(t, configStr, ":443")
897-
assert.Contains(t, configStr, ":3000")
898-
assert.Contains(t, configStr, ":8080")
907+
// Verify all three ports have separate servers (port isolation)
908+
httpApp := apps["http"].(map[string]interface{})
909+
servers := httpApp["servers"].(map[string]interface{})
910+
assert.Contains(t, servers, "ingress-443")
911+
assert.Contains(t, servers, "ingress-3000")
912+
assert.Contains(t, servers, "ingress-8080")
913+
}
914+
915+
func TestGenerateConfig_PortIsolation(t *testing.T) {
916+
// Test that wildcard ingresses on different ports don't conflict
917+
// by verifying each port gets its own isolated server with routes
918+
tmpDir, err := os.MkdirTemp("", "ingress-config-port-isolation-test-*")
919+
require.NoError(t, err)
920+
defer os.RemoveAll(tmpDir)
921+
922+
p := paths.New(tmpDir)
923+
require.NoError(t, os.MkdirAll(p.CaddyDir(), 0755))
924+
require.NoError(t, os.MkdirAll(p.CaddyDataDir(), 0755))
925+
926+
// Create generator with ACME configured
927+
acmeConfig := ACMEConfig{
928+
Email: "admin@example.com",
929+
DNSProvider: DNSProviderCloudflare,
930+
CloudflareAPIToken: "test-token",
931+
AllowedDomains: "*.example.com",
932+
}
933+
generator := NewCaddyConfigGenerator(p, "0.0.0.0", "127.0.0.1", 2019, acmeConfig, APIIngressConfig{}, 5353)
934+
935+
ctx := context.Background()
936+
// Create wildcard ingresses on different ports that would conflict
937+
// if they were in the same server (same hostname pattern, different target ports)
938+
ingresses := []Ingress{
939+
{
940+
ID: "ing-legacy",
941+
Name: "wildcard-legacy",
942+
Rules: []IngressRule{
943+
{
944+
// Old style: HTTPS on 443 -> backend on 80
945+
Match: IngressMatch{Hostname: "{instance}.example.com", Port: 443},
946+
Target: IngressTarget{Instance: "{instance}", Port: 80},
947+
TLS: true,
948+
},
949+
},
950+
},
951+
{
952+
ID: "ing-app",
953+
Name: "wildcard-app",
954+
Rules: []IngressRule{
955+
{
956+
// New style: HTTPS on 3000 -> backend on 3000
957+
Match: IngressMatch{Hostname: "{instance}.example.com", Port: 3000},
958+
Target: IngressTarget{Instance: "{instance}", Port: 3000},
959+
TLS: true,
960+
},
961+
},
962+
},
963+
}
964+
965+
data, err := generator.GenerateConfig(ctx, ingresses)
966+
require.NoError(t, err)
967+
968+
// Parse the config
969+
var config map[string]interface{}
970+
err = json.Unmarshal(data, &config)
971+
require.NoError(t, err)
972+
973+
apps := config["apps"].(map[string]interface{})
974+
httpApp := apps["http"].(map[string]interface{})
975+
servers := httpApp["servers"].(map[string]interface{})
976+
977+
// Should have 2 separate servers (one per port)
978+
assert.Len(t, servers, 2)
979+
980+
// Verify server for port 443 exists and routes to port 80
981+
server443 := servers["ingress-443"].(map[string]interface{})
982+
routes443 := server443["routes"].([]interface{})
983+
assert.GreaterOrEqual(t, len(routes443), 1)
984+
985+
// First route should be the wildcard route to port 80
986+
route443 := routes443[0].(map[string]interface{})
987+
handle443 := route443["handle"].([]interface{})[0].(map[string]interface{})
988+
dynamicUpstreams443 := handle443["dynamic_upstreams"].(map[string]interface{})
989+
assert.Equal(t, "80", dynamicUpstreams443["port"])
990+
991+
// Verify server for port 3000 exists and routes to port 3000
992+
server3000 := servers["ingress-3000"].(map[string]interface{})
993+
routes3000 := server3000["routes"].([]interface{})
994+
assert.GreaterOrEqual(t, len(routes3000), 1)
995+
996+
// First route should be the wildcard route to port 3000
997+
route3000 := routes3000[0].(map[string]interface{})
998+
handle3000 := route3000["handle"].([]interface{})[0].(map[string]interface{})
999+
dynamicUpstreams3000 := handle3000["dynamic_upstreams"].(map[string]interface{})
1000+
assert.Equal(t, "3000", dynamicUpstreams3000["port"])
1001+
1002+
// Verify each server only listens on its own port
1003+
listen443 := server443["listen"].([]interface{})
1004+
assert.Len(t, listen443, 1)
1005+
assert.Equal(t, "0.0.0.0:443", listen443[0])
1006+
1007+
listen3000 := server3000["listen"].([]interface{})
1008+
assert.Len(t, listen3000, 1)
1009+
assert.Equal(t, "0.0.0.0:3000", listen3000[0])
8991010
}
9001011

9011012
func TestDeduplicateStrings(t *testing.T) {

0 commit comments

Comments
 (0)