Skip to content

Commit 3f2b1b5

Browse files
markusthoemmesmarkusthoemmes
authored
Replace SetClient usage with transport configuration (#2006)
Instead of having a `SetClient` function that can be called whenever in the lifecycle of an apko build, this uses the `WithTransport` option consistently and removes `SetClient`.
1 parent 6e25ab2 commit 3f2b1b5

File tree

3 files changed

+49
-105
lines changed

3 files changed

+49
-105
lines changed

‎pkg/apk/apk/implementation.go‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -183,14 +183,6 @@ var initDeviceFiles = []deviceFile{
183183
{"/dev/console", 5, 1, 0o620},
184184
}
185185

186-
// SetClient set the http client to use for downloading packages.
187-
// In general, you can leave this unset, and it will use the default http.Client.
188-
// It is useful for fine-grained control, for proxying, or for setting alternate
189-
// paths.
190-
func (a *APK) SetClient(client *http.Client) {
191-
a.client = client
192-
}
193-
194186
// ListInitFiles list the files that are installed during the InitDB phase.
195187
func (a *APK) ListInitFiles() []tar.Header {
196188
headers := make([]tar.Header, 0, 20)

‎pkg/apk/apk/implementation_test.go‎

Lines changed: 18 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,8 @@ func TestSetRepositories_Empty(t *testing.T) {
490490

491491
func TestInitKeyring(t *testing.T) {
492492
src := apkfs.NewMemFS()
493-
a, err := New(t.Context(), WithFS(src), WithIgnoreMknodErrors(ignoreMknodErrors))
493+
tr := &testLocalTransport{root: testPrimaryPkgDir, basenameOnly: true}
494+
a, err := New(t.Context(), WithFS(src), WithIgnoreMknodErrors(ignoreMknodErrors), WithTransport(tr))
494495
require.NoError(t, err)
495496

496497
dir, err := os.MkdirTemp("", "go-apk")
@@ -504,10 +505,6 @@ func TestInitKeyring(t *testing.T) {
504505
keyfiles := []string{
505506
keyPath, "https://alpinelinux.org/keys/alpine-devel%40lists.alpinelinux.org-4a6a0840.rsa.pub",
506507
}
507-
// ensure we send things from local
508-
a.SetClient(&http.Client{
509-
Transport: &testLocalTransport{root: testPrimaryPkgDir, basenameOnly: true},
510-
})
511508

512509
require.NoError(t, a.InitKeyring(context.Background(), keyfiles, nil))
513510
// InitKeyring should have copied the local key and remote key to the right place
@@ -533,9 +530,7 @@ func TestInitKeyring(t *testing.T) {
533530
keyfiles = []string{
534531
"https://user:pass@alpinelinux.org/keys/alpine-devel%40lists.alpinelinux.org-4a6a0840.rsa.pub",
535532
}
536-
a.SetClient(&http.Client{
537-
Transport: &testLocalTransport{root: testPrimaryPkgDir, basenameOnly: true, requireBasicAuth: true},
538-
})
533+
tr.requireBasicAuth = true
539534
require.NoError(t, a.InitKeyring(context.Background(), keyfiles, nil))
540535

541536
t.Run("auth", func(t *testing.T) {
@@ -675,12 +670,12 @@ func TestFetchPackage(t *testing.T) {
675670
pkg = NewRepositoryPackage(&testPkg, repoWithIndex)
676671
ctx = context.Background()
677672
)
678-
prepLayout := func(t *testing.T, cache string) *APK {
673+
prepLayout := func(t *testing.T, tr http.RoundTripper, cache string) *APK {
679674
src := apkfs.NewMemFS()
680675
err := src.MkdirAll("usr/lib/apk/db", 0o755)
681676
require.NoError(t, err, "unable to mkdir /usr/lib/apk/db")
682677

683-
opts := []Option{WithFS(src), WithIgnoreMknodErrors(ignoreMknodErrors)}
678+
opts := []Option{WithFS(src), WithIgnoreMknodErrors(ignoreMknodErrors), WithTransport(tr)}
684679
if cache != "" {
685680
opts = append(opts, WithCache(cache, false, NewCache(false)))
686681
}
@@ -693,27 +688,21 @@ func TestFetchPackage(t *testing.T) {
693688
return a
694689
}
695690
t.Run("no cache", func(t *testing.T) {
696-
a := prepLayout(t, "")
697-
a.SetClient(&http.Client{
698-
Transport: &testLocalTransport{root: testPrimaryPkgDir, basenameOnly: true},
699-
})
691+
a := prepLayout(t, &testLocalTransport{root: testPrimaryPkgDir, basenameOnly: true}, "")
700692
_, err := a.FetchPackage(ctx, pkg)
701693
require.NoErrorf(t, err, "unable to install package")
702694
})
703695
t.Run("cache miss no network", func(t *testing.T) {
704696
// we use a transport that always returns a 404 so we know we're not hitting the network
705697
// it should fail for a cache hit
706698
tmpDir := t.TempDir()
707-
a := prepLayout(t, tmpDir)
708-
a.SetClient(&http.Client{
709-
Transport: &testLocalTransport{fail: true},
710-
})
699+
a := prepLayout(t, &testLocalTransport{fail: true}, tmpDir)
711700
_, err := a.FetchPackage(ctx, pkg)
712701
require.Error(t, err, "should fail when no cache and no network")
713702
})
714703
t.Run("cache miss network should fill cache", func(t *testing.T) {
715704
tmpDir := t.TempDir()
716-
a := prepLayout(t, tmpDir)
705+
a := prepLayout(t, &testLocalTransport{root: testPrimaryPkgDir, basenameOnly: true}, tmpDir)
717706
// fill the cache
718707
repoDir := filepath.Join(tmpDir, url.QueryEscape(testAlpineRepos), testArch)
719708
err := os.MkdirAll(repoDir, 0o755)
@@ -722,10 +711,6 @@ func TestFetchPackage(t *testing.T) {
722711
cacheApkFile := filepath.Join(repoDir, testPkgFilename)
723712
cacheApkDir := strings.TrimSuffix(cacheApkFile, ".apk")
724713

725-
a.SetClient(&http.Client{
726-
Transport: &testLocalTransport{root: testPrimaryPkgDir, basenameOnly: true},
727-
})
728-
729714
_, err = a.expandPackage(ctx, pkg)
730715
require.NoErrorf(t, err, "unable to install pkg")
731716
// check that the package file is in place
@@ -755,7 +740,7 @@ func TestFetchPackage(t *testing.T) {
755740
})
756741
t.Run("handle missing cache files when expanding APK", func(t *testing.T) {
757742
tmpDir := t.TempDir()
758-
a := prepLayout(t, tmpDir)
743+
a := prepLayout(t, http.DefaultTransport, tmpDir)
759744

760745
// Fill the cache
761746
exp, err := a.expandPackage(ctx, pkg)
@@ -793,7 +778,9 @@ func TestFetchPackage(t *testing.T) {
793778
})
794779
t.Run("cache hit no etag", func(t *testing.T) {
795780
tmpDir := t.TempDir()
796-
a := prepLayout(t, tmpDir)
781+
a := prepLayout(t,
782+
&testLocalTransport{root: testAlternatePkgDir, basenameOnly: true, headers: map[string][]string{http.CanonicalHeaderKey("etag"): {testEtag}}},
783+
tmpDir)
797784
// fill the cache
798785
repoDir := filepath.Join(tmpDir, url.QueryEscape(testAlpineRepos), testArch)
799786
err := os.MkdirAll(repoDir, 0o755)
@@ -805,10 +792,6 @@ func TestFetchPackage(t *testing.T) {
805792
err = os.WriteFile(cacheApkFile, contents, 0o644) //nolint:gosec // we're writing a test file
806793
require.NoError(t, err, "unable to write cache apk file")
807794

808-
a.SetClient(&http.Client{
809-
// use a different root, so we get a different file
810-
Transport: &testLocalTransport{root: testAlternatePkgDir, basenameOnly: true, headers: map[string][]string{http.CanonicalHeaderKey("etag"): {testEtag}}},
811-
})
812795
_, err = a.FetchPackage(ctx, pkg)
813796
require.NoErrorf(t, err, "unable to install pkg")
814797
// check that the package file is in place
@@ -821,7 +804,9 @@ func TestFetchPackage(t *testing.T) {
821804
})
822805
t.Run("cache hit etag match", func(t *testing.T) {
823806
tmpDir := t.TempDir()
824-
a := prepLayout(t, tmpDir)
807+
a := prepLayout(t,
808+
&testLocalTransport{root: testAlternatePkgDir, basenameOnly: true, headers: map[string][]string{http.CanonicalHeaderKey("etag"): {testEtag}}},
809+
tmpDir)
825810
// fill the cache
826811
repoDir := filepath.Join(tmpDir, url.QueryEscape(testAlpineRepos), testArch)
827812
err := os.MkdirAll(repoDir, 0o755)
@@ -835,10 +820,6 @@ func TestFetchPackage(t *testing.T) {
835820
err = os.WriteFile(cacheApkFile+".etag", []byte(testEtag), 0o644) //nolint:gosec // we're writing a test file
836821
require.NoError(t, err, "unable to write etag")
837822

838-
a.SetClient(&http.Client{
839-
// use a different root, so we get a different file
840-
Transport: &testLocalTransport{root: testAlternatePkgDir, basenameOnly: true, headers: map[string][]string{http.CanonicalHeaderKey("etag"): {testEtag}}},
841-
})
842823
_, err = a.FetchPackage(ctx, pkg)
843824
require.NoErrorf(t, err, "unable to install pkg")
844825
// check that the package file is in place
@@ -851,7 +832,9 @@ func TestFetchPackage(t *testing.T) {
851832
})
852833
t.Run("cache hit etag miss", func(t *testing.T) {
853834
tmpDir := t.TempDir()
854-
a := prepLayout(t, tmpDir)
835+
a := prepLayout(t,
836+
&testLocalTransport{root: testAlternatePkgDir, basenameOnly: true, headers: map[string][]string{http.CanonicalHeaderKey("etag"): {testEtag + "abcdefg"}}},
837+
tmpDir)
855838
// fill the cache
856839
repoDir := filepath.Join(tmpDir, url.QueryEscape(testAlpineRepos), testArch)
857840
err := os.MkdirAll(repoDir, 0o755)
@@ -865,10 +848,6 @@ func TestFetchPackage(t *testing.T) {
865848
err = os.WriteFile(cacheApkFile+".etag", []byte(testEtag), 0o644) //nolint:gosec // we're writing a test file
866849
require.NoError(t, err, "unable to write etag")
867850

868-
a.SetClient(&http.Client{
869-
// use a different root, so we get a different file
870-
Transport: &testLocalTransport{root: testAlternatePkgDir, basenameOnly: true, headers: map[string][]string{http.CanonicalHeaderKey("etag"): {testEtag + "abcdefg"}}},
871-
})
872851
_, err = a.FetchPackage(ctx, pkg)
873852
require.NoErrorf(t, err, "unable to install pkg")
874853
// check that the package file is in place

‎pkg/apk/apk/repo_test.go‎

Lines changed: 31 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ guyM+Ks3c29KlRf3iX35Gt0CAwEAAQ==
9292
)
9393

9494
func TestGetRepositoryIndexes(t *testing.T) {
95-
prepLayout := func(t *testing.T, cache string, repos []string) *APK {
95+
prepLayout := func(t *testing.T, tr http.RoundTripper, cache string, repos []string) *APK {
9696
src := apkfs.NewMemFS()
9797
err := src.MkdirAll("etc/apk", 0o755)
9898
require.NoError(t, err, "unable to mkdir /etc/apk")
@@ -113,7 +113,7 @@ func TestGetRepositoryIndexes(t *testing.T) {
113113
require.NoErrorf(t, err, "unable to write repositories")
114114
}
115115

116-
opts := []Option{WithFS(src), WithIgnoreMknodErrors(ignoreMknodErrors)}
116+
opts := []Option{WithFS(src), WithIgnoreMknodErrors(ignoreMknodErrors), WithTransport(tr)}
117117
if cache != "" {
118118
opts = append(opts, WithCache(cache, false, NewCache(false)))
119119
}
@@ -124,19 +124,13 @@ func TestGetRepositoryIndexes(t *testing.T) {
124124
return a
125125
}
126126
t.Run("no cache", func(t *testing.T) {
127-
a := prepLayout(t, "", nil)
128-
a.SetClient(&http.Client{
129-
Transport: &testLocalTransport{root: testPrimaryPkgDir, basenameOnly: true},
130-
})
127+
a := prepLayout(t, &testLocalTransport{root: testPrimaryPkgDir, basenameOnly: true}, "", nil)
131128
indexes, err := a.GetRepositoryIndexes(context.Background(), false)
132129
require.NoErrorf(t, err, "unable to get indexes")
133130
require.Greater(t, len(indexes), 0, "no indexes found")
134131
})
135132
t.Run("RSA256 signed", func(t *testing.T) {
136-
a := prepLayout(t, "", nil)
137-
a.SetClient(&http.Client{
138-
Transport: &testLocalTransport{root: testRSA256IndexPkgDir, basenameOnly: true},
139-
})
133+
a := prepLayout(t, &testLocalTransport{root: testRSA256IndexPkgDir, basenameOnly: true}, "", nil)
140134
indexes, err := a.GetRepositoryIndexes(context.Background(), false)
141135
require.NoErrorf(t, err, "unable to get indexes")
142136
require.Greater(t, len(indexes), 0, "no indexes found")
@@ -145,22 +139,16 @@ func TestGetRepositoryIndexes(t *testing.T) {
145139
// we use a transport that always returns a 404 so we know we're not hitting the network
146140
// it should fail for a cache hit
147141
tmpDir := t.TempDir()
148-
a := prepLayout(t, tmpDir, nil)
149-
a.SetClient(&http.Client{
150-
Transport: &testLocalTransport{fail: true},
151-
})
142+
a := prepLayout(t, &testLocalTransport{fail: true}, tmpDir, nil)
152143
_, err := a.GetRepositoryIndexes(context.Background(), false)
153144
require.Error(t, err, "should fail when no cache and no network")
154145
})
155146
t.Run("we can fetch, but do not cache indices without etag", func(t *testing.T) {
156147
// we use a transport that can read from the network
157148
// it should fail for a cache hit
158149
tmpDir := t.TempDir()
159-
a := prepLayout(t, tmpDir, nil)
150+
a := prepLayout(t, &testLocalTransport{root: testPrimaryPkgDir, basenameOnly: true}, tmpDir, nil)
160151

161-
a.SetClient(&http.Client{
162-
Transport: &testLocalTransport{root: testPrimaryPkgDir, basenameOnly: true},
163-
})
164152
indexes, err := a.GetRepositoryIndexes(context.Background(), false)
165153
require.NoErrorf(t, err, "unable to get indexes")
166154
require.Greater(t, len(indexes), 0, "no indexes found")
@@ -176,19 +164,16 @@ func TestGetRepositoryIndexes(t *testing.T) {
176164
// we use a transport that can read from the network
177165
// it should fail for a cache hit
178166
tmpDir := t.TempDir()
179-
a := prepLayout(t, tmpDir, []string{testAlpineRepos})
167+
a := prepLayout(t, &testLocalTransport{
168+
root: testPrimaryPkgDir,
169+
basenameOnly: true,
170+
headers: map[string][]string{
171+
http.CanonicalHeaderKey("etag"): {"an-etag"},
172+
},
173+
}, tmpDir, []string{testAlpineRepos})
180174
// fill the cache
181175
repoDir := filepath.Join(tmpDir, url.QueryEscape(testAlpineRepos), testArch)
182176

183-
a.SetClient(&http.Client{
184-
Transport: &testLocalTransport{
185-
root: testPrimaryPkgDir,
186-
basenameOnly: true,
187-
headers: map[string][]string{
188-
http.CanonicalHeaderKey("etag"): {"an-etag"},
189-
},
190-
},
191-
})
192177
indexes, err := a.GetRepositoryIndexes(context.Background(), false)
193178
require.NoErrorf(t, err, "unable to get indexes")
194179
require.Greater(t, len(indexes), 0, "no indexes found")
@@ -201,15 +186,12 @@ func TestGetRepositoryIndexes(t *testing.T) {
201186
})
202187
t.Run("repo url with http basic auth", func(t *testing.T) {
203188
tmpDir := t.TempDir()
204-
a := prepLayout(t, tmpDir, []string{"https://user:pass@dl-cdn.alpinelinux.org/alpine/v3.16/main"})
189+
a := prepLayout(t, &testLocalTransport{
190+
root: testPrimaryPkgDir,
191+
basenameOnly: true,
192+
requireBasicAuth: true,
193+
}, tmpDir, []string{"https://user:pass@dl-cdn.alpinelinux.org/alpine/v3.16/main"})
205194

206-
a.SetClient(&http.Client{
207-
Transport: &testLocalTransport{
208-
root: testPrimaryPkgDir,
209-
basenameOnly: true,
210-
requireBasicAuth: true,
211-
},
212-
})
213195
ctx := context.Background()
214196
indexes, err := a.GetRepositoryIndexes(ctx, false)
215197
require.NoErrorf(t, err, "unable to get indexes")
@@ -219,13 +201,11 @@ func TestGetRepositoryIndexes(t *testing.T) {
219201
// it should succeed for a cache hit
220202
tmpDir := t.TempDir()
221203
testEtag := "test-etag"
204+
tr := &testLocalTransport{root: testPrimaryPkgDir, basenameOnly: true, headers: map[string][]string{http.CanonicalHeaderKey("etag"): {testEtag}}}
222205

223206
// get our APK struct
224-
a := prepLayout(t, tmpDir, nil)
207+
a := prepLayout(t, tr, tmpDir, nil)
225208

226-
a.SetClient(&http.Client{
227-
Transport: &testLocalTransport{root: testPrimaryPkgDir, basenameOnly: true, headers: map[string][]string{http.CanonicalHeaderKey("etag"): {testEtag}}},
228-
})
229209
// Use the client to fill the cache.
230210
indexes, err := a.GetRepositoryIndexes(context.Background(), false)
231211
require.NoErrorf(t, err, "unable to get indexes")
@@ -235,9 +215,8 @@ func TestGetRepositoryIndexes(t *testing.T) {
235215

236216
// Update the transport to serve the same etag, but different content to
237217
// verify that we serve from the cache instead of the response.
238-
a.SetClient(&http.Client{
239-
Transport: &testLocalTransport{root: testAlternatePkgDir, basenameOnly: true, headers: map[string][]string{http.CanonicalHeaderKey("etag"): {testEtag}}},
240-
})
218+
tr.root = testAlternatePkgDir
219+
241220
indexes, err = a.GetRepositoryIndexes(context.Background(), false)
242221
require.NoErrorf(t, err, "unable to get indexes")
243222
require.Greater(t, len(indexes), 0, "no indexes found")
@@ -251,13 +230,11 @@ func TestGetRepositoryIndexes(t *testing.T) {
251230
// it should succeed for a cache hit
252231
tmpDir := t.TempDir()
253232
testEtag := "test-etag"
233+
tr := &testLocalTransport{root: testPrimaryPkgDir, basenameOnly: true, headers: map[string][]string{http.CanonicalHeaderKey("etag"): {testEtag}}}
254234

255235
// get our APK struct
256-
a := prepLayout(t, tmpDir, nil)
236+
a := prepLayout(t, tr, tmpDir, nil)
257237

258-
a.SetClient(&http.Client{
259-
Transport: &testLocalTransport{root: testPrimaryPkgDir, basenameOnly: true, headers: map[string][]string{http.CanonicalHeaderKey("etag"): {testEtag}}},
260-
})
261238
// Use the client to fill the cache.
262239
indexes, err := a.GetRepositoryIndexes(context.Background(), false)
263240
require.NoErrorf(t, err, "unable to get indexes")
@@ -268,9 +245,8 @@ func TestGetRepositoryIndexes(t *testing.T) {
268245
// Update the transport to serve a different etag and different content,
269246
// to verify that when the etag changes we use the data from the
270247
// response.
271-
a.SetClient(&http.Client{
272-
Transport: &testLocalTransport{root: testAlternatePkgDir, basenameOnly: true, headers: map[string][]string{http.CanonicalHeaderKey("etag"): {testEtag + "change"}}},
273-
})
248+
tr.root = testAlternatePkgDir
249+
tr.headers = map[string][]string{http.CanonicalHeaderKey("etag"): {testEtag + "change"}}
274250

275251
indexes, err = a.GetRepositoryIndexes(context.Background(), false)
276252
require.NoErrorf(t, err, "unable to get indexes")
@@ -288,14 +264,11 @@ func TestGetRepositoryIndexes(t *testing.T) {
288264
eg := errgroup.Group{}
289265
for i := range 100 {
290266
eg.Go(func() error {
291-
a := prepLayout(t, tmpDir, nil)
292-
a.SetClient(&http.Client{
293-
Transport: &testLocalTransport{
294-
root: testPrimaryPkgDir,
295-
basenameOnly: true,
296-
headers: map[string][]string{http.CanonicalHeaderKey("etag"): {fmt.Sprint(i)}},
297-
},
298-
})
267+
a := prepLayout(t, &testLocalTransport{
268+
root: testPrimaryPkgDir,
269+
basenameOnly: true,
270+
headers: map[string][]string{http.CanonicalHeaderKey("etag"): {fmt.Sprint(i)}},
271+
}, tmpDir, nil)
299272
indexes, err := a.GetRepositoryIndexes(context.Background(), false)
300273
require.NoErrorf(t, err, "unable to get indexes")
301274
require.Greater(t, len(indexes), 0, "no indexes found")

0 commit comments

Comments
 (0)