fix: produce valid listener names for wildcard and unusual DNS names
A DNS name like *.home.hrajfrisbee.cz put "*" into the listener name; the API server rejected the patch, which blocked every certificate on that Gateway. - lowercase the DNS part, spell "*" as "wildcard", map other characters outside [a-z0-9] to "-" - names over 253 chars are cut and get a hash suffix instead of a blind cut - names that were valid before are unchanged Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -55,8 +55,10 @@ A reverse watch maps Gateway events back to the targeting Certificates through t
|
|||||||
same index. Manual edits to `auto-*` listeners are reverted only in the fields
|
same index. Manual edits to `auto-*` listeners are reverted only in the fields
|
||||||
`ListenersEqual` compares, and only while at least one Certificate targets the Gateway.
|
`ListenersEqual` compares, and only while at least one Certificate targets the Gateway.
|
||||||
|
|
||||||
Listeners named `auto-{certName}-{dnsName with dots as dashes}` belong to the
|
Listeners named `auto-{certName}-{sanitized dnsName}` belong to the operator. The DNS
|
||||||
operator. Any other listener is never modified or removed.
|
name is lowercased, `*` becomes `wildcard`, anything else outside `[a-z0-9]` becomes
|
||||||
|
`-`; names over 253 chars are cut and get a hash suffix. Changing this scheme renames
|
||||||
|
existing listeners on live Gateways. Any other listener is never modified or removed.
|
||||||
|
|
||||||
`docs/flows.md` has the full control-path reference (startup, watch wiring, decision
|
`docs/flows.md` has the full control-path reference (startup, watch wiring, decision
|
||||||
tree, scenarios).
|
tree, scenarios).
|
||||||
@@ -241,8 +243,9 @@ Gitea Actions, workflows in `.gitea/workflows/`:
|
|||||||
it does not clean up the old Gateway. A sibling Certificate's next reconcile fixes
|
it does not clean up the old Gateway. A sibling Certificate's next reconcile fixes
|
||||||
it; with no sibling the listener stays forever.
|
it; with no sibling the listener stays forever.
|
||||||
- **A single bad listener blocks the whole Gateway.** The patch replaces the full
|
- **A single bad listener blocks the whole Gateway.** The patch replaces the full
|
||||||
listeners array, so a wildcard DNS name (`*` ends up in the listener name) or a name
|
listeners array, so a listener-name collision (same cert name in two namespaces,
|
||||||
collision makes the API server reject every update for that Gateway.
|
or two DNS names that sanitize alike, such as `a.b.com` and `a-b.com`) makes the
|
||||||
|
API server reject every update for that Gateway.
|
||||||
- **`ListenersEqual` is partial**: name, hostname, port and the first secret ref only.
|
- **`ListenersEqual` is partial**: name, hostname, port and the first secret ref only.
|
||||||
Changing what `BuildListenersFromCertificates` generates in other fields will not
|
Changing what `BuildListenersFromCertificates` generates in other fields will not
|
||||||
roll out to existing listeners.
|
roll out to existing listeners.
|
||||||
|
|||||||
+11
-8
@@ -325,19 +325,22 @@ The comparison is done via a name-keyed map (`listenerMap`, `patch.go:118`), so
|
|||||||
|
|
||||||
## 7. Listener Naming and Sanitization
|
## 7. Listener Naming and Sanitization
|
||||||
|
|
||||||
`listenerNameFor` (`patch.go:143`):
|
`listenerNameFor` and `sanitizeDNSName` (`patch.go`):
|
||||||
|
|
||||||
```
|
```
|
||||||
input: prefix="auto-" certName="my-app-cert" dnsName="app.example.com"
|
input: prefix="auto-" certName="my-app-cert" dnsName="app.example.com"
|
||||||
|
|
||||||
step 1: replace every "." with "-"
|
step 1: sanitize the DNS name
|
||||||
|
lowercase; "*" ──► "wildcard"; every other character outside
|
||||||
|
[a-z0-9] ──► "-"; trim leading and trailing "-"
|
||||||
"app.example.com" ──► "app-example-com"
|
"app.example.com" ──► "app-example-com"
|
||||||
|
|
||||||
step 2: concatenate
|
step 2: concatenate
|
||||||
"auto-" + "my-app-cert" + "-" + "app-example-com"
|
"auto-" + "my-app-cert" + "-" + "app-example-com"
|
||||||
──► "auto-my-app-cert-app-example-com"
|
──► "auto-my-app-cert-app-example-com"
|
||||||
|
|
||||||
step 3: if len > 253 bytes: name = name[:253] (hard byte slice)
|
step 3: if len > 253: cut to 244, trim trailing "-" and ".", append
|
||||||
|
"-" + first 8 hex chars of sha256(full name)
|
||||||
```
|
```
|
||||||
|
|
||||||
Example outputs for a cert with two DNS names:
|
Example outputs for a cert with two DNS names:
|
||||||
@@ -345,9 +348,9 @@ Example outputs for a cert with two DNS names:
|
|||||||
| `dnsName` | listener `Name` |
|
| `dnsName` | listener `Name` |
|
||||||
|-----------|-----------------|
|
|-----------|-----------------|
|
||||||
| `app.example.com` | `auto-my-app-cert-app-example-com` |
|
| `app.example.com` | `auto-my-app-cert-app-example-com` |
|
||||||
| `*.example.com` | `auto-my-app-cert-*-example-com` |
|
| `*.example.com` | `auto-my-app-cert-wildcard-example-com` |
|
||||||
|
|
||||||
The second entry is a risk: `*` is not valid in a Gateway API `SectionName` (which must match RFC 1123). The API server will reject the patch and the reconciler will requeue forever (see §10, item 7).
|
The listener `Hostname` keeps the original DNS name, wildcard included.
|
||||||
|
|
||||||
The sanitization is also lossy: `a.b.com` and `a-b-com` produce the same result. If a certificate lists both, two listeners with the same name are appended; the API server rejects the patch (listener names must be unique), so no listener on that Gateway is updated.
|
The sanitization is also lossy: `a.b.com` and `a-b-com` produce the same result. If a certificate lists both, two listeners with the same name are appended; the API server rejects the patch (listener names must be unique), so no listener on that Gateway is updated.
|
||||||
|
|
||||||
@@ -490,9 +493,9 @@ Branch (F) of Reconcile treats all Gateway `Get` errors the same (including `Not
|
|||||||
`ctrl.NewManager` at `main.go:50` does not set `LeaderElection: true`. The `deploy/deployment_gateway-cert-operator.yaml` runs `replicas: 1` which makes this safe today. Scaling to multiple replicas without enabling leader election would result in concurrent Gateway patches.
|
`ctrl.NewManager` at `main.go:50` does not set `LeaderElection: true`. The `deploy/deployment_gateway-cert-operator.yaml` runs `replicas: 1` which makes this safe today. Scaling to multiple replicas without enabling leader election would result in concurrent Gateway patches.
|
||||||
|
|
||||||
**7. `listenerNameFor` edge cases**
|
**7. `listenerNameFor` edge cases**
|
||||||
- **Wildcard hostnames**: `*.example.com` produces `SectionName` `auto-…-*-example-com`. The `*` character is not valid in an RFC 1123 label; the Gateway API server will reject the Patch, causing an error-retry loop.
|
- **Wildcard hostnames** (fixed): `*` used to end up in the listener name and the API server rejected the patch. It is now spelled out as `wildcard`.
|
||||||
- **Collisions after sanitization**: `a.b.com` and `a-b-com` produce identical sanitized names. The same happens across certificates (cert `a` with `b-c.com` and cert `a-b` with `c.com`) and across namespaces, since the name omits the cert namespace. Both listeners are appended and the API server rejects the patch for the whole Gateway.
|
- **Collisions after sanitization**: `a.b.com` and `a-b-com` produce identical sanitized names, as do `*.x.com` and `wildcard.x.com`. The same happens across certificates (cert `a` with `b-c.com` and cert `a-b` with `c.com`) and across namespaces, since the name omits the cert namespace. Both listeners are appended and the API server rejects the patch for the whole Gateway.
|
||||||
- **Truncation at 253 bytes**: `patch.go:147` slices on bytes, not runes (safe for ASCII DNS names). Two DNS names sharing the first 253 characters after sanitization will map to the same listener name.
|
- **Truncation at 253 bytes** (fixed): long names are cut and get a hash of the full name appended, so they stay valid and distinct.
|
||||||
|
|
||||||
**Dead symbols in `patch.go`**
|
**Dead symbols in `patch.go`**
|
||||||
`gatewayGroup` and `gatewayKind` are declared at `patch.go:16-17` but never referenced. They can be removed without effect.
|
`gatewayGroup` and `gatewayKind` are declared at `patch.go:16-17` but never referenced. They can be removed without effect.
|
||||||
|
|||||||
@@ -3,7 +3,32 @@
|
|||||||
Plan: `docs/plans/2026-10-08-2046-listener-name-sanitize.md`
|
Plan: `docs/plans/2026-10-08-2046-listener-name-sanitize.md`
|
||||||
|
|
||||||
- [x] Step 0 — Save and commit the plan
|
- [x] Step 0 — Save and commit the plan
|
||||||
- [ ] Step 1 — Regression tests first
|
- [x] Step 1 — Regression tests first
|
||||||
- [ ] Step 2 — Fix
|
- [x] Step 2 — Fix
|
||||||
- [ ] Step 3 — Docs
|
- [x] Step 3 — Docs
|
||||||
- [ ] Step 4 — Ship
|
- [ ] Step 4 — Ship
|
||||||
|
|
||||||
|
## Steps 1–3 — Tests, fix, docs
|
||||||
|
|
||||||
|
Tests were written first and failed on the old code (wildcard, uppercase, `_`,
|
||||||
|
trailing dot, long names), then passed after the change to `listenerNameFor`.
|
||||||
|
|
||||||
|
```
|
||||||
|
$ go test -race -run 'TestListenerNameFor|TestBuildListeners' -v ./internal/gateway/
|
||||||
|
--- PASS: TestListenerNameFor_validSectionName/wildcard_label_is_spelled_out
|
||||||
|
--- PASS: TestListenerNameFor_longNames
|
||||||
|
--- PASS: TestBuildListenersFromCertificates_wildcard
|
||||||
|
$ make check
|
||||||
|
ok .../internal/gateway coverage: 98.6% of statements
|
||||||
|
```
|
||||||
|
|
||||||
|
Deviations from the plan:
|
||||||
|
|
||||||
|
- A trailing `-` or `.` is trimmed from the whole name, not only from the DNS part,
|
||||||
|
so a DNS name that sanitizes to nothing cannot leave the name ending in `-`.
|
||||||
|
- One extra case: a trailing dot (`app.example.com.`) is dropped.
|
||||||
|
- Steps 1–3 are one commit, since the tests do not pass without the fix.
|
||||||
|
|
||||||
|
Worth noting: names that were valid before are unchanged, so existing listeners are
|
||||||
|
not renamed. Not verified against a cluster; the production case is covered by the
|
||||||
|
test using the exact cert and DNS name from the error log.
|
||||||
|
|||||||
@@ -1,6 +1,8 @@
|
|||||||
package gateway
|
package gateway
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"crypto/sha256"
|
||||||
|
"encoding/hex"
|
||||||
"fmt"
|
"fmt"
|
||||||
"sort"
|
"sort"
|
||||||
"strings"
|
"strings"
|
||||||
@@ -132,17 +134,39 @@ func listenerMap(listeners []gwapiv1.Listener) map[string]listenerKey {
|
|||||||
return m
|
return m
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// maxSectionNameLen is the Gateway API limit for listener names.
|
||||||
|
const maxSectionNameLen = 253
|
||||||
|
|
||||||
// listenerNameFor generates a deterministic listener name from cert name and DNS name.
|
// listenerNameFor generates a deterministic listener name from cert name and DNS name.
|
||||||
// Truncates to stay within Gateway API's 253-char limit for section names.
|
// The result is a valid Gateway API SectionName: names that would exceed the limit
|
||||||
|
// are cut and get a hash of the full name appended, so distinct long names stay distinct.
|
||||||
func listenerNameFor(prefix, certName, dnsName string) string {
|
func listenerNameFor(prefix, certName, dnsName string) string {
|
||||||
// Replace dots with dashes for DNS names, combine with cert name.
|
name := strings.TrimRight(fmt.Sprintf("%s%s-%s", prefix, certName, sanitizeDNSName(dnsName)), "-.")
|
||||||
sanitized := strings.ReplaceAll(dnsName, ".", "-")
|
if len(name) <= maxSectionNameLen {
|
||||||
name := fmt.Sprintf("%s%s-%s", prefix, certName, sanitized)
|
|
||||||
if len(name) > 253 {
|
|
||||||
name = name[:253]
|
|
||||||
}
|
|
||||||
return name
|
return name
|
||||||
}
|
}
|
||||||
|
sum := sha256.Sum256([]byte(name))
|
||||||
|
suffix := "-" + hex.EncodeToString(sum[:4])
|
||||||
|
return strings.TrimRight(name[:maxSectionNameLen-len(suffix)], "-.") + suffix
|
||||||
|
}
|
||||||
|
|
||||||
|
// sanitizeDNSName maps a certificate DNS name onto the characters allowed in a
|
||||||
|
// listener name. A wildcard label is spelled out so "*.example.com" and
|
||||||
|
// "example.com" stay distinct.
|
||||||
|
func sanitizeDNSName(dnsName string) string {
|
||||||
|
var b strings.Builder
|
||||||
|
for _, r := range strings.ToLower(dnsName) {
|
||||||
|
switch {
|
||||||
|
case r >= 'a' && r <= 'z', r >= '0' && r <= '9':
|
||||||
|
b.WriteRune(r)
|
||||||
|
case r == '*':
|
||||||
|
b.WriteString("wildcard")
|
||||||
|
default:
|
||||||
|
b.WriteByte('-')
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return strings.Trim(b.String(), "-")
|
||||||
|
}
|
||||||
|
|
||||||
func ptrTo[T any](v T) *T {
|
func ptrTo[T any](v T) *T {
|
||||||
return &v
|
return &v
|
||||||
|
|||||||
@@ -1,6 +1,8 @@
|
|||||||
package gateway
|
package gateway
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"regexp"
|
||||||
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
@@ -272,7 +274,20 @@ func TestListenersEqual_managedFields(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestListenerNameFor_sanitizesDots(t *testing.T) {
|
// sectionNamePattern is the Gateway API SectionName validation rule.
|
||||||
|
var sectionNamePattern = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`)
|
||||||
|
|
||||||
|
func assertValidSectionName(t *testing.T, name string) {
|
||||||
|
t.Helper()
|
||||||
|
if len(name) > 253 {
|
||||||
|
t.Errorf("name is %d chars, limit is 253", len(name))
|
||||||
|
}
|
||||||
|
if !sectionNamePattern.MatchString(name) {
|
||||||
|
t.Errorf("name %q is not a valid SectionName", name)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestListenerNameFor_validSectionName(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
@@ -284,14 +299,57 @@ func TestListenerNameFor_sanitizesDots(t *testing.T) {
|
|||||||
{name: "dots become dashes", certName: "web", dnsName: "app.example.com", want: "auto-web-app-example-com"},
|
{name: "dots become dashes", certName: "web", dnsName: "app.example.com", want: "auto-web-app-example-com"},
|
||||||
{name: "single label", certName: "web", dnsName: "localhost", want: "auto-web-localhost"},
|
{name: "single label", certName: "web", dnsName: "localhost", want: "auto-web-localhost"},
|
||||||
{name: "existing dashes are kept", certName: "my-cert", dnsName: "my-app.example.com", want: "auto-my-cert-my-app-example-com"},
|
{name: "existing dashes are kept", certName: "my-cert", dnsName: "my-app.example.com", want: "auto-my-cert-my-app-example-com"},
|
||||||
|
{
|
||||||
|
name: "wildcard label is spelled out",
|
||||||
|
certName: "wildcard-home-hrajfrisbee",
|
||||||
|
dnsName: "*.home.hrajfrisbee.cz",
|
||||||
|
want: "auto-wildcard-home-hrajfrisbee-wildcard-home-hrajfrisbee-cz",
|
||||||
|
},
|
||||||
|
{name: "uppercase is lowered", certName: "web", dnsName: "App.Example.COM", want: "auto-web-app-example-com"},
|
||||||
|
{name: "other characters become dashes", certName: "web", dnsName: "my_app.example.com", want: "auto-web-my-app-example-com"},
|
||||||
|
{name: "trailing dot is dropped", certName: "web", dnsName: "app.example.com.", want: "auto-web-app-example-com"},
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, tc := range tests {
|
for _, tc := range tests {
|
||||||
t.Run(tc.name, func(t *testing.T) {
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
if got := listenerNameFor(testPrefix, tc.certName, tc.dnsName); got != tc.want {
|
got := listenerNameFor(testPrefix, tc.certName, tc.dnsName)
|
||||||
|
if got != tc.want {
|
||||||
t.Errorf("listenerNameFor() = %q, want %q", got, tc.want)
|
t.Errorf("listenerNameFor() = %q, want %q", got, tc.want)
|
||||||
}
|
}
|
||||||
|
assertValidSectionName(t, got)
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestListenerNameFor_longNames(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
// 62 dashes right where the name is cut, so a blind cut would end in "-".
|
||||||
|
shared := strings.Repeat("a", 200) + strings.Repeat("-", 62)
|
||||||
|
first := listenerNameFor(testPrefix, "web", shared+"x.example.com")
|
||||||
|
second := listenerNameFor(testPrefix, "web", shared+"y.example.com")
|
||||||
|
|
||||||
|
assertValidSectionName(t, first)
|
||||||
|
assertValidSectionName(t, second)
|
||||||
|
if first == second {
|
||||||
|
t.Errorf("two long DNS names sharing a prefix got the same listener name %q", first)
|
||||||
|
}
|
||||||
|
if again := listenerNameFor(testPrefix, "web", shared+"x.example.com"); again != first {
|
||||||
|
t.Errorf("name is not deterministic: %q then %q", first, again)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestBuildListenersFromCertificates_wildcard(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
got := BuildListenersFromCertificates(
|
||||||
|
[]cmv1.Certificate{cert("default", "wild", "wild-tls", "*.example.com")}, testPrefix)
|
||||||
|
if len(got) != 1 {
|
||||||
|
t.Fatalf("got %d listeners, want 1", len(got))
|
||||||
|
}
|
||||||
|
assertValidSectionName(t, string(got[0].Name))
|
||||||
|
if got[0].Hostname == nil || *got[0].Hostname != "*.example.com" {
|
||||||
|
t.Errorf("hostname = %v, want the wildcard kept as *.example.com", got[0].Hostname)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user