Add Retry-After header value to get renewal info result

[mocheryl]

Welcome

• [X] Yes, I've searched similar issues on GitHub and didn't find any.

How do you use lego?

Library

Detailed Description

The draft states that the server should return information in HTTP Retry-After header on when to make the next renewal inquiry. I've checked on Let's Encrypt's staging environment and saw that they indeed provide this information however the Lego library doesn't account for that. Could you include this duration in the call so I can use it to better decide on the pooling interval? I've made this diff to get the process going. Would you rather I make a PR instead?

diff --git a/certificate/renewal.go b/certificate/renewal.go
index 4f420301..fe759c1d 100644
--- a/certificate/renewal.go
+++ b/certificate/renewal.go
@@ -21,6 +21,10 @@ type RenewalInfoRequest struct {
 // RenewalInfoResponse is a wrapper around acme.RenewalInfoResponse that provides a method for determining when to renew a certificate.
 type RenewalInfoResponse struct {
    acme.RenewalInfoResponse
+
+   // RetryAfter header indicating the polling interval that the ACME server recommends.
+   // Conforming clients SHOULD query the renewalInfo URL again after the RetryAfter period has passed, as the server may provide a different suggestedWindow.
+   RetryAfter time.Duration
 }

 // ShouldRenewAt determines the optimal renewal time based on the current time (UTC),renewal window suggest by ARI, and the client's willingness to sleep.
@@ -81,6 +85,14 @@ func (c *Certifier) GetRenewalInfo(req RenewalInfoRequest) (*RenewalInfoResponse
    if err != nil {
        return nil, err
    }
+
+   if retry := resp.Header.Get(`Retry-After`); retry != `` {
+       info.RetryAfter, err = time.ParseDuration(retry + `s`)
+       if err != nil {
+           return nil, err
+       }
+   }
+
    return &info, nil
 }

diff --git a/certificate/renewal_test.go b/certificate/renewal_test.go
index e883a40c..dfd36a7d 100644
--- a/certificate/renewal_test.go
+++ b/certificate/renewal_test.go
@@ -50,6 +50,7 @@ func TestCertifier_GetRenewalInfo(t *testing.T) {
        }

        w.Header().Set("Content-Type", "application/json")
+       w.Header().Set("Retry-After", "21600")
        w.WriteHeader(http.StatusOK)
        _, wErr := w.Write([]byte(`{
                "suggestedWindow": {
@@ -76,6 +77,7 @@ func TestCertifier_GetRenewalInfo(t *testing.T) {
    assert.Equal(t, "2020-03-17T17:51:09Z", ri.SuggestedWindow.Start.Format(time.RFC3339))
    assert.Equal(t, "2020-03-17T18:21:09Z", ri.SuggestedWindow.End.Format(time.RFC3339))
    assert.Equal(t, "https://aricapable.ca/docs/renewal-advice/", ri.ExplanationURL)
+   assert.Equal(t, time.Duration(21600000000000), ri.RetryAfter)
 }

 func TestCertifier_GetRenewalInfo_errors(t *testing.T) {
@@ -142,6 +144,7 @@ func TestRenewalInfoResponse_ShouldRenew(t *testing.T) {
                },
                ExplanationURL: "",
            },
+           0,
        }

        rt := ri.ShouldRenewAt(now, 0)
@@ -158,6 +161,7 @@ func TestRenewalInfoResponse_ShouldRenew(t *testing.T) {
                },
                ExplanationURL: "",
            },
+           0,
        }

        rt := ri.ShouldRenewAt(now, 0)
@@ -173,6 +177,7 @@ func TestRenewalInfoResponse_ShouldRenew(t *testing.T) {
                },
                ExplanationURL: "",
            },
+           0,
        }

        rt := ri.ShouldRenewAt(now, 2*time.Hour)
@@ -189,6 +194,7 @@ func TestRenewalInfoResponse_ShouldRenew(t *testing.T) {
                },
                ExplanationURL: "",
            },
+           0,
        }

        rt := ri.ShouldRenewAt(now, 59*time.Minute)

[ldez]

Hello,

The only constraint for a client is to follow suggestedWindow (MUST). The support Retry-After is recommended (SHOULD) but not required.

Your patch gets the Retry-After information but doesn't use it.

You can open a PR, you will have to add the missing elements.

Note: please use " for string, and explicit field name inside tests.

[mocheryl]

Maybe I wasn't clear enough. By "the Lego library doesn't account for that" I meant that the library part of the project (as opposed to the command line tool), specifically the GetRenewalInfo method, ignores the header value. This diff fixes that. It is up to whoever implements the library to decide if and how to use the newly provided value. In my case, I can use it in my custom-made service to set the next time when to rerun the renewal check process. If I'm understood now and we agree on the issue, can I begin with a PR?

[ldez]

ok, let's go for a PR.