Skip to content

Commit

Permalink
fix: avoid duplicate warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
@favonia
favonia committed Jan 21, 2025
1 parent 05a1f4d commit 06a7f77
Show file tree
Hide file tree
Showing 10 changed files with 323 additions and 210 deletions.
24 changes: 16 additions & 8 deletions internal/api/base.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,24 @@ type WAFList struct {
// Describe formats WAFList as a string.
func (l WAFList) Describe() string { return fmt.Sprintf("%s/%s", string(l.AccountID), l.Name) }

// Record bundles an ID and an IP address, representing a DNS record.
// RecordParams bundles parameters of a DNS record.
type RecordParams struct {
TTL
Proxied bool
Comment string
}

// Record represents a DNS record.
type Record struct {
ID ID
ID
IP netip.Addr
RecordParams
}

// WAFListItem bundles an ID and an IP range, representing an item in a WAF list.
type WAFListItem struct {
ID ID
Prefix netip.Prefix
ID
netip.Prefix
}

// DeletionMode tells the deletion updater whether a careful re-reading of lists
Expand All @@ -58,17 +66,17 @@ type Handle interface {
//
// The second return value indicates whether the list was cached.
ListRecords(ctx context.Context, ppfmt pp.PP, ipNet ipnet.Type, domain domain.Domain,
expectedTTL TTL, expectedProxied bool, expectedRecordComment string,
expectedParams RecordParams,
) ([]Record, bool, bool)

// UpdateRecord updates one DNS record.
UpdateRecord(ctx context.Context, ppfmt pp.PP, ipNet ipnet.Type, domain domain.Domain, id ID, ip netip.Addr,
expectedTTL TTL, expectedProxied bool, expectedRecordComment string,
UpdateRecord(ctx context.Context, ppfmt pp.PP, ipNet ipnet.Type, domain domain.Domain,
id ID, ip netip.Addr, currentParams, expectedParams RecordParams,
) bool

// CreateRecord creates one DNS record. It returns the ID of the new record.
CreateRecord(ctx context.Context, ppfmt pp.PP, ipNet ipnet.Type, domain domain.Domain,
ip netip.Addr, ttl TTL, proxied bool, recordComment string) (ID, bool)
ip netip.Addr, params RecordParams) (ID, bool)

// DeleteRecord deletes one DNS record, assuming we will not update or create any DNS records.
DeleteRecord(ctx context.Context, ppfmt pp.PP, ipNet ipnet.Type, domain domain.Domain, id ID, mode DeletionMode) bool
Expand Down
76 changes: 46 additions & 30 deletions internal/api/cloudflare_records.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ zoneSearch:

// ListRecords calls cloudflare.ListDNSRecords.
func (h CloudflareHandle) ListRecords(ctx context.Context, ppfmt pp.PP, ipNet ipnet.Type, domain domain.Domain,
expectedTTL TTL, expectedProxied bool, expectedRecordComment string,
expectedParams RecordParams,
) ([]Record, bool, bool) {
if rmap := h.cache.listRecords[ipNet].Get(domain.DNSNameASCII()); rmap != nil {
return *rmap.Value(), true, true
Expand Down Expand Up @@ -152,31 +152,39 @@ func (h CloudflareHandle) ListRecords(ctx context.Context, ppfmt pp.PP, ipNet ip
return nil, false, false
}

if TTL(r.TTL) != expectedTTL {
if TTL(r.TTL) != expectedParams.TTL {
ppfmt.Noticef(pp.EmojiUserWarning,
"The TTL of the %s record of %s (ID: %s) differs from the value of TTL (%s) and will be kept",
ipNet.RecordType(), domain.Describe(), id, expectedTTL.Describe(),
ipNet.RecordType(), domain.Describe(), id, expectedParams.TTL.Describe(),
)
hintMismatchedRecordAttributes(ppfmt)
}

Check warning on line 161 in internal/api/cloudflare_records.go

View check run for this annotation

Codecov / codecov/patch

internal/api/cloudflare_records.go#L156-L161 

Added lines #L156 - L161 were not covered by tests
// by default, proxied = false
if r.Proxied == nil && expectedProxied ||
r.Proxied != nil && *r.Proxied != expectedProxied {
if r.Proxied == nil && expectedParams.Proxied ||
r.Proxied != nil && *r.Proxied != expectedParams.Proxied {
ppfmt.Noticef(pp.EmojiUserWarning,
"The proxy status of the %s record of %s (ID: %s) differs from the value of PROXIED (%v for this domain) and will be kept", //nolint:lll
ipNet.RecordType(), domain.Describe(), id, expectedProxied,
ipNet.RecordType(), domain.Describe(), id, expectedParams.Proxied,
)
hintMismatchedRecordAttributes(ppfmt)
}

Check warning on line 170 in internal/api/cloudflare_records.go

View check run for this annotation

Codecov / codecov/patch

internal/api/cloudflare_records.go#L165-L170 

Added lines #L165 - L170 were not covered by tests
if r.Comment != expectedRecordComment {
if r.Comment != expectedParams.Comment {
ppfmt.Noticef(pp.EmojiUserWarning,
"The comment of the %s record of %s (ID: %s) to be updated differs from the value of RECORD_COMMENT (%q) and will be kept", //nolint:lll
ipNet.RecordType(), domain.Describe(), id, expectedRecordComment,
"The comment of the %s record of %s (ID: %s) differs from the value of RECORD_COMMENT (%q) and will be kept", //nolint:lll
ipNet.RecordType(), domain.Describe(), id, expectedParams.Comment,
)
hintMismatchedRecordAttributes(ppfmt)
}

Check warning on line 177 in internal/api/cloudflare_records.go

View check run for this annotation

Codecov / codecov/patch

internal/api/cloudflare_records.go#L172-L177 

Added lines #L172 - L177 were not covered by tests

rs = append(rs, Record{ID: ID(r.ID), IP: ip})
rs = append(rs, Record{
ID: ID(r.ID),
IP: ip,
RecordParams: RecordParams{
TTL: TTL(r.TTL),
Proxied: r.Proxied != nil && *r.Proxied,
Comment: r.Comment,
},
})
}

h.cache.listRecords[ipNet].DeleteExpired()
Expand Down Expand Up @@ -215,7 +223,7 @@ func (h CloudflareHandle) DeleteRecord(ctx context.Context, ppfmt pp.PP,
// UpdateRecord calls cloudflare.UpdateDNSRecord.
func (h CloudflareHandle) UpdateRecord(ctx context.Context, ppfmt pp.PP,
ipNet ipnet.Type, domain domain.Domain, id ID, ip netip.Addr,
expectedTTL TTL, expectedProxied bool, expectedRecordComment string,
currentParams, expectedParams RecordParams,
) bool {
zone, ok := h.ZoneIDOfDomain(ctx, ppfmt, domain)
if !ok {
Expand All @@ -228,7 +236,7 @@ func (h CloudflareHandle) UpdateRecord(ctx context.Context, ppfmt pp.PP,
Content: ip.String(),
}

r, err := h.cf.UpdateDNSRecord(ctx, cloudflare.ZoneIdentifier(string(zone)), params)
resp, err := h.cf.UpdateDNSRecord(ctx, cloudflare.ZoneIdentifier(string(zone)), params)
if err != nil {
ppfmt.Noticef(pp.EmojiError, "Failed to update a stale %s record of %s (ID: %s): %v",
ipNet.RecordType(), domain.Describe(), id, err)
Expand All @@ -239,34 +247,42 @@ func (h CloudflareHandle) UpdateRecord(ctx context.Context, ppfmt pp.PP,
return false
}

if TTL(r.TTL) != expectedTTL {
if TTL(resp.TTL) != currentParams.TTL && TTL(resp.TTL) != expectedParams.TTL {
ppfmt.Noticef(pp.EmojiUserWarning,
"The TTL of the %s record of %s (ID: %s) to be updated differs from the value of TTL (%s) and will be kept",
ipNet.RecordType(), domain.Describe(), id, expectedTTL.Describe(),
"The TTL of the %s record of %s (ID: %s) differs from the value of TTL (%s) and will be kept",
ipNet.RecordType(), domain.Describe(), ip, expectedParams.TTL.Describe(),

Check warning on line 253 in internal/api/cloudflare_records.go

View check run for this annotation

Codecov / codecov/patch

internal/api/cloudflare_records.go#L251-L253 

Added lines #L251 - L253 were not covered by tests
)
hintMismatchedRecordAttributes(ppfmt)
}
// by default, proxied = false
if r.Proxied == nil && expectedProxied ||
r.Proxied != nil && *r.Proxied != expectedProxied {
if resp.Proxied == nil && currentParams.Proxied && expectedParams.Proxied ||
resp.Proxied != nil && *resp.Proxied != currentParams.Proxied && *resp.Proxied != expectedParams.Proxied {
ppfmt.Noticef(pp.EmojiUserWarning,
"The proxy status of the %s record of %s (ID: %s) to be updated differs from the value of PROXIED (%v for this domain) and will be kept", //nolint:lll
ipNet.RecordType(), domain.Describe(), id, expectedProxied,
"The proxy status of the %s record of %s (ID: %s) differs from the value of PROXIED (%v for this domain) and will be kept", //nolint:lll
ipNet.RecordType(), domain.Describe(), id, expectedParams.Proxied,

Check warning on line 262 in internal/api/cloudflare_records.go

View check run for this annotation

Codecov / codecov/patch

internal/api/cloudflare_records.go#L260-L262 

Added lines #L260 - L262 were not covered by tests
)
hintMismatchedRecordAttributes(ppfmt)
}
if r.Comment != expectedRecordComment {
if resp.Comment != currentParams.Comment && resp.Comment != expectedParams.Comment {
ppfmt.Noticef(pp.EmojiUserWarning,
"The comment of the %s record of %s (ID: %s) to be updated differs from the value of RECORD_COMMENT (%q) and will be kept", //nolint:lll
ipNet.RecordType(), domain.Describe(), id, expectedRecordComment,
"The comment of the %s record of %s (ID: %s) differs from the value of RECORD_COMMENT (%q) and will be kept", //nolint:lll
ipNet.RecordType(), domain.Describe(), id, expectedParams.Comment,

Check warning on line 269 in internal/api/cloudflare_records.go

View check run for this annotation

Codecov / codecov/patch

internal/api/cloudflare_records.go#L267-L269 

Added lines #L267 - L269 were not covered by tests
)
hintMismatchedRecordAttributes(ppfmt)
}

if rs := h.cache.listRecords[ipNet].Get(domain.DNSNameASCII()); rs != nil {
for i, r := range *rs.Value() {
if r.ID == id {
(*rs.Value())[i].IP = ip
(*rs.Value())[i] = Record{
ID: id,
IP: ip,
RecordParams: RecordParams{
TTL: TTL(resp.TTL),
Proxied: resp.Proxied != nil && *resp.Proxied,
Comment: resp.Comment,
},
}
}
}
}
Expand All @@ -276,24 +292,24 @@ func (h CloudflareHandle) UpdateRecord(ctx context.Context, ppfmt pp.PP,

// CreateRecord calls cloudflare.CreateDNSRecord.
func (h CloudflareHandle) CreateRecord(ctx context.Context, ppfmt pp.PP,
ipNet ipnet.Type, domain domain.Domain, ip netip.Addr, ttl TTL, proxied bool, recordComment string,
ipNet ipnet.Type, domain domain.Domain, ip netip.Addr, params RecordParams,
) (ID, bool) {
zone, ok := h.ZoneIDOfDomain(ctx, ppfmt, domain)
if !ok {
return "", false
}

//nolint:exhaustruct // Other fields are intentionally omitted
params := cloudflare.CreateDNSRecordParams{
ps := cloudflare.CreateDNSRecordParams{
Name: domain.DNSNameASCII(),
Type: ipNet.RecordType(),
Content: ip.String(),
TTL: ttl.Int(),
Proxied: &proxied,
Comment: recordComment,
TTL: params.TTL.Int(),
Proxied: &params.Proxied,
Comment: params.Comment,
}

res, err := h.cf.CreateDNSRecord(ctx, cloudflare.ZoneIdentifier(string(zone)), params)
res, err := h.cf.CreateDNSRecord(ctx, cloudflare.ZoneIdentifier(string(zone)), ps)
if err != nil {
ppfmt.Noticef(pp.EmojiError, "Failed to add a new %s record of %s: %v",
ipNet.RecordType(), domain.Describe(), err)
Expand All @@ -305,7 +321,7 @@ func (h CloudflareHandle) CreateRecord(ctx context.Context, ppfmt pp.PP,
}

if rs := h.cache.listRecords[ipNet].Get(domain.DNSNameASCII()); rs != nil {
*rs.Value() = append([]Record{{ID: ID(res.ID), IP: ip}}, *rs.Value()...)
*rs.Value() = append([]Record{{ID: ID(res.ID), IP: ip, RecordParams: params}}, *rs.Value()...)
}

return ID(res.ID), true
Expand Down
Loading

0 comments on commit 06a7f77

Please sign in to comment.