Skip to content

Commit e9e0d48

Browse files
malancasmalancas
authored
Allow users to configure retry behavior in DefaultFetcher (#684)
* allow users to set request timeout as struct field Signed-off-by: Meredith Lancaster <malancas@github.com> * add optional request retry and backoff Signed-off-by: Meredith Lancaster <malancas@github.com> * add retry setters Signed-off-by: Meredith Lancaster <malancas@github.com> * use v5 of backoff library Signed-off-by: Meredith Lancaster <malancas@github.com> * add more flexible retry option setters Signed-off-by: Meredith Lancaster <malancas@github.com> * move request setup out of retry operation func Signed-off-by: Meredith Lancaster <malancas@github.com> * add tests for retry config Signed-off-by: Meredith Lancaster <malancas@github.com> * comments Signed-off-by: Meredith Lancaster <malancas@github.com> --------- Signed-off-by: Meredith Lancaster <malancas@github.com>
1 parent 0239656 commit e9e0d48

File tree

5 files changed

+164
-33
lines changed

5 files changed

+164
-33
lines changed

‎go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ module github.com/theupdateframework/go-tuf/v2
33
go 1.23.0
44

55
require (
6+
github.com/cenkalti/backoff/v5 v5.0.2
67
github.com/go-logr/stdr v1.2.2
78
github.com/secure-systems-lab/go-securesystemslib v0.9.0
89
github.com/sigstore/sigstore v1.8.4

‎go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
22
github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw=
3+
github.com/cenkalti/backoff/v5 v5.0.2 h1:rIfFVxEf1QsI7E1ZHfp/B4DF/6QBAUhmgkxc0H7Zss8=
4+
github.com/cenkalti/backoff/v5 v5.0.2/go.mod h1:rkhZdG3JZukswDf7f0cwqPNk4K0sa+F97BxZthm/crw=
35
github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44=
46
github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
57
github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=

‎metadata/config/config.go

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ import (
2222
"net/http"
2323
"net/url"
2424
"os"
25+
"time"
2526

27+
"github.com/cenkalti/backoff/v5"
2628
"github.com/theupdateframework/go-tuf/v2/metadata/fetcher"
2729
)
2830

@@ -92,7 +94,7 @@ func (cfg *UpdaterConfig) EnsurePathsExist() error {
9294

9395
func (cfg *UpdaterConfig) SetDefaultFetcherHTTPClient(client *http.Client) error {
9496
// Check if the configured fetcher is the default fetcher
95-
// since we are only configuring a timeout value for the default fetcher
97+
// since we are only configuring a http.Client value for the default fetcher
9698
df, ok := cfg.Fetcher.(*fetcher.DefaultFetcher)
9799
if !ok {
98100
return fmt.Errorf("fetcher is not type fetcher.DefaultFetcher")
@@ -104,7 +106,7 @@ func (cfg *UpdaterConfig) SetDefaultFetcherHTTPClient(client *http.Client) error
104106

105107
func (cfg *UpdaterConfig) SetDefaultFetcherTransport(rt http.RoundTripper) error {
106108
// Check if the configured fetcher is the default fetcher
107-
// since we are only configuring a timeout value for the default fetcher
109+
// since we are only configuring a Transport value for the default fetcher
108110
df, ok := cfg.Fetcher.(*fetcher.DefaultFetcher)
109111
if !ok {
110112
return fmt.Errorf("fetcher is not type fetcher.DefaultFetcher")
@@ -115,3 +117,28 @@ func (cfg *UpdaterConfig) SetDefaultFetcherTransport(rt http.RoundTripper) error
115117
cfg.Fetcher = df
116118
return nil
117119
}
120+
121+
// SetDefaultFetcherRetry sets the constant retry interval and count for the default fetcher
122+
func (cfg *UpdaterConfig) SetDefaultFetcherRetry(retryInterval time.Duration, retryCount uint) error {
123+
// Check if the configured fetcher is the default fetcher
124+
// since we are only configuring the retry interval and count for the default fetcher
125+
df, ok := cfg.Fetcher.(*fetcher.DefaultFetcher)
126+
if !ok {
127+
return fmt.Errorf("fetcher is not type fetcher.DefaultFetcher")
128+
}
129+
df.SetRetry(retryInterval, retryCount)
130+
cfg.Fetcher = df
131+
return nil
132+
}
133+
134+
func (cfg *UpdaterConfig) SetRetryOptions(retryOptions ...backoff.RetryOption) error {
135+
// Check if the configured fetcher is the default fetcher
136+
// since we are only configuring retry options for the default fetcher
137+
df, ok := cfg.Fetcher.(*fetcher.DefaultFetcher)
138+
if !ok {
139+
return fmt.Errorf("fetcher is not type fetcher.DefaultFetcher")
140+
}
141+
df.SetRetryOptions(retryOptions...)
142+
cfg.Fetcher = df
143+
return nil
144+
}

‎metadata/fetcher/fetcher.go

Lines changed: 51 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,14 @@
1818
package fetcher
1919

2020
import (
21+
"context"
2122
"fmt"
2223
"io"
2324
"net/http"
2425
"strconv"
26+
"time"
2527

28+
"github.com/cenkalti/backoff/v5"
2629
"github.com/theupdateframework/go-tuf/v2/metadata"
2730
)
2831

@@ -39,11 +42,11 @@ type Fetcher interface {
3942

4043
// DefaultFetcher implements Fetcher
4144
type DefaultFetcher struct {
45+
// httpClient configuration
4246
httpUserAgent string
4347
client httpClient
44-
// for implementation of retry logic in a future pull request
45-
// retryInterval time.Duration
46-
// retryCount int
48+
// retry logic configuration
49+
retryOptions []backoff.RetryOption
4750
}
4851

4952
func (d *DefaultFetcher) SetHTTPUserAgent(httpUserAgent string) {
@@ -61,48 +64,59 @@ func (d *DefaultFetcher) DownloadFile(urlPath string, maxLength int64) ([]byte,
6164
if d.httpUserAgent != "" {
6265
req.Header.Set("User-Agent", d.httpUserAgent)
6366
}
64-
// Execute the request.
65-
res, err := d.client.Do(req)
66-
if err != nil {
67-
return nil, err
68-
}
69-
defer res.Body.Close()
70-
// Handle HTTP status codes.
71-
if res.StatusCode != http.StatusOK {
72-
return nil, &metadata.ErrDownloadHTTP{StatusCode: res.StatusCode, URL: urlPath}
73-
}
74-
var length int64
75-
// Get content length from header (might not be accurate, -1 or not set).
76-
if header := res.Header.Get("Content-Length"); header != "" {
77-
length, err = strconv.ParseInt(header, 10, 0)
67+
68+
operation := func() ([]byte, error) {
69+
// Execute the request.
70+
res, err := d.client.Do(req)
71+
if err != nil {
72+
return nil, err
73+
}
74+
defer res.Body.Close()
75+
// Handle HTTP status codes.
76+
if res.StatusCode != http.StatusOK {
77+
return nil, &metadata.ErrDownloadHTTP{StatusCode: res.StatusCode, URL: urlPath}
78+
}
79+
var length int64
80+
// Get content length from header (might not be accurate, -1 or not set).
81+
if header := res.Header.Get("Content-Length"); header != "" {
82+
length, err = strconv.ParseInt(header, 10, 0)
83+
if err != nil {
84+
return nil, err
85+
}
86+
// Error if the reported size is greater than what is expected.
87+
if length > maxLength {
88+
return nil, &metadata.ErrDownloadLengthMismatch{Msg: fmt.Sprintf("download failed for %s, length %d is larger than expected %d", urlPath, length, maxLength)}
89+
}
90+
}
91+
// Although the size has been checked above, use a LimitReader in case
92+
// the reported size is inaccurate, or size is -1 which indicates an
93+
// unknown length. We read maxLength + 1 in order to check if the read data
94+
// surpassed our set limit.
95+
data, err := io.ReadAll(io.LimitReader(res.Body, maxLength+1))
7896
if err != nil {
7997
return nil, err
8098
}
8199
// Error if the reported size is greater than what is expected.
100+
length = int64(len(data))
82101
if length > maxLength {
83102
return nil, &metadata.ErrDownloadLengthMismatch{Msg: fmt.Sprintf("download failed for %s, length %d is larger than expected %d", urlPath, length, maxLength)}
84103
}
104+
105+
return data, nil
85106
}
86-
// Although the size has been checked above, use a LimitReader in case
87-
// the reported size is inaccurate, or size is -1 which indicates an
88-
// unknown length. We read maxLength + 1 in order to check if the read data
89-
// surpassed our set limit.
90-
data, err := io.ReadAll(io.LimitReader(res.Body, maxLength+1))
107+
data, err := backoff.Retry(context.Background(), operation, d.retryOptions...)
91108
if err != nil {
92109
return nil, err
93110
}
94-
// Error if the reported size is greater than what is expected.
95-
length = int64(len(data))
96-
if length > maxLength {
97-
return nil, &metadata.ErrDownloadLengthMismatch{Msg: fmt.Sprintf("download failed for %s, length %d is larger than expected %d", urlPath, length, maxLength)}
98-
}
99111

100112
return data, nil
101113
}
102114

103115
func NewDefaultFetcher() *DefaultFetcher {
104116
return &DefaultFetcher{
105117
client: http.DefaultClient,
118+
// default to attempting the HTTP request once
119+
retryOptions: []backoff.RetryOption{backoff.WithMaxTries(1)},
106120
}
107121
}
108122

@@ -136,3 +150,13 @@ func (f *DefaultFetcher) SetTransport(rt http.RoundTripper) error {
136150
f.client = hc
137151
return nil
138152
}
153+
154+
func (f *DefaultFetcher) SetRetry(retryInterval time.Duration, retryCount uint) {
155+
constantBackOff := backoff.WithBackOff(backoff.NewConstantBackOff(retryInterval))
156+
maxTryCount := backoff.WithMaxTries(retryCount)
157+
f.SetRetryOptions(constantBackOff, maxTryCount)
158+
}
159+
160+
func (f *DefaultFetcher) SetRetryOptions(retryOptions ...backoff.RetryOption) {
161+
f.retryOptions = retryOptions
162+
}

0 commit comments

Comments
 (0)