Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Formally require Go 1.15 #1493

Merged
merged 6 commits into from
Mar 19, 2022
Merged

Formally require Go 1.15 #1493

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d2ea8f1
Update an obsolete comment
mtrmac Mar 15, 2022
f0d818b
Use Testing.T.Cleanup() to simplify tests
mtrmac Mar 15, 2022
fa54b28
Modify makeRequestToResolvedURL and makeRequestToResolvedURLOnce to a…
mtrmac Mar 15, 2022
5a4b8a4
Use url.Redacted() in log output
mtrmac Mar 15, 2022
9a99049
Use testing.T.TempDir()
mtrmac Mar 15, 2022
a310b13
Simplify blobifocache/internal/test.GenericCache
mtrmac Mar 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions copy/sign_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
14 changes: 4 additions & 10 deletions directory/directory_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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")
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand Down
43 changes: 12 additions & 31 deletions directory/directory_transport_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
"/",
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
}
Expand All @@ -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))
Expand All @@ -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))
Expand All @@ -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))
Expand All @@ -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())
Expand Down
7 changes: 2 additions & 5 deletions directory/explicitfilepath/path_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package explicitfilepath

import (
"fmt"
"io/ioutil"
"os"
"path/filepath"
"testing"
Expand Down Expand Up @@ -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!
Expand Down
8 changes: 2 additions & 6 deletions docker/archive/transport_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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))
Expand Down
40 changes: 25 additions & 15 deletions docker/docker_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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 {
Expand All @@ -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()
Expand All @@ -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")
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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, "")
}
Expand All @@ -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
}
Expand Down
Loading