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/docker_client.go b/docker/docker_client.go index 833323b42..9837235d8 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.Redacted(), delay.Seconds()) select { case <-ctx.Done(): return nil, ctx.Err() @@ -533,12 +537,12 @@ 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 } - 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") @@ -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.Redacted()) res, err := c.client.Do(req) if err != nil { return nil, err @@ -653,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 @@ -705,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 @@ -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.Redacted(), 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.Redacted(), 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.Redacted(), 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.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 e7af8f93d..e3275aa45 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 } @@ -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,8 +275,8 @@ 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()) - res2, err := d.c.makeRequestToResolvedURL(ctx, http.MethodDelete, uploadLocation.String(), nil, nil, -1, v2Auth, extraScope) + 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) } else { @@ -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 cb520d670..c08e5538a 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)) @@ -524,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 @@ -537,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 { @@ -546,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/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/go.mod b/go.mod index a94854c86..201f237c3 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/containers/image/v5 -go 1.13 +go 1.15 require ( github.com/14rcole/gopopulate v0.0.0-20180821133914-b175b219e774 // indirect 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/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/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 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..50b478459 100644 --- a/pkg/blobinfocache/boltdb/boltdb_test.go +++ b/pkg/blobinfocache/boltdb/boltdb_test.go @@ -1,29 +1,22 @@ 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{} -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, 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 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/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) { 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 2620a586f..87a3d9a5b 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) @@ -189,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 @@ -209,8 +202,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) @@ -218,49 +210,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..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" @@ -215,8 +214,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 +266,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 +344,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) @@ -369,9 +354,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 +393,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 +448,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 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{{