Skip to content

Commit 7c9c60f

Browse files
authored
Merge pull request #3327 from robnester-rh/EC-1778
fix(EC-1778): use go-gather WithTransport options for HTTP transports
2 parents cec8e51 + e6200bf commit 7c9c60f

4 files changed

Lines changed: 67 additions & 28 deletions

File tree

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@ require (
88
github.com/MakeNowJust/heredoc v1.0.0
99
github.com/Maldris/go-billy-afero v0.0.0-20200815120323-e9d3de59c99a
1010
github.com/conforma/crds/api v0.1.7
11-
github.com/conforma/go-gather v1.1.0
11+
github.com/conforma/go-gather v1.2.0
1212
github.com/docker/docker v28.5.2+incompatible
1313
github.com/evanphx/json-patch v5.9.11+incompatible
1414
github.com/gkampitakis/go-snaps v0.5.19
15-
github.com/go-git/go-git/v5 v5.17.1
15+
github.com/go-git/go-git/v5 v5.18.0
1616
github.com/go-logr/logr v1.4.3
1717
github.com/go-openapi/strfmt v0.26.1
1818
github.com/google/go-cmp v0.7.0

go.sum

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -333,8 +333,8 @@ github.com/conforma/crds/api v0.1.7 h1:IBsIw35A5QkP5i1Se6/BASaFvTnW+xTlWEqJMdX7E
333333
github.com/conforma/crds/api v0.1.7/go.mod h1:vaUCN54r99HME4555KRKOUPGJxBNAlX1agZ4jlGI2YA=
334334
github.com/conforma/go-containerregistry v0.20.7-0.20251103083939-3459088e4bae h1:KoJk+DwW+UReJ47IFxJvEqlc3jOTzsXum5mQ85MEogg=
335335
github.com/conforma/go-containerregistry v0.20.7-0.20251103083939-3459088e4bae/go.mod h1:sdc2GWaYnuP7aY7cx4nTEW16xVJsVuqjZ+zyx5S0XeI=
336-
github.com/conforma/go-gather v1.1.0 h1:j7n9J66gW9d4Imlvx1sBJUC2fmquf/u9jxAJjIu14EY=
337-
github.com/conforma/go-gather v1.1.0/go.mod h1:75GiwN11ojMb6i00hTAMaFhTkaiDf34a93BdWhnC4NM=
336+
github.com/conforma/go-gather v1.2.0 h1:HYmUtkGPQRUOb2mEBQ51FTSO0SpZ//12mNp2AXcVxBs=
337+
github.com/conforma/go-gather v1.2.0/go.mod h1:7LbJh5AAljGbOoUWQy+wBKu7MKecyG45bLbbe7Bo0/A=
338338
github.com/containerd/containerd/v2 v2.2.2 h1:mjVQdtfryzT7lOqs5EYUFZm8ioPVjOpkSoG1GJPxEMY=
339339
github.com/containerd/containerd/v2 v2.2.2/go.mod h1:5Jhevmv6/2J+Iu/A2xXAdUIdI5Ah/hfyO7okJ4AFIdY=
340340
github.com/containerd/errdefs v1.0.0 h1:tg5yIfIlQIrxYtu9ajqY42W3lpS19XqdxRQeEwYG8PI=
@@ -480,8 +480,8 @@ github.com/go-git/go-billy/v5 v5.8.0 h1:I8hjc3LbBlXTtVuFNJuwYuMiHvQJDq1AT6u4DwDz
480480
github.com/go-git/go-billy/v5 v5.8.0/go.mod h1:RpvI/rw4Vr5QA+Z60c6d6LXH0rYJo0uD5SqfmrrheCY=
481481
github.com/go-git/go-git-fixtures/v4 v4.3.2-0.20231010084843-55a94097c399 h1:eMje31YglSBqCdIqdhKBW8lokaMrL3uTkpGYlE2OOT4=
482482
github.com/go-git/go-git-fixtures/v4 v4.3.2-0.20231010084843-55a94097c399/go.mod h1:1OCfN199q1Jm3HZlxleg+Dw/mwps2Wbk9frAWm+4FII=
483-
github.com/go-git/go-git/v5 v5.17.1 h1:WnljyxIzSj9BRRUlnmAU35ohDsjRK0EKmL0evDqi5Jk=
484-
github.com/go-git/go-git/v5 v5.17.1/go.mod h1:pW/VmeqkanRFqR6AljLcs7EA7FbZaN5MQqO7oZADXpo=
483+
github.com/go-git/go-git/v5 v5.18.0 h1:O831KI+0PR51hM2kep6T8k+w0/LIAD490gvqMCvL5hM=
484+
github.com/go-git/go-git/v5 v5.18.0/go.mod h1:pW/VmeqkanRFqR6AljLcs7EA7FbZaN5MQqO7oZADXpo=
485485
github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU=
486486
github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=
487487
github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=

internal/downloader/downloader.go

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package downloader
1919
import (
2020
"context"
2121
"fmt"
22+
net_http "net/http"
2223
"regexp"
2324
"strings"
2425
"sync"
@@ -37,25 +38,53 @@ type key int
3738

3839
const downloadImplKey key = 0
3940

41+
// downloadImpl defines the interface for downloading files.
4042
type downloadImpl interface {
4143
Download(context.Context, string, []string) error
4244
}
4345

4446
var log = logrus.StandardLogger()
4547

48+
// ociGatherer and httpGatherer hold gatherer instances configured with
49+
// tracing and retry transports via WithTransport. Initialized once by
50+
// _initialize via sync.OnceFunc.
51+
var (
52+
ociGatherer *goci.OCIGatherer
53+
httpGatherer *ghttp.HTTPGatherer
54+
)
55+
56+
// gatherFunc dispatches sources to the appropriate gatherer. OCI and HTTP
57+
// scheme prefixes route to custom gatherers; all other sources fall through
58+
// to the go-gather registry.
4659
var gatherFunc = func(ctx context.Context, source, destination string) (metadata.Metadata, error) {
4760
initialize()
61+
62+
// Dispatch to custom gatherers only for unambiguous scheme prefixes.
63+
// Bare hostnames (e.g. quay.io/..., 127.0.0.1/...) fall through to
64+
// the registry, which checks git matchers before OCI matchers.
65+
switch {
66+
case strings.HasPrefix(source, "oci://") || strings.HasPrefix(source, "oci::"):
67+
return ociGatherer.Gather(ctx, source, destination)
68+
case strings.HasPrefix(source, "https://") || strings.HasPrefix(source, "http://"):
69+
if httpGatherer.Matcher(source) {
70+
return httpGatherer.Gather(ctx, source, destination)
71+
}
72+
}
73+
4874
g, err := registry.GetGatherer(source)
4975
if err != nil {
5076
return nil, err
5177
}
5278
return g.Gather(ctx, source, destination)
5379
}
5480

81+
// _initialize builds the transport stack (optional tracing + retry) and
82+
// constructs the OCI and HTTP gatherer instances.
5583
var _initialize = func() {
84+
var base net_http.RoundTripper = net_http.DefaultTransport
85+
5686
if log.IsLevelEnabled(logrus.TraceLevel) {
57-
goci.Transport = http.NewTracingRoundTripperWithLogger(goci.Transport)
58-
ghttp.Transport = http.NewTracingRoundTripperWithLogger(ghttp.Transport)
87+
base = http.NewTracingRoundTripperWithLogger(base)
5988
}
6089

6190
backoff := retry.ExponentialBackoff(http.DefaultBackoff.Duration, http.DefaultBackoff.Factor, http.DefaultBackoff.Jitter)
@@ -69,13 +98,16 @@ var _initialize = func() {
6998
return policy
7099
}
71100

72-
ociTransport := retry.NewTransport(goci.Transport)
101+
ociTransport := retry.NewTransport(base)
73102
ociTransport.Policy = policyfn
74-
goci.Transport = ociTransport
103+
oci := goci.NewOCIGatherer(goci.WithTransport(ociTransport))
75104

76-
httpTransport := retry.NewTransport(ghttp.Transport)
105+
httpTransport := retry.NewTransport(base)
77106
httpTransport.Policy = policyfn
78-
ghttp.Transport = httpTransport
107+
h := ghttp.NewHTTPGatherer(ghttp.WithTransport(httpTransport))
108+
109+
ociGatherer = oci
110+
httpGatherer = h
79111
}
80112

81113
var initialize = sync.OnceFunc(_initialize)

internal/downloader/downloader_test.go

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -159,12 +159,11 @@ func TestOCITracing(t *testing.T) {
159159
t.Cleanup(trace.Stop)
160160

161161
log.Level = logrus.TraceLevel
162-
initialize = _initialize // we want it to re-execute for the test
163162
t.Cleanup(func() {
164163
log.Level = logrus.InfoLevel
165164
initialize = sync.OnceFunc(_initialize)
166-
goci.Transport = http.DefaultTransport
167-
ghttp.Transport = http.DefaultTransport
165+
ociGatherer = nil
166+
httpGatherer = nil
168167
})
169168
registry := httptest.NewServer(registry.New())
170169
t.Cleanup(registry.Close)
@@ -179,7 +178,12 @@ func TestOCITracing(t *testing.T) {
179178
require.NoError(t, err)
180179
require.NoError(t, remote.Push(ref, img))
181180

182-
_, err = gatherFunc(context.Background(), ref.String(), t.TempDir())
181+
// Initialize directly then call ociGatherer.Gather to test the OCI
182+
// tracing path. Bare localhost refs fall through to the registry's
183+
// default gatherer (no tracing), so we invoke the gatherer directly.
184+
_initialize()
185+
186+
_, err = ociGatherer.Gather(context.Background(), ref.String(), t.TempDir())
183187
require.NoError(t, err)
184188

185189
trace.Stop()
@@ -215,12 +219,12 @@ func TestHTTPTracing(t *testing.T) {
215219
t.Cleanup(trace.Stop)
216220

217221
log.Level = logrus.TraceLevel
218-
initialize = _initialize // we want it to re-execute for the test
222+
initialize = _initialize
219223
t.Cleanup(func() {
220224
log.Level = logrus.InfoLevel
221225
initialize = sync.OnceFunc(_initialize)
222-
goci.Transport = http.DefaultTransport
223-
ghttp.Transport = http.DefaultTransport
226+
ociGatherer = nil
227+
httpGatherer = nil
224228
})
225229
var requests []string
226230
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -261,32 +265,35 @@ func TestHTTPTracing(t *testing.T) {
261265
assert.Equal(t, 3, found)
262266
}
263267

268+
// TestOCIClientConfiguration verifies _initialize constructs an OCI gatherer.
269+
// OCIGatherer.transport is unexported, so we cannot verify the retry transport
270+
// directly (unlike TestHTTPClientConfiguration). TestOCITracing provides
271+
// integration-level verification that the transport stack is wired correctly.
264272
func TestOCIClientConfiguration(t *testing.T) {
265-
defaultMaxRetry := echttp.DefaultRetry.MaxRetry
266273
t.Cleanup(func() {
267-
echttp.DefaultRetry.MaxRetry = defaultMaxRetry
274+
ociGatherer = nil
275+
httpGatherer = nil
268276
})
269-
echttp.DefaultRetry.MaxRetry = rand.Int() //nolint:gosec // G404 - no need for a secure random here
270277

271278
_initialize()
272279

273-
assert.IsType(t, &retry.Transport{}, goci.Transport)
274-
275-
transport := goci.Transport.(*retry.Transport)
276-
assert.Equal(t, echttp.DefaultRetry.MaxRetry, transport.Policy().(*retry.GenericPolicy).MaxRetry)
280+
assert.IsType(t, &goci.OCIGatherer{}, ociGatherer)
277281
}
278282

279283
func TestHTTPClientConfiguration(t *testing.T) {
280284
defaultMaxRetry := echttp.DefaultRetry.MaxRetry
281285
t.Cleanup(func() {
282286
echttp.DefaultRetry.MaxRetry = defaultMaxRetry
287+
ociGatherer = nil
288+
httpGatherer = nil
283289
})
284290
echttp.DefaultRetry.MaxRetry = rand.Int() //nolint:gosec // G404 - no need for a secure random here
285291

286292
_initialize()
287293

288-
assert.IsType(t, &retry.Transport{}, ghttp.Transport)
294+
require.IsType(t, &ghttp.HTTPGatherer{}, httpGatherer)
295+
assert.IsType(t, &retry.Transport{}, httpGatherer.Client.Transport)
289296

290-
transport := ghttp.Transport.(*retry.Transport)
297+
transport := httpGatherer.Client.Transport.(*retry.Transport)
291298
assert.Equal(t, echttp.DefaultRetry.MaxRetry, transport.Policy().(*retry.GenericPolicy).MaxRetry)
292299
}

0 commit comments

Comments
 (0)