From 7fcd0f902b4adf9cf93ecdd1f5c241b2d11f76f4 Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Tue, 30 Aug 2022 01:23:45 +0000 Subject: [PATCH] chore(lint): add `goerr113` linter - Change database id to be `uint` instead of `int` - Define and use sentinel errors - Return `string` instead of `error` when appropriate (linode) --- .golangci.yml | 5 +++-- internal/data/memory.go | 12 +++++------ internal/data/persistence.go | 9 +++----- internal/health/check.go | 15 +++++++++---- internal/health/client.go | 4 ++-- internal/persistence/json/database.go | 21 ++++++++++++++----- .../providers/digitalocean/provider.go | 5 +++-- .../settings/providers/dreamhost/provider.go | 2 +- .../settings/providers/linode/provider.go | 16 +++++++------- internal/update/interfaces.go | 6 +++--- internal/update/run.go | 12 ++++++----- internal/update/update.go | 7 ++++--- 12 files changed, 67 insertions(+), 47 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 858c6d7e6..7420d2fcd 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -16,8 +16,9 @@ issues: - gomnd - path: _test\.go linters: - - dupl - containedctx + - dupl + - goerr113 linters: enable: @@ -51,7 +52,7 @@ linters: - gomoddirectives - goprintffuncname - gosec - # - goerr113 # TODO + - goerr113 - grouper - importas - interfacebloat diff --git a/internal/data/memory.go b/internal/data/memory.go index 4ffa9dbc2..8d11001c6 100644 --- a/internal/data/memory.go +++ b/internal/data/memory.go @@ -1,19 +1,19 @@ package data import ( + "errors" "fmt" "github.com/qdm12/ddns-updater/internal/records" ) -func (db *Database) Select(id int) (record records.Record, err error) { +var ErrRecordNotFound = errors.New("record not found") + +func (db *Database) Select(id uint) (record records.Record, err error) { db.RLock() defer db.RUnlock() - if id < 0 { - return record, fmt.Errorf("id %d cannot be lower than 0", id) - } - if id > len(db.data)-1 { - return record, fmt.Errorf("no record config found for id %d", id) + if int(id) > len(db.data)-1 { + return record, fmt.Errorf("%w: for id %d", ErrRecordNotFound, id) } return db.data[id], nil } diff --git a/internal/data/persistence.go b/internal/data/persistence.go index 0f847c61c..d24e23310 100644 --- a/internal/data/persistence.go +++ b/internal/data/persistence.go @@ -11,14 +11,11 @@ func (db *Database) GetEvents(domain, host string) (events []models.HistoryEvent return db.persistentDB.GetEvents(domain, host) } -func (db *Database) Update(id int, record records.Record) (err error) { +func (db *Database) Update(id uint, record records.Record) (err error) { db.Lock() defer db.Unlock() - if id < 0 { - return fmt.Errorf("id %d cannot be lower than 0", id) - } - if id > len(db.data)-1 { - return fmt.Errorf("no record config found for id %d", id) + if int(id) > len(db.data)-1 { + return fmt.Errorf("%w: for id %d", ErrRecordNotFound, id) } currentCount := len(db.data[id].History) newCount := len(record.History) diff --git a/internal/health/check.go b/internal/health/check.go index d139a12c1..6289d8141 100644 --- a/internal/health/check.go +++ b/internal/health/check.go @@ -1,6 +1,7 @@ package health import ( + "errors" "fmt" "net" "strings" @@ -17,12 +18,18 @@ func MakeIsHealthy(db AllSelecter, lookupIP lookupIPFunc, logger logging.Logger) } } +var ( + ErrRecordUpdateFailed = errors.New("record update failed") + ErrRecordIPNotSet = errors.New("record IP not set") + ErrLookupMismatch = errors.New("lookup IP addresses do not match") +) + // isHealthy checks all the records were updated successfully and returns an error if not. func isHealthy(db AllSelecter, lookupIP lookupIPFunc) (err error) { records := db.SelectAll() for _, record := range records { if record.Status == constants.FAIL { - return fmt.Errorf("%s", record.String()) + return fmt.Errorf("%w: %s", ErrRecordUpdateFailed, record.String()) } else if record.Settings.Proxied() { continue } @@ -33,7 +40,7 @@ func isHealthy(db AllSelecter, lookupIP lookupIPFunc) (err error) { } currentIP := record.History.GetCurrentIP() if currentIP == nil { - return fmt.Errorf("no database set IP address found for %s", hostname) + return fmt.Errorf("%w: for hostname %s", ErrRecordIPNotSet, hostname) } found := false lookedUpIPsString := make([]string, len(lookedUpIPs)) @@ -45,8 +52,8 @@ func isHealthy(db AllSelecter, lookupIP lookupIPFunc) (err error) { } } if !found { - return fmt.Errorf("lookup IP addresses for %s are %s instead of %s", - hostname, strings.Join(lookedUpIPsString, ","), currentIP) + return fmt.Errorf("%w: %s instead of %s for %s", + ErrLookupMismatch, strings.Join(lookedUpIPsString, ","), currentIP, hostname) } } return nil diff --git a/internal/health/client.go b/internal/health/client.go index 7aca9e526..b382267fc 100644 --- a/internal/health/client.go +++ b/internal/health/client.go @@ -25,7 +25,7 @@ func NewClient() *Client { } } -var ErrParseHealthServerAddress = errors.New("cannot parse health server address") +var ErrUnhealthy = errors.New("program is unhealthy") // Query sends an HTTP request to the other instance of // the program, and to its internal healthcheck server. @@ -49,5 +49,5 @@ func (c *Client) Query(ctx context.Context, port uint16) error { return fmt.Errorf("reading body from response with status %s: %w", resp.Status, err) } - return fmt.Errorf(string(b)) + return fmt.Errorf("%w: %s", ErrUnhealthy, string(b)) } diff --git a/internal/persistence/json/database.go b/internal/persistence/json/database.go index 7a666359b..34217c5ac 100644 --- a/internal/persistence/json/database.go +++ b/internal/persistence/json/database.go @@ -2,6 +2,7 @@ package json import ( "encoding/json" + "errors" "fmt" "sync" "time" @@ -56,25 +57,35 @@ func NewDatabase(dataDir string) (*Database, error) { return &db, nil } +var ( + ErrDomainEmpty = errors.New("domain is empty") + ErrHostIsEmpty = errors.New("host is empty") + ErrIPRecordsMisordered = errors.New("IP records are not ordered correctly by time") + ErrIPEmpty = errors.New("IP is empty") + ErrIPTimeEmpty = errors.New("time of IP is empty") +) + func (db *Database) Check() error { for _, record := range db.data.Records { switch { case len(record.Domain) == 0: - return fmt.Errorf("domain is empty for record %s", record) + return fmt.Errorf("%w: for record %s", ErrDomainEmpty, record) case len(record.Host) == 0: - return fmt.Errorf("host is empty for record %s", record) + return fmt.Errorf("%w: for record %s", ErrHostIsEmpty, record) } var t time.Time for i, event := range record.Events { if event.Time.Before(t) { - return fmt.Errorf("IP records are not ordered correctly by time") + return fmt.Errorf("%w", ErrIPRecordsMisordered) } t = event.Time switch { case event.IP == nil: - return fmt.Errorf("IP %d of %d is empty for record %s", i+1, len(record.Events), record) + return fmt.Errorf("%w: IP %d of %d for record %s", + ErrIPEmpty, i+1, len(record.Events), record) case event.Time.IsZero(): - return fmt.Errorf("time of IP %d of %d is empty for record %s", i+1, len(record.Events), record) + return fmt.Errorf("%w: IP %d of %d for record %s", + ErrIPTimeEmpty, i+1, len(record.Events), record) } } } diff --git a/internal/settings/providers/digitalocean/provider.go b/internal/settings/providers/digitalocean/provider.go index 64e0ccba4..15757c503 100644 --- a/internal/settings/providers/digitalocean/provider.go +++ b/internal/settings/providers/digitalocean/provider.go @@ -200,9 +200,10 @@ func (p *Provider) Update(ctx context.Context, client *http.Client, ip net.IP) ( newIP = net.ParseIP(responseData.DomainRecord.Data) if newIP == nil { - return nil, fmt.Errorf("IP address received %q is malformed", responseData.DomainRecord.Data) + return nil, fmt.Errorf("%w: %s", errors.ErrIPReceivedMalformed, responseData.DomainRecord.Data) } else if !newIP.Equal(ip) { - return nil, fmt.Errorf("updated IP address %s is not the IP address %s sent for update", newIP, ip) + return nil, fmt.Errorf("%w: sent %s but received %s ", + errors.ErrIPReceivedMismatch, ip, newIP) } return newIP, nil } diff --git a/internal/settings/providers/dreamhost/provider.go b/internal/settings/providers/dreamhost/provider.go index 8a5767eab..b25a5a7f0 100644 --- a/internal/settings/providers/dreamhost/provider.go +++ b/internal/settings/providers/dreamhost/provider.go @@ -53,7 +53,7 @@ func New(data json.RawMessage, domain, host string, ipVersion ipversion.IPVersio func (p *Provider) isValid() error { if !p.matcher.DreamhostKey(p.key) { - return fmt.Errorf("invalid key format") + return fmt.Errorf("%w: %s", errors.ErrMalformedKey, p.key) } return nil } diff --git a/internal/settings/providers/linode/provider.go b/internal/settings/providers/linode/provider.go index 0fedd18b0..8a56e86a7 100644 --- a/internal/settings/providers/linode/provider.go +++ b/internal/settings/providers/linode/provider.go @@ -153,7 +153,7 @@ func (p *Provider) getDomainID(ctx context.Context, client *http.Client) (domain if response.StatusCode != http.StatusOK { err = fmt.Errorf("%w: %d", errors.ErrBadHTTPStatus, response.StatusCode) - return 0, fmt.Errorf("%w: %s", err, p.getError(response.Body)) + return 0, fmt.Errorf("%w: %s", err, p.getErrorMessage(response.Body)) } decoder := json.NewDecoder(response.Body) @@ -212,7 +212,7 @@ func (p *Provider) getRecordID(ctx context.Context, client *http.Client, if response.StatusCode != http.StatusOK { err = fmt.Errorf("%w: %d", errors.ErrBadHTTPStatus, response.StatusCode) - return 0, fmt.Errorf("%w: %s", err, p.getError(response.Body)) + return 0, fmt.Errorf("%w: %s", err, p.getErrorMessage(response.Body)) } decoder := json.NewDecoder(response.Body) @@ -276,7 +276,7 @@ func (p *Provider) createRecord(ctx context.Context, client *http.Client, if response.StatusCode != http.StatusOK { err = fmt.Errorf("%w: %d", errors.ErrBadHTTPStatus, response.StatusCode) - return fmt.Errorf("%w: %s", err, p.getError(response.Body)) + return fmt.Errorf("%w: %s", err, p.getErrorMessage(response.Body)) } var responseData domainRecord @@ -329,7 +329,7 @@ func (p *Provider) updateRecord(ctx context.Context, client *http.Client, if response.StatusCode != http.StatusOK { err = fmt.Errorf("%w: %d", errors.ErrBadHTTPStatus, response.StatusCode) - return fmt.Errorf("%w: %s", err, p.getError(response.Body)) + return fmt.Errorf("%w: %s", err, p.getErrorMessage(response.Body)) } data.IP = "" @@ -348,14 +348,14 @@ func (p *Provider) updateRecord(ctx context.Context, client *http.Client, return nil } -func (p *Provider) getError(body io.Reader) (err error) { +func (p *Provider) getErrorMessage(body io.Reader) (message string) { var errorObj linodeErrors b, err := io.ReadAll(body) if err != nil { - return err + return fmt.Sprintf("reading body: %s", err) } if err := json.Unmarshal(b, &errorObj); err != nil { - return fmt.Errorf("%s", utils.ToSingleLine(string(b))) + return utils.ToSingleLine(string(b)) } - return fmt.Errorf("%v", errorObj) + return fmt.Sprintf("%v", errorObj) } diff --git a/internal/update/interfaces.go b/internal/update/interfaces.go index 82446da3d..b905f04bb 100644 --- a/internal/update/interfaces.go +++ b/internal/update/interfaces.go @@ -15,11 +15,11 @@ type PublicIPFetcher interface { } type UpdaterInterface interface { - Update(ctx context.Context, recordID int, ip net.IP, now time.Time) (err error) + Update(ctx context.Context, recordID uint, ip net.IP, now time.Time) (err error) } type Database interface { - Select(recordID int) (record records.Record, err error) + Select(recordID uint) (record records.Record, err error) SelectAll() (records []records.Record) - Update(recordID int, record records.Record) (err error) + Update(recordID uint, record records.Record) (err error) } diff --git a/internal/update/run.go b/internal/update/run.go index e1408a541..80fa537fe 100644 --- a/internal/update/run.go +++ b/internal/update/run.go @@ -117,10 +117,11 @@ func (r *Runner) getNewIPs(ctx context.Context, doIP, doIPv4, doIPv6 bool, ipv6M } func (r *Runner) getRecordIDsToUpdate(ctx context.Context, records []librecords.Record, - ip, ipv4, ipv6 net.IP, now time.Time, ipv6Mask net.IPMask) (recordIDs map[int]struct{}) { - recordIDs = make(map[int]struct{}) - for id, record := range records { + ip, ipv4, ipv6 net.IP, now time.Time, ipv6Mask net.IPMask) (recordIDs map[uint]struct{}) { + recordIDs = make(map[uint]struct{}) + for i, record := range records { if shouldUpdate := r.shouldUpdateRecord(ctx, record, ip, ipv4, ipv6, now, ipv6Mask); shouldUpdate { + id := uint(i) recordIDs[id] = struct{}{} } } @@ -239,7 +240,7 @@ func getIPMatchingVersion(ip, ipv4, ipv6 net.IP, ipVersion ipversion.IPVersion) return nil } -func setInitialUpToDateStatus(db Database, id int, updateIP net.IP, now time.Time) error { +func setInitialUpToDateStatus(db Database, id uint, updateIP net.IP, now time.Time) error { record, err := db.Select(id) if err != nil { return err @@ -268,7 +269,8 @@ func (r *Runner) updateNecessary(ctx context.Context, ipv6Mask net.IPMask) (erro now := r.timeNow() recordIDs := r.getRecordIDsToUpdate(ctx, records, ip, ipv4, ipv6, now, ipv6Mask) - for id, record := range records { + for i, record := range records { + id := uint(i) _, requireUpdate := recordIDs[id] if requireUpdate || record.Status != constants.UNSET { continue diff --git a/internal/update/update.go b/internal/update/update.go index f558ce7ca..b2bd7b27b 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -33,7 +33,7 @@ func NewUpdater(db Database, client *http.Client, notify notifyFunc, logger logg } } -func (u *Updater) Update(ctx context.Context, id int, ip net.IP, now time.Time) (err error) { +func (u *Updater) Update(ctx context.Context, id uint, ip net.IP, now time.Time) (err error) { record, err := u.db.Select(id) if err != nil { return err @@ -50,10 +50,11 @@ func (u *Updater) Update(ctx context.Context, id int, ip net.IP, now time.Time) if errors.Is(err, settingserrors.ErrAbuse) { lastBan := time.Unix(now.Unix(), 0) record.LastBan = &lastBan - message := record.Settings.BuildDomainName() + ": " + record.Message + + domainName := record.Settings.BuildDomainName() + message := domainName + ": " + record.Message + ", no more updates will be attempted for an hour" u.notify(message) - err = fmt.Errorf(message) + err = fmt.Errorf("%w: for domain %s, no more update will be attempted for 1h", err, domainName) } else { record.LastBan = nil // clear a previous ban }