From f9f909dd38f940d5cf41f73a07b0df884eac8e99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 8 Jun 2022 16:01:25 +0200 Subject: [PATCH 1/3] Update go.mod to Go 1.17 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > go mod tidy -go=1.17 Signed-off-by: Miloslav Trmač --- go.mod | 74 ++++++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 59 insertions(+), 15 deletions(-) diff --git a/go.mod b/go.mod index 3e875d325e..305d0d8d2d 100644 --- a/go.mod +++ b/go.mod @@ -1,13 +1,9 @@ module github.com/containers/image/v5 -go 1.16 +go 1.17 require ( - github.com/14rcole/gopopulate v0.0.0-20180821133914-b175b219e774 // indirect - github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect github.com/BurntSushi/toml v1.1.0 - github.com/cespare/xxhash/v2 v2.1.2 // indirect - github.com/containerd/cgroups v1.0.3 // indirect github.com/containers/libtrust v0.0.0-20200511145503-9c3a6c22cd9a github.com/containers/ocicrypt v1.1.5 github.com/containers/storage v1.41.0 @@ -15,41 +11,89 @@ require ( github.com/docker/docker v20.10.17+incompatible github.com/docker/docker-credential-helpers v0.6.4 github.com/docker/go-connections v0.4.0 - github.com/docker/libtrust v0.0.0-20160708172513-aabc10ec26b7 // indirect github.com/ghodss/yaml v1.0.0 - github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect - github.com/gorilla/mux v1.7.4 // indirect - github.com/hashicorp/errwrap v1.1.0 // indirect github.com/hashicorp/go-multierror v1.1.1 github.com/imdario/mergo v0.3.13 github.com/klauspost/compress v1.15.6 github.com/klauspost/pgzip v1.2.5 github.com/manifoldco/promptui v0.9.0 - github.com/moby/term v0.0.0-20210610120745-9d4ed1856297 // indirect github.com/opencontainers/go-digest v1.0.0 github.com/opencontainers/image-spec v1.0.3-0.20211202193544-a5463b7f9c84 - github.com/opencontainers/runc v1.1.2 // indirect github.com/opencontainers/selinux v1.10.1 github.com/ostreedev/ostree-go v0.0.0-20210805093236-719684c64e4f github.com/pkg/errors v0.9.1 github.com/proglottis/gpgme v0.1.2 - github.com/prometheus/common v0.30.0 // indirect - github.com/prometheus/procfs v0.7.3 // indirect github.com/sirupsen/logrus v1.8.1 github.com/stretchr/testify v1.7.2 github.com/sylabs/sif/v2 v2.7.1 github.com/ulikunitz/xz v0.5.10 github.com/vbatts/tar-split v0.11.2 github.com/vbauerster/mpb/v7 v7.4.2 - github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect github.com/xeipuuv/gojsonschema v1.2.0 go.etcd.io/bbolt v1.3.6 - go.opencensus.io v0.23.0 // indirect golang.org/x/crypto v0.0.0-20211215153901-e495a2d5b3d3 golang.org/x/net v0.0.0-20220225172249-27dd8689420f golang.org/x/sync v0.0.0-20210220032951-036812b2e83c golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 +) + +require ( + github.com/14rcole/gopopulate v0.0.0-20180821133914-b175b219e774 // indirect + github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect + github.com/Microsoft/go-winio v0.5.2 // indirect + github.com/Microsoft/hcsshim v0.9.2 // indirect + github.com/VividCortex/ewma v1.2.0 // indirect + github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d // indirect + github.com/beorn7/perks v1.0.1 // indirect + github.com/cespare/xxhash/v2 v2.1.2 // indirect + github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e // indirect + github.com/containerd/cgroups v1.0.3 // indirect + github.com/containerd/stargz-snapshotter/estargz v0.11.4 // indirect + github.com/cyphar/filepath-securejoin v0.2.3 // indirect + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/docker/go-metrics v0.0.1 // indirect + github.com/docker/go-units v0.4.0 // indirect + github.com/docker/libtrust v0.0.0-20160708172513-aabc10ec26b7 // indirect + github.com/gogo/protobuf v1.3.2 // indirect + github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect + github.com/golang/protobuf v1.5.2 // indirect + github.com/google/go-intervals v0.0.2 // indirect + github.com/google/uuid v1.3.0 // indirect + github.com/gorilla/mux v1.7.4 // indirect + github.com/hashicorp/errwrap v1.1.0 // indirect + github.com/json-iterator/go v1.1.12 // indirect + github.com/mattn/go-runewidth v0.0.13 // indirect + github.com/mattn/go-shellwords v1.0.12 // indirect + github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 // indirect + github.com/miekg/pkcs11 v1.1.1 // indirect + github.com/mistifyio/go-zfs v2.1.2-0.20190413222219-f784269be439+incompatible // indirect + github.com/moby/sys/mountinfo v0.6.1 // indirect + github.com/moby/term v0.0.0-20210610120745-9d4ed1856297 // indirect + github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect + github.com/modern-go/reflect2 v1.0.2 // indirect + github.com/opencontainers/runc v1.1.2 // indirect + github.com/opencontainers/runtime-spec v1.0.3-0.20210326190908-1c3f411f0417 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/prometheus/client_golang v1.11.0 // indirect + github.com/prometheus/client_model v0.2.0 // indirect + github.com/prometheus/common v0.30.0 // indirect + github.com/prometheus/procfs v0.7.3 // indirect + github.com/rivo/uniseg v0.2.0 // indirect + github.com/stefanberger/go-pkcs11uri v0.0.0-20201008174630-78d3cae3a980 // indirect + github.com/syndtr/gocapability v0.0.0-20200815063812-42c35b437635 // indirect + github.com/tchap/go-patricia v2.3.0+incompatible // indirect + github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect + github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect + go.mozilla.org/pkcs7 v0.0.0-20200128120323-432b2356ecb1 // indirect + go.opencensus.io v0.23.0 // indirect + golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a // indirect + golang.org/x/text v0.3.7 // indirect golang.org/x/time v0.0.0-20210723032227-1f47c861a9ac // indirect google.golang.org/genproto v0.0.0-20220304144024-325a89244dc8 // indirect + google.golang.org/grpc v1.44.0 // indirect + google.golang.org/protobuf v1.27.1 // indirect gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect + gopkg.in/square/go-jose.v2 v2.5.1 // indirect + gopkg.in/yaml.v2 v2.4.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect ) From 225404ed0fdac2cc802785ad44e5fb88974ccf4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 8 Jun 2022 16:05:30 +0200 Subject: [PATCH 2/3] Stop using net/tls.Config.PreferServerCipherSuites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As of Go 1.17 it is deprecated and ignored. Signed-off-by: Miloslav Trmač --- docker/docker_client.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docker/docker_client.go b/docker/docker_client.go index daac45f878..29c2568696 100644 --- a/docker/docker_client.go +++ b/docker/docker_client.go @@ -163,9 +163,8 @@ func newBearerTokenFromJSONBlob(blob []byte) (*bearerToken, error) { func serverDefault() *tls.Config { return &tls.Config{ // Avoid fallback to SSL protocols < TLS1.0 - MinVersion: tls.VersionTLS10, - PreferServerCipherSuites: true, - CipherSuites: tlsconfig.DefaultServerAcceptedCiphers, + MinVersion: tls.VersionTLS10, + CipherSuites: tlsconfig.DefaultServerAcceptedCiphers, } } From a56436cb15f54dae0f348f30df54d480f080ed8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 8 Jun 2022 16:41:49 +0200 Subject: [PATCH 3/3] Use testing.T.Setenv instead of os.Setenv in tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... to simplify and benefit from Go 1.17. In some cases, wrap tests in testing.T.Run() to decrease the scope, or to make the relationship between the test and the cleanup clearer. In some cases it's still a bit awkward because there is no testing.T.Unsetenv, but still worth it. Signed-off-by: Miloslav Trmač --- copy/sign_test.go | 4 +- openshift/openshift-copies_test.go | 15 +---- pkg/blobinfocache/default_test.go | 88 ++++++++++-------------------- pkg/docker/config/config_test.go | 74 ++++++++++--------------- signature/mechanism_test.go | 4 +- 5 files changed, 60 insertions(+), 125 deletions(-) diff --git a/copy/sign_test.go b/copy/sign_test.go index 5c93d25fa9..5fa2df3264 100644 --- a/copy/sign_test.go +++ b/copy/sign_test.go @@ -3,7 +3,6 @@ package copy import ( "context" "io" - "os" "testing" "github.com/containers/image/v5/directory" @@ -36,8 +35,7 @@ func TestCreateSignature(t *testing.T) { t.Skipf("Signing not supported: %v", err) } - os.Setenv("GNUPGHOME", testGPGHomeDirectory) - defer os.Unsetenv("GNUPGHOME") + t.Setenv("GNUPGHOME", testGPGHomeDirectory) // Signing a directory: reference, which does not have a DockerReference(), fails. tempDir := t.TempDir() diff --git a/openshift/openshift-copies_test.go b/openshift/openshift-copies_test.go index f4fabb8e98..8346db1a41 100644 --- a/openshift/openshift-copies_test.go +++ b/openshift/openshift-copies_test.go @@ -1,7 +1,6 @@ package openshift import ( - "os" "testing" "github.com/stretchr/testify/assert" @@ -13,20 +12,10 @@ const fixtureKubeConfigPath = "testdata/admin.kubeconfig" // These are only smoke tests based on the skopeo integration test cluster. Error handling, non-trivial configuration merging, // and any other situations are not currently covered. -// Set up KUBECONFIG to point at the fixture, and return a handler to clean it up. +// Set up KUBECONFIG to point at the fixture. // Callers MUST NOT call testing.T.Parallel(). 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") - t.Cleanup(func() { - if hasKC { - os.Setenv("KUBECONFIG", oldKC) - } else { - os.Unsetenv("KUBECONFIG") - } - }) - os.Setenv("KUBECONFIG", fixtureKubeConfigPath) + t.Setenv("KUBECONFIG", fixtureKubeConfigPath) } func TestClientConfigLoadingRules(t *testing.T) { diff --git a/pkg/blobinfocache/default_test.go b/pkg/blobinfocache/default_test.go index bc3ca8ebba..83a3fea98a 100644 --- a/pkg/blobinfocache/default_test.go +++ b/pkg/blobinfocache/default_test.go @@ -1,6 +1,7 @@ package blobinfocache import ( + "fmt" "os" "path/filepath" "testing" @@ -20,28 +21,8 @@ func TestBlobInfoCacheDir(t *testing.T) { const homeDir = "/fake/home/directory" const xdgDataHome = "/fake/home/directory/XDG" - // 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. - oldXRD, hasXRD := os.LookupEnv("XDG_RUNTIME_DIR") - defer func() { - if hasXRD { - os.Setenv("XDG_RUNTIME_DIR", oldXRD) - } else { - os.Unsetenv("XDG_RUNTIME_DIR") - } - }() - // FIXME: This should be a shared helper in internal/testing - oldHome, hasHome := os.LookupEnv("HOME") - defer func() { - if hasHome { - os.Setenv("HOME", oldHome) - } else { - os.Unsetenv("HOME") - } - }() - - os.Setenv("HOME", homeDir) - os.Setenv("XDG_DATA_HOME", xdgDataHome) + t.Setenv("HOME", homeDir) + t.Setenv("XDG_DATA_HOME", xdgDataHome) // The default paths and explicit overrides for _, c := range []struct { @@ -83,7 +64,7 @@ func TestBlobInfoCacheDir(t *testing.T) { } // Paths used by unprivileged users - for _, c := range []struct { + for caseIndex, c := range []struct { xdgDH, home, expected string }{ {"", homeDir, filepath.Join(homeDir, ".local", "share", "containers", "cache")}, // HOME only @@ -91,25 +72,28 @@ func TestBlobInfoCacheDir(t *testing.T) { {xdgDataHome, homeDir, filepath.Join(xdgDataHome, "containers", "cache")}, // both {"", "", ""}, // neither } { - if c.xdgDH != "" { - os.Setenv("XDG_DATA_HOME", c.xdgDH) - } else { - os.Unsetenv("XDG_DATA_HOME") - } - if c.home != "" { - os.Setenv("HOME", c.home) - } else { - os.Unsetenv("HOME") - } - for _, sys := range []*types.SystemContext{nil, {}} { - path, err := blobInfoCacheDir(sys, 1) - if c.expected != "" { - require.NoError(t, err) - assert.Equal(t, c.expected, path) - } else { - assert.Error(t, err) + t.Run(fmt.Sprintf("unprivileged %d", caseIndex), func(t *testing.T) { + // Always use t.Setenv() to ensure the environment variable is restored to the original value after the test. + // Then, in cases where the test needs the variable unset (not just set to empty), use a raw os.Unsetenv() + // to override the situation. (Sadly there isn’t a t.Unsetenv() as of Go 1.17.) + t.Setenv("XDG_DATA_HOME", c.xdgDH) + if c.xdgDH == "" { + os.Unsetenv("XDG_DATA_HOME") } - } + t.Setenv("HOME", c.home) + if c.home == "" { + os.Unsetenv("HOME") + } + for _, sys := range []*types.SystemContext{nil, {}} { + path, err := blobInfoCacheDir(sys, 1) + if c.expected != "" { + require.NoError(t, err) + assert.Equal(t, c.expected, path) + } else { + assert.Error(t, err) + } + } + }) } } @@ -123,26 +107,10 @@ func TestDefaultCache(t *testing.T) { assert.Equal(t, boltdb.New(filepath.Join(normalDir, blobInfoCacheFilename)), c) // Error running blobInfoCacheDir: - // 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. - oldXRD, hasXRD := os.LookupEnv("XDG_RUNTIME_DIR") - defer func() { - if hasXRD { - os.Setenv("XDG_RUNTIME_DIR", oldXRD) - } else { - os.Unsetenv("XDG_RUNTIME_DIR") - } - }() - // FIXME: This should be a shared helper in internal/testing - oldHome, hasHome := os.LookupEnv("HOME") - defer func() { - if hasHome { - os.Setenv("HOME", oldHome) - } else { - os.Unsetenv("HOME") - } - }() + // Use t.Setenv() just as a way to set up cleanup to original values; then os.Unsetenv() to test a situation where the values are not set. + t.Setenv("HOME", "") os.Unsetenv("HOME") + t.Setenv("XDG_DATA_HOME", "") os.Unsetenv("XDG_DATA_HOME") c = DefaultCache(nil) assert.IsType(t, memory.New(), c) diff --git a/pkg/docker/config/config_test.go b/pkg/docker/config/config_test.go index 5287454bbb..4277bba824 100644 --- a/pkg/docker/config/config_test.go +++ b/pkg/docker/config/config_test.go @@ -25,18 +25,7 @@ func TestGetPathToAuth(t *testing.T) { 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. - oldXRD, hasXRD := os.LookupEnv("XDG_RUNTIME_DIR") - defer func() { - if hasXRD { - os.Setenv("XDG_RUNTIME_DIR", oldXRD) - } else { - os.Unsetenv("XDG_RUNTIME_DIR") - } - }() - - for _, c := range []struct { + for caseIndex, c := range []struct { sys *types.SystemContext os string xrd string @@ -61,40 +50,38 @@ func TestGetPathToAuth(t *testing.T) { {nil, linux, tmpDir + "/thisdoesnotexist", "", false}, {nil, darwin, tmpDir + "/thisdoesnotexist", darwinDefault, false}, } { - if c.xrd != "" { - os.Setenv("XDG_RUNTIME_DIR", c.xrd) - } else { - os.Unsetenv("XDG_RUNTIME_DIR") - } - res, lf, err := getPathToAuthWithOS(c.sys, c.os) - if c.expected == "" { - assert.Error(t, err) - } else { - require.NoError(t, err) - assert.Equal(t, c.expected, res) - assert.Equal(t, c.legacyFormat, lf) - } + t.Run(fmt.Sprintf("%d", caseIndex), func(t *testing.T) { + // Always use t.Setenv() to ensure XDG_RUNTIME_DIR is restored to the original value after the test. + // Then, in cases where the test needs XDG_RUNTIME_DIR unset (not just set to empty), use a raw os.Unsetenv() + // to override the situation. (Sadly there isn’t a t.Unsetenv() as of Go 1.17.) + t.Setenv("XDG_RUNTIME_DIR", c.xrd) + if c.xrd == "" { + os.Unsetenv("XDG_RUNTIME_DIR") + } + res, lf, err := getPathToAuthWithOS(c.sys, c.os) + if c.expected == "" { + assert.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, c.expected, res) + assert.Equal(t, c.legacyFormat, lf) + } + }) } } func TestGetAuth(t *testing.T) { - origXDG := os.Getenv("XDG_RUNTIME_DIR") tmpXDGRuntimeDir := t.TempDir() t.Logf("using temporary XDG_RUNTIME_DIR directory: %q", tmpXDGRuntimeDir) - // override XDG_RUNTIME_DIR - os.Setenv("XDG_RUNTIME_DIR", tmpXDGRuntimeDir) - defer os.Setenv("XDG_RUNTIME_DIR", origXDG) + t.Setenv("XDG_RUNTIME_DIR", tmpXDGRuntimeDir) // override PATH for executing credHelper curtDir, err := os.Getwd() require.NoError(t, err) origPath := os.Getenv("PATH") newPath := fmt.Sprintf("%s:%s", filepath.Join(curtDir, "testdata"), origPath) - os.Setenv("PATH", newPath) + t.Setenv("PATH", newPath) t.Logf("using PATH: %q", newPath) - defer func() { - os.Setenv("PATH", origPath) - }() tmpHomeDir := t.TempDir() t.Logf("using temporary home directory: %q", tmpHomeDir) @@ -405,12 +392,9 @@ func TestGetAuthPreferNewConfig(t *testing.T) { } func TestGetAuthFailsOnBadInput(t *testing.T) { - origXDG := os.Getenv("XDG_RUNTIME_DIR") tmpXDGRuntimeDir := t.TempDir() t.Logf("using temporary XDG_RUNTIME_DIR directory: %q", tmpXDGRuntimeDir) - // override XDG_RUNTIME_DIR - os.Setenv("XDG_RUNTIME_DIR", tmpXDGRuntimeDir) - defer os.Setenv("XDG_RUNTIME_DIR", origXDG) + t.Setenv("XDG_RUNTIME_DIR", tmpXDGRuntimeDir) tmpHomeDir := t.TempDir() t.Logf("using temporary home directory: %q", tmpHomeDir) @@ -466,11 +450,8 @@ func TestGetAllCredentials(t *testing.T) { require.NoError(t, err) origPath := os.Getenv("PATH") newPath := fmt.Sprintf("%s:%s", filepath.Join(path, "testdata"), origPath) - os.Setenv("PATH", newPath) + t.Setenv("PATH", newPath) t.Logf("using PATH: %q", newPath) - defer func() { - os.Setenv("PATH", origPath) - }() err = os.Chmod(filepath.Join(path, "testdata", "docker-credential-helper-registry"), os.ModePerm) require.NoError(t, err) sys := types.SystemContext{ @@ -480,11 +461,12 @@ func TestGetAllCredentials(t *testing.T) { } // Make sure that we can handle no-creds-found errors. - os.Setenv("DOCKER_CONFIG", filepath.Join(path, "testdata")) - authConfigs, err := GetAllCredentials(nil) - require.NoError(t, err) - require.Empty(t, authConfigs) - os.Unsetenv("DOCKER_CONFIG") + t.Run("no credentials found", func(t *testing.T) { + t.Setenv("DOCKER_CONFIG", filepath.Join(path, "testdata")) + authConfigs, err := GetAllCredentials(nil) + require.NoError(t, err) + require.Empty(t, authConfigs) + }) for _, data := range [][]struct { writeKey string diff --git a/signature/mechanism_test.go b/signature/mechanism_test.go index 95f2403802..6c2b3393d2 100644 --- a/signature/mechanism_test.go +++ b/signature/mechanism_test.go @@ -97,9 +97,7 @@ func TestNewGPGSigningMechanismInDirectory(t *testing.T) { } // If we use the default directory mechanism, GNUPGHOME is respected. - origGNUPGHOME := os.Getenv("GNUPGHOME") - defer os.Setenv("GNUPGHOME", origGNUPGHOME) - os.Setenv("GNUPGHOME", testGPGHomeDirectory) + t.Setenv("GNUPGHOME", testGPGHomeDirectory) mech, err = newGPGSigningMechanismInDirectory("") require.NoError(t, err) defer mech.Close()