Conversation
Allow routing wildcard hostnames (e.g., *.pds.miren.garden) to apps. Lookup chains exact match → wildcard subdomain → wildcard bare domain, all using existing entity index for O(1) lookups. Includes CLI validation, TLS cert provisioning for wildcard subdomains, and tests.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds wildcard routing: Client.LookupWithWildcard and ValidateWildcardHost, CLI host validation, tests, and docs. Replaces the route-watcher/autocert integration with a controller-based TLS model: removes RouteWatcher/RouteSet, introduces CertificateProvider and HTTPChallengeProvider interfaces, adds ServeTLSWithController, and wires an AutocertController plus coordinator support for ACME HTTP-01 (including readiness signaling). Exposes LoadOrGenerateFallbackCert as LoadOrGenerateFallbackCert. HTTP ingress lookup now uses LookupWithWildcard. Tests and documentation updated to cover wildcard behavior and the new certificate controller. Comment |
Update traffic-routing guide with wildcard route section, add TLS note for wildcard subdomain cert provisioning, add changelog entry, and update route terminology to mention wildcard patterns.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
components/autotls/autotls.go (1)
39-62: Consider sharing the wildcard-candidate generation with ingress lookup.
HasRoutenow reimplements the same exact → first-label wildcard → bare-domain wildcard order asapi/ingress/client.go. Pulling that host-to-candidate logic into one helper would reduce drift between TLS certificate decisions and request routing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/autotls/autotls.go` around lines 39 - 62, HasRoute duplicates the host→candidate generation logic used by the ingress lookup; extract that logic into a shared helper (e.g., HostCandidates(host string) []string) that returns candidates in the exact → first-label-wildcard → bare-domain-wildcard order, then change RouteSet.HasRoute to iterate those candidates against rs.hosts (and update api/ingress/client.go to call the same helper) so both TLS decisioning and routing use the identical candidate generation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/ingress/client.go`:
- Around line 97-111: Update ValidateWildcardHost to reject any '*' characters
that are not part of a single leading '*.' pattern: if host doesn't start with
"*." and contains any '*' return an error; if it starts with "*." continue
existing checks (remainder non-empty, no additional '*', and remainder contains
a dot). Also ensure callers like SetRoute call ValidateWildcardHost(host) and
propagate the error, and update TestValidateWildcardHost to mark cases like
"foo.*.com", "*" and "**" as expecting an error.
---
Nitpick comments:
In `@components/autotls/autotls.go`:
- Around line 39-62: HasRoute duplicates the host→candidate generation logic
used by the ingress lookup; extract that logic into a shared helper (e.g.,
HostCandidates(host string) []string) that returns candidates in the exact →
first-label-wildcard → bare-domain-wildcard order, then change RouteSet.HasRoute
to iterate those candidates against rs.hosts (and update api/ingress/client.go
to call the same helper) so both TLS decisioning and routing use the identical
candidate generation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8beabeae-7b26-4bef-a631-34d5c7c9e945
📒 Files selected for processing (5)
api/ingress/client.goapi/ingress/client_test.gocli/commands/route_set.gocomponents/autotls/autotls.goservers/httpingress/httpingress.go
| // ValidateWildcardHost validates a wildcard host pattern. | ||
| // Valid patterns: *.example.com, *.sub.example.com | ||
| // Invalid: *.com, foo.*.com, **, * | ||
| func ValidateWildcardHost(host string) error { | ||
| if !strings.HasPrefix(host, "*.") { | ||
| return nil | ||
| } | ||
| remainder := host[2:] | ||
| if remainder == "" || strings.Contains(remainder, "*") { | ||
| return fmt.Errorf("invalid wildcard pattern: %s (must be *.domain.tld)", host) | ||
| } | ||
| if !strings.Contains(remainder, ".") { | ||
| return fmt.Errorf("invalid wildcard pattern: %s (must have at least two domain labels after *)", host) | ||
| } | ||
| return nil |
There was a problem hiding this comment.
Reject any * outside a single leading *. pattern.
ValidateWildcardHost currently returns nil for foo.*.com, *, and ** because it exits early unless the host starts with *.. That contradicts the helper’s own docstring and the PR contract, and right now the CLI path is the only place this validation runs. Please fail on any other * placement and enforce the same check from SetRoute so non-CLI callers can’t persist bad hosts.
🔧 Possible fix
func ValidateWildcardHost(host string) error {
- if !strings.HasPrefix(host, "*.") {
+ if !strings.Contains(host, "*") {
return nil
}
+ if !strings.HasPrefix(host, "*.") || strings.Count(host, "*") != 1 {
+ return fmt.Errorf("invalid wildcard pattern: %s (must be *.domain.tld)", host)
+ }
remainder := host[2:]
- if remainder == "" || strings.Contains(remainder, "*") {
+ if remainder == "" {
return fmt.Errorf("invalid wildcard pattern: %s (must be *.domain.tld)", host)
}
if !strings.Contains(remainder, ".") {
return fmt.Errorf("invalid wildcard pattern: %s (must have at least two domain labels after *)", host)
}
return nil
}// Also enforce this in SetRoute:
if err := ValidateWildcardHost(host); err != nil {
return nil, err
}TestValidateWildcardHost should then flip foo.*.com to wantErr: true and add * / ** cases.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // ValidateWildcardHost validates a wildcard host pattern. | |
| // Valid patterns: *.example.com, *.sub.example.com | |
| // Invalid: *.com, foo.*.com, **, * | |
| func ValidateWildcardHost(host string) error { | |
| if !strings.HasPrefix(host, "*.") { | |
| return nil | |
| } | |
| remainder := host[2:] | |
| if remainder == "" || strings.Contains(remainder, "*") { | |
| return fmt.Errorf("invalid wildcard pattern: %s (must be *.domain.tld)", host) | |
| } | |
| if !strings.Contains(remainder, ".") { | |
| return fmt.Errorf("invalid wildcard pattern: %s (must have at least two domain labels after *)", host) | |
| } | |
| return nil | |
| // ValidateWildcardHost validates a wildcard host pattern. | |
| // Valid patterns: *.example.com, *.sub.example.com | |
| // Invalid: *.com, foo.*.com, **, * | |
| func ValidateWildcardHost(host string) error { | |
| if !strings.Contains(host, "*") { | |
| return nil | |
| } | |
| if !strings.HasPrefix(host, "*.") || strings.Count(host, "*") != 1 { | |
| return fmt.Errorf("invalid wildcard pattern: %s (must be *.domain.tld)", host) | |
| } | |
| remainder := host[2:] | |
| if remainder == "" { | |
| return fmt.Errorf("invalid wildcard pattern: %s (must be *.domain.tld)", host) | |
| } | |
| if !strings.Contains(remainder, ".") { | |
| return fmt.Errorf("invalid wildcard pattern: %s (must have at least two domain labels after *)", host) | |
| } | |
| return nil | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/ingress/client.go` around lines 97 - 111, Update ValidateWildcardHost to
reject any '*' characters that are not part of a single leading '*.' pattern: if
host doesn't start with "*." and contains any '*' return an error; if it starts
with "*." continue existing checks (remainder non-empty, no additional '*', and
remainder contains a dot). Also ensure callers like SetRoute call
ValidateWildcardHost(host) and propagate the error, and update
TestValidateWildcardHost to mark cases like "foo.*.com", "*" and "**" as
expecting an error.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/docs/tls.md (1)
15-15: Consider clarifying that wildcard routes also match the bare domain.The phrase "each matching subdomain" may not fully convey that wildcard routes also match the bare domain itself. Based on the detailed explanation in
traffic-routing.md(lines 31-33),*.myapp.example.commatches both subdomains andmyapp.example.com(the bare domain).📝 Optional refinement for clarity
-For wildcard routes (e.g., `*.myapp.example.com`), TLS certificates are provisioned for each matching subdomain as requests arrive. See [Wildcard Routes](/traffic-routing#wildcard-routes) for details. +For wildcard routes (e.g., `*.myapp.example.com`), TLS certificates are provisioned for each matching hostname (including subdomains and the bare domain) as requests arrive. See [Wildcard Routes](/traffic-routing#wildcard-routes) for details.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/docs/tls.md` at line 15, Update the sentence about wildcard routes so it explicitly states that a pattern like '*.myapp.example.com' matches both all subdomains and the bare domain ('myapp.example.com'); edit the line containing the example '*.myapp.example.com' (and the phrase "each matching subdomain") to say "each matching subdomain and the bare domain (e.g., 'myapp.example.com')" and ensure the existing "Wildcard Routes" link remains for details.docs/docs/terminology.md (1)
43-43: Clarify that wildcard routes match both subdomains and the bare domain.The phrase "match all subdomains" is slightly imprecise. According to the detailed documentation in
traffic-routing.md(lines 31-33), wildcard routes like*.myapp.example.commatch both subdomains (foo.myapp.example.com) and the bare domain itself (myapp.example.com).📝 Suggested refinement for precision
-Maps a hostname to an application. Routes determine how HTTP traffic reaches your apps. Your first app gets a default route automatically. Routes can use wildcard patterns (e.g., `*.example.com`) to match all subdomains of a domain. +Maps a hostname to an application. Routes determine how HTTP traffic reaches your apps. Your first app gets a default route automatically. Routes can use wildcard patterns (e.g., `*.example.com`) to match subdomains and the bare domain.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/docs/terminology.md` at line 43, Update the sentence describing wildcard routes so it explicitly states that patterns like `*.example.com` match both subdomains and the bare (root) domain; e.g., change the line containing "Routes can use wildcard patterns (e.g., `*.example.com`) to match all subdomains of a domain." to say they "match both subdomains and the bare domain (for example, `*.myapp.example.com` matches `foo.myapp.example.com` and `myapp.example.com`)" and ensure consistency with the explanation in traffic-routing.md.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/docs/terminology.md`:
- Line 43: Update the sentence describing wildcard routes so it explicitly
states that patterns like `*.example.com` match both subdomains and the bare
(root) domain; e.g., change the line containing "Routes can use wildcard
patterns (e.g., `*.example.com`) to match all subdomains of a domain." to say
they "match both subdomains and the bare domain (for example,
`*.myapp.example.com` matches `foo.myapp.example.com` and `myapp.example.com`)"
and ensure consistency with the explanation in traffic-routing.md.
In `@docs/docs/tls.md`:
- Line 15: Update the sentence about wildcard routes so it explicitly states
that a pattern like '*.myapp.example.com' matches both all subdomains and the
bare domain ('myapp.example.com'); edit the line containing the example
'*.myapp.example.com' (and the phrase "each matching subdomain") to say "each
matching subdomain and the bare domain (e.g., 'myapp.example.com')" and ensure
the existing "Wildcard Routes" link remains for details.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eb1fc769-9dca-4060-bad9-f719c4ec706f
📒 Files selected for processing (4)
docs/docs/changelog.mddocs/docs/terminology.mddocs/docs/tls.mddocs/docs/traffic-routing.md
✅ Files skipped from review due to trivial changes (1)
- docs/docs/changelog.md
phinze
left a comment
phinze
left a comment
There was a problem hiding this comment.
Claude and I sat down with this one and traced through the lookup chains, the autotls cert provisioning path, and the validation logic end-to-end. Clean feature, well-structured.
The three-step fallback in LookupWithWildcard (exact → wildcard subdomain → wildcard bare domain) is a nice design — deterministic priority with no iteration or regex, just index lookups. And mirroring the same logic in RouteSet.HasRoute for cert provisioning means wildcards get per-subdomain certs on demand rather than a blanket wildcard cert, which is the right call.
One thing we noted: on the HTTP ingress hot path, a wildcard-routed subdomain now does 2 serial entity lookups (exact miss → wildcard hit), and a complete miss does 3 before falling through to the default route check. These are local RPC calls so latency is low, but if we ever want to tighten this up, a batched LookupAny(ctx, ...keys) that resolves multiple index keys in one RPC round-trip would collapse the chain. Not urgent — exact matches (the common case) are still 1 call — just flagging it as a future optimization opportunity.
When `miren route set` links a subdomain to an app, the first HTTPS request would time out because autocert only provisions certs lazily on the first TLS handshake — blocking the request for 30-90s while ACME completes. Introduces AutocertController, which plugs into the existing controller framework to watch http_route entities and eagerly call autocert.Manager.GetCertificate() when routes appear. This replaces the bespoke RouteWatcher + ServeTLS machinery with the same reconcile pattern the DNS-01 certificate controller already uses. The coordinator now always creates a certificate controller: DNS-01 via lego when a DNS provider is configured, HTTP-01 via autocert otherwise. A HostPolicy on the autocert manager restricts cert provisioning to domains with active routes, and a ready channel handles the startup ordering between the controller manager and the port-80 ACME challenge server. MIR-771
Matches the port-80 HTTP server's timeout for consistency and slowloris protection.
HTTP-01 challenges can't provision wildcard certs (that requires DNS-01), so wildcard routes like *.example.com need special handling. When Reconcile sees a wildcard route, it stores the wildcard pattern in allowedHosts for HostPolicy matching, but eagerly provisions only the base domain cert. Individual subdomains provision inline when they first arrive via GetCertificate. isAllowedHost checks both exact matches and wildcard coverage, so foo.example.com is allowed when *.example.com is in the set.
Wildcard routes (*.example.com) can't be eagerly provisioned — HTTP-01 can't issue wildcard certs and we don't know which subdomains will be requested. Just store the pattern in allowedHosts so HostPolicy accepts subdomains, and let them provision inline on first TLS handshake. The bare domain (example.com) is not covered by its wildcard, matching DNS semantics — it needs its own explicit route to get a cert.
…t-when-miren-route-set-is-run Prefetch TLS certs eagerly when routes are configured
|
@phinze agree on the round trips, we can do that as a follow up. I think we might want to also add a cache on the check side since most of the pain will come from something doing requests over and over, so even a 5 min TTL cache would eliminate most of the pain. |
…le.com Wildcard routes match exactly one DNS label, so *.foo.example.com matches blah.foo.example.com but not foo.example.com. Removes the bare-domain fallback from LookupWithWildcard and isAllowedHost to align routing and TLS cert provisioning on the same semantics.
evanphx
deleted the
mir-781-add-ability-to-route-wildcard-routes-to-an-app
branch
2 participants
Summary
LookupWithWildcardto ingress client: chains exact match → wildcard subdomain (*.example.com) → wildcard bare domain → nil, all using existing entity index for O(1) lookupsValidateWildcardHostto reject invalid patterns like*.comorfoo.*.comRouteSet.HasRoutein autotls to provision TLS certs for wildcard subdomainsmiren route set *.example.com app)Test plan
hack/it ./api/ingress/...— 27 tests pass (wildcard subdomain, bare domain, exact priority, case insensitivity, validation)hack/it ./components/autotls/...— 8 tests passmake test— 3582 tests pass, 20 skippedmake lint— 0 issues