Skip to content

Commit

Permalink
refactor: minor misc code refactoring (#867)
Browse files Browse the repository at this point in the history
  • Loading branch information
favonia authored Aug 11, 2024
1 parent a9b08e6 commit f0b2945
Show file tree
Hide file tree
Showing 32 changed files with 323 additions and 288 deletions.
1 change: 1 addition & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ linters:
- gocognit # can detect complicated code, but never leads to actual code changes
- gocyclo # can detect complicated code, but never leads to actual code changes
- maintidx # can detect complicated code, but never leads to actual code changes
- nestif # can detect complicated code, but never leads to actual code changes

- depguard # useless; I do not introduce a dependency carelessly
- gosmopolitan # interesting for i18n checking, useless for this project
Expand Down
35 changes: 23 additions & 12 deletions cmd/ddns/ddns.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func formatName() string {
return fmt.Sprintf("Cloudflare DDNS (%s)", Version)
}

func initConfig(ctx context.Context, ppfmt pp.PP) (*config.Config, setter.Setter, bool) {
func initConfig(ppfmt pp.PP) (*config.Config, setter.Setter, bool) {
c := config.Default()

// Read the config
Expand All @@ -40,7 +40,7 @@ func initConfig(ctx context.Context, ppfmt pp.PP) (*config.Config, setter.Setter
c.Print(ppfmt)

// Get the handler
h, ok := c.Auth.New(ctx, ppfmt, c.CacheExpiration)
h, ok := c.Auth.New(ppfmt, c.CacheExpiration)
if !ok {
return c, nil, false
}
Expand Down Expand Up @@ -87,11 +87,11 @@ func realMain() int { //nolint:funlen
config.CheckRoot(ppfmt)

// Read the config and get the handler and the setter
c, s, configOk := initConfig(ctx, ppfmt)
c, s, configOK := initConfig(ppfmt)
// Ping monitors regardless of whether initConfig succeeded
monitor.StartAll(ctx, ppfmt, c.Monitors, formatName())
// Bail out now if initConfig failed
if !configOk {
if !configOK {
monitor.ExitStatusAll(ctx, ppfmt, c.Monitors, 1, "Configuration errors")
notifier.SendAll(ctx, ppfmt, c.Notifiers,
"Cloudflare DDNS was misconfigured and could not start. Please check the logging for details.")
Expand Down Expand Up @@ -124,20 +124,30 @@ func realMain() int { //nolint:funlen
if first && !c.UpdateOnStart {
monitor.SuccessAll(ctx, ppfmt, c.Monitors, "Started (no action)")
} else {
if c.UpdateCron != nil && !s.SanityCheck(ctx, ppfmt) {
monitor.SuccessAll(ctx, ppfmt, c.Monitors, "Invalid Cloudflare API token")
notifier.SendAll(ctx, ppfmt, c.Notifiers,
"The Cloudflare API token is invalid. "+
"Please check the value of CF_API_TOKEN or CF_API_TOKEN_FILE.",
)
return 1
if c.UpdateCron != nil { // no need to do sanity check if it's a one-time update
if ok, certain := s.SanityCheck(ctxWithSignals, ppfmt); !ok && certain {
monitor.FailureAll(ctx, ppfmt, c.Monitors, "Invalid Cloudflare API token")
notifier.SendAll(ctx, ppfmt, c.Notifiers,
"The Cloudflare API token is invalid. "+
"Please check the value of CF_API_TOKEN or CF_API_TOKEN_FILE.",
)
return 1
}
}

if ctxWithSignals.Err() != nil {
goto signaled
}

msg := updater.UpdateIPs(ctxWithSignals, ppfmt, c, s)
monitor.PingMessageAll(ctx, ppfmt, c.Monitors, msg)
notifier.SendMessageAll(ctx, ppfmt, c.Notifiers, msg)
}

if ctxWithSignals.Err() != nil {
goto signaled
}

// Check if cron was disabled
if c.UpdateCron == nil {
ppfmt.Infof(pp.EmojiBye, "Bye!")
Expand Down Expand Up @@ -168,8 +178,9 @@ func realMain() int { //nolint:funlen
// Display the remaining time interval
cron.PrintCountdown(ppfmt, "Checking the IP addresses", time.Now(), next)

signaled:
// Wait for the next signal or the alarm, whichever comes first
if !sig.SleepUntil(ppfmt, next) {
if sig.ReportSignalsUntil(ppfmt, next) {
stopUpdating(ctx, ppfmt, c, s)
monitor.ExitStatusAll(ctx, ppfmt, c.Monitors, 0, "Stopped")
if c.UpdateCron != nil {
Expand Down
7 changes: 4 additions & 3 deletions internal/api/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ type WAFListItem struct {
// Currently, the only implementation is Cloudflare.
type Handle interface {
// Perform basic checking (e.g., the validity of tokens).
// It returns false when we should give up all future operations.
SanityCheck(ctx context.Context, ppfmt pp.PP) bool
// The first return value indicates whether it passes the test.
// The second return value indicates whether we are certain about the result.
SanityCheck(ctx context.Context, ppfmt pp.PP) (bool, bool)

// ListRecords lists all matching DNS records.
//
Expand Down Expand Up @@ -70,7 +71,7 @@ type Handle interface {
// An Auth contains authentication information.
type Auth interface {
// New uses the authentication information to create a Handle.
New(ctx context.Context, ppfmt pp.PP, cacheExpiration time.Duration) (Handle, bool)
New(ppfmt pp.PP, cacheExpiration time.Duration) (Handle, bool)

// Check whether DNS records are supported.
SupportsRecords() bool
Expand Down
100 changes: 66 additions & 34 deletions internal/api/cloudflare.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,22 @@ import (
"github.com/cloudflare/cloudflare-go"
"github.com/jellydator/ttlcache/v3"

"github.com/favonia/cloudflare-ddns/internal/cron"
"github.com/favonia/cloudflare-ddns/internal/ipnet"
"github.com/favonia/cloudflare-ddns/internal/pp"
)

type sanityCheckType int

const (
sanityCheckToken sanityCheckType = iota
sanityCheckAccount
)

// CloudflareCache holds the previous repsonses from the Cloudflare API.
type CloudflareCache = struct {
// sanity check
sanityCheck *ttlcache.Cache[struct{}, bool] // whether token is valid
sanityCheck *ttlcache.Cache[sanityCheckType, bool] // whether token or account is valid
// domains to zones
listZones *ttlcache.Cache[string, []string] // zone names to zone IDs
zoneOfDomain *ttlcache.Cache[string, string] // domain names to the zone ID
Expand Down Expand Up @@ -52,7 +60,7 @@ type CloudflareAuth struct {
}

// New creates a [CloudflareHandle] from the authentication data.
func (t CloudflareAuth) New(_ context.Context, ppfmt pp.PP, cacheExpiration time.Duration) (Handle, bool) {
func (t CloudflareAuth) New(ppfmt pp.PP, cacheExpiration time.Duration) (Handle, bool) {
handle, err := cloudflare.NewWithAPIToken(t.Token)
if err != nil {
ppfmt.Errorf(pp.EmojiUserError, "Failed to prepare the Cloudflare authentication: %v", err)
Expand All @@ -68,7 +76,7 @@ func (t CloudflareAuth) New(_ context.Context, ppfmt pp.PP, cacheExpiration time
cf: handle,
accountID: t.AccountID,
cache: CloudflareCache{
sanityCheck: newCache[struct{}, bool](cacheExpiration),
sanityCheck: newCache[sanityCheckType, bool](cacheExpiration),
listZones: newCache[string, []string](cacheExpiration),
zoneOfDomain: newCache[string, string](cacheExpiration),
listRecords: map[ipnet.Type]*ttlcache.Cache[string, *[]Record]{
Expand Down Expand Up @@ -108,60 +116,84 @@ func (h CloudflareHandle) FlushCache() {
// errTimeout for checking if it's timeout.
var errTimeout = errors.New("timeout")

// SanityCheck verifies Cloudflare tokens.
//
// Ideally, we should also verify accountID here, but that is impossible without
// more permissions included in the API token.
func (h CloudflareHandle) SanityCheck(ctx context.Context, ppfmt pp.PP) bool {
if valid := h.cache.sanityCheck.Get(struct{}{}); valid != nil {
return valid.Value()
func (h CloudflareHandle) skipSanityCheckToken() {
h.cache.sanityCheck.Set(sanityCheckToken, true, ttlcache.DefaultTTL)
}

func (h CloudflareHandle) skipSanityCheck() {
h.skipSanityCheckToken()
h.cache.sanityCheck.Set(sanityCheckAccount, true, ttlcache.DefaultTTL)
}

// SanityCheckToken verifies the Cloudflare token.
func (h CloudflareHandle) SanityCheckToken(ctx context.Context, ppfmt pp.PP) (bool, bool) {
if valid := h.cache.sanityCheck.Get(sanityCheckToken); valid != nil {
return valid.Value(), true
}

quickCtx, cancel := context.WithTimeoutCause(ctx, time.Second, errTimeout)
defer cancel()

ok := true
res, err := h.cf.VerifyAPIToken(quickCtx)
if err != nil {
// Check if the token is permanently invalid...
var aerr *cloudflare.AuthorizationError
var rerr *cloudflare.RequestError
if errors.As(err, &aerr) || errors.As(err, &rerr) {
ppfmt.Errorf(pp.EmojiUserError, "The Cloudflare API token is invalid: %v", err)
ok = false
goto permanently
if quickCtx.Err() != nil {
return true, false
}
if !errors.Is(context.Cause(quickCtx), errTimeout) {
ppfmt.Warningf(pp.EmojiWarning, "Failed to verify the Cloudflare API token; will retry later: %v", err)

var requestError *cloudflare.RequestError
var authorizationError *cloudflare.AuthorizationError

// known error messages
// 400:6003:"Invalid request headers"
// 400:6111:"Invalid format for Authorization header"
// 401:1000:"Invalid API Token"

switch {
case errors.As(err, &requestError),
errors.As(err, &requestError) && requestError.InternalErrorCodeIs(6111), //nolint:mnd
errors.As(err, &authorizationError) && authorizationError.InternalErrorCodeIs(1000): //nolint:mnd

ppfmt.Errorf(pp.EmojiUserError,
"The Cloudflare API token is invalid; "+
"please check the value of CF_API_TOKEN or CF_API_TOKEN_FILE")
goto certainlyBad

default:
// We will try again later.
return true, false
}
return true // It could be that the network is temporarily down.
}

// The API call succeeded, but the token might be in a bad status.
switch res.Status {
case "active":
case "disabled", "expired":
ppfmt.Errorf(pp.EmojiUserError, "The Cloudflare API token is %s", res.Status)
ok = false
goto permanently
goto certainlyBad
default:
ppfmt.Warningf(pp.EmojiImpossible,
"The Cloudflare API token is in an undocumented state %q; please report this at %s",
res.Status, pp.IssueReportingURL)
goto permanently
return true, false
}

if !res.ExpiresOn.IsZero() {
ppfmt.Warningf(pp.EmojiAlarm, "The token will expire at %s",
res.ExpiresOn.In(time.Local).Format(time.RFC1123Z))
}
now := time.Now()
remainingLifespan := max(res.ExpiresOn.Sub(now), 0)

permanently:
if !ok {
ppfmt.Errorf(pp.EmojiUserError, "Please double-check the value of CF_API_TOKEN or CF_API_TOKEN_FILE")
ppfmt.Warningf(pp.EmojiAlarm, "The Cloudflare API token will expire at %s (%v left)",
cron.DescribeIntuitively(now, res.ExpiresOn), remainingLifespan)
}
h.cache.sanityCheck.Set(struct{}{}, ok, ttlcache.DefaultTTL)
return ok

h.cache.sanityCheck.Set(sanityCheckToken, true, ttlcache.DefaultTTL)
return true, true

certainlyBad:
return false, true
}

func (h CloudflareHandle) forcePassSanityCheck() {
h.cache.sanityCheck.Set(struct{}{}, true, ttlcache.DefaultTTL)
// SanityCheck verifies both the Cloudflare API token and account ID.
// It returns false only when the token or the account ID is certainly bad.
func (h CloudflareHandle) SanityCheck(ctx context.Context, ppfmt pp.PP) (bool, bool) {
return h.SanityCheckToken(ctx, ppfmt)
}
12 changes: 6 additions & 6 deletions internal/api/cloudflare_records.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func (h CloudflareHandle) ListZones(ctx context.Context, ppfmt pp.PP, name strin
}

// The operation went through. No need to perform any sanity checking in near future!
h.forcePassSanityCheck()
h.skipSanityCheckToken()

ids := make([]string, 0, len(res.Result))
for _, zone := range res.Result {
Expand Down Expand Up @@ -80,7 +80,7 @@ zoneSearch:
}

// The operation went through. No need to perform any sanity checking in near future!
h.forcePassSanityCheck()
h.skipSanityCheckToken()

switch len(zones) {
case 0: // len(zones) == 0
Expand Down Expand Up @@ -119,7 +119,7 @@ func (h CloudflareHandle) ListRecords(ctx context.Context, ppfmt pp.PP,
}

// The operation went through. No need to perform any sanity checking in near future!
h.forcePassSanityCheck()
h.skipSanityCheckToken()

//nolint:exhaustruct // Other fields are intentionally unspecified
raw, _, err := h.cf.ListDNSRecords(ctx,
Expand Down Expand Up @@ -173,7 +173,7 @@ func (h CloudflareHandle) DeleteRecord(ctx context.Context, ppfmt pp.PP,
}

// The operation went through. No need to perform any sanity checking in near future!
h.forcePassSanityCheck()
h.skipSanityCheckToken()

if rs := h.cache.listRecords[ipNet].Get(domain.DNSNameASCII()); rs != nil {
*rs.Value() = slices.DeleteFunc(*rs.Value(), func(r Record) bool { return r.ID == id })
Expand Down Expand Up @@ -207,7 +207,7 @@ func (h CloudflareHandle) UpdateRecord(ctx context.Context, ppfmt pp.PP,
}

// The operation went through. No need to perform any sanity checking in near future!
h.forcePassSanityCheck()
h.skipSanityCheckToken()

if rs := h.cache.listRecords[ipNet].Get(domain.DNSNameASCII()); rs != nil {
for i, r := range *rs.Value() {
Expand Down Expand Up @@ -250,7 +250,7 @@ func (h CloudflareHandle) CreateRecord(ctx context.Context, ppfmt pp.PP,
}

// The operation went through. No need to perform any sanity checking in near future!
h.forcePassSanityCheck()
h.skipSanityCheckToken()

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

0 comments on commit f0b2945

Please sign in to comment.