fix(dns): reconciliar reescrevia os mesmos registos a cada passagem
Duas coisas apanhadas ao correr o reconcile contra a zona real depois de a
rota passar a funcionar.
1) Nao era idempotente: 0 criados, 4 corrigidos, 11 iguais -- e outra vez 4
corrigidos na passagem seguinte, para sempre, com o temporizador a fazer o
mesmo em ciclo. Os 4 eram os SRV. A Cloudflare devolve o content de um SRV
SEM a prioridade ("1 993 host" para um registo guardado como "0 1 993
host") e guarda-a em data. O adaptador so recorria a data quando o content
vinha vazio, portanto lia sempre um valor diferente do da spec.
Para SRV e TLSA a data passa a ser a autoridade.
2) A spec de cada dominio incluia o A e o AAAA do mail host. Um segundo
dominio servido pelo mesmo servidor aponta o MX para um host que nao lhe
pertence, por isso reconciliar o cienciaembarcada.com.br tentava escrever
mail.lucasrguerra.dev.br -- zona alheia. A Cloudflare respondia "an
identical record already exists" e a consola mostrava dois erros para um
dominio que estava correcto. O mesmo valia para o TLSA, que tambem tem o
nome do mail host.
Os registos de endereco passam a ser publicados so pela zona que contem o
host; o MX e os SRV, que apontam para ele, continuam a ser de cada dominio.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
9ce5fcafaa
commit
47c3700380
7 files changed
+153
-20
No files matched your search
@@ -50,16 +50,15 @@ func BuildDnsRecordSpecs(spec DnsRecordSpec) []DnsRecord {
|
|||||||
mailHost = fmt.Sprintf("mail.%s", domainName)
|
mailHost = fmt.Sprintf("mail.%s", domainName)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The address records for the mail host belong to whichever zone actually
|
||||||
|
// contains that host. A second domain served by the same server points its
|
||||||
|
// MX at a host it does not own, and publishing an A for it meant trying to
|
||||||
|
// write into somebody else's zone: the provider answered "an identical
|
||||||
|
// record already exists" and the console showed errors for a domain that
|
||||||
|
// was correctly configured.
|
||||||
|
ownsMailHost := hostInsideDomain(mailHost, domainName)
|
||||||
|
|
||||||
records := []DnsRecord{
|
records := []DnsRecord{
|
||||||
// 1. A record for mail host
|
|
||||||
{
|
|
||||||
Type: "A",
|
|
||||||
Name: mailHost,
|
|
||||||
Value: serverIPv4,
|
|
||||||
TTL: 1, // Auto
|
|
||||||
Proxied: false,
|
|
||||||
Comment: "LambdaMail Mail Host A Record",
|
|
||||||
},
|
|
||||||
// 3. MX record
|
// 3. MX record
|
||||||
{
|
{
|
||||||
Type: "MX",
|
Type: "MX",
|
||||||
@@ -185,8 +184,20 @@ func BuildDnsRecordSpecs(spec DnsRecordSpec) []DnsRecord {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// 1. A record for the mail host, only when this domain contains it.
|
||||||
|
if ownsMailHost {
|
||||||
|
records = append(records, DnsRecord{
|
||||||
|
Type: "A",
|
||||||
|
Name: mailHost,
|
||||||
|
Value: serverIPv4,
|
||||||
|
TTL: 1, // Auto
|
||||||
|
Proxied: false,
|
||||||
|
Comment: "LambdaMail Mail Host A Record",
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
// 2. Conditional AAAA record if IPv6 is supplied
|
// 2. Conditional AAAA record if IPv6 is supplied
|
||||||
if serverIPv6 != "" {
|
if ownsMailHost && serverIPv6 != "" {
|
||||||
records = append(records, DnsRecord{
|
records = append(records, DnsRecord{
|
||||||
Type: "AAAA",
|
Type: "AAAA",
|
||||||
Name: mailHost,
|
Name: mailHost,
|
||||||
@@ -200,7 +211,7 @@ func BuildDnsRecordSpecs(spec DnsRecordSpec) []DnsRecord {
|
|||||||
// 11. Conditional TLSA records if DANE is enabled. More than one is normal
|
// 11. Conditional TLSA records if DANE is enabled. More than one is normal
|
||||||
// during a rollover: the current association and the next one are both
|
// during a rollover: the current association and the next one are both
|
||||||
// published so the certificate can change without a gap (RFC 7671 8.1).
|
// published so the certificate can change without a gap (RFC 7671 8.1).
|
||||||
if spec.DaneEnabled {
|
if ownsMailHost && spec.DaneEnabled {
|
||||||
for _, hash := range spec.TlsaHashes {
|
for _, hash := range spec.TlsaHashes {
|
||||||
if hash == "" {
|
if hash == "" {
|
||||||
continue
|
continue
|
||||||
@@ -268,3 +279,16 @@ func (r DnsRecord) EqualsNormalized(other DnsRecord) bool {
|
|||||||
}
|
}
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// hostInsideDomain reports whether a host name sits inside a domain's zone,
|
||||||
|
// and so whether this domain is the one that should publish its addresses.
|
||||||
|
func hostInsideDomain(host, domain string) bool {
|
||||||
|
host = strings.ToLower(strings.TrimSuffix(strings.TrimSpace(host), "."))
|
||||||
|
domain = strings.ToLower(strings.TrimSuffix(strings.TrimSpace(domain), "."))
|
||||||
|
if host == "" || domain == "" {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
// The suffix must fall on a label boundary: "notexample.test" is not
|
||||||
|
// inside "example.test".
|
||||||
|
return host == domain || strings.HasSuffix(host, "."+domain)
|
||||||
|
}
|
||||||
@@ -132,3 +132,66 @@ func TestOnlyTheAvailableDkimKeyIsPublished(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A second domain served by the same server does not own the mail host.
|
||||||
|
//
|
||||||
|
// Every domain's spec included an A and an AAAA for the mail host, so
|
||||||
|
// reconciling cienciaembarcada.com.br tried to create records for
|
||||||
|
// mail.lucasrguerra.dev.br - a name in somebody else's zone. Cloudflare
|
||||||
|
// answered "An identical record already exists" and the console showed the
|
||||||
|
// operator two errors about a domain that was in fact perfectly configured.
|
||||||
|
//
|
||||||
|
// The address records belong to whichever zone actually contains the host.
|
||||||
|
func TestBuildDnsRecordSpecs_SkipsMailHostAddressesOutsideTheDomain(t *testing.T) {
|
||||||
|
specs := BuildDnsRecordSpecs(DnsRecordSpec{
|
||||||
|
DomainName: "cienciaembarcada.test",
|
||||||
|
MailHost: "mail.lucasrguerra.test",
|
||||||
|
ServerIPv4: "192.0.2.1",
|
||||||
|
ServerIPv6: "2001:db8::1",
|
||||||
|
DaneEnabled: true,
|
||||||
|
TlsaHashes: []string{"hash123"},
|
||||||
|
})
|
||||||
|
|
||||||
|
for _, r := range specs {
|
||||||
|
if r.Type == "A" || r.Type == "AAAA" || r.Type == "TLSA" {
|
||||||
|
t.Errorf("spec publishes %s for %q, which is not inside cienciaembarcada.test",
|
||||||
|
r.Type, r.Name)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The records that point AT the mail host are still the domain's own and
|
||||||
|
// must stay, or the domain stops receiving mail.
|
||||||
|
var mx bool
|
||||||
|
for _, r := range specs {
|
||||||
|
if r.Type == "MX" && r.Value == "mail.lucasrguerra.test" {
|
||||||
|
mx = true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if !mx {
|
||||||
|
t.Error("the domain lost its MX; it would stop receiving mail")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The ordinary case must keep them: a domain whose mail host is its own
|
||||||
|
// subdomain still needs the address records published.
|
||||||
|
func TestBuildDnsRecordSpecs_KeepsMailHostAddressesInsideTheDomain(t *testing.T) {
|
||||||
|
specs := BuildDnsRecordSpecs(DnsRecordSpec{
|
||||||
|
DomainName: "example.test",
|
||||||
|
MailHost: "mail.example.test",
|
||||||
|
ServerIPv4: "192.0.2.1",
|
||||||
|
ServerIPv6: "2001:db8::1",
|
||||||
|
})
|
||||||
|
|
||||||
|
var a, aaaa bool
|
||||||
|
for _, r := range specs {
|
||||||
|
if r.Type == "A" && r.Name == "mail.example.test" {
|
||||||
|
a = true
|
||||||
|
}
|
||||||
|
if r.Type == "AAAA" && r.Name == "mail.example.test" {
|
||||||
|
aaaa = true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if !a || !aaaa {
|
||||||
|
t.Errorf("lost the mail host addresses (A=%v AAAA=%v)", a, aaaa)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -130,12 +130,15 @@ func (a *CloudflareAdapter) ListRecords(ctx context.Context, zoneID string) ([]e
|
|||||||
if r.Proxied != nil {
|
if r.Proxied != nil {
|
||||||
rec.Proxied = *r.Proxied
|
rec.Proxied = *r.Proxied
|
||||||
}
|
}
|
||||||
// Cloudflare returns SRV and TLSA with content already rendered,
|
// For SRV and TLSA the structured data is the authority, not the
|
||||||
// but older zones and some responses only populate data; fall back
|
// rendered content. Cloudflare renders SRV content WITHOUT the
|
||||||
// to it so a structured record is not seen as an empty value and
|
// priority - a record stored as "0 1 993 host" reads back as
|
||||||
// rewritten on every reconcile.
|
// "1 993 host" - so trusting content made every SRV differ from
|
||||||
if rec.Value == "" && len(r.Data) > 0 {
|
// the spec and be rewritten on every single reconcile.
|
||||||
rec.Value = renderStructuredValue(r.Type, r.Data)
|
if len(r.Data) > 0 {
|
||||||
|
if rendered := renderStructuredValue(r.Type, r.Data); rendered != "" {
|
||||||
|
rec.Value = rendered
|
||||||
|
}
|
||||||
}
|
}
|
||||||
out = append(out, rec)
|
out = append(out, rec)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -235,3 +235,46 @@ func TestCloudflareAdapter_ListRecords_RendersStructuredValues(t *testing.T) {
|
|||||||
t.Errorf("value = %q, want the rendered presentation form", records[0].Value)
|
t.Errorf("value = %q, want the rendered presentation form", records[0].Value)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Cloudflare renders an SRV record's content WITHOUT the priority: the zone
|
||||||
|
// holding "0 1 993 mail.example.test" answers content "1 993 mail.example.test"
|
||||||
|
// and keeps the priority in data. Taking content at face value made the record
|
||||||
|
// differ from the spec on every comparison, so every reconcile rewrote all four
|
||||||
|
// SRV records - against the live zone, on a timer, forever.
|
||||||
|
func TestCloudflareAdapter_ListRecords_KeepsSrvPriority(t *testing.T) {
|
||||||
|
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
json.NewEncoder(w).Encode(map[string]interface{}{
|
||||||
|
"success": true,
|
||||||
|
"result": []map[string]interface{}{{
|
||||||
|
"id": "rec_1", "type": "SRV", "name": "_imaps._tcp.example.test",
|
||||||
|
// Exactly what the API returns: the priority is missing here
|
||||||
|
// and present in data.
|
||||||
|
"content": "1 993 mail.example.test",
|
||||||
|
"data": map[string]interface{}{
|
||||||
|
"priority": float64(0), "weight": float64(1),
|
||||||
|
"port": float64(993), "target": "mail.example.test",
|
||||||
|
},
|
||||||
|
"ttl": float64(1),
|
||||||
|
}},
|
||||||
|
"result_info": map[string]interface{}{"total_pages": float64(1)},
|
||||||
|
})
|
||||||
|
}))
|
||||||
|
defer ts.Close()
|
||||||
|
|
||||||
|
adapter := NewCloudflareAdapter("token")
|
||||||
|
adapter.baseURL = ts.URL
|
||||||
|
|
||||||
|
records, err := adapter.ListRecords(context.Background(), "zone1")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ListRecords: %v", err)
|
||||||
|
}
|
||||||
|
if len(records) != 1 {
|
||||||
|
t.Fatalf("got %d records, want 1", len(records))
|
||||||
|
}
|
||||||
|
|
||||||
|
const want = "0 1 993 mail.example.test"
|
||||||
|
if records[0].Value != want {
|
||||||
|
t.Errorf("read SRV value %q, want %q: the record will be rewritten on every reconcile",
|
||||||
|
records[0].Value, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -4,7 +4,7 @@
|
|||||||
"aliasTarget": "Destination address",
|
"aliasTarget": "Destination address",
|
||||||
"aliasesTitle": "Aliases",
|
"aliasesTitle": "Aliases",
|
||||||
"auditLog": "Audit log",
|
"auditLog": "Audit log",
|
||||||
"backToWebmail": "Back to webmail",
|
"backToWebmail": "Webmail",
|
||||||
"cancelJob": "Cancel",
|
"cancelJob": "Cancel",
|
||||||
"certificateState": "Certificate",
|
"certificateState": "Certificate",
|
||||||
"checksPassed": "{passed} of {total} passed",
|
"checksPassed": "{passed} of {total} passed",
|
||||||
|
|||||||
@@ -4,7 +4,7 @@
|
|||||||
"aliasTarget": "Dirección de destino",
|
"aliasTarget": "Dirección de destino",
|
||||||
"aliasesTitle": "Alias",
|
"aliasesTitle": "Alias",
|
||||||
"auditLog": "Registro de auditoría",
|
"auditLog": "Registro de auditoría",
|
||||||
"backToWebmail": "Volver al webmail",
|
"backToWebmail": "Webmail",
|
||||||
"cancelJob": "Cancelar",
|
"cancelJob": "Cancelar",
|
||||||
"certificateState": "Certificado",
|
"certificateState": "Certificado",
|
||||||
"checksPassed": "{passed} de {total} aprobadas",
|
"checksPassed": "{passed} de {total} aprobadas",
|
||||||
|
|||||||
@@ -4,7 +4,7 @@
|
|||||||
"aliasTarget": "Endereço de destino",
|
"aliasTarget": "Endereço de destino",
|
||||||
"aliasesTitle": "Apelidos",
|
"aliasesTitle": "Apelidos",
|
||||||
"auditLog": "Trilha de auditoria",
|
"auditLog": "Trilha de auditoria",
|
||||||
"backToWebmail": "Voltar ao webmail",
|
"backToWebmail": "Webmail",
|
||||||
"cancelJob": "Cancelar",
|
"cancelJob": "Cancelar",
|
||||||
"certificateState": "Certificado",
|
"certificateState": "Certificado",
|
||||||
"checksPassed": "{passed} de {total} aprovadas",
|
"checksPassed": "{passed} de {total} aprovadas",
|
||||||
|
|||||||
Reference in new issue
Block a user