From ce1cdb0c2cc7557884054a2714041547c7502d6f Mon Sep 17 00:00:00 2001 From: Francisco Delmar Kurpiel Date: Sat, 4 Oct 2025 18:49:44 +0200 Subject: [PATCH 1/5] refactor(tests): Use t.Context() instead of context.Background() Replace all instances of `context.Background()` with `t.Context()` within the test files. This change aligns with modern Go (1.20+) testing best practices. The `t.Context()` function provides a context that is automatically canceled when the test or subtest completes. Using this context for operations like API calls ensures that they are properly terminated if the test finishes, is skipped, or times out. This improves test hygiene by preventing potential resource leaks from dangling goroutines and makes test execution more robust, especially when running with a timeout. --- client_test.go | 6 +++--- retry/exp_backoff_test.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/client_test.go b/client_test.go index 1fe0c53..2d3c02c 100644 --- a/client_test.go +++ b/client_test.go @@ -28,7 +28,7 @@ const ( func TestCreateCNAME(t *testing.T) { t.Parallel() - ctx := context.Background() + ctx := t.Context() client, testZoneID := getClient(ctx, t) cname := "CNAME" comment := "integration test" @@ -79,7 +79,7 @@ func TestUpdate(t *testing.T) { changedComment := "integration test" cname := "cname" - ctx := context.Background() + ctx := t.Context() client, testZoneID := getClient(ctx, t) // create a DNS record @@ -139,7 +139,7 @@ func TestUpdate(t *testing.T) { // Test a few cases of error to make sure error handling works. func TestConflict(t *testing.T) { t.Parallel() - ctx := context.Background() + ctx := t.Context() client, testZoneID := getClient(ctx, t) cases := []*struct { diff --git a/retry/exp_backoff_test.go b/retry/exp_backoff_test.go index 92f54b3..0179b56 100644 --- a/retry/exp_backoff_test.go +++ b/retry/exp_backoff_test.go @@ -15,7 +15,7 @@ import ( ) func TestRetry(t *testing.T) { - ctx := context.Background() + ctx := t.Context() someErr := errors.New("some error") cases := []*struct { @@ -101,7 +101,7 @@ func TestContextCancel(t *testing.T) { logger := log.New(testtarget.ForTest(t, true), log.WithDebugEnabledFn(func() bool { return true })) - ctx, done := context.WithTimeout(context.Background(), 100*time.Millisecond) + ctx, done := context.WithTimeout(t.Context(), 100*time.Millisecond) defer done() err := retry.ExpBackoff(ctx, logger, time.Hour, time.Hour, 2, 100, func() error { From 404190723131f4ffe54c8124f64ba49d4d302d70 Mon Sep 17 00:00:00 2001 From: Francisco Delmar Kurpiel Date: Sat, 4 Oct 2025 19:01:11 +0200 Subject: [PATCH 2/5] ci: Update Go to 1.23 and golangci-lint to v2.5.0 This commit updates the versions of Go and golangci-lint used in the GitHub Actions CI workflow. The Go version is bumped from 1.22 to 1.23 for both the test and linting jobs. The version of golangci-lint is also upgraded from v2.1.6 to v2.5.0. These changes ensure the project is tested and linted against the latest stable toolchain, leveraging new language features, performance improvements, and updated static analysis rules. --- .github/workflows/ci.yml | 6 +++--- go.mod | 2 -- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 057dc68..01103a1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,7 +12,7 @@ jobs: test: strategy: matrix: - go: ["1.22", "stable"] + go: ["1.23", "stable"] name: test runs-on: ubuntu-latest steps: @@ -31,8 +31,8 @@ jobs: golangci: strategy: matrix: - go: ["1.22", "stable"] - lint: ["v2.1.6"] + go: ["1.23", "stable"] + lint: ["v2.5.0"] name: lint runs-on: ubuntu-latest steps: diff --git a/go.mod b/go.mod index 6f8ba5c..95a780a 100644 --- a/go.mod +++ b/go.mod @@ -2,8 +2,6 @@ module github.com/simplesurance/cfdns go 1.23.0 -toolchain go1.24.3 - require ( github.com/fatih/color v1.18.0 golang.org/x/time v0.12.0 From 4dfd9f33b4a564b36c09583f0887e181c686e8c1 Mon Sep 17 00:00:00 2001 From: Francisco Delmar Kurpiel Date: Sat, 4 Oct 2025 20:55:09 +0200 Subject: [PATCH 3/5] Whitespace police --- .github/workflows/ci.yml | 2 +- client.go | 11 +++++++++++ client_test.go | 23 ++++++++++++++++++----- createrecord.go | 3 ++- deleterecord.go | 2 +- iterator.go | 3 +++ listrecords.go | 2 +- listzones.go | 4 ++-- log/logger.go | 4 ++++ log/testtarget/testtarget.go | 1 + log/texttarget/textlogger.go | 2 ++ request.go | 3 +++ retry/exp_backoff.go | 4 ++++ retry/exp_backoff_test.go | 8 ++++++-- updaterecord.go | 3 ++- 15 files changed, 61 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 01103a1..d6daee1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,7 +12,7 @@ jobs: test: strategy: matrix: - go: ["1.23", "stable"] + go: ["1.24", "stable"] name: test runs-on: ubuntu-latest steps: diff --git a/client.go b/client.go index 94c946f..07eb1dd 100644 --- a/client.go +++ b/client.go @@ -38,6 +38,7 @@ var errResponseTooLarge = retry.PermanentError{ type Client struct { *settings + creds Credentials } @@ -66,10 +67,13 @@ func sendRequestRetry[TRESP commonResponseSetter]( error, ) { var resp *response[TRESP] + reterr := retry.ExpBackoff(ctx, logger, retryFirstDelay, retryMaxDelay, retryFactor, retryMaxAttempts, func() error { var err error + resp, err = sendRequest[TRESP](ctx, client, logger, req) + return err }) @@ -105,8 +109,10 @@ func sendRequest[TRESP commonResponseSetter]( // request timeout reqCtx := ctx + if client.requestTimeout > 0 { var reqCtxCancel func() + reqCtx, reqCtxCancel = context.WithTimeout(reqCtx, client.requestTimeout) defer reqCtxCancel() } @@ -145,6 +151,7 @@ func sendRequest[TRESP commonResponseSetter]( if resp.StatusCode >= 400 { err = handleErrorResponse(resp, logger) logFullRequestResponse(logger, reqNoAuth, reqBody, resp, rawResponseFromErr(err)) + return nil, err } @@ -171,6 +178,7 @@ func handleSuccessResponse[TRESP commonResponseSetter](httpResp *http.Response, ret.headers = httpResp.Header var err error + ret.rawBody, err = readResponseBody(httpResp.Body) if err != nil { // error response already specifies is can retry or not @@ -183,6 +191,7 @@ func handleSuccessResponse[TRESP commonResponseSetter](httpResp *http.Response, if len(ret.rawBody) == maxResponseLength { logger.W(fmt.Sprintf("Response from CloudFlare rejected because is bigger than %d", maxResponseLength)) + return nil, retry.PermanentError{ Cause: errors.Join(err, HTTPError{ Code: httpResp.StatusCode, @@ -238,6 +247,7 @@ func handleErrorResponse(resp *http.Response, _ *log.Logger) error { } var cfcommon cfResponseCommon + err = json.Unmarshal(respBody, &cfcommon) if err != nil { return retry.PermanentError{Cause: fmt.Errorf("CloudFlare returned an error, unmarshaling the error body as json failed: %w; %w", err, httpErr)} @@ -283,6 +293,7 @@ func logFullRequestResponse( func requestURL(treq *request) string { urlstring := baseURL + "/" + treq.path + theurl, err := url.Parse(urlstring) if err != nil { // this only happens in case of coding error on cfapi diff --git a/client_test.go b/client_test.go index 2d3c02c..2eb720d 100644 --- a/client_test.go +++ b/client_test.go @@ -28,13 +28,15 @@ const ( func TestCreateCNAME(t *testing.T) { t.Parallel() - ctx := t.Context() + + ctx := context.Background() client, testZoneID := getClient(ctx, t) cname := "CNAME" comment := "integration test" // create a DNS record recName := testRecordName(t) + resp, err := client.CreateRecord(ctx, &cfdns.CreateRecordRequest{ ZoneID: testZoneID, Name: recName, @@ -52,6 +54,7 @@ func TestCreateCNAME(t *testing.T) { // assert that it is present var recs []*cfdns.ListRecordsResponseItem + recs, err = cfdns.ReadAll(ctx, client.ListRecords(&cfdns.ListRecordsRequest{ ZoneID: testZoneID, Name: resp.Name, @@ -75,15 +78,17 @@ func TestCreateCNAME(t *testing.T) { func TestUpdate(t *testing.T) { t.Parallel() + originalComment := "integration test" changedComment := "integration test" cname := "cname" - ctx := t.Context() + ctx := context.Background() client, testZoneID := getClient(ctx, t) // create a DNS record recName := testRecordName(t) + resp, err := client.CreateRecord(ctx, &cfdns.CreateRecordRequest{ ZoneID: testZoneID, Name: recName, @@ -139,7 +144,8 @@ func TestUpdate(t *testing.T) { // Test a few cases of error to make sure error handling works. func TestConflict(t *testing.T) { t.Parallel() - ctx := t.Context() + + ctx := context.Background() client, testZoneID := getClient(ctx, t) cases := []*struct { @@ -160,13 +166,14 @@ func TestConflict(t *testing.T) { } for _, tc := range cases { - tc := tc //nolint:copyloopvar t.Run(tc.typ, func(t *testing.T) { t.Parallel() + comment := "integration test" // create a DNS record recName := testRecordName(t) + resp, err := client.CreateRecord(ctx, &cfdns.CreateRecordRequest{ ZoneID: testZoneID, Name: recName, @@ -238,6 +245,7 @@ func getClient(ctx context.Context, t *testing.T) (_ *cfdns.Client, testZoneID s } t.Fatalf("Zone %s not found on CloudFlare", testzone) + return nil, "" } @@ -250,7 +258,9 @@ func testRecordName(t *testing.T) string { testzone := os.Getenv(envTestZone) rnd := make([]byte, 4) - if _, err := rand.Read(rnd); err != nil { + + _, err := rand.Read(rnd) + if err != nil { t.Fatalf("Error reading random number: %v", err) } @@ -292,6 +302,7 @@ func cleanup( } t.Logf("Error listing records when looking for old test data: %v", err) + return } @@ -304,6 +315,7 @@ func cleanup( if err != nil { t.Errorf("Record %s (%s %s %s) has a time part %q that is invalid", record.ID, record.Name, record.Type, record.Content, matches[1]) + continue } @@ -326,6 +338,7 @@ func cleanup( func requireNotNil(t *testing.T, v any) { t.Helper() + if v == nil { t.Fatalf("Unexpected nil value") } diff --git a/createrecord.go b/createrecord.go index 2dde10f..52fbf74 100644 --- a/createrecord.go +++ b/createrecord.go @@ -41,7 +41,6 @@ func (c *Client) CreateRecord( TTL: ttl, }, }) - if err != nil { return nil, err } @@ -50,6 +49,7 @@ func (c *Client) CreateRecord( log(fmt.Sprintf("Record %s %s %s created with ID=%s", req.Name, req.Type, req.Content, resp.body.Result.ID)) }) + return &CreateRecordResponse{ ID: resp.body.Result.ID, Name: resp.body.Result.Name, @@ -84,6 +84,7 @@ type createRecordAPIRequest struct { type createRecordAPIResponse struct { cfResponseCommon + Result struct { ID string `json:"id"` Name string `json:"name"` diff --git a/deleterecord.go b/deleterecord.go index 8e87eae..ed962a0 100644 --- a/deleterecord.go +++ b/deleterecord.go @@ -28,7 +28,6 @@ func (c *Client) DeleteRecord( queryParams: url.Values{}, body: nil, }) - if err != nil { return nil, err } @@ -36,6 +35,7 @@ func (c *Client) DeleteRecord( c.logger.D(func(log log.DebugFn) { log(fmt.Sprintf("Record %s deleted", req.RecordID)) }) + return &DeleteRecordResponse{}, err } diff --git a/iterator.go b/iterator.go index a795d20..a425629 100644 --- a/iterator.go +++ b/iterator.go @@ -23,6 +23,7 @@ type fetchFn[T any] func(ctx context.Context) (batch []*T, last bool, _ error) func (it *Iterator[T]) Next(ctx context.Context) (retElm *T, err error) { if len(it.elements) == 0 && !it.isLast { var elements []*T + elements, it.isLast, err = it.fetchNext(ctx) if err != nil { return nil, err @@ -37,6 +38,7 @@ func (it *Iterator[T]) Next(ctx context.Context) (retElm *T, err error) { retElm = it.elements[0] it.elements = it.elements[1:] + return retElm, nil } @@ -44,6 +46,7 @@ func (it *Iterator[T]) Next(ctx context.Context) (retElm *T, err error) { // and return them as an array. func ReadAll[T any](ctx context.Context, it *Iterator[T]) ([]*T, error) { ret := []*T{} + for { item, err := it.Next(ctx) if err != nil { diff --git a/listrecords.go b/listrecords.go index 1ea0c1a..5579d15 100644 --- a/listrecords.go +++ b/listrecords.go @@ -52,7 +52,6 @@ func (c *Client) ListRecords( queryParams: queryParams, body: nil, }) - if err != nil { return nil, false, err } @@ -100,6 +99,7 @@ type ListRecordsResponseItem struct { type listRecordsAPIResponse struct { cfResponseCommon + Result []listRecordsAPIResponseItem `json:"result"` } diff --git a/listzones.go b/listzones.go index d482b88..33c1241 100644 --- a/listzones.go +++ b/listzones.go @@ -9,7 +9,7 @@ import ( "github.com/simplesurance/cfdns/log" ) -// Listzones lists zones on CloudFlare. +// ListZones lists zones on CloudFlare. // // API Reference: https://developers.cloudflare.com/api/operations/zones-get func (c *Client) ListZones( @@ -44,7 +44,6 @@ func (c *Client) ListZones( queryParams: queryParams, body: nil, }) - if err != nil { return nil, false, err } @@ -75,6 +74,7 @@ type ListZonesResponseItem struct { type listZoneAPIResponse struct { cfResponseCommon + Result []listZoneAPIResponseItem `json:"result"` } diff --git a/log/logger.go b/log/logger.go index 5607da4..e4712b6 100644 --- a/log/logger.go +++ b/log/logger.go @@ -66,6 +66,7 @@ func (l *Logger) I(msg string, opt ...Option) { if helper := l.driver.PreLog(); helper != nil { helper() } + l.log(msg, Info, opt...) } @@ -73,6 +74,7 @@ func (l *Logger) W(msg string, opt ...Option) { if helper := l.driver.PreLog(); helper != nil { helper() } + l.log(msg, Warn, opt...) } @@ -80,6 +82,7 @@ func (l *Logger) E(msg string, opt ...Option) { if helper := l.driver.PreLog(); helper != nil { helper() } + l.log(msg, Error, opt...) } @@ -87,6 +90,7 @@ func (l *Logger) d(msg string, opt ...Option) { if helper := l.driver.PreLog(); helper != nil { helper() } + l.log(msg, Debug, opt...) } diff --git a/log/testtarget/testtarget.go b/log/testtarget/testtarget.go index 996182d..92bd873 100644 --- a/log/testtarget/testtarget.go +++ b/log/testtarget/testtarget.go @@ -40,6 +40,7 @@ func (t testDriver) Send(l *log.Entry) { keys := slices.Collect(maps.Keys(l.Tags)) slices.Sort(keys) + for _, key := range keys { fmt.Fprintf(msg, "\n- %s: %v", key, format(l.Tags[key])) } diff --git a/log/texttarget/textlogger.go b/log/texttarget/textlogger.go index 1304f22..035aebd 100644 --- a/log/texttarget/textlogger.go +++ b/log/texttarget/textlogger.go @@ -44,6 +44,7 @@ type logger struct { func (l *logger) Send(entry *log.Entry) { w := l.outw + mux := l.outMux if entry.Severity == log.Error { w = l.errw @@ -77,6 +78,7 @@ func (l *logger) Send(entry *log.Entry) { mux.Lock() defer mux.Unlock() + fmt.Fprintln(w) } diff --git a/request.go b/request.go index 00f73dc..39640f0 100644 --- a/request.go +++ b/request.go @@ -35,9 +35,11 @@ func (e HTTPError) Error() string { fmt.Fprintf(msg, "HTTP %d\n", e.Code) headers := slices.Collect(maps.Keys(e.Headers)) slices.Sort(headers) + for _, k := range headers { fmt.Fprintf(msg, "%s: %s\n", k, e.Headers.Get(k)) } + fmt.Fprintln(msg) fmt.Fprintf(msg, "%s", e.RawBody) @@ -57,6 +59,7 @@ var _ error = HTTPError{} type CloudFlareError struct { cfResponseCommon + HTTPError HTTPError } diff --git a/retry/exp_backoff.go b/retry/exp_backoff.go index d779dd2..087b192 100644 --- a/retry/exp_backoff.go +++ b/retry/exp_backoff.go @@ -80,18 +80,22 @@ func ExpBackoff( select { case <-ctx.Done(): err := ctx.Err() + logger.D(func(lg log.DebugFn) { lg("context was canceled", log.WithInt("attempt", attempt), log.WithDuration("total_delay", time.Since(start)), log.WithError(err)) }) + return err case <-time.After(time.Duration(delay * float64(time.Second))): delay *= factor + logger.D(func(lg log.DebugFn) { lg(fmt.Sprintf("next delay: %f seconds", delay)) }) + if delay > maxDelay.Seconds() { delay = maxDelay.Seconds() } diff --git a/retry/exp_backoff_test.go b/retry/exp_backoff_test.go index 0179b56..a94d0bf 100644 --- a/retry/exp_backoff_test.go +++ b/retry/exp_backoff_test.go @@ -15,7 +15,7 @@ import ( ) func TestRetry(t *testing.T) { - ctx := t.Context() + ctx := context.Background() someErr := errors.New("some error") cases := []*struct { @@ -74,17 +74,20 @@ func TestRetry(t *testing.T) { if tc.permErrorAt != 0 && fCallCount == tc.permErrorAt { t.Logf("test function returning permanent error (c=%d l=%d)", fCallCount, tc.permErrorAt) + return retry.PermanentError{Cause: someErr} } if tc.tempErrorCount != 0 && fCallCount <= tc.tempErrorCount { t.Logf("test function returning temporary error (c=%d l=%d)", fCallCount, tc.tempErrorCount) + return someErr } t.Logf("test function returning nil (c=%d pl=%d tl=%d)", fCallCount, tc.permErrorAt, tc.tempErrorCount) + return nil }) if tc.wantError { @@ -92,6 +95,7 @@ func TestRetry(t *testing.T) { } else { assertNoError(t, err) } + assertEquals(t, tc.wantFunctionCalls, fCallCount) }) } @@ -101,7 +105,7 @@ func TestContextCancel(t *testing.T) { logger := log.New(testtarget.ForTest(t, true), log.WithDebugEnabledFn(func() bool { return true })) - ctx, done := context.WithTimeout(t.Context(), 100*time.Millisecond) + ctx, done := context.WithTimeout(context.Background(), 100*time.Millisecond) defer done() err := retry.ExpBackoff(ctx, logger, time.Hour, time.Hour, 2, 100, func() error { diff --git a/updaterecord.go b/updaterecord.go index 31d1961..15ab8d0 100644 --- a/updaterecord.go +++ b/updaterecord.go @@ -44,7 +44,6 @@ func (c *Client) UpdateRecord( TTL: ttl, }, }) - if err != nil { return nil, err } @@ -53,6 +52,7 @@ func (c *Client) UpdateRecord( log(fmt.Sprintf("Record %s (%s %s %s) updated", req.RecordID, req.Name, req.Type, req.Content)) }) + return &UpdateRecordResponse{ ModifiedOn: resp.body.Result.ModifiedOn, }, err @@ -86,6 +86,7 @@ type updateRecordAPIRequest struct { type updateRecordAPIResponse struct { cfResponseCommon + Result struct { ModifiedOn time.Time `json:"modified_on"` } `json:"result"` From e3cee9b2d28915fe57c33cef5001eee3d06389d2 Mon Sep 17 00:00:00 2001 From: Francisco Delmar Kurpiel Date: Sat, 4 Oct 2025 21:01:18 +0200 Subject: [PATCH 4/5] Consistently supporting golangci-lint 1.23 to 1.25 --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d6daee1..01103a1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,7 +12,7 @@ jobs: test: strategy: matrix: - go: ["1.24", "stable"] + go: ["1.23", "stable"] name: test runs-on: ubuntu-latest steps: From 41056fb32a0ce8fa7c9df61071dded8a848fe814 Mon Sep 17 00:00:00 2001 From: Francisco Delmar Kurpiel Date: Sun, 5 Oct 2025 16:11:58 +0200 Subject: [PATCH 5/5] Fixing test assertion --- client_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client_test.go b/client_test.go index 2eb720d..1333899 100644 --- a/client_test.go +++ b/client_test.go @@ -80,7 +80,7 @@ func TestUpdate(t *testing.T) { t.Parallel() originalComment := "integration test" - changedComment := "integration test" + changedComment := "integration test - changed" cname := "cname" ctx := context.Background()