From d2ea8f14dd00a6e2ee07ce5190b08c55f7ab63f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 15 Mar 2022 22:01:54 +0100 Subject: [PATCH 1/6] Update an obsolete comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- docker/docker_client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/docker_client.go b/docker/docker_client.go index 833323b42..0ba88d13d 100644 --- a/docker/docker_client.go +++ b/docker/docker_client.go @@ -538,7 +538,7 @@ func (c *dockerClient) makeRequestToResolvedURLOnce(ctx context.Context, method, if err != nil { return nil, err } - if streamLen != -1 { // Do not blindly overwrite if streamLen == -1, http.NewRequest above can figure out the length of bytes.Reader and similar objects without us having to compute it. + if streamLen != -1 { // Do not blindly overwrite if streamLen == -1, http.NewRequestWithContext above can figure out the length of bytes.Reader and similar objects without us having to compute it. req.ContentLength = streamLen } req.Header.Set("Docker-Distribution-API-Version", "registry/2.0") From f0d818b27c57c1db344719478c4e61daaf84d119 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 15 Mar 2022 22:32:37 +0100 Subject: [PATCH 2/6] Use Testing.T.Cleanup() to simplify tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- go.mod | 2 +- openshift/openshift-copies_test.go | 19 +++---- signature/policy_eval_signedby_test.go | 52 +++++++---------- signature/policy_eval_test.go | 78 +++++++++----------------- 4 files changed, 54 insertions(+), 97 deletions(-) diff --git a/go.mod b/go.mod index a94854c86..4dbb5f4cf 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/containers/image/v5 -go 1.13 +go 1.14 require ( github.com/14rcole/gopopulate v0.0.0-20180821133914-b175b219e774 // indirect diff --git a/openshift/openshift-copies_test.go b/openshift/openshift-copies_test.go index 74ec5623b..f4fabb8e9 100644 --- a/openshift/openshift-copies_test.go +++ b/openshift/openshift-copies_test.go @@ -15,24 +15,22 @@ const fixtureKubeConfigPath = "testdata/admin.kubeconfig" // Set up KUBECONFIG to point at the fixture, and return a handler to clean it up. // Callers MUST NOT call testing.T.Parallel(). -func setupKubeConfigForSerialTest() func() { +func setupKubeConfigForSerialTest(t *testing.T) { // Environment is per-process, so this looks very unsafe; actually it seems fine because tests are not // run in parallel unless they opt in by calling t.Parallel(). So don’t do that. oldKC, hasKC := os.LookupEnv("KUBECONFIG") - cleanup := func() { + t.Cleanup(func() { if hasKC { os.Setenv("KUBECONFIG", oldKC) } else { os.Unsetenv("KUBECONFIG") } - } + }) os.Setenv("KUBECONFIG", fixtureKubeConfigPath) - return cleanup } func TestClientConfigLoadingRules(t *testing.T) { - cleanup := setupKubeConfigForSerialTest() - defer cleanup() + setupKubeConfigForSerialTest(t) rules := newOpenShiftClientConfigLoadingRules() res, err := rules.Load() @@ -66,8 +64,7 @@ func TestClientConfigLoadingRules(t *testing.T) { } func TestDirectClientConfig(t *testing.T) { - cleanup := setupKubeConfigForSerialTest() - defer cleanup() + setupKubeConfigForSerialTest(t) rules := newOpenShiftClientConfigLoadingRules() config, err := rules.Load() @@ -87,8 +84,7 @@ func TestDirectClientConfig(t *testing.T) { } func TestDeferredLoadingClientConfig(t *testing.T) { - cleanup := setupKubeConfigForSerialTest() - defer cleanup() + setupKubeConfigForSerialTest(t) rules := newOpenShiftClientConfigLoadingRules() deferred := newNonInteractiveDeferredLoadingClientConfig(rules) @@ -105,8 +101,7 @@ func TestDeferredLoadingClientConfig(t *testing.T) { } func TestDefaultClientConfig(t *testing.T) { - cleanup := setupKubeConfigForSerialTest() - defer cleanup() + setupKubeConfigForSerialTest(t) config := defaultClientConfig() res, err := config.ClientConfig() diff --git a/signature/policy_eval_signedby_test.go b/signature/policy_eval_signedby_test.go index 2620a586f..34a52d1b2 100644 --- a/signature/policy_eval_signedby_test.go +++ b/signature/policy_eval_signedby_test.go @@ -16,27 +16,26 @@ import ( ) // dirImageMock returns a types.UnparsedImage for a directory, claiming a specified dockerReference. -// The caller must call the returned close callback when done. -func dirImageMock(t *testing.T, dir, dockerReference string) (types.UnparsedImage, func()) { +func dirImageMock(t *testing.T, dir, dockerReference string) types.UnparsedImage { ref, err := reference.ParseNormalizedNamed(dockerReference) require.NoError(t, err) return dirImageMockWithRef(t, dir, refImageReferenceMock{ref}) } // dirImageMockWithRef returns a types.UnparsedImage for a directory, claiming a specified ref. -// The caller must call the returned close callback when done. -func dirImageMockWithRef(t *testing.T, dir string, ref types.ImageReference) (types.UnparsedImage, func()) { +func dirImageMockWithRef(t *testing.T, dir string, ref types.ImageReference) types.UnparsedImage { srcRef, err := directory.NewReference(dir) require.NoError(t, err) src, err := srcRef.NewImageSource(context.Background(), nil) require.NoError(t, err) + t.Cleanup(func() { + err := src.Close() + require.NoError(t, err) + }) return image.UnparsedInstance(&dirImageSourceMock{ - ImageSource: src, - ref: ref, - }, nil), func() { - err := src.Close() - require.NoError(t, err) - } + ImageSource: src, + ref: ref, + }, nil) } // dirImageSourceMock inherits dirImageSource, but overrides its Reference method. @@ -52,8 +51,7 @@ func (d *dirImageSourceMock) Reference() types.ImageReference { func TestPRSignedByIsSignatureAuthorAccepted(t *testing.T) { ktGPG := SBKeyTypeGPGKeys prm := NewPRMMatchExact() - testImage, closer := dirImageMock(t, "fixtures/dir-img-valid", "testing/manifest:latest") - defer closer() + testImage := dirImageMock(t, "fixtures/dir-img-valid", "testing/manifest:latest") testImageSig, err := ioutil.ReadFile("fixtures/dir-img-valid/signature-1") require.NoError(t, err) @@ -157,8 +155,7 @@ func TestPRSignedByIsSignatureAuthorAccepted(t *testing.T) { assertSARRejectedPolicyRequirement(t, sar, parsedSig, err) // Error reading image manifest - image, closer := dirImageMock(t, "fixtures/dir-img-no-manifest", "testing/manifest:latest") - defer closer() + image := dirImageMock(t, "fixtures/dir-img-no-manifest", "testing/manifest:latest") sig, err = ioutil.ReadFile("fixtures/dir-img-no-manifest/signature-1") require.NoError(t, err) pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) @@ -167,8 +164,7 @@ func TestPRSignedByIsSignatureAuthorAccepted(t *testing.T) { assertSARRejected(t, sar, parsedSig, err) // Error computing manifest digest - image, closer = dirImageMock(t, "fixtures/dir-img-manifest-digest-error", "testing/manifest:latest") - defer closer() + image = dirImageMock(t, "fixtures/dir-img-manifest-digest-error", "testing/manifest:latest") sig, err = ioutil.ReadFile("fixtures/dir-img-manifest-digest-error/signature-1") require.NoError(t, err) pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) @@ -177,8 +173,7 @@ func TestPRSignedByIsSignatureAuthorAccepted(t *testing.T) { assertSARRejected(t, sar, parsedSig, err) // A valid signature with a non-matching manifest - image, closer = dirImageMock(t, "fixtures/dir-img-modified-manifest", "testing/manifest:latest") - defer closer() + image = dirImageMock(t, "fixtures/dir-img-modified-manifest", "testing/manifest:latest") sig, err = ioutil.ReadFile("fixtures/dir-img-modified-manifest/signature-1") require.NoError(t, err) pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) @@ -209,8 +204,7 @@ func TestPRSignedByIsRunningImageAllowed(t *testing.T) { prm := NewPRMMatchExact() // A simple success case: single valid signature. - image, closer := dirImageMock(t, "fixtures/dir-img-valid", "testing/manifest:latest") - defer closer() + image := dirImageMock(t, "fixtures/dir-img-valid", "testing/manifest:latest") pr, err := NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) require.NoError(t, err) allowed, err := pr.isRunningImageAllowed(context.Background(), image) @@ -219,48 +213,42 @@ func TestPRSignedByIsRunningImageAllowed(t *testing.T) { // Error reading signatures invalidSigDir := createInvalidSigDir(t) defer os.RemoveAll(invalidSigDir) - image, closer = dirImageMock(t, invalidSigDir, "testing/manifest:latest") - defer closer() + image = dirImageMock(t, invalidSigDir, "testing/manifest:latest") pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) require.NoError(t, err) allowed, err = pr.isRunningImageAllowed(context.Background(), image) assertRunningRejected(t, allowed, err) // No signatures - image, closer = dirImageMock(t, "fixtures/dir-img-unsigned", "testing/manifest:latest") - defer closer() + image = dirImageMock(t, "fixtures/dir-img-unsigned", "testing/manifest:latest") pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) require.NoError(t, err) allowed, err = pr.isRunningImageAllowed(context.Background(), image) assertRunningRejectedPolicyRequirement(t, allowed, err) // 1 invalid signature: use dir-img-valid, but a non-matching Docker reference - image, closer = dirImageMock(t, "fixtures/dir-img-valid", "testing/manifest:notlatest") - defer closer() + image = dirImageMock(t, "fixtures/dir-img-valid", "testing/manifest:notlatest") pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) require.NoError(t, err) allowed, err = pr.isRunningImageAllowed(context.Background(), image) assertRunningRejectedPolicyRequirement(t, allowed, err) // 2 valid signatures - image, closer = dirImageMock(t, "fixtures/dir-img-valid-2", "testing/manifest:latest") - defer closer() + image = dirImageMock(t, "fixtures/dir-img-valid-2", "testing/manifest:latest") pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) require.NoError(t, err) allowed, err = pr.isRunningImageAllowed(context.Background(), image) assertRunningAllowed(t, allowed, err) // One invalid, one valid signature (in this order) - image, closer = dirImageMock(t, "fixtures/dir-img-mixed", "testing/manifest:latest") - defer closer() + image = dirImageMock(t, "fixtures/dir-img-mixed", "testing/manifest:latest") pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) require.NoError(t, err) allowed, err = pr.isRunningImageAllowed(context.Background(), image) assertRunningAllowed(t, allowed, err) // 2 invalid signatures: use dir-img-valid-2, but a non-matching Docker reference - image, closer = dirImageMock(t, "fixtures/dir-img-valid-2", "testing/manifest:notlatest") - defer closer() + image = dirImageMock(t, "fixtures/dir-img-valid-2", "testing/manifest:notlatest") pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) require.NoError(t, err) allowed, err = pr.isRunningImageAllowed(context.Background(), image) diff --git a/signature/policy_eval_test.go b/signature/policy_eval_test.go index 8cabb3eea..c56be7de0 100644 --- a/signature/policy_eval_test.go +++ b/signature/policy_eval_test.go @@ -215,8 +215,7 @@ func TestPolicyContextRequirementsForImageRef(t *testing.T) { } // pcImageMock returns a types.UnparsedImage for a directory, claiming a specified dockerReference and implementing PolicyConfigurationIdentity/PolicyConfigurationNamespaces. -// The caller must call the returned close callback when done. -func pcImageMock(t *testing.T, dir, dockerReference string) (types.UnparsedImage, func()) { +func pcImageMock(t *testing.T, dir, dockerReference string) types.UnparsedImage { ref, err := reference.ParseNormalizedNamed(dockerReference) require.NoError(t, err) return dirImageMockWithRef(t, dir, pcImageReferenceMock{"docker", ref}) @@ -268,85 +267,73 @@ func TestPolicyContextGetSignaturesWithAcceptedAuthor(t *testing.T) { }() // Success - img, closer := pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:latest") - defer closer() + img := pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:latest") sigs, err := pc.GetSignaturesWithAcceptedAuthor(context.Background(), img) require.NoError(t, err) assert.Equal(t, []*Signature{expectedSig}, sigs) // Two signatures // FIXME? Use really different signatures for this? - img, closer = pcImageMock(t, "fixtures/dir-img-valid-2", "testing/manifest:latest") - defer closer() + img = pcImageMock(t, "fixtures/dir-img-valid-2", "testing/manifest:latest") sigs, err = pc.GetSignaturesWithAcceptedAuthor(context.Background(), img) require.NoError(t, err) assert.Equal(t, []*Signature{expectedSig, expectedSig}, sigs) // No signatures - img, closer = pcImageMock(t, "fixtures/dir-img-unsigned", "testing/manifest:latest") - defer closer() + img = pcImageMock(t, "fixtures/dir-img-unsigned", "testing/manifest:latest") sigs, err = pc.GetSignaturesWithAcceptedAuthor(context.Background(), img) require.NoError(t, err) assert.Empty(t, sigs) // Only invalid signatures - img, closer = pcImageMock(t, "fixtures/dir-img-modified-manifest", "testing/manifest:latest") - defer closer() + img = pcImageMock(t, "fixtures/dir-img-modified-manifest", "testing/manifest:latest") sigs, err = pc.GetSignaturesWithAcceptedAuthor(context.Background(), img) require.NoError(t, err) assert.Empty(t, sigs) // 1 invalid, 1 valid signature (in this order) - img, closer = pcImageMock(t, "fixtures/dir-img-mixed", "testing/manifest:latest") - defer closer() + img = pcImageMock(t, "fixtures/dir-img-mixed", "testing/manifest:latest") sigs, err = pc.GetSignaturesWithAcceptedAuthor(context.Background(), img) require.NoError(t, err) assert.Equal(t, []*Signature{expectedSig}, sigs) // Two sarAccepted results for one signature - img, closer = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:twoAccepts") - defer closer() + img = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:twoAccepts") sigs, err = pc.GetSignaturesWithAcceptedAuthor(context.Background(), img) require.NoError(t, err) assert.Equal(t, []*Signature{expectedSig}, sigs) // sarAccepted+sarRejected for a signature - img, closer = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:acceptReject") - defer closer() + img = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:acceptReject") sigs, err = pc.GetSignaturesWithAcceptedAuthor(context.Background(), img) require.NoError(t, err) assert.Empty(t, sigs) // sarAccepted+sarUnknown for a signature - img, closer = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:acceptUnknown") - defer closer() + img = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:acceptUnknown") sigs, err = pc.GetSignaturesWithAcceptedAuthor(context.Background(), img) require.NoError(t, err) assert.Equal(t, []*Signature{expectedSig}, sigs) // sarRejected+sarUnknown for a signature - img, closer = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:rejectUnknown") - defer closer() + img = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:rejectUnknown") sigs, err = pc.GetSignaturesWithAcceptedAuthor(context.Background(), img) require.NoError(t, err) assert.Empty(t, sigs) // sarUnknown only - img, closer = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:unknown") - defer closer() + img = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:unknown") sigs, err = pc.GetSignaturesWithAcceptedAuthor(context.Background(), img) require.NoError(t, err) assert.Empty(t, sigs) - img, closer = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:unknown2") - defer closer() + img = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:unknown2") sigs, err = pc.GetSignaturesWithAcceptedAuthor(context.Background(), img) require.NoError(t, err) assert.Empty(t, sigs) // Empty list of requirements (invalid) - img, closer = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:invalidEmptyRequirements") - defer closer() + img = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:invalidEmptyRequirements") sigs, err = pc.GetSignaturesWithAcceptedAuthor(context.Background(), img) require.NoError(t, err) assert.Empty(t, sigs) @@ -358,8 +345,7 @@ func TestPolicyContextGetSignaturesWithAcceptedAuthor(t *testing.T) { require.NoError(t, err) err = destroyedPC.Destroy() require.NoError(t, err) - img, closer = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:latest") - defer closer() + img = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:latest") sigs, err = destroyedPC.GetSignaturesWithAcceptedAuthor(context.Background(), img) assert.Error(t, err) assert.Nil(t, sigs) @@ -370,8 +356,7 @@ func TestPolicyContextGetSignaturesWithAcceptedAuthor(t *testing.T) { // Error reading signatures. invalidSigDir := createInvalidSigDir(t) defer os.RemoveAll(invalidSigDir) - img, closer = pcImageMock(t, invalidSigDir, "testing/manifest:latest") - defer closer() + img = pcImageMock(t, invalidSigDir, "testing/manifest:latest") sigs, err = pc.GetSignaturesWithAcceptedAuthor(context.Background(), img) assert.Error(t, err) assert.Nil(t, sigs) @@ -410,63 +395,53 @@ func TestPolicyContextIsRunningImageAllowed(t *testing.T) { }() // Success - img, closer := pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:latest") - defer closer() + img := pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:latest") res, err := pc.IsRunningImageAllowed(context.Background(), img) assertRunningAllowed(t, res, err) // Two signatures // FIXME? Use really different signatures for this? - img, closer = pcImageMock(t, "fixtures/dir-img-valid-2", "testing/manifest:latest") - defer closer() + img = pcImageMock(t, "fixtures/dir-img-valid-2", "testing/manifest:latest") res, err = pc.IsRunningImageAllowed(context.Background(), img) assertRunningAllowed(t, res, err) // No signatures - img, closer = pcImageMock(t, "fixtures/dir-img-unsigned", "testing/manifest:latest") - defer closer() + img = pcImageMock(t, "fixtures/dir-img-unsigned", "testing/manifest:latest") res, err = pc.IsRunningImageAllowed(context.Background(), img) assertRunningRejectedPolicyRequirement(t, res, err) // Only invalid signatures - img, closer = pcImageMock(t, "fixtures/dir-img-modified-manifest", "testing/manifest:latest") - defer closer() + img = pcImageMock(t, "fixtures/dir-img-modified-manifest", "testing/manifest:latest") res, err = pc.IsRunningImageAllowed(context.Background(), img) assertRunningRejectedPolicyRequirement(t, res, err) // 1 invalid, 1 valid signature (in this order) - img, closer = pcImageMock(t, "fixtures/dir-img-mixed", "testing/manifest:latest") - defer closer() + img = pcImageMock(t, "fixtures/dir-img-mixed", "testing/manifest:latest") res, err = pc.IsRunningImageAllowed(context.Background(), img) assertRunningAllowed(t, res, err) // Two allowed results - img, closer = pcImageMock(t, "fixtures/dir-img-mixed", "testing/manifest:twoAllows") - defer closer() + img = pcImageMock(t, "fixtures/dir-img-mixed", "testing/manifest:twoAllows") res, err = pc.IsRunningImageAllowed(context.Background(), img) assertRunningAllowed(t, res, err) // Allow + deny results - img, closer = pcImageMock(t, "fixtures/dir-img-mixed", "testing/manifest:allowDeny") - defer closer() + img = pcImageMock(t, "fixtures/dir-img-mixed", "testing/manifest:allowDeny") res, err = pc.IsRunningImageAllowed(context.Background(), img) assertRunningRejectedPolicyRequirement(t, res, err) // prReject works - img, closer = pcImageMock(t, "fixtures/dir-img-mixed", "testing/manifest:reject") - defer closer() + img = pcImageMock(t, "fixtures/dir-img-mixed", "testing/manifest:reject") res, err = pc.IsRunningImageAllowed(context.Background(), img) assertRunningRejectedPolicyRequirement(t, res, err) // prInsecureAcceptAnything works - img, closer = pcImageMock(t, "fixtures/dir-img-mixed", "testing/manifest:acceptAnything") - defer closer() + img = pcImageMock(t, "fixtures/dir-img-mixed", "testing/manifest:acceptAnything") res, err = pc.IsRunningImageAllowed(context.Background(), img) assertRunningAllowed(t, res, err) // Empty list of requirements (invalid) - img, closer = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:invalidEmptyRequirements") - defer closer() + img = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:invalidEmptyRequirements") res, err = pc.IsRunningImageAllowed(context.Background(), img) assertRunningRejectedPolicyRequirement(t, res, err) @@ -475,8 +450,7 @@ func TestPolicyContextIsRunningImageAllowed(t *testing.T) { require.NoError(t, err) err = destroyedPC.Destroy() require.NoError(t, err) - img, closer = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:latest") - defer closer() + img = pcImageMock(t, "fixtures/dir-img-valid", "testing/manifest:latest") res, err = destroyedPC.IsRunningImageAllowed(context.Background(), img) assertRunningRejected(t, res, err) // Not testing the pcInUse->pcReady transition, that would require custom PolicyRequirement From fa54b28a4d7a7e97fe899f958326b2bd3f91a11a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 15 Mar 2022 23:04:07 +0100 Subject: [PATCH 3/6] Modify makeRequestToResolvedURL and makeRequestToResolvedURLOnce to accept an *url.URL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is, sadly, wasteful, because NewRequestWithContext() only accepts a string and parses it again, but it gives us more type safety, and simplifies at least some callers. Most importantly, this will also allow us to call url.Redacted() for logging. Signed-off-by: Miloslav Trmač --- docker/docker_client.go | 34 ++++++++++++++++++++++------------ docker/docker_image_dest.go | 6 +++--- docker/docker_image_src.go | 5 +++-- 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/docker/docker_client.go b/docker/docker_client.go index 0ba88d13d..6f9b06797 100644 --- a/docker/docker_client.go +++ b/docker/docker_client.go @@ -463,7 +463,11 @@ func (c *dockerClient) makeRequest(ctx context.Context, method, path string, hea return nil, err } - url := fmt.Sprintf("%s://%s%s", c.scheme, c.registry, path) + urlString := fmt.Sprintf("%s://%s%s", c.scheme, c.registry, path) + url, err := url.Parse(urlString) + if err != nil { + return nil, err + } return c.makeRequestToResolvedURL(ctx, method, url, headers, stream, -1, auth, extraScope) } @@ -500,7 +504,7 @@ func parseRetryAfter(res *http.Response, fallbackDelay time.Duration) time.Durat // makeRequest should generally be preferred. // In case of an HTTP 429 status code in the response, it may automatically retry a few times. // TODO(runcom): too many arguments here, use a struct -func (c *dockerClient) makeRequestToResolvedURL(ctx context.Context, method, url string, headers map[string][]string, stream io.Reader, streamLen int64, auth sendAuth, extraScope *authScope) (*http.Response, error) { +func (c *dockerClient) makeRequestToResolvedURL(ctx context.Context, method string, url *url.URL, headers map[string][]string, stream io.Reader, streamLen int64, auth sendAuth, extraScope *authScope) (*http.Response, error) { delay := backoffInitialDelay attempts := 0 for { @@ -518,7 +522,7 @@ func (c *dockerClient) makeRequestToResolvedURL(ctx context.Context, method, url if delay > backoffMaxDelay { delay = backoffMaxDelay } - logrus.Debugf("Too many requests to %s: sleeping for %f seconds before next attempt", url, delay.Seconds()) + logrus.Debugf("Too many requests to %s: sleeping for %f seconds before next attempt", url.String(), delay.Seconds()) select { case <-ctx.Done(): return nil, ctx.Err() @@ -533,8 +537,8 @@ func (c *dockerClient) makeRequestToResolvedURL(ctx context.Context, method, url // streamLen, if not -1, specifies the length of the data expected on stream. // makeRequest should generally be preferred. // Note that no exponential back off is performed when receiving an http 429 status code. -func (c *dockerClient) makeRequestToResolvedURLOnce(ctx context.Context, method, url string, headers map[string][]string, stream io.Reader, streamLen int64, auth sendAuth, extraScope *authScope) (*http.Response, error) { - req, err := http.NewRequestWithContext(ctx, method, url, stream) +func (c *dockerClient) makeRequestToResolvedURLOnce(ctx context.Context, method string, url *url.URL, headers map[string][]string, stream io.Reader, streamLen int64, auth sendAuth, extraScope *authScope) (*http.Response, error) { + req, err := http.NewRequestWithContext(ctx, method, url.String(), stream) if err != nil { return nil, err } @@ -553,7 +557,7 @@ func (c *dockerClient) makeRequestToResolvedURLOnce(ctx context.Context, method, return nil, err } } - logrus.Debugf("%s %s", method, url) + logrus.Debugf("%s %s", method, url.String()) res, err := c.client.Do(req) if err != nil { return nil, err @@ -735,14 +739,17 @@ func (c *dockerClient) detectPropertiesHelper(ctx context.Context) error { c.client = &http.Client{Transport: tr} ping := func(scheme string) error { - url := fmt.Sprintf(resolvedPingV2URL, scheme, c.registry) + url, err := url.Parse(fmt.Sprintf(resolvedPingV2URL, scheme, c.registry)) + if err != nil { + return err + } resp, err := c.makeRequestToResolvedURL(ctx, http.MethodGet, url, nil, nil, -1, noAuth, nil) if err != nil { - logrus.Debugf("Ping %s err %s (%#v)", url, err.Error(), err) + logrus.Debugf("Ping %s err %s (%#v)", url.String(), err.Error(), err) return err } defer resp.Body.Close() - logrus.Debugf("Ping %s status %d", url, resp.StatusCode) + logrus.Debugf("Ping %s status %d", url.String(), resp.StatusCode) if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusUnauthorized { return httpResponseToError(resp, "") } @@ -762,14 +769,17 @@ func (c *dockerClient) detectPropertiesHelper(ctx context.Context) error { } // best effort to understand if we're talking to a V1 registry pingV1 := func(scheme string) bool { - url := fmt.Sprintf(resolvedPingV1URL, scheme, c.registry) + url, err := url.Parse(fmt.Sprintf(resolvedPingV1URL, scheme, c.registry)) + if err != nil { + return false + } resp, err := c.makeRequestToResolvedURL(ctx, http.MethodGet, url, nil, nil, -1, noAuth, nil) if err != nil { - logrus.Debugf("Ping %s err %s (%#v)", url, err.Error(), err) + logrus.Debugf("Ping %s err %s (%#v)", url.String(), err.Error(), err) return false } defer resp.Body.Close() - logrus.Debugf("Ping %s status %d", url, resp.StatusCode) + logrus.Debugf("Ping %s status %d", url.String(), resp.StatusCode) if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusUnauthorized { return false } diff --git a/docker/docker_image_dest.go b/docker/docker_image_dest.go index e7af8f93d..8e99b97e0 100644 --- a/docker/docker_image_dest.go +++ b/docker/docker_image_dest.go @@ -182,7 +182,7 @@ func (d *dockerImageDestination) PutBlob(ctx context.Context, stream io.Reader, // This error text should never be user-visible, we terminate only after makeRequestToResolvedURL // returns, so there isn’t a way for the error text to be provided to any of our callers. defer uploadReader.Terminate(errors.New("Reading data from an already terminated upload")) - res, err = d.c.makeRequestToResolvedURL(ctx, http.MethodPatch, uploadLocation.String(), map[string][]string{"Content-Type": {"application/octet-stream"}}, uploadReader, inputInfo.Size, v2Auth, nil) + res, err = d.c.makeRequestToResolvedURL(ctx, http.MethodPatch, uploadLocation, map[string][]string{"Content-Type": {"application/octet-stream"}}, uploadReader, inputInfo.Size, v2Auth, nil) if err != nil { logrus.Debugf("Error uploading layer chunked %v", err) return nil, err @@ -207,7 +207,7 @@ func (d *dockerImageDestination) PutBlob(ctx context.Context, stream io.Reader, locationQuery := uploadLocation.Query() locationQuery.Set("digest", blobDigest.String()) uploadLocation.RawQuery = locationQuery.Encode() - res, err = d.c.makeRequestToResolvedURL(ctx, http.MethodPut, uploadLocation.String(), map[string][]string{"Content-Type": {"application/octet-stream"}}, nil, -1, v2Auth, nil) + res, err = d.c.makeRequestToResolvedURL(ctx, http.MethodPut, uploadLocation, map[string][]string{"Content-Type": {"application/octet-stream"}}, nil, -1, v2Auth, nil) if err != nil { return types.BlobInfo{}, err } @@ -277,7 +277,7 @@ func (d *dockerImageDestination) mountBlob(ctx context.Context, srcRepo referenc return errors.Wrap(err, "determining upload URL after a mount attempt") } logrus.Debugf("... started an upload instead of mounting, trying to cancel at %s", uploadLocation.String()) - res2, err := d.c.makeRequestToResolvedURL(ctx, http.MethodDelete, uploadLocation.String(), nil, nil, -1, v2Auth, extraScope) + res2, err := d.c.makeRequestToResolvedURL(ctx, http.MethodDelete, uploadLocation, nil, nil, -1, v2Auth, extraScope) if err != nil { logrus.Debugf("Error trying to cancel an inadvertent upload: %s", err) } else { diff --git a/docker/docker_image_src.go b/docker/docker_image_src.go index cb520d670..b940e35ca 100644 --- a/docker/docker_image_src.go +++ b/docker/docker_image_src.go @@ -253,13 +253,14 @@ func (s *dockerImageSource) getExternalBlob(ctx context.Context, urls []string) return nil, 0, errors.New("internal error: getExternalBlob called with no URLs") } for _, u := range urls { - if u, err := url.Parse(u); err != nil || (u.Scheme != "http" && u.Scheme != "https") { + url, err := url.Parse(u) + if err != nil || (url.Scheme != "http" && url.Scheme != "https") { continue // unsupported url. skip this url. } // NOTE: we must not authenticate on additional URLs as those // can be abused to leak credentials or tokens. Please // refer to CVE-2020-15157 for more information. - resp, err = s.c.makeRequestToResolvedURL(ctx, http.MethodGet, u, nil, nil, -1, noAuth, nil) + resp, err = s.c.makeRequestToResolvedURL(ctx, http.MethodGet, url, nil, nil, -1, noAuth, nil) if err == nil { if resp.StatusCode != http.StatusOK { err = errors.Errorf("error fetching external blob from %q: %d (%s)", u, resp.StatusCode, http.StatusText(resp.StatusCode)) From 5a4b8a40384c892d538bc0d62c3b722b37677a7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 15 Mar 2022 23:07:05 +0100 Subject: [PATCH 4/6] Use url.Redacted() in log output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... to be at least a bit protected against credentials in logs. I did try to find all uses, but it's possible I have missed some. Signed-off-by: Miloslav Trmač --- docker/docker_client.go | 16 ++++++++-------- docker/docker_image_dest.go | 15 +++++++-------- docker/docker_image_src.go | 6 +++--- docker/lookaside.go | 2 +- go.mod | 2 +- openshift/openshift.go | 2 +- 6 files changed, 21 insertions(+), 22 deletions(-) diff --git a/docker/docker_client.go b/docker/docker_client.go index 6f9b06797..9837235d8 100644 --- a/docker/docker_client.go +++ b/docker/docker_client.go @@ -522,7 +522,7 @@ func (c *dockerClient) makeRequestToResolvedURL(ctx context.Context, method stri if delay > backoffMaxDelay { delay = backoffMaxDelay } - logrus.Debugf("Too many requests to %s: sleeping for %f seconds before next attempt", url.String(), delay.Seconds()) + logrus.Debugf("Too many requests to %s: sleeping for %f seconds before next attempt", url.Redacted(), delay.Seconds()) select { case <-ctx.Done(): return nil, ctx.Err() @@ -557,7 +557,7 @@ func (c *dockerClient) makeRequestToResolvedURLOnce(ctx context.Context, method return nil, err } } - logrus.Debugf("%s %s", method, url.String()) + logrus.Debugf("%s %s", method, url.Redacted()) res, err := c.client.Do(req) if err != nil { return nil, err @@ -657,7 +657,7 @@ func (c *dockerClient) getBearerTokenOAuth2(ctx context.Context, challenge chall authReq.Body = ioutil.NopCloser(bytes.NewBufferString(params.Encode())) authReq.Header.Add("User-Agent", c.userAgent) authReq.Header.Add("Content-Type", "application/x-www-form-urlencoded") - logrus.Debugf("%s %s", authReq.Method, authReq.URL.String()) + logrus.Debugf("%s %s", authReq.Method, authReq.URL.Redacted()) res, err := c.client.Do(authReq) if err != nil { return nil, err @@ -709,7 +709,7 @@ func (c *dockerClient) getBearerToken(ctx context.Context, challenge challenge, } authReq.Header.Add("User-Agent", c.userAgent) - logrus.Debugf("%s %s", authReq.Method, authReq.URL.String()) + logrus.Debugf("%s %s", authReq.Method, authReq.URL.Redacted()) res, err := c.client.Do(authReq) if err != nil { return nil, err @@ -745,11 +745,11 @@ func (c *dockerClient) detectPropertiesHelper(ctx context.Context) error { } resp, err := c.makeRequestToResolvedURL(ctx, http.MethodGet, url, nil, nil, -1, noAuth, nil) if err != nil { - logrus.Debugf("Ping %s err %s (%#v)", url.String(), err.Error(), err) + logrus.Debugf("Ping %s err %s (%#v)", url.Redacted(), err.Error(), err) return err } defer resp.Body.Close() - logrus.Debugf("Ping %s status %d", url.String(), resp.StatusCode) + logrus.Debugf("Ping %s status %d", url.Redacted(), resp.StatusCode) if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusUnauthorized { return httpResponseToError(resp, "") } @@ -775,11 +775,11 @@ func (c *dockerClient) detectPropertiesHelper(ctx context.Context) error { } resp, err := c.makeRequestToResolvedURL(ctx, http.MethodGet, url, nil, nil, -1, noAuth, nil) if err != nil { - logrus.Debugf("Ping %s err %s (%#v)", url.String(), err.Error(), err) + logrus.Debugf("Ping %s err %s (%#v)", url.Redacted(), err.Error(), err) return false } defer resp.Body.Close() - logrus.Debugf("Ping %s status %d", url.String(), resp.StatusCode) + logrus.Debugf("Ping %s status %d", url.Redacted(), resp.StatusCode) if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusUnauthorized { return false } diff --git a/docker/docker_image_dest.go b/docker/docker_image_dest.go index 8e99b97e0..e3275aa45 100644 --- a/docker/docker_image_dest.go +++ b/docker/docker_image_dest.go @@ -257,9 +257,8 @@ func (d *dockerImageDestination) mountBlob(ctx context.Context, srcRepo referenc "from": {reference.Path(srcRepo)}, }.Encode(), } - mountPath := u.String() - logrus.Debugf("Trying to mount %s", mountPath) - res, err := d.c.makeRequest(ctx, http.MethodPost, mountPath, nil, nil, v2Auth, extraScope) + logrus.Debugf("Trying to mount %s", u.Redacted()) + res, err := d.c.makeRequest(ctx, http.MethodPost, u.String(), nil, nil, v2Auth, extraScope) if err != nil { return err } @@ -276,7 +275,7 @@ func (d *dockerImageDestination) mountBlob(ctx context.Context, srcRepo referenc if err != nil { return errors.Wrap(err, "determining upload URL after a mount attempt") } - logrus.Debugf("... started an upload instead of mounting, trying to cancel at %s", uploadLocation.String()) + logrus.Debugf("... started an upload instead of mounting, trying to cancel at %s", uploadLocation.Redacted()) res2, err := d.c.makeRequestToResolvedURL(ctx, http.MethodDelete, uploadLocation, nil, nil, -1, v2Auth, extraScope) if err != nil { logrus.Debugf("Error trying to cancel an inadvertent upload: %s", err) @@ -600,9 +599,9 @@ func (d *dockerImageDestination) putOneSignature(url *url.URL, signature []byte) return nil case "http", "https": - return errors.Errorf("Writing directly to a %s sigstore %s is not supported. Configure a sigstore-staging: location", url.Scheme, url.String()) + return errors.Errorf("Writing directly to a %s sigstore %s is not supported. Configure a sigstore-staging: location", url.Scheme, url.Redacted()) default: - return errors.Errorf("Unsupported scheme when writing signature to %s", url.String()) + return errors.Errorf("Unsupported scheme when writing signature to %s", url.Redacted()) } } @@ -620,9 +619,9 @@ func (c *dockerClient) deleteOneSignature(url *url.URL) (missing bool, err error return false, err case "http", "https": - return false, errors.Errorf("Writing directly to a %s sigstore %s is not supported. Configure a sigstore-staging: location", url.Scheme, url.String()) + return false, errors.Errorf("Writing directly to a %s sigstore %s is not supported. Configure a sigstore-staging: location", url.Scheme, url.Redacted()) default: - return false, errors.Errorf("Unsupported scheme when deleting signature from %s", url.String()) + return false, errors.Errorf("Unsupported scheme when deleting signature from %s", url.Redacted()) } } diff --git a/docker/docker_image_src.go b/docker/docker_image_src.go index b940e35ca..c08e5538a 100644 --- a/docker/docker_image_src.go +++ b/docker/docker_image_src.go @@ -525,7 +525,7 @@ func (s *dockerImageSource) getOneSignature(ctx context.Context, url *url.URL) ( return sig, false, nil case "http", "https": - logrus.Debugf("GET %s", url) + logrus.Debugf("GET %s", url.Redacted()) req, err := http.NewRequestWithContext(ctx, http.MethodGet, url.String(), nil) if err != nil { return nil, false, err @@ -538,7 +538,7 @@ func (s *dockerImageSource) getOneSignature(ctx context.Context, url *url.URL) ( if res.StatusCode == http.StatusNotFound { return nil, true, nil } else if res.StatusCode != http.StatusOK { - return nil, false, errors.Errorf("Error reading signature from %s: status %d (%s)", url.String(), res.StatusCode, http.StatusText(res.StatusCode)) + return nil, false, errors.Errorf("Error reading signature from %s: status %d (%s)", url.Redacted(), res.StatusCode, http.StatusText(res.StatusCode)) } sig, err := iolimits.ReadAtMost(res.Body, iolimits.MaxSignatureBodySize) if err != nil { @@ -547,7 +547,7 @@ func (s *dockerImageSource) getOneSignature(ctx context.Context, url *url.URL) ( return sig, false, nil default: - return nil, false, errors.Errorf("Unsupported scheme when reading signature from %s", url.String()) + return nil, false, errors.Errorf("Unsupported scheme when reading signature from %s", url.Redacted()) } } diff --git a/docker/lookaside.go b/docker/lookaside.go index 515e59327..22d84931c 100644 --- a/docker/lookaside.go +++ b/docker/lookaside.go @@ -82,7 +82,7 @@ func SignatureStorageBaseURL(sys *types.SystemContext, ref types.ImageReference, } else { // returns default directory if no sigstore specified in configuration file url = builtinDefaultSignatureStorageDir(rootless.GetRootlessEUID()) - logrus.Debugf(" No signature storage configuration found for %s, using built-in default %s", dr.PolicyConfigurationIdentity(), url.String()) + logrus.Debugf(" No signature storage configuration found for %s, using built-in default %s", dr.PolicyConfigurationIdentity(), url.Redacted()) } // NOTE: Keep this in sync with docs/signature-protocols.md! // FIXME? Restrict to explicitly supported schemes? diff --git a/go.mod b/go.mod index 4dbb5f4cf..201f237c3 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/containers/image/v5 -go 1.14 +go 1.15 require ( github.com/14rcole/gopopulate v0.0.0-20180821133914-b175b219e774 // indirect diff --git a/openshift/openshift.go b/openshift/openshift.go index c7c6cf694..67612d800 100644 --- a/openshift/openshift.go +++ b/openshift/openshift.go @@ -95,7 +95,7 @@ func (c *openshiftClient) doRequest(ctx context.Context, method, path string, re req.Header.Set("Content-Type", "application/json") } - logrus.Debugf("%s %s", method, url.String()) + logrus.Debugf("%s %s", method, url.Redacted()) res, err := c.httpClient.Do(req) if err != nil { return nil, err From 9a9904944d467301a04b978fb1a74c6add2e9c63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 16 Mar 2022 00:36:21 +0100 Subject: [PATCH 5/6] Use testing.T.TempDir() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... to simplify tests. Signed-off-by: Miloslav Trmač --- copy/sign_test.go | 4 +- directory/directory_test.go | 14 +--- directory/directory_transport_test.go | 43 +++------- directory/explicitfilepath/path_test.go | 7 +- docker/archive/transport_test.go | 8 +- docker/lookaside_test.go | 12 +-- oci/archive/oci_transport_test.go | 48 ++++-------- oci/layout/oci_dest_test.go | 6 +- oci/layout/oci_transport_test.go | 54 ++++--------- ostree/ostree_transport_test.go | 32 ++------ pkg/blobcache/blobcache_test.go | 22 +----- pkg/blobinfocache/boltdb/boltdb_test.go | 11 +-- pkg/blobinfocache/default_test.go | 7 +- pkg/docker/config/config_test.go | 78 +++---------------- .../system_registries_v2_test.go | 4 +- pkg/tlsclientconfig/tlsclientconfig_test.go | 5 +- sif/transport_test.go | 6 +- signature/mechanism_test.go | 8 +- signature/policy_config_test.go | 4 +- signature/policy_eval_signedby_test.go | 7 +- signature/policy_eval_test.go | 2 - storage/storage_test.go | 29 ++----- 22 files changed, 97 insertions(+), 314 deletions(-) diff --git a/copy/sign_test.go b/copy/sign_test.go index 6c95d438f..0e74c7cde 100644 --- a/copy/sign_test.go +++ b/copy/sign_test.go @@ -39,9 +39,7 @@ func TestCreateSignature(t *testing.T) { defer os.Unsetenv("GNUPGHOME") // Signing a directory: reference, which does not have a DockerReference(), fails. - tempDir, err := ioutil.TempDir("", "signature-dir-dest") - require.NoError(t, err) - defer os.RemoveAll(tempDir) + tempDir := t.TempDir() dirRef, err := directory.NewReference(tempDir) require.NoError(t, err) dirDest, err := dirRef.NewImageDestination(context.Background(), nil) diff --git a/directory/directory_test.go b/directory/directory_test.go index 2ab20b2a4..ce5429a01 100644 --- a/directory/directory_test.go +++ b/directory/directory_test.go @@ -18,7 +18,6 @@ import ( func TestDestinationReference(t *testing.T) { ref, tmpDir := refToTempDir(t) - defer os.RemoveAll(tmpDir) dest, err := ref.NewImageDestination(context.Background(), nil) require.NoError(t, err) @@ -28,8 +27,7 @@ func TestDestinationReference(t *testing.T) { } func TestGetPutManifest(t *testing.T) { - ref, tmpDir := refToTempDir(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempDir(t) man := []byte("test-manifest") list := []byte("test-manifest-list") @@ -64,8 +62,7 @@ func TestGetPutBlob(t *testing.T) { providedBlob := []byte("provided-blob") providedDigest := digest.Digest("sha256:provided-test-digest") - ref, tmpDir := refToTempDir(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempDir(t) cache := memory.New() dest, err := ref.NewImageDestination(context.Background(), nil) @@ -114,8 +111,7 @@ func TestPutBlobDigestFailure(t *testing.T) { const digestErrorString = "Simulated digest error" const blobDigest = digest.Digest("sha256:test-digest") - ref, tmpDir := refToTempDir(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempDir(t) dirRef, ok := ref.(dirReference) require.True(t, ok) blobPath := dirRef.layerPath(blobDigest) @@ -152,8 +148,7 @@ func TestPutBlobDigestFailure(t *testing.T) { } func TestGetPutSignatures(t *testing.T) { - ref, tmpDir := refToTempDir(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempDir(t) man := []byte("test-manifest") list := []byte("test-manifest-list") @@ -200,7 +195,6 @@ func TestGetPutSignatures(t *testing.T) { func TestSourceReference(t *testing.T) { ref, tmpDir := refToTempDir(t) - defer os.RemoveAll(tmpDir) src, err := ref.NewImageSource(context.Background(), nil) require.NoError(t, err) diff --git a/directory/directory_transport_test.go b/directory/directory_transport_test.go index 50c5c1d69..30b210ebe 100644 --- a/directory/directory_transport_test.go +++ b/directory/directory_transport_test.go @@ -50,9 +50,7 @@ func TestNewReference(t *testing.T) { // testNewReference is a test shared for Transport.ParseReference and NewReference. func testNewReference(t *testing.T, fn func(string) (types.ImageReference, error)) { - tmpDir, err := ioutil.TempDir("", "dir-transport-test") - require.NoError(t, err) - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() for _, path := range []string{ "/", @@ -68,42 +66,35 @@ func testNewReference(t *testing.T, fn func(string) (types.ImageReference, error assert.Equal(t, path, dirRef.path, path) } - _, err = fn(tmpDir + "/thisparentdoesnotexist/something") + _, err := fn(tmpDir + "/thisparentdoesnotexist/something") assert.Error(t, err) } // refToTempDir creates a temporary directory and returns a reference to it. -// The caller should -// defer os.RemoveAll(tmpDir) -func refToTempDir(t *testing.T) (ref types.ImageReference, tmpDir string) { - tmpDir, err := ioutil.TempDir("", "dir-transport-test") - require.NoError(t, err) - ref, err = NewReference(tmpDir) +func refToTempDir(t *testing.T) (types.ImageReference, string) { + tmpDir := t.TempDir() + ref, err := NewReference(tmpDir) require.NoError(t, err) return ref, tmpDir } func TestReferenceTransport(t *testing.T) { - ref, tmpDir := refToTempDir(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempDir(t) assert.Equal(t, Transport, ref.Transport()) } func TestReferenceStringWithinTransport(t *testing.T) { ref, tmpDir := refToTempDir(t) - defer os.RemoveAll(tmpDir) assert.Equal(t, tmpDir, ref.StringWithinTransport()) } func TestReferenceDockerReference(t *testing.T) { - ref, tmpDir := refToTempDir(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempDir(t) assert.Nil(t, ref.DockerReference()) } func TestReferencePolicyConfigurationIdentity(t *testing.T) { ref, tmpDir := refToTempDir(t) - defer os.RemoveAll(tmpDir) assert.Equal(t, tmpDir, ref.PolicyConfigurationIdentity()) // A non-canonical path. Test just one, the various other cases are @@ -120,7 +111,6 @@ func TestReferencePolicyConfigurationIdentity(t *testing.T) { func TestReferencePolicyConfigurationNamespaces(t *testing.T) { ref, tmpDir := refToTempDir(t) - defer os.RemoveAll(tmpDir) // We don't really know enough to make a full equality test here. ns := ref.PolicyConfigurationNamespaces() require.NotNil(t, ns) @@ -150,8 +140,7 @@ func TestReferencePolicyConfigurationNamespaces(t *testing.T) { } func TestReferenceNewImage(t *testing.T) { - ref, tmpDir := refToTempDir(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempDir(t) dest, err := ref.NewImageDestination(context.Background(), nil) require.NoError(t, err) @@ -169,8 +158,7 @@ func TestReferenceNewImage(t *testing.T) { } func TestReferenceNewImageNoValidManifest(t *testing.T) { - ref, tmpDir := refToTempDir(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempDir(t) dest, err := ref.NewImageDestination(context.Background(), nil) require.NoError(t, err) @@ -185,24 +173,21 @@ func TestReferenceNewImageNoValidManifest(t *testing.T) { } func TestReferenceNewImageSource(t *testing.T) { - ref, tmpDir := refToTempDir(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempDir(t) src, err := ref.NewImageSource(context.Background(), nil) assert.NoError(t, err) defer src.Close() } func TestReferenceNewImageDestination(t *testing.T) { - ref, tmpDir := refToTempDir(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempDir(t) dest, err := ref.NewImageDestination(context.Background(), nil) assert.NoError(t, err) defer dest.Close() } func TestReferenceDeleteImage(t *testing.T) { - ref, tmpDir := refToTempDir(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempDir(t) err := ref.DeleteImage(context.Background(), nil) assert.Error(t, err) } @@ -211,7 +196,6 @@ func TestReferenceManifestPath(t *testing.T) { dhex := digest.Digest("sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef") ref, tmpDir := refToTempDir(t) - defer os.RemoveAll(tmpDir) dirRef, ok := ref.(dirReference) require.True(t, ok) assert.Equal(t, tmpDir+"/manifest.json", dirRef.manifestPath(nil)) @@ -222,7 +206,6 @@ func TestReferenceLayerPath(t *testing.T) { const hex = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" ref, tmpDir := refToTempDir(t) - defer os.RemoveAll(tmpDir) dirRef, ok := ref.(dirReference) require.True(t, ok) assert.Equal(t, tmpDir+"/"+hex, dirRef.layerPath("sha256:"+hex)) @@ -232,7 +215,6 @@ func TestReferenceSignaturePath(t *testing.T) { dhex := digest.Digest("sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef") ref, tmpDir := refToTempDir(t) - defer os.RemoveAll(tmpDir) dirRef, ok := ref.(dirReference) require.True(t, ok) assert.Equal(t, tmpDir+"/signature-1", dirRef.signaturePath(0, nil)) @@ -243,7 +225,6 @@ func TestReferenceSignaturePath(t *testing.T) { func TestReferenceVersionPath(t *testing.T) { ref, tmpDir := refToTempDir(t) - defer os.RemoveAll(tmpDir) dirRef, ok := ref.(dirReference) require.True(t, ok) assert.Equal(t, tmpDir+"/version", dirRef.versionPath()) diff --git a/directory/explicitfilepath/path_test.go b/directory/explicitfilepath/path_test.go index 3c7434a1b..f1eedcaca 100644 --- a/directory/explicitfilepath/path_test.go +++ b/directory/explicitfilepath/path_test.go @@ -2,7 +2,6 @@ package explicitfilepath import ( "fmt" - "io/ioutil" "os" "path/filepath" "testing" @@ -125,12 +124,10 @@ func testPathsAreSameFile(t *testing.T, path1, path2, description string) { } func runPathResolvingTestCase(t *testing.T, f func(string) (string, error), c pathResolvingTestCase, suffix string) { - topDir, err := ioutil.TempDir("", "pathResolving") - require.NoError(t, err) + topDir := t.TempDir() defer func() { - // Clean up after the "Unreadable directory" case; os.RemoveAll just fails. + // Clean up after the "Unreadable directory" case; os.RemoveAll just fails without this. _ = os.Chmod(filepath.Join(topDir, "unreadable"), 0755) // Ignore errors, especially if this does not exist. - os.RemoveAll(topDir) }() input := c.setup(t, topDir) + suffix // Do not call filepath.Join() on input, it calls filepath.Clean() internally! diff --git a/docker/archive/transport_test.go b/docker/archive/transport_test.go index 2b4b853b0..4c06bdc88 100644 --- a/docker/archive/transport_test.go +++ b/docker/archive/transport_test.go @@ -251,9 +251,7 @@ func TestReferenceNewImageSource(t *testing.T) { } func TestReferenceNewImageDestination(t *testing.T) { - tmpDir, err := ioutil.TempDir("", "docker-archive-test") - require.NoError(t, err) - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() ref, err := ParseReference(filepath.Join(tmpDir, "no-reference")) require.NoError(t, err) @@ -269,9 +267,7 @@ func TestReferenceNewImageDestination(t *testing.T) { } func TestReferenceDeleteImage(t *testing.T) { - tmpDir, err := ioutil.TempDir("", "docker-archive-test") - require.NoError(t, err) - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() for i, suffix := range []string{"", ":some-reference", ":@0"} { testFile := filepath.Join(tmpDir, fmt.Sprintf("file%d.tar", i)) diff --git a/docker/lookaside_test.go b/docker/lookaside_test.go index 28da515d5..ddfff67b6 100644 --- a/docker/lookaside_test.go +++ b/docker/lookaside_test.go @@ -29,9 +29,7 @@ func TestSignatureStorageBaseURL(t *testing.T) { // No match found // expect default user storage base - emptyDir, err := ioutil.TempDir("", "empty-dir") - require.NoError(t, err) - defer os.RemoveAll(emptyDir) + emptyDir := t.TempDir() base, err := SignatureStorageBaseURL(&types.SystemContext{RegistriesDirPath: emptyDir}, dockerRefFromString(t, "//this/is/not/in/the:configuration"), false) assert.NoError(t, err) @@ -55,9 +53,7 @@ func TestRegistriesDirPath(t *testing.T) { const nondefaultPath = "/this/is/not/the/default/registries.d" const variableReference = "$HOME" const rootPrefix = "/root/prefix" - tempHome, err := ioutil.TempDir("", "tempHome") - require.NoError(t, err) - defer os.RemoveAll(tempHome) + tempHome := t.TempDir() var userRegistriesDir = filepath.FromSlash(".config/containers/registries.d") userRegistriesDirPath := filepath.Join(tempHome, userRegistriesDir) for _, c := range []struct { @@ -115,9 +111,7 @@ func TestRegistriesDirPath(t *testing.T) { } func TestLoadAndMergeConfig(t *testing.T) { - tmpDir, err := ioutil.TempDir("", "merge-config") - require.NoError(t, err) - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() // No registries.d exists config, err := loadAndMergeConfig(filepath.Join(tmpDir, "thisdoesnotexist")) diff --git a/oci/archive/oci_transport_test.go b/oci/archive/oci_transport_test.go index 046e2c2b9..88e6c7ab8 100644 --- a/oci/archive/oci_transport_test.go +++ b/oci/archive/oci_transport_test.go @@ -49,9 +49,7 @@ func TestParseReference(t *testing.T) { // testParseReference is a test shared for Transport.ParseReference and ParseReference. func testParseReference(t *testing.T, fn func(string) (types.ImageReference, error)) { - tmpDir, err := ioutil.TempDir("", "oci-transport-test") - require.NoError(t, err) - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() for _, path := range []string{ "/", @@ -76,7 +74,7 @@ func testParseReference(t *testing.T, fn func(string) (types.ImageReference, err } } - _, err = fn(tmpDir + ":invalid'image!value@") + _, err := fn(tmpDir + ":invalid'image!value@") assert.Error(t, err) } @@ -86,9 +84,7 @@ func TestNewReference(t *testing.T) { noImageValue = "" ) - tmpDir, err := ioutil.TempDir("", "oci-transport-test") - require.NoError(t, err) - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() ref, err := NewReference(tmpDir, imageValue) require.NoError(t, err) @@ -115,11 +111,8 @@ func TestNewReference(t *testing.T) { } // refToTempOCI creates a temporary directory and returns an reference to it. -// The caller should -// defer os.RemoveAll(tmpDir) -func refToTempOCI(t *testing.T) (ref types.ImageReference, tmpDir string) { - tmpDir, err := ioutil.TempDir("", "oci-transport-test") - require.NoError(t, err) +func refToTempOCI(t *testing.T) (types.ImageReference, string) { + tmpDir := t.TempDir() m := `{ "schemaVersion": 2, "manifests": [ @@ -138,9 +131,9 @@ func refToTempOCI(t *testing.T) (ref types.ImageReference, tmpDir string) { ] } ` - err = ioutil.WriteFile(filepath.Join(tmpDir, "index.json"), []byte(m), 0644) + err := ioutil.WriteFile(filepath.Join(tmpDir, "index.json"), []byte(m), 0644) require.NoError(t, err) - ref, err = NewReference(tmpDir, "imageValue") + ref, err := NewReference(tmpDir, "imageValue") require.NoError(t, err) return ref, tmpDir } @@ -148,9 +141,7 @@ func refToTempOCI(t *testing.T) (ref types.ImageReference, tmpDir string) { // refToTempOCIArchive creates a temporary directory, copies the contents of that directory // to a temporary tar file and returns a reference to the temporary tar file func refToTempOCIArchive(t *testing.T) (ref types.ImageReference, tmpTarFile string) { - tmpDir, err := ioutil.TempDir("", "oci-transport-test") - defer os.RemoveAll(tmpDir) - require.NoError(t, err) + tmpDir := t.TempDir() m := `{ "schemaVersion": 2, "manifests": [ @@ -169,7 +160,7 @@ func refToTempOCIArchive(t *testing.T) (ref types.ImageReference, tmpTarFile str ] } ` - err = ioutil.WriteFile(filepath.Join(tmpDir, "index.json"), []byte(m), 0644) + err := ioutil.WriteFile(filepath.Join(tmpDir, "index.json"), []byte(m), 0644) require.NoError(t, err) tarFile, err := ioutil.TempFile("", "oci-transport-test.tar") require.NoError(t, err) @@ -181,15 +172,12 @@ func refToTempOCIArchive(t *testing.T) (ref types.ImageReference, tmpTarFile str } func TestReferenceTransport(t *testing.T) { - ref, tmpDir := refToTempOCI(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempOCI(t) assert.Equal(t, Transport, ref.Transport()) } func TestReferenceStringWithinTransport(t *testing.T) { - tmpDir, err := ioutil.TempDir("", "oci-transport-test") - require.NoError(t, err) - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() for _, c := range []struct{ input, result string }{ {"/dir1:notlatest:notlatest", "/dir1:notlatest:notlatest"}, // Explicit image @@ -208,14 +196,12 @@ func TestReferenceStringWithinTransport(t *testing.T) { } func TestReferenceDockerReference(t *testing.T) { - ref, tmpDir := refToTempOCI(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempOCI(t) assert.Nil(t, ref.DockerReference()) } func TestReferencePolicyConfigurationIdentity(t *testing.T) { ref, tmpDir := refToTempOCI(t) - defer os.RemoveAll(tmpDir) assert.Equal(t, tmpDir, ref.PolicyConfigurationIdentity()) // A non-canonical path. Test just one, the various other cases are @@ -232,7 +218,6 @@ func TestReferencePolicyConfigurationIdentity(t *testing.T) { func TestReferencePolicyConfigurationNamespaces(t *testing.T) { ref, tmpDir := refToTempOCI(t) - defer os.RemoveAll(tmpDir) // We don't really know enough to make a full equality test here. ns := ref.PolicyConfigurationNamespaces() require.NotNil(t, ns) @@ -263,8 +248,7 @@ func TestReferencePolicyConfigurationNamespaces(t *testing.T) { } func TestReferenceNewImage(t *testing.T) { - ref, tmpDir := refToTempOCI(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempOCI(t) _, err := ref.NewImage(context.Background(), nil) assert.Error(t, err) } @@ -277,16 +261,14 @@ func TestReferenceNewImageSource(t *testing.T) { } func TestReferenceNewImageDestination(t *testing.T) { - ref, tmpDir := refToTempOCI(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempOCI(t) dest, err := ref.NewImageDestination(context.Background(), nil) assert.NoError(t, err) defer dest.Close() } func TestReferenceDeleteImage(t *testing.T) { - ref, tmpDir := refToTempOCI(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempOCI(t) err := ref.DeleteImage(context.Background(), nil) assert.Error(t, err) } diff --git a/oci/layout/oci_dest_test.go b/oci/layout/oci_dest_test.go index bf092ffdf..f9c3882f0 100644 --- a/oci/layout/oci_dest_test.go +++ b/oci/layout/oci_dest_test.go @@ -29,8 +29,7 @@ func TestPutBlobDigestFailure(t *testing.T) { const digestErrorString = "Simulated digest error" const blobDigest = "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f" - ref, tmpDir := refToTempOCI(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempOCI(t) dirRef, ok := ref.(ociReference) require.True(t, ok) blobPath, err := dirRef.blobPath(blobDigest, "") @@ -70,7 +69,6 @@ func TestPutBlobDigestFailure(t *testing.T) { // TestPutManifestAppendsToExistingManifest tests that new manifests are getting added to existing index. func TestPutManifestAppendsToExistingManifest(t *testing.T) { ref, tmpDir := refToTempOCI(t) - defer os.RemoveAll(tmpDir) ociRef, ok := ref.(ociReference) require.True(t, ok) @@ -94,7 +92,6 @@ func TestPutManifestAppendsToExistingManifest(t *testing.T) { // TestPutManifestTwice tests that existing manifest gets updated and not appended. func TestPutManifestTwice(t *testing.T) { ref, tmpDir := refToTempOCI(t) - defer os.RemoveAll(tmpDir) ociRef, ok := ref.(ociReference) require.True(t, ok) @@ -110,7 +107,6 @@ func TestPutManifestTwice(t *testing.T) { func TestPutTwoDifferentTags(t *testing.T) { ref, tmpDir := refToTempOCI(t) - defer os.RemoveAll(tmpDir) ociRef, ok := ref.(ociReference) require.True(t, ok) diff --git a/oci/layout/oci_transport_test.go b/oci/layout/oci_transport_test.go index 9f4c6d329..cf2584270 100644 --- a/oci/layout/oci_transport_test.go +++ b/oci/layout/oci_transport_test.go @@ -61,9 +61,7 @@ func TestParseReference(t *testing.T) { // testParseReference is a test shared for Transport.ParseReference and ParseReference. func testParseReference(t *testing.T, fn func(string) (types.ImageReference, error)) { - tmpDir, err := ioutil.TempDir("", "oci-transport-test") - require.NoError(t, err) - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() for _, path := range []string{ "/", @@ -88,7 +86,7 @@ func testParseReference(t *testing.T, fn func(string) (types.ImageReference, err } } - _, err = fn(tmpDir + ":invalid'image!value@") + _, err := fn(tmpDir + ":invalid'image!value@") assert.Error(t, err) } @@ -98,9 +96,7 @@ func TestNewReference(t *testing.T) { noImageValue = "" ) - tmpDir, err := ioutil.TempDir("", "oci-transport-test") - require.NoError(t, err) - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() ref, err := NewReference(tmpDir, imageValue) require.NoError(t, err) @@ -127,11 +123,8 @@ func TestNewReference(t *testing.T) { } // refToTempOCI creates a temporary directory and returns an reference to it. -// The caller should -// defer os.RemoveAll(tmpDir) -func refToTempOCI(t *testing.T) (ref types.ImageReference, tmpDir string) { - tmpDir, err := ioutil.TempDir("", "oci-transport-test") - require.NoError(t, err) +func refToTempOCI(t *testing.T) (types.ImageReference, string) { + tmpDir := t.TempDir() m := `{ "schemaVersion": 2, "manifests": [ @@ -150,23 +143,20 @@ func refToTempOCI(t *testing.T) (ref types.ImageReference, tmpDir string) { ] } ` - err = ioutil.WriteFile(filepath.Join(tmpDir, "index.json"), []byte(m), 0644) + err := ioutil.WriteFile(filepath.Join(tmpDir, "index.json"), []byte(m), 0644) require.NoError(t, err) - ref, err = NewReference(tmpDir, "imageValue") + ref, err := NewReference(tmpDir, "imageValue") require.NoError(t, err) return ref, tmpDir } func TestReferenceTransport(t *testing.T) { - ref, tmpDir := refToTempOCI(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempOCI(t) assert.Equal(t, Transport, ref.Transport()) } func TestReferenceStringWithinTransport(t *testing.T) { - tmpDir, err := ioutil.TempDir("", "oci-transport-test") - require.NoError(t, err) - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() for _, c := range []struct{ input, result string }{ {"/dir1:notlatest:notlatest", "/dir1:notlatest:notlatest"}, // Explicit image @@ -185,14 +175,12 @@ func TestReferenceStringWithinTransport(t *testing.T) { } func TestReferenceDockerReference(t *testing.T) { - ref, tmpDir := refToTempOCI(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempOCI(t) assert.Nil(t, ref.DockerReference()) } func TestReferencePolicyConfigurationIdentity(t *testing.T) { ref, tmpDir := refToTempOCI(t) - defer os.RemoveAll(tmpDir) assert.Equal(t, tmpDir, ref.PolicyConfigurationIdentity()) // A non-canonical path. Test just one, the various other cases are @@ -209,7 +197,6 @@ func TestReferencePolicyConfigurationIdentity(t *testing.T) { func TestReferencePolicyConfigurationNamespaces(t *testing.T) { ref, tmpDir := refToTempOCI(t) - defer os.RemoveAll(tmpDir) // We don't really know enough to make a full equality test here. ns := ref.PolicyConfigurationNamespaces() require.NotNil(t, ns) @@ -240,37 +227,32 @@ func TestReferencePolicyConfigurationNamespaces(t *testing.T) { } func TestReferenceNewImage(t *testing.T) { - ref, tmpDir := refToTempOCI(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempOCI(t) _, err := ref.NewImage(context.Background(), nil) assert.Error(t, err) } func TestReferenceNewImageSource(t *testing.T) { - ref, tmpDir := refToTempOCI(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempOCI(t) _, err := ref.NewImageSource(context.Background(), nil) assert.NoError(t, err) } func TestReferenceNewImageDestination(t *testing.T) { - ref, tmpDir := refToTempOCI(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempOCI(t) dest, err := ref.NewImageDestination(context.Background(), nil) assert.NoError(t, err) defer dest.Close() } func TestReferenceDeleteImage(t *testing.T) { - ref, tmpDir := refToTempOCI(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempOCI(t) err := ref.DeleteImage(context.Background(), nil) assert.Error(t, err) } func TestReferenceOCILayoutPath(t *testing.T) { ref, tmpDir := refToTempOCI(t) - defer os.RemoveAll(tmpDir) ociRef, ok := ref.(ociReference) require.True(t, ok) assert.Equal(t, tmpDir+"/oci-layout", ociRef.ociLayoutPath()) @@ -278,7 +260,6 @@ func TestReferenceOCILayoutPath(t *testing.T) { func TestReferenceIndexPath(t *testing.T) { ref, tmpDir := refToTempOCI(t) - defer os.RemoveAll(tmpDir) ociRef, ok := ref.(ociReference) require.True(t, ok) assert.Equal(t, tmpDir+"/index.json", ociRef.indexPath()) @@ -288,7 +269,6 @@ func TestReferenceBlobPath(t *testing.T) { const hex = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" ref, tmpDir := refToTempOCI(t) - defer os.RemoveAll(tmpDir) ociRef, ok := ref.(ociReference) require.True(t, ok) bp, err := ociRef.blobPath("sha256:"+hex, "") @@ -299,8 +279,7 @@ func TestReferenceBlobPath(t *testing.T) { func TestReferenceSharedBlobPathShared(t *testing.T) { const hex = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" - ref, tmpDir := refToTempOCI(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempOCI(t) ociRef, ok := ref.(ociReference) require.True(t, ok) bp, err := ociRef.blobPath("sha256:"+hex, "/external/path") @@ -311,8 +290,7 @@ func TestReferenceSharedBlobPathShared(t *testing.T) { func TestReferenceBlobPathInvalid(t *testing.T) { const hex = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" - ref, tmpDir := refToTempOCI(t) - defer os.RemoveAll(tmpDir) + ref, _ := refToTempOCI(t) ociRef, ok := ref.(ociReference) require.True(t, ok) _, err := ociRef.blobPath(hex, "") diff --git a/ostree/ostree_transport_test.go b/ostree/ostree_transport_test.go index 6eabc13be..da2f8031b 100644 --- a/ostree/ostree_transport_test.go +++ b/ostree/ostree_transport_test.go @@ -55,9 +55,7 @@ var imageNameTestcases = []struct{ input, normalized, branchName string }{ } func TestTransportParseReference(t *testing.T) { - tmpDir, err := ioutil.TempDir("", "ostreeParseReference") - require.NoError(t, err) - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() for _, c := range imageNameTestcases { for _, suffix := range repoSuffixes { @@ -104,9 +102,7 @@ func TestTransportValidatePolicyConfigurationScope(t *testing.T) { } func TestNewReference(t *testing.T) { - tmpDir, err := ioutil.TempDir("", "ostreeNewReference") - require.NoError(t, err) - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() for _, c := range imageNameTestcases { for _, suffix := range repoSuffixes { @@ -161,9 +157,7 @@ func TestReferenceTransport(t *testing.T) { } func TestReferenceStringWithinTransport(t *testing.T) { - tmpDir, err := ioutil.TempDir("", "ostreeStringWithinTransport") - require.NoError(t, err) - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() for _, c := range validReferenceTestCases { ref, err := Transport.ParseReference(withTmpDir(c.input, tmpDir)) @@ -179,9 +173,7 @@ func TestReferenceStringWithinTransport(t *testing.T) { } func TestReferenceDockerReference(t *testing.T) { - tmpDir, err := ioutil.TempDir("", "ostreeDockerReference") - require.NoError(t, err) - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() for _, c := range validReferenceTestCases { ref, err := Transport.ParseReference(withTmpDir(c.input, tmpDir)) @@ -192,9 +184,7 @@ func TestReferenceDockerReference(t *testing.T) { } func TestReferencePolicyConfigurationIdentity(t *testing.T) { - tmpDir, err := ioutil.TempDir("", "ostreePolicyConfigurationIdentity") - require.NoError(t, err) - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() for _, c := range validReferenceTestCases { ref, err := Transport.ParseReference(withTmpDir(c.input, tmpDir)) @@ -204,9 +194,7 @@ func TestReferencePolicyConfigurationIdentity(t *testing.T) { } func TestReferencePolicyConfigurationNamespaces(t *testing.T) { - tmpDir, err := ioutil.TempDir("", "ostreePolicyConfigurationNamespaces") - require.NoError(t, err) - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() // Test both that DockerReferenceIdentity returns the expected value (fullName+suffix), // and that DockerReferenceNamespaces starts with the expected value (fullName), i.e. that the two functions are @@ -257,9 +245,7 @@ func TestReferenceNewImageSource(t *testing.T) { } func TestReferenceNewImageDestination(t *testing.T) { - otherTmpDir, err := ioutil.TempDir("", "ostree-transport-test") - require.NoError(t, err) - defer os.RemoveAll(otherTmpDir) + otherTmpDir := t.TempDir() for _, c := range []struct { sys *types.SystemContext @@ -281,9 +267,7 @@ func TestReferenceNewImageDestination(t *testing.T) { } func TestReferenceDeleteImage(t *testing.T) { - tmpDir, err := ioutil.TempDir("", "ostreeDeleteImage") - require.NoError(t, err) - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() ref, err := Transport.ParseReference(withTmpDir("busybox@$TMP/this-repo-does-not-exist", tmpDir)) require.NoError(t, err) diff --git a/pkg/blobcache/blobcache_test.go b/pkg/blobcache/blobcache_test.go index 2393350c3..b9fad2606 100644 --- a/pkg/blobcache/blobcache_test.go +++ b/pkg/blobcache/blobcache_test.go @@ -58,15 +58,7 @@ func makeLayer(filename string, repeat int, compression archive.Compression) ([] } func TestBlobCache(t *testing.T) { - cacheDir, err := ioutil.TempDir("", "blobcache") - if err != nil { - t.Fatalf("error creating persistent cache directory: %v", err) - } - defer func() { - if err := os.RemoveAll(cacheDir); err != nil { - t.Fatalf("error removing persistent cache directory %q: %v", cacheDir, err) - } - }() + cacheDir := t.TempDir() systemContext := types.SystemContext{} @@ -119,11 +111,7 @@ func TestBlobCache(t *testing.T) { t.Fatalf("error encoding image manifest: %v", err) } // Write this image to a "dir" destination with blob caching using this directory. - srcdir, err := ioutil.TempDir("", "blobcache-source") - if err != nil { - t.Fatalf("error creating temporary source directory: %v", err) - } - defer os.RemoveAll(srcdir) + srcdir := t.TempDir() srcRef, err := directory.NewReference(srcdir) if err != nil { t.Fatalf("error creating source image name reference for %q: %v", srcdir, err) @@ -215,11 +203,7 @@ func TestBlobCache(t *testing.T) { } // Now that we've deleted some of the contents, try to copy from the source image // to a second image. It should fail because the source is missing some blobs. - destdir, err := ioutil.TempDir("", "blobcache-destination") - if err != nil { - t.Fatalf("error creating temporary destination directory: %v", err) - } - defer os.RemoveAll(destdir) + destdir := t.TempDir() destRef, err := directory.NewReference(destdir) if err != nil { t.Fatalf("error creating destination image reference for %q: %v", destdir, err) diff --git a/pkg/blobinfocache/boltdb/boltdb_test.go b/pkg/blobinfocache/boltdb/boltdb_test.go index a91ab6d7d..e53f6e9dd 100644 --- a/pkg/blobinfocache/boltdb/boltdb_test.go +++ b/pkg/blobinfocache/boltdb/boltdb_test.go @@ -1,14 +1,11 @@ package boltdb import ( - "io/ioutil" - "os" "path/filepath" "testing" "github.com/containers/image/v5/internal/blobinfocache" "github.com/containers/image/v5/pkg/blobinfocache/internal/test" - "github.com/stretchr/testify/require" ) var _ blobinfocache.BlobInfoCache2 = &cache{} @@ -18,12 +15,8 @@ func newTestCache(t *testing.T) (blobinfocache.BlobInfoCache2, func(t *testing.T // an existing but empty file, and incorrectly fails without releasing the lock - which in turn causes // any future writes to hang. Creating a temporary directory allows us to use a path to a // non-existent file, thus replicating the expected conditions for creating a new DB. - dir, err := ioutil.TempDir("", "boltdb") - require.NoError(t, err) - return new2(filepath.Join(dir, "db")), func(t *testing.T) { - err = os.RemoveAll(dir) - require.NoError(t, err) - } + dir := t.TempDir() + return new2(filepath.Join(dir, "db")), func(t *testing.T) {} } func TestNew(t *testing.T) { diff --git a/pkg/blobinfocache/default_test.go b/pkg/blobinfocache/default_test.go index 3ade76fd9..bc3ca8ebb 100644 --- a/pkg/blobinfocache/default_test.go +++ b/pkg/blobinfocache/default_test.go @@ -1,7 +1,6 @@ package blobinfocache import ( - "io/ioutil" "os" "path/filepath" "testing" @@ -115,9 +114,7 @@ func TestBlobInfoCacheDir(t *testing.T) { } func TestDefaultCache(t *testing.T) { - tmpDir, err := ioutil.TempDir("", "TestDefaultCache") - require.NoError(t, err) - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() // Success normalDir := filepath.Join(tmpDir, "normal") @@ -152,7 +149,7 @@ func TestDefaultCache(t *testing.T) { // Error creating the parent directory: unwritableDir := filepath.Join(tmpDir, "unwritable") - err = os.Mkdir(unwritableDir, 0700) + err := os.Mkdir(unwritableDir, 0700) require.NoError(t, err) defer func() { err = os.Chmod(unwritableDir, 0700) // To make it possible to remove it again diff --git a/pkg/docker/config/config_test.go b/pkg/docker/config/config_test.go index f6786a640..0f5e1ea4f 100644 --- a/pkg/docker/config/config_test.go +++ b/pkg/docker/config/config_test.go @@ -24,9 +24,7 @@ func TestGetPathToAuth(t *testing.T) { // on any state of the filesystem. darwinDefault := filepath.Join(os.Getenv("HOME"), ".config", "containers", "auth.json") - tmpDir, err := ioutil.TempDir("", "TestGetPathToAuth") - require.NoError(t, err) - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() // Environment is per-process, so this looks very unsafe; actually it seems fine because tests are not // run in parallel unless they opt in by calling t.Parallel(). So don’t do that. @@ -82,20 +80,11 @@ func TestGetPathToAuth(t *testing.T) { func TestGetAuth(t *testing.T) { origXDG := os.Getenv("XDG_RUNTIME_DIR") - tmpXDGRuntimeDir, err := ioutil.TempDir("", "test_docker_client_get_auth") - if err != nil { - t.Fatal(err) - } + tmpXDGRuntimeDir := t.TempDir() t.Logf("using temporary XDG_RUNTIME_DIR directory: %q", tmpXDGRuntimeDir) // override XDG_RUNTIME_DIR os.Setenv("XDG_RUNTIME_DIR", tmpXDGRuntimeDir) - defer func() { - err := os.RemoveAll(tmpXDGRuntimeDir) - if err != nil { - t.Logf("failed to cleanup temporary home directory %q: %v", tmpXDGRuntimeDir, err) - } - os.Setenv("XDG_RUNTIME_DIR", origXDG) - }() + defer os.Setenv("XDG_RUNTIME_DIR", origXDG) // override PATH for executing credHelper curtDir, err := os.Getwd() @@ -108,17 +97,8 @@ func TestGetAuth(t *testing.T) { os.Setenv("PATH", origPath) }() - tmpHomeDir, err := ioutil.TempDir("", "test_docker_client_get_auth") - if err != nil { - t.Fatal(err) - } + tmpHomeDir := t.TempDir() t.Logf("using temporary home directory: %q", tmpHomeDir) - defer func() { - err := os.RemoveAll(tmpHomeDir) - if err != nil { - t.Logf("failed to cleanup temporary home directory %q: %v", tmpHomeDir, err) - } - }() configDir1 := filepath.Join(tmpXDGRuntimeDir, "containers") if runtime.GOOS == "windows" || runtime.GOOS == "darwin" { @@ -338,17 +318,8 @@ func TestGetAuth(t *testing.T) { } func TestGetAuthFromLegacyFile(t *testing.T) { - tmpDir, err := ioutil.TempDir("", "test_docker_client_get_auth") - if err != nil { - t.Fatal(err) - } + tmpDir := t.TempDir() t.Logf("using temporary home directory: %q", tmpDir) - defer func() { - err := os.RemoveAll(tmpDir) - if err != nil { - t.Logf("failed to cleanup temporary home directory %q: %v", tmpDir, err) - } - }() configPath := filepath.Join(tmpDir, ".dockercfg") contents, err := ioutil.ReadFile(filepath.Join("testdata", "legacy.json")) @@ -397,17 +368,8 @@ func TestGetAuthFromLegacyFile(t *testing.T) { } func TestGetAuthPreferNewConfig(t *testing.T) { - tmpDir, err := ioutil.TempDir("", "test_docker_client_get_auth") - if err != nil { - t.Fatal(err) - } + tmpDir := t.TempDir() t.Logf("using temporary home directory: %q", tmpDir) - defer func() { - err := os.RemoveAll(tmpDir) - if err != nil { - t.Logf("failed to cleanup temporary home directory %q: %v", tmpDir, err) - } - }() configDir := filepath.Join(tmpDir, ".docker") if err := os.Mkdir(configDir, 0750); err != nil { @@ -445,32 +407,14 @@ func TestGetAuthPreferNewConfig(t *testing.T) { func TestGetAuthFailsOnBadInput(t *testing.T) { origXDG := os.Getenv("XDG_RUNTIME_DIR") - tmpXDGRuntimeDir, err := ioutil.TempDir("", "test_docker_client_get_auth") - if err != nil { - t.Fatal(err) - } + tmpXDGRuntimeDir := t.TempDir() t.Logf("using temporary XDG_RUNTIME_DIR directory: %q", tmpXDGRuntimeDir) // override XDG_RUNTIME_DIR os.Setenv("XDG_RUNTIME_DIR", tmpXDGRuntimeDir) - defer func() { - err := os.RemoveAll(tmpXDGRuntimeDir) - if err != nil { - t.Logf("failed to cleanup temporary home directory %q: %v", tmpXDGRuntimeDir, err) - } - os.Setenv("XDG_RUNTIME_DIR", origXDG) - }() + defer os.Setenv("XDG_RUNTIME_DIR", origXDG) - tmpHomeDir, err := ioutil.TempDir("", "test_docker_client_get_auth") - if err != nil { - t.Fatal(err) - } + tmpHomeDir := t.TempDir() t.Logf("using temporary home directory: %q", tmpHomeDir) - defer func() { - err := os.RemoveAll(tmpHomeDir) - if err != nil { - t.Logf("failed to cleanup temporary home directory %q: %v", tmpHomeDir, err) - } - }() configDir := filepath.Join(tmpXDGRuntimeDir, "containers") if runtime.GOOS == "windows" || runtime.GOOS == "darwin" { @@ -890,9 +834,7 @@ func TestSetGetCredentials(t *testing.T) { password = "password" ) - tmpDir, err := ioutil.TempDir("", "auth-test-") - require.NoError(t, err) - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() for _, tc := range []struct { name string diff --git a/pkg/sysregistriesv2/system_registries_v2_test.go b/pkg/sysregistriesv2/system_registries_v2_test.go index 5ed7b6351..e93db41ee 100644 --- a/pkg/sysregistriesv2/system_registries_v2_test.go +++ b/pkg/sysregistriesv2/system_registries_v2_test.go @@ -225,9 +225,7 @@ func TestNewConfigWrapper(t *testing.T) { const nondefaultPath = "/this/is/not/the/default/registries.conf" const variableReference = "$HOME" const rootPrefix = "/root/prefix" - tempHome, err := ioutil.TempDir("", "tempHome") - require.NoError(t, err) - defer os.RemoveAll(tempHome) + tempHome := t.TempDir() var userRegistriesFile = filepath.FromSlash(".config/containers/registries.conf") userRegistriesFilePath := filepath.Join(tempHome, userRegistriesFile) diff --git a/pkg/tlsclientconfig/tlsclientconfig_test.go b/pkg/tlsclientconfig/tlsclientconfig_test.go index 979eb91ae..3b65fe022 100644 --- a/pkg/tlsclientconfig/tlsclientconfig_test.go +++ b/pkg/tlsclientconfig/tlsclientconfig_test.go @@ -5,7 +5,6 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/asn1" - "io/ioutil" "os" "sort" "testing" @@ -93,11 +92,9 @@ func TestSetupCertificates(t *testing.T) { assert.Equal(t, &tls.Config{}, &tlsc) // Directory not accessible - unreadableDir, err := ioutil.TempDir("", "containers-image-tlsclientconfig") - require.NoError(t, err) + unreadableDir := t.TempDir() defer func() { _ = os.Chmod(unreadableDir, 0700) - _ = os.Remove(unreadableDir) }() err = os.Chmod(unreadableDir, 000) require.NoError(t, err) diff --git a/sif/transport_test.go b/sif/transport_test.go index 3ac6b79bd..c68dd045f 100644 --- a/sif/transport_test.go +++ b/sif/transport_test.go @@ -49,11 +49,9 @@ func TestNewReference(t *testing.T) { // testNewReference is a test shared for Transport.ParseReference and NewReference. func testNewReference(t *testing.T, fn func(string) (types.ImageReference, error)) { - tmpDir, err := ioutil.TempDir("", "sif-transport-test") - require.NoError(t, err) - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() tmpFile := filepath.Join(tmpDir, "image.sif") - err = ioutil.WriteFile(tmpFile, nil, 0600) + err := ioutil.WriteFile(tmpFile, nil, 0600) require.NoError(t, err) for _, file := range []string{ diff --git a/signature/mechanism_test.go b/signature/mechanism_test.go index a503223c2..4b5cf23dc 100644 --- a/signature/mechanism_test.go +++ b/signature/mechanism_test.go @@ -63,9 +63,7 @@ func TestNewGPGSigningMechanismInDirectory(t *testing.T) { // Similarly, using a newly created empty directory makes TestKeyFingerprint // unavailable - emptyDir, err := ioutil.TempDir("", "signing-empty-directory") - require.NoError(t, err) - defer os.RemoveAll(emptyDir) + emptyDir := t.TempDir() mech, err = newGPGSigningMechanismInDirectory(emptyDir) require.NoError(t, err) defer mech.Close() @@ -77,9 +75,7 @@ func TestNewGPGSigningMechanismInDirectory(t *testing.T) { // If pubring.gpg is unreadable in the directory, either initializing // the mechanism fails (with openpgp), or it succeeds (sadly, gpgme) and // later verification fails. - unreadableDir, err := ioutil.TempDir("", "signing-unreadable-directory") - require.NoError(t, err) - defer os.RemoveAll(unreadableDir) + unreadableDir := t.TempDir() f, err := os.OpenFile(filepath.Join(unreadableDir, "pubring.gpg"), os.O_RDONLY|os.O_CREATE, 0000) require.NoError(t, err) f.Close() diff --git a/signature/policy_config_test.go b/signature/policy_config_test.go index 96141d746..68890b275 100644 --- a/signature/policy_config_test.go +++ b/signature/policy_config_test.go @@ -120,9 +120,7 @@ func TestDefaultPolicyPath(t *testing.T) { const nondefaultPath = "/this/is/not/the/default/path.json" const variableReference = "$HOME" const rootPrefix = "/root/prefix" - tempHome, err := ioutil.TempDir("", "tempHome") - require.NoError(t, err) - defer os.RemoveAll(tempHome) + tempHome := t.TempDir() userDefaultPolicyPath := filepath.Join(tempHome, userPolicyFile) for _, c := range []struct { diff --git a/signature/policy_eval_signedby_test.go b/signature/policy_eval_signedby_test.go index 34a52d1b2..87a3d9a5b 100644 --- a/signature/policy_eval_signedby_test.go +++ b/signature/policy_eval_signedby_test.go @@ -184,11 +184,9 @@ func TestPRSignedByIsSignatureAuthorAccepted(t *testing.T) { // createInvalidSigDir creates a directory suitable for dirImageMock, in which image.Signatures() // fails. -// The caller should eventually call os.RemoveAll on the returned path. func createInvalidSigDir(t *testing.T) string { - dir, err := ioutil.TempDir("", "skopeo-test-unreadable-signature") - require.NoError(t, err) - err = ioutil.WriteFile(path.Join(dir, "manifest.json"), []byte("{}"), 0644) + dir := t.TempDir() + err := ioutil.WriteFile(path.Join(dir, "manifest.json"), []byte("{}"), 0644) require.NoError(t, err) // Creating a 000-permissions file would work for unprivileged accounts, but root (in particular, // in the Docker container we use for testing) would still have access. So, create a symlink @@ -212,7 +210,6 @@ func TestPRSignedByIsRunningImageAllowed(t *testing.T) { // Error reading signatures invalidSigDir := createInvalidSigDir(t) - defer os.RemoveAll(invalidSigDir) image = dirImageMock(t, invalidSigDir, "testing/manifest:latest") pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) require.NoError(t, err) diff --git a/signature/policy_eval_test.go b/signature/policy_eval_test.go index c56be7de0..db86f6f5a 100644 --- a/signature/policy_eval_test.go +++ b/signature/policy_eval_test.go @@ -3,7 +3,6 @@ package signature import ( "context" "fmt" - "os" "testing" "github.com/containers/image/v5/docker" @@ -355,7 +354,6 @@ func TestPolicyContextGetSignaturesWithAcceptedAuthor(t *testing.T) { // Error reading signatures. invalidSigDir := createInvalidSigDir(t) - defer os.RemoveAll(invalidSigDir) img = pcImageMock(t, invalidSigDir, "testing/manifest:latest") sigs, err = pc.GetSignaturesWithAcceptedAuthor(context.Background(), img) assert.Error(t, err) diff --git a/storage/storage_test.go b/storage/storage_test.go index ecfe9b239..19cd0b3e2 100644 --- a/storage/storage_test.go +++ b/storage/storage_test.go @@ -37,12 +37,11 @@ import ( ) var ( - topwd = "" - _ types.ImageDestination = &storageImageDestination{} - _ private.ImageDestination = (*storageImageDestination)(nil) - _ types.ImageSource = &storageImageSource{} - _ types.ImageReference = &storageReference{} - _ types.ImageTransport = &storageTransport{} + _ types.ImageDestination = &storageImageDestination{} + _ private.ImageDestination = (*storageImageDestination)(nil) + _ types.ImageSource = &storageImageSource{} + _ types.ImageReference = &storageReference{} + _ types.ImageTransport = &storageTransport{} ) const ( @@ -53,31 +52,17 @@ func TestMain(m *testing.M) { if reexec.Init() { return } - wd, err := ioutil.TempDir("", "test.") - if err != nil { - os.Exit(1) - } - topwd = wd debug := false flag.BoolVar(&debug, "debug", false, "print debug statements") flag.Parse() if debug { logrus.SetLevel(logrus.DebugLevel) } - code := m.Run() - os.RemoveAll(wd) - os.Exit(code) + os.Exit(m.Run()) } func newStoreWithGraphDriverOptions(t *testing.T, options []string) storage.Store { - wd, err := ioutil.TempDir(topwd, "test.") - if err != nil { - t.Fatal(err) - } - err = os.MkdirAll(wd, 0700) - if err != nil { - t.Fatal(err) - } + wd := t.TempDir() run := filepath.Join(wd, "run") root := filepath.Join(wd, "root") Transport.SetDefaultUIDMap([]idtools.IDMap{{ From a310b13598f5b51f4ceebdb0fdd02cb23a127c7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 16 Mar 2022 00:39:06 +0100 Subject: [PATCH 6/6] Simplify blobifocache/internal/test.GenericCache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We no longer need a cleanup callback. Signed-off-by: Miloslav Trmač --- pkg/blobinfocache/boltdb/boltdb_test.go | 4 ++-- pkg/blobinfocache/internal/test/test.go | 7 +++---- pkg/blobinfocache/memory/memory_test.go | 4 ++-- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/pkg/blobinfocache/boltdb/boltdb_test.go b/pkg/blobinfocache/boltdb/boltdb_test.go index e53f6e9dd..50b478459 100644 --- a/pkg/blobinfocache/boltdb/boltdb_test.go +++ b/pkg/blobinfocache/boltdb/boltdb_test.go @@ -10,13 +10,13 @@ import ( var _ blobinfocache.BlobInfoCache2 = &cache{} -func newTestCache(t *testing.T) (blobinfocache.BlobInfoCache2, func(t *testing.T)) { +func newTestCache(t *testing.T) blobinfocache.BlobInfoCache2 { // We need a separate temporary directory here, because bolt.Open(…, &bolt.Options{Readonly:true}) can't deal with // an existing but empty file, and incorrectly fails without releasing the lock - which in turn causes // any future writes to hang. Creating a temporary directory allows us to use a path to a // non-existent file, thus replicating the expected conditions for creating a new DB. dir := t.TempDir() - return new2(filepath.Join(dir, "db")), func(t *testing.T) {} + return new2(filepath.Join(dir, "db")) } func TestNew(t *testing.T) { diff --git a/pkg/blobinfocache/internal/test/test.go b/pkg/blobinfocache/internal/test/test.go index 4e7c1d06d..ce4ce5e30 100644 --- a/pkg/blobinfocache/internal/test/test.go +++ b/pkg/blobinfocache/internal/test/test.go @@ -24,8 +24,8 @@ const ( ) // GenericCache runs an implementation-independent set of tests, given a -// newTestCache, which can be called repeatedly and always returns a (cache, cleanup callback) pair -func GenericCache(t *testing.T, newTestCache func(t *testing.T) (blobinfocache.BlobInfoCache2, func(t *testing.T))) { +// newTestCache, which can be called repeatedly and always returns a fresh cache instance +func GenericCache(t *testing.T, newTestCache func(t *testing.T) blobinfocache.BlobInfoCache2) { for _, s := range []struct { name string fn func(t *testing.T, cache blobinfocache.BlobInfoCache2) @@ -37,8 +37,7 @@ func GenericCache(t *testing.T, newTestCache func(t *testing.T) (blobinfocache.B {"CandidateLocations2", testGenericCandidateLocations2}, } { t.Run(s.name, func(t *testing.T) { - cache, cleanup := newTestCache(t) - defer cleanup(t) + cache := newTestCache(t) s.fn(t, cache) }) } diff --git a/pkg/blobinfocache/memory/memory_test.go b/pkg/blobinfocache/memory/memory_test.go index bc8d5035f..322f008f3 100644 --- a/pkg/blobinfocache/memory/memory_test.go +++ b/pkg/blobinfocache/memory/memory_test.go @@ -9,8 +9,8 @@ import ( var _ blobinfocache.BlobInfoCache2 = &cache{} -func newTestCache(t *testing.T) (blobinfocache.BlobInfoCache2, func(t *testing.T)) { - return new2(), func(t *testing.T) {} +func newTestCache(t *testing.T) blobinfocache.BlobInfoCache2 { + return new2() } func TestNew(t *testing.T) {