diff --git a/platform/wait/wait_test.go b/platform/wait/wait_test.go index 9d1a4ac34..b401c819f 100644 --- a/platform/wait/wait_test.go +++ b/platform/wait/wait_test.go @@ -2,65 +2,119 @@ package wait import ( "errors" + "sync/atomic" "testing" "time" + + "github.com/stretchr/testify/require" ) +// TODO(ldez): rewrite those tests when upgrading to go1.25 as minimum Go version. + func TestFor_timeout(t *testing.T) { + var io atomic.Int64 + c := make(chan error) + go func() { - c <- For("", 3*time.Second, 1*time.Second, func() (bool, error) { + c <- For("test", 3*time.Second, 1*time.Second, func() (bool, error) { + io.Add(1) + if io.Load() == 1 { + return false, nil + } + return false, nil }) }() timeout := time.After(6 * time.Second) + select { case <-timeout: t.Fatal("timeout exceeded") case err := <-c: - if err == nil { - t.Errorf("expected timeout error; got %v", err) - } - t.Logf("%v", err) + require.EqualError(t, err, "test: time limit exceeded") } + + require.EqualValues(t, 3, io.Load()) +} + +func TestFor_timeout_with_error(t *testing.T) { + var io atomic.Int64 + + c := make(chan error) + + go func() { + c <- For("test", 3*time.Second, 1*time.Second, func() (bool, error) { + io.Add(1) + + // This allows be sure that the latest previous error is returned. + if io.Load() == 1 { + return false, errors.New("oops") + } + + return false, nil + }) + }() + + timeout := time.After(6 * time.Second) + + select { + case <-timeout: + t.Fatal("timeout exceeded") + case err := <-c: + require.EqualError(t, err, "test: time limit exceeded: last error: oops") + } + + require.EqualValues(t, 3, io.Load()) } func TestFor_stop(t *testing.T) { + var io atomic.Int64 + c := make(chan error) + go func() { - c <- For("", 3*time.Second, 1*time.Second, func() (bool, error) { + c <- For("test", 3*time.Second, 1*time.Second, func() (bool, error) { + io.Add(1) + return true, nil }) }() timeout := time.After(6 * time.Second) + select { case <-timeout: t.Fatal("timeout exceeded") case err := <-c: - if err != nil { - t.Errorf("expected no timeout error; got %v", err) - } + require.NoError(t, err) } + + require.EqualValues(t, 1, io.Load()) } -func TestFor_stop_error(t *testing.T) { +func TestFor_stop_with_error(t *testing.T) { + var io atomic.Int64 + c := make(chan error) + go func() { - c <- For("", 3*time.Second, 1*time.Second, func() (bool, error) { + c <- For("test", 3*time.Second, 1*time.Second, func() (bool, error) { + io.Add(1) + return true, errors.New("oops") }) }() timeout := time.After(6 * time.Second) + select { case <-timeout: t.Fatal("timeout exceeded") case err := <-c: - if err == nil { - t.Errorf("expected error; got %v", err) - } - t.Logf("%v", err) + require.EqualError(t, err, "oops") } + + require.EqualValues(t, 1, io.Load()) } diff --git a/providers/dns/cloudns/cloudns.go b/providers/dns/cloudns/cloudns.go index ef6524c4d..f199cd1a1 100644 --- a/providers/dns/cloudns/cloudns.go +++ b/providers/dns/cloudns/cloudns.go @@ -8,6 +8,7 @@ import ( "net/http" "time" + "github.com/cenkalti/backoff/v5" "github.com/go-acme/lego/v4/challenge" "github.com/go-acme/lego/v4/challenge/dns01" "github.com/go-acme/lego/v4/log" @@ -162,14 +163,22 @@ func (d *DNSProvider) Timeout() (timeout, interval time.Duration) { // waitNameservers At the time of writing 4 servers are found as authoritative, but 8 are reported during the sync. // If this is not done, the secondary verification done by Let's Encrypt server will fail quire a bit. func (d *DNSProvider) waitNameservers(ctx context.Context, domain string, zone *internal.Zone) error { - return wait.For("Nameserver sync on "+domain, d.config.PropagationTimeout, d.config.PollingInterval, func() (bool, error) { - syncProgress, err := d.client.GetUpdateStatus(ctx, zone.Name) - if err != nil { - return false, err - } + return wait.Retry(context.Background(), + func() error { + syncProgress, err := d.client.GetUpdateStatus(ctx, zone.Name) + if err != nil { + return fmt.Errorf("nameserver sync on %s: %w", domain, err) + } - log.Infof("[%s] Sync %d/%d complete", domain, syncProgress.Updated, syncProgress.Total) + log.Infof("[%s] Sync %d/%d complete", domain, syncProgress.Updated, syncProgress.Total) - return syncProgress.Complete, nil - }) + if !syncProgress.Complete { + return fmt.Errorf("nameserver sync on %s not complete", domain) + } + + return nil + }, + backoff.WithBackOff(backoff.NewConstantBackOff(d.config.PollingInterval)), + backoff.WithMaxElapsedTime(d.config.PropagationTimeout), + ) } diff --git a/providers/dns/f5xc/f5xc.go b/providers/dns/f5xc/f5xc.go index 2ed1f0c4f..e854bf020 100644 --- a/providers/dns/f5xc/f5xc.go +++ b/providers/dns/f5xc/f5xc.go @@ -8,6 +8,7 @@ import ( "net/http" "time" + "github.com/cenkalti/backoff/v5" "github.com/go-acme/lego/v4/challenge/dns01" "github.com/go-acme/lego/v4/platform/config/env" "github.com/go-acme/lego/v4/platform/wait" @@ -128,29 +129,41 @@ func (d *DNSProvider) Present(domain, token, keyAuth string) error { }, } - return wait.For("f5xc create", 60*time.Second, 2*time.Second, func() (bool, error) { + return d.waitFor(context.Background(), func() error { _, err = d.client.CreateRRSet(context.Background(), dns01.UnFqdn(authZone), d.config.GroupName, rrSet) if err != nil { - return false, fmt.Errorf("f5xc: create RR set: %w", err) + return fmt.Errorf("create RR set: %w", err) } - return true, nil + return nil }) } // Update RRSet. existingRRSet.RRSet.TXTRecord.Values = append(existingRRSet.RRSet.TXTRecord.Values, info.Value) - return wait.For("f5xc replace", 60*time.Second, 2*time.Second, func() (bool, error) { + return d.waitFor(context.Background(), func() error { _, err = d.client.ReplaceRRSet(context.Background(), dns01.UnFqdn(authZone), d.config.GroupName, subDomain, "TXT", existingRRSet.RRSet) if err != nil { - return false, fmt.Errorf("f5xc: replace RR set: %w", err) + return fmt.Errorf("replace RR set: %w", err) } - return true, nil + return nil }) } +func (d *DNSProvider) waitFor(ctx context.Context, operation func() error) error { + err := wait.Retry(ctx, operation, + backoff.WithBackOff(backoff.NewConstantBackOff(2*time.Second)), + backoff.WithMaxElapsedTime(60*time.Second), + ) + if err != nil { + return fmt.Errorf("f5xc: %w", err) + } + + return nil +} + // CleanUp removes the TXT record matching the specified parameters. func (d *DNSProvider) CleanUp(domain, token, keyAuth string) error { info := dns01.GetChallengeInfo(domain, keyAuth) diff --git a/providers/dns/gcloud/googlecloud.go b/providers/dns/gcloud/googlecloud.go index 94cc3df1e..60f38592d 100644 --- a/providers/dns/gcloud/googlecloud.go +++ b/providers/dns/gcloud/googlecloud.go @@ -11,6 +11,7 @@ import ( "time" "cloud.google.com/go/compute/metadata" + "github.com/cenkalti/backoff/v5" "github.com/go-acme/lego/v4/challenge" "github.com/go-acme/lego/v4/challenge/dns01" "github.com/go-acme/lego/v4/log" @@ -266,24 +267,28 @@ func (d *DNSProvider) applyChanges(zone string, change *gdns.Change) error { chgID := chg.Id // wait for change to be acknowledged - return wait.For("apply change", 30*time.Second, 3*time.Second, func() (bool, error) { - if d.config.Debug { - data, _ := json.Marshal(change) - log.Printf("change (Get): %s", string(data)) - } + return wait.Retry(context.Background(), + func() error { + if d.config.Debug { + data, _ := json.Marshal(change) + log.Printf("change (Get): %s", string(data)) + } - chg, err = d.client.Changes.Get(d.config.Project, zone, chgID).Do() - if err != nil { - data, _ := json.Marshal(change) - return false, fmt.Errorf("failed to get changes [zone %s, change %s]: %w", zone, string(data), err) - } + chg, err = d.client.Changes.Get(d.config.Project, zone, chgID).Do() + if err != nil { + data, _ := json.Marshal(change) + return fmt.Errorf("failed to get changes [zone %s, change %s]: %w", zone, string(data), err) + } - if chg.Status == changeStatusDone { - return true, nil - } + if chg.Status != changeStatusDone { + return fmt.Errorf("status: %s", chg.Status) + } - return false, fmt.Errorf("status: %s", chg.Status) - }) + return nil + }, + backoff.WithBackOff(backoff.NewConstantBackOff(3*time.Second)), + backoff.WithMaxElapsedTime(30*time.Second), + ) } // CleanUp removes the TXT record matching the specified parameters. diff --git a/providers/dns/hetzner/internal/hetznerv1/hetznerv1.go b/providers/dns/hetzner/internal/hetznerv1/hetznerv1.go index 603177a0d..827eb5457 100644 --- a/providers/dns/hetzner/internal/hetznerv1/hetznerv1.go +++ b/providers/dns/hetzner/internal/hetznerv1/hetznerv1.go @@ -9,6 +9,7 @@ import ( "strconv" "time" + "github.com/cenkalti/backoff/v5" "github.com/go-acme/lego/v4/challenge/dns01" "github.com/go-acme/lego/v4/platform/config/env" "github.com/go-acme/lego/v4/platform/wait" @@ -123,9 +124,9 @@ func (d *DNSProvider) Present(domain, token, keyAuth string) error { return fmt.Errorf("hetzner: add RRSet records: %w", err) } - err = d.waitAction(ctx, "action: add RRSet records", action.ID) + err = d.waitAction(ctx, action.ID) if err != nil { - return fmt.Errorf("hetzner: wait (add): %w", err) + return fmt.Errorf("hetzner: wait (add RRSet records): %w", err) } return nil @@ -164,9 +165,9 @@ func (d *DNSProvider) CleanUp(domain, token, keyAuth string) error { return fmt.Errorf("hetzner: remove RRSet records: %w", err) } - err = d.waitAction(ctx, "action: remove RRSet records", action.ID) + err = d.waitAction(ctx, action.ID) if err != nil { - return fmt.Errorf("hetzner: wait (remove): %w", err) + return fmt.Errorf("hetzner: wait (remove RRSet records): %w", err) } return nil @@ -178,24 +179,26 @@ func (d *DNSProvider) Timeout() (timeout, interval time.Duration) { return d.config.PropagationTimeout, d.config.PollingInterval } -func (d *DNSProvider) waitAction(ctx context.Context, msg string, actionID int) error { - return wait.For(msg, d.config.PropagationTimeout, d.config.PollingInterval, func() (bool, error) { - result, err := d.client.GetAction(ctx, actionID) - if err != nil { - return false, fmt.Errorf("get action %d: %w", actionID, err) - } +func (d *DNSProvider) waitAction(ctx context.Context, actionID int) error { + return wait.Retry(ctx, + func() error { + result, err := d.client.GetAction(ctx, actionID) + if err != nil { + return backoff.Permanent(fmt.Errorf("get action %d: %w", actionID, err)) + } - switch result.Status { - case internal.StatusRunning: - return false, fmt.Errorf("action %d is %s", actionID, internal.StatusRunning) + switch result.Status { + case internal.StatusRunning: + return fmt.Errorf("action %d is %s", actionID, internal.StatusRunning) - case internal.StatusSuccess: - return true, nil + case internal.StatusError: + return fmt.Errorf("action %d: %s: %w", actionID, internal.StatusError, result.ErrorInfo) - case internal.StatusError: - return true, fmt.Errorf("action %d: %s: %w", actionID, internal.StatusError, result.ErrorInfo) - } - - return true, nil - }) + default: + return nil + } + }, + backoff.WithBackOff(backoff.NewConstantBackOff(d.config.PollingInterval)), + backoff.WithMaxElapsedTime(d.config.PropagationTimeout), + ) } diff --git a/providers/dns/hetzner/internal/hetznerv1/hetznerv1_test.go b/providers/dns/hetzner/internal/hetznerv1/hetznerv1_test.go index 597907e08..e43dce068 100644 --- a/providers/dns/hetzner/internal/hetznerv1/hetznerv1_test.go +++ b/providers/dns/hetzner/internal/hetznerv1/hetznerv1_test.go @@ -164,7 +164,7 @@ func TestDNSProvider_Present_error(t *testing.T) { provider.config.PropagationTimeout = 1 * time.Second err := provider.Present("example.com", "", "foobar") - require.EqualError(t, err, "hetzner: wait (add): action 1: error: action_failed: Action failed") + require.EqualError(t, err, "hetzner: wait (add RRSet records): action 1: error: action_failed: Action failed") } func TestDNSProvider_Present_running(t *testing.T) { @@ -180,7 +180,7 @@ func TestDNSProvider_Present_running(t *testing.T) { provider.config.PropagationTimeout = 1 * time.Second err := provider.Present("example.com", "", "foobar") - require.EqualError(t, err, "hetzner: wait (add): action: add RRSet records: time limit exceeded: last error: action 1 is running") + require.EqualError(t, err, "hetzner: wait (add RRSet records): action 1 is running") } func TestDNSProvider_CleanUp(t *testing.T) { @@ -209,7 +209,7 @@ func TestDNSProvider_CleanUp_error(t *testing.T) { provider.config.PropagationTimeout = 1 * time.Second err := provider.CleanUp("example.com", "", "foobar") - require.EqualError(t, err, "hetzner: wait (remove): action 1: error: action_failed: Action failed") + require.EqualError(t, err, "hetzner: wait (remove RRSet records): action 1: error: action_failed: Action failed") } func TestDNSProvider_CleanUp_running(t *testing.T) { @@ -225,5 +225,5 @@ func TestDNSProvider_CleanUp_running(t *testing.T) { provider.config.PropagationTimeout = 1 * time.Second err := provider.CleanUp("example.com", "", "foobar") - require.EqualError(t, err, "hetzner: wait (remove): action: remove RRSet records: time limit exceeded: last error: action 1 is running") + require.EqualError(t, err, "hetzner: wait (remove RRSet records): action 1 is running") } diff --git a/providers/dns/huaweicloud/huaweicloud.go b/providers/dns/huaweicloud/huaweicloud.go index 32f4d3446..430abce74 100644 --- a/providers/dns/huaweicloud/huaweicloud.go +++ b/providers/dns/huaweicloud/huaweicloud.go @@ -2,6 +2,7 @@ package huaweicloud import ( + "context" "errors" "fmt" "strconv" @@ -9,6 +10,7 @@ import ( "sync" "time" + "github.com/cenkalti/backoff/v5" "github.com/go-acme/lego/v4/challenge" "github.com/go-acme/lego/v4/challenge/dns01" "github.com/go-acme/lego/v4/platform/config/env" @@ -148,19 +150,27 @@ func (d *DNSProvider) Present(domain, token, keyAuth string) error { d.recordIDs[token] = recordSetID d.recordIDsMu.Unlock() - err = wait.For("record set sync on "+domain, d.config.PropagationTimeout, d.config.PollingInterval, func() (bool, error) { - rs, errShow := d.client.ShowRecordSet(&hwmodel.ShowRecordSetRequest{ - ZoneId: zoneID, - RecordsetId: recordSetID, - }) - if errShow != nil { - return false, fmt.Errorf("show record set: %w", errShow) - } + err = wait.Retry(context.Background(), + func() error { + rs, errShow := d.client.ShowRecordSet(&hwmodel.ShowRecordSetRequest{ + ZoneId: zoneID, + RecordsetId: recordSetID, + }) + if errShow != nil { + return fmt.Errorf("show record set: %w", errShow) + } - return !strings.HasSuffix(ptr.Deref(rs.Status), "PENDING_"), nil - }) + if !strings.HasSuffix(ptr.Deref(rs.Status), "PENDING_") { + return nil + } + + return fmt.Errorf("status: %s", ptr.Deref(rs.Status)) + }, + backoff.WithBackOff(backoff.NewConstantBackOff(d.config.PollingInterval)), + backoff.WithMaxElapsedTime(d.config.PropagationTimeout), + ) if err != nil { - return fmt.Errorf("huaweicloud: %w", err) + return fmt.Errorf("huaweicloud: record set sync on %s: %w", domain, err) } return nil diff --git a/providers/dns/nifcloud/nifcloud.go b/providers/dns/nifcloud/nifcloud.go index e73333c52..ccdadf615 100644 --- a/providers/dns/nifcloud/nifcloud.go +++ b/providers/dns/nifcloud/nifcloud.go @@ -9,6 +9,7 @@ import ( "net/url" "time" + "github.com/cenkalti/backoff/v5" "github.com/go-acme/lego/v4/challenge" "github.com/go-acme/lego/v4/challenge/dns01" "github.com/go-acme/lego/v4/platform/config/env" @@ -179,11 +180,20 @@ func (d *DNSProvider) changeRecord(action, fqdn, value string, ttl int) error { statusID := resp.ChangeInfo.ID - return wait.For("nifcloud", 120*time.Second, 4*time.Second, func() (bool, error) { - resp, err := d.client.GetChange(ctx, statusID) - if err != nil { - return false, fmt.Errorf("failed to query change status: %w", err) - } - return resp.ChangeInfo.Status == "INSYNC", nil - }) + return wait.Retry(context.Background(), + func() error { + resp, err := d.client.GetChange(ctx, statusID) + if err != nil { + return fmt.Errorf("get change: %w", err) + } + + if resp.ChangeInfo.Status != "INSYNC" { + return fmt.Errorf("change status: %s", resp.ChangeInfo.Status) + } + + return nil + }, + backoff.WithBackOff(backoff.NewConstantBackOff(4*time.Second)), + backoff.WithMaxElapsedTime(120*time.Second), + ) } diff --git a/providers/dns/route53/route53.go b/providers/dns/route53/route53.go index db578eb00..56cb37cb5 100644 --- a/providers/dns/route53/route53.go +++ b/providers/dns/route53/route53.go @@ -17,6 +17,7 @@ import ( "github.com/aws/aws-sdk-go-v2/service/route53" awstypes "github.com/aws/aws-sdk-go-v2/service/route53/types" "github.com/aws/aws-sdk-go-v2/service/sts" + "github.com/cenkalti/backoff/v5" "github.com/go-acme/lego/v4/challenge" "github.com/go-acme/lego/v4/challenge/dns01" "github.com/go-acme/lego/v4/platform/config/env" @@ -249,18 +250,22 @@ func (d *DNSProvider) changeRecord(ctx context.Context, action awstypes.ChangeAc changeID := resp.ChangeInfo.Id if d.config.WaitForRecordSetsChanged { - return wait.For("route53", d.config.PropagationTimeout, d.config.PollingInterval, func() (bool, error) { - resp, err := d.client.GetChange(ctx, &route53.GetChangeInput{Id: changeID}) - if err != nil { - return false, fmt.Errorf("failed to query change status: %w", err) - } + return wait.Retry(context.Background(), + func() error { + resp, err := d.client.GetChange(ctx, &route53.GetChangeInput{Id: changeID}) + if err != nil { + return fmt.Errorf("failed to query change status: %w", err) + } - if resp.ChangeInfo.Status == awstypes.ChangeStatusInsync { - return true, nil - } + if resp.ChangeInfo.Status != awstypes.ChangeStatusInsync { + return fmt.Errorf("unable to retrieve change: ID=%s, status=%s", ptr.Deref(changeID), resp.ChangeInfo.Status) + } - return false, fmt.Errorf("unable to retrieve change: ID=%s", ptr.Deref(changeID)) - }) + return nil + }, + backoff.WithBackOff(backoff.NewConstantBackOff(d.config.PollingInterval)), + backoff.WithMaxElapsedTime(d.config.PropagationTimeout), + ) } return nil diff --git a/providers/dns/variomedia/variomedia.go b/providers/dns/variomedia/variomedia.go index 548d8bab8..f8254335c 100644 --- a/providers/dns/variomedia/variomedia.go +++ b/providers/dns/variomedia/variomedia.go @@ -10,6 +10,7 @@ import ( "sync" "time" + "github.com/cenkalti/backoff/v5" "github.com/go-acme/lego/v4/challenge" "github.com/go-acme/lego/v4/challenge/dns01" "github.com/go-acme/lego/v4/log" @@ -179,14 +180,22 @@ func (d *DNSProvider) CleanUp(domain, token, keyAuth string) error { } func (d *DNSProvider) waitJob(ctx context.Context, domain, id string) error { - return wait.For("variomedia: apply change on "+domain, d.config.PropagationTimeout, d.config.PollingInterval, func() (bool, error) { - result, err := d.client.GetJob(ctx, id) - if err != nil { - return false, err - } + return wait.Retry(context.Background(), + func() error { + result, err := d.client.GetJob(ctx, id) + if err != nil { + return fmt.Errorf("apply change on %s: %w", domain, err) + } - log.Infof("variomedia: [%s] %s: %s %s", domain, result.Data.ID, result.Data.Attributes.JobType, result.Data.Attributes.Status) + log.Infof("variomedia: [%s] %s: %s %s", domain, result.Data.ID, result.Data.Attributes.JobType, result.Data.Attributes.Status) - return result.Data.Attributes.Status == "done", nil - }) + if result.Data.Attributes.Status != "done" { + return fmt.Errorf("apply change on %s: status: %s", domain, result.Data.Attributes.Status) + } + + return nil + }, + backoff.WithBackOff(backoff.NewConstantBackOff(d.config.PollingInterval)), + backoff.WithMaxElapsedTime(d.config.PropagationTimeout), + ) } diff --git a/providers/dns/vinyldns/wrapper.go b/providers/dns/vinyldns/wrapper.go index f17b3de31..84d6e20a6 100644 --- a/providers/dns/vinyldns/wrapper.go +++ b/providers/dns/vinyldns/wrapper.go @@ -1,8 +1,10 @@ package vinyldns import ( + "context" "fmt" + "github.com/cenkalti/backoff/v5" "github.com/go-acme/lego/v4/challenge/dns01" "github.com/go-acme/lego/v4/platform/wait" "github.com/vinyldns/go-vinyldns/vinyldns" @@ -95,20 +97,22 @@ func (d *DNSProvider) deleteRecordSet(existingRecord *vinyldns.RecordSet) error } func (d *DNSProvider) waitForChanges(operation string, resp *vinyldns.RecordSetUpdateResponse) error { - return wait.For("vinyldns", d.config.PropagationTimeout, d.config.PollingInterval, - func() (bool, error) { + return wait.Retry(context.Background(), + func() error { change, err := d.client.RecordSetChange(resp.Zone.ID, resp.RecordSet.ID, resp.ChangeID) if err != nil { - return false, fmt.Errorf("failed to query change status: %w", err) + return fmt.Errorf("failed to query change status: %w", err) } - if change.Status == "Complete" { - return true, nil + if change.Status != "Complete" { + return fmt.Errorf("waiting operation: %s, zoneID: %s, recordsetID: %s, changeID: %s", + operation, resp.Zone.ID, resp.RecordSet.ID, resp.ChangeID) } - return false, fmt.Errorf("waiting operation: %s, zoneID: %s, recordsetID: %s, changeID: %s", - operation, resp.Zone.ID, resp.RecordSet.ID, resp.ChangeID) + return nil }, + backoff.WithBackOff(backoff.NewConstantBackOff(d.config.PollingInterval)), + backoff.WithMaxElapsedTime(d.config.PropagationTimeout), ) }