Merge pull request #1824 from mtrmac/golangci-lint

Update to golangci-lint 1.51.0

Makefile

@@ -55,7 +55,7 @@ tools: .install.gitvalidation .install.golangci-lint .install.golint
 
 .install.golangci-lint:
 	if [ ! -x "$(GOBIN)/golangci-lint" ]; then \
-		curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b $(GOBIN) v1.49.0; \
+		curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b $(GOBIN) v1.51.0; \
 	fi
 
 .install.golint:

copy/compression.go

@@ -200,7 +200,9 @@ func (ic *imageCopier) bpcDecompressCompressed(stream *sourceStream, detected bp
 }
 
 // bpcPreserveOriginal returns a *bpCompressionStepData for not changing the original blob.
-func (ic *imageCopier) bpcPreserveOriginal(stream *sourceStream, detected bpDetectCompressionStepData,
+// This does not change the sourceStream parameter; we include it for symmetry with other
+// pipeline steps.
+func (ic *imageCopier) bpcPreserveOriginal(_ *sourceStream, detected bpDetectCompressionStepData,
 	layerCompressionChangeSupported bool) *bpCompressionStepData {
 	logrus.Debugf("Using original blob without modification")
 	// Remember if the original blob was compressed, and if so how, so that if

copy/copy.go

@@ -145,7 +145,7 @@ type Options struct {
 
 	// Preserve digests, and fail if we cannot.
 	PreserveDigests bool
-	// manifest MIME type of image set by user. "" is default and means use the autodetection to the the manifest MIME type
+	// manifest MIME type of image set by user. "" is default and means use the autodetection to the manifest MIME type
 	ForceManifestMIMEType string
 	ImageListSelection    ImageListSelection // set to either CopySystemImage (the default), CopyAllImages, or CopySpecificImages to control which instances we copy when the source reference is a list; ignored if the source reference is not a list
 	Instances             []digest.Digest    // if ImageListSelection is CopySpecificImages, copy only these instances and the list itself

copy/progress_bars.go

@@ -120,7 +120,7 @@ func (bar *progressBar) mark100PercentComplete() {
 		bar.SetCurrent(bar.originalSize) // This triggers the completion condition.
 	} else {
 		// -1 = unknown size
-		// 0 is somewhat of a a special case: Unlike c/image, where 0 is a definite known
+		// 0 is somewhat of a special case: Unlike c/image, where 0 is a definite known
 		// size (possible at least in theory), in mpb, zero-sized progress bars are treated
 		// as unknown size, in particular they are not configured to be marked as
 		// complete on bar.Current() reaching bar.total (because that would happen already

directory/directory_transport.go

@@ -89,7 +89,7 @@ func (ref dirReference) Transport() types.ImageTransport {
 // StringWithinTransport returns a string representation of the reference, which MUST be such that
 // reference.Transport().ParseReference(reference.StringWithinTransport()) returns an equivalent reference.
 // NOTE: The returned string is not promised to be equal to the original input to ParseReference;
-// e.g. default attribute values omitted by the user may be filled in in the return value, or vice versa.
+// e.g. default attribute values omitted by the user may be filled in the return value, or vice versa.
 // WARNING: Do not use the return value in the UI to describe an image, it does not contain the Transport().Name() prefix.
 func (ref dirReference) StringWithinTransport() string {
 	return ref.path

directory/explicitfilepath/path.go

@@ -9,7 +9,7 @@ import (
 // ResolvePathToFullyExplicit returns the input path converted to an absolute, no-symlinks, cleaned up path.
 // To do so, all elements of the input path must exist; as a special case, the final component may be
 // a non-existent name (but not a symlink pointing to a non-existent name)
-// This is intended as a a helper for implementations of types.ImageReference.PolicyConfigurationIdentity etc.
+// This is intended as a helper for implementations of types.ImageReference.PolicyConfigurationIdentity etc.
 func ResolvePathToFullyExplicit(path string) (string, error) {
 	switch _, err := os.Lstat(path); {
 	case err == nil:

docker/archive/transport.go

@@ -136,7 +136,7 @@ func (ref archiveReference) Transport() types.ImageTransport {
 // StringWithinTransport returns a string representation of the reference, which MUST be such that
 // reference.Transport().ParseReference(reference.StringWithinTransport()) returns an equivalent reference.
 // NOTE: The returned string is not promised to be equal to the original input to ParseReference;
-// e.g. default attribute values omitted by the user may be filled in in the return value, or vice versa.
+// e.g. default attribute values omitted by the user may be filled in the return value, or vice versa.
 // WARNING: Do not use the return value in the UI to describe an image, it does not contain the Transport().Name() prefix.
 func (ref archiveReference) StringWithinTransport() string {
 	switch {

docker/daemon/daemon_transport.go

@@ -118,7 +118,7 @@ func (ref daemonReference) Transport() types.ImageTransport {
 // StringWithinTransport returns a string representation of the reference, which MUST be such that
 // reference.Transport().ParseReference(reference.StringWithinTransport()) returns an equivalent reference.
 // NOTE: The returned string is not promised to be equal to the original input to ParseReference;
-// e.g. default attribute values omitted by the user may be filled in in the return value, or vice versa.
+// e.g. default attribute values omitted by the user may be filled in the return value, or vice versa.
 // WARNING: Do not use the return value in the UI to describe an image, it does not contain the Transport().Name() prefix;
 // instead, see transports.ImageName().
 func (ref daemonReference) StringWithinTransport() string {

docker/docker_client.go

@@ -586,7 +586,7 @@ func (c *dockerClient) makeRequestToResolvedURL(ctx context.Context, method stri
 		case <-time.After(delay):
 			// Nothing
 		}
-		delay = delay * 2 // If the registry does not specify a delay, back off exponentially.
+		delay *= 2 // If the registry does not specify a delay, back off exponentially.
 	}
 }
 

docker/docker_client_test.go

@@ -381,6 +381,7 @@ func TestIsManifestUnknownError(t *testing.T) {
 	} {
 		resp, err := http.ReadResponse(bufio.NewReader(bytes.NewReader([]byte(c.response))), nil)
 		require.NoError(t, err, c.name)
+		defer resp.Body.Close()
 		err = fmt.Errorf("wrapped: %w", registryHTTPResponseToError(resp))
 
 		res := isManifestUnknownError(err)

docker/docker_image_dest_test.go

@@ -28,6 +28,7 @@ func TestIsManifestInvalidError(t *testing.T) {
 		"{\"errors\":[{\"code\":\"TAG_INVALID\",\"message\":\"manifest tag did not match URI\"}]}\n"
 	resp, err := http.ReadResponse(bufio.NewReader(bytes.NewReader([]byte(response))), nil)
 	require.NoError(t, err)
+	defer resp.Body.Close()
 	err = registryHTTPResponseToError(resp)
 
 	res := isManifestInvalidError(err)

docker/docker_image_src.go

@@ -335,7 +335,7 @@ func parseMediaType(contentType string) (string, map[string]string, error) {
 func (s *dockerImageSource) GetBlobAt(ctx context.Context, info types.BlobInfo, chunks []private.ImageSourceChunk) (chan io.ReadCloser, chan error, error) {
 	headers := make(map[string][]string)
 
-	var rangeVals []string
+	rangeVals := make([]string, 0, len(chunks))
 	for _, c := range chunks {
 		rangeVals = append(rangeVals, fmt.Sprintf("%d-%d", c.Offset, c.Offset+c.Length-1))
 	}

docker/docker_transport.go

@@ -92,7 +92,7 @@ func (ref dockerReference) Transport() types.ImageTransport {
 // StringWithinTransport returns a string representation of the reference, which MUST be such that
 // reference.Transport().ParseReference(reference.StringWithinTransport()) returns an equivalent reference.
 // NOTE: The returned string is not promised to be equal to the original input to ParseReference;
-// e.g. default attribute values omitted by the user may be filled in in the return value, or vice versa.
+// e.g. default attribute values omitted by the user may be filled in the return value, or vice versa.
 // WARNING: Do not use the return value in the UI to describe an image, it does not contain the Transport().Name() prefix.
 func (ref dockerReference) StringWithinTransport() string {
 	return "//" + reference.FamiliarString(ref.ref)

docker/errors.go

@@ -40,7 +40,7 @@ func httpResponseToError(res *http.Response, context string) error {
 		return ErrUnauthorizedForCredentials{Err: err}
 	default:
 		if context != "" {
-			context = context + ": "
+			context += ": "
 		}
 		return fmt.Errorf("%sinvalid status code from registry %d (%s)", context, res.StatusCode, http.StatusText(res.StatusCode))
 	}

docker/errors_test.go

@@ -176,6 +176,7 @@ func TestRegistryHTTPResponseToError(t *testing.T) {
 	} {
 		res, err := http.ReadResponse(bufio.NewReader(bytes.NewReader([]byte(c.response))), nil)
 		require.NoError(t, err, c.name)
+		defer res.Body.Close()
 
 		err = registryHTTPResponseToError(res)
 		assert.Equal(t, c.errorString, err.Error(), c.name)

docker/reference/normalize.go

@@ -104,7 +104,7 @@ func splitDockerDomain(name string) (domain, remainder string) {
 }
 
 // familiarizeName returns a shortened version of the name familiar
-// to to the Docker UI. Familiar names have the default domain
+// to the Docker UI. Familiar names have the default domain
 // "docker.io" and "library/" repository prefix removed.
 // For example, "docker.io/library/redis" will have the familiar
 // name "redis" and "docker.io/dmcgowan/myapp" will be "dmcgowan/myapp".

docker/reference/normalize_test.go

@@ -69,7 +69,7 @@ func TestValidateRemoteName(t *testing.T) {
 		// Allow multiple hyphens as well.
 		"docker---rules/docker",
 
-		//Username doc and image name docker being tested.
+		// Username doc and image name docker being tested.
 		"doc/docker",
 
 		// single character names are now allowed.
@@ -114,7 +114,7 @@ func TestValidateRemoteName(t *testing.T) {
 		// No repository.
 		"docker/",
 
-		//namespace too long
+		// namespace too long
 		"this_is_not_a_valid_namespace_because_its_length_is_greater_than_255_this_is_not_a_valid_namespace_because_its_length_is_greater_than_255_this_is_not_a_valid_namespace_because_its_length_is_greater_than_255_this_is_not_a_valid_namespace_because_its_length_is_greater_than_255/docker",
 	}
 	for _, repositoryName := range invalidRepositoryNames {
@@ -258,7 +258,6 @@ func TestParseRepositoryInfo(t *testing.T) {
 			if expected, actual := tcase.RemoteName, Path(r); expected != actual {
 				t.Fatalf("Invalid remoteName for %q. Expected %q, got %q", r, expected, actual)
 			}
-
 		}
 	}
 }

docker/reference/reference.go

@@ -175,7 +175,7 @@ func splitDomain(name string) (string, string) {
 // hostname and name string. If no valid hostname is
 // found, the hostname is empty and the full value
 // is returned as name
-// DEPRECATED: Use Domain or Path
+// Deprecated: Use Domain or Path
 func SplitHostname(named Named) (string, string) {
 	if r, ok := named.(namedRepository); ok {
 		return r.Domain(), r.Path()

docker/reference/reference_test.go

@@ -103,10 +103,10 @@ func TestReferenceParse(t *testing.T) {
 		},
 		// FIXME "Uppercase" is incorrectly handled as a domain-name here, therefore passes.
 		// See https://github.com/docker/distribution/pull/1778, and https://github.com/docker/docker/pull/20175
-		//{
-		//	input: "Uppercase/lowercase:tag",
-		//	err:   ErrNameContainsUppercase,
-		//},
+		//	{
+		//		input: "Uppercase/lowercase:tag",
+		//		err:   ErrNameContainsUppercase,
+		//	},
 		{
 			input: "test:5000/Uppercase/lowercase:tag",
 			err:   ErrNameContainsUppercase,
@@ -231,7 +231,6 @@ func TestReferenceParse(t *testing.T) {
 		} else if ok {
 			failf("unexpected digested type")
 		}
-
 	}
 }
 
@@ -462,7 +461,6 @@ func TestSerialization(t *testing.T) {
 		if _, ok := fieldInterface.(Reference); ok {
 			failf("field should not implement Reference interface")
 		}
-
 	}
 }
 

docker/reference/regexp.go

@@ -1,9 +1,10 @@
 package reference
 
 import (
-	storageRegexp "github.com/containers/storage/pkg/regexp"
 	"regexp"
 	"strings"
+
+	storageRegexp "github.com/containers/storage/pkg/regexp"
 )
 
 const (

docker/wwwauthenticate.go

@@ -149,7 +149,7 @@ func expectTokenOrQuoted(s string) (value string, rest string) {
 			p := make([]byte, len(s)-1)
 			j := copy(p, s[:i])
 			escape := true
-			for i = i + 1; i < len(s); i++ {
+			for i++; i < len(s); i++ {
 				b := s[i]
 				switch {
 				case escape:

internal/image/docker_schema2_test.go

@@ -439,7 +439,7 @@ func modifiedLayerInfos(t *testing.T, input []types.BlobInfo) ([]types.BlobInfo,
 		require.NoError(t, err)
 		oldDigest[len(oldDigest)-1] ^= 1
 		b2.Digest = digest.NewDigestFromEncoded(b2.Digest.Algorithm(), hex.EncodeToString(oldDigest))
-		b2.Size = b2.Size ^ 1
+		b2.Size ^= 1
 		modified = append(modified, b2)
 	}
 

internal/iolimits/iolimits_test.go

@@ -10,6 +10,7 @@ import (
 )
 
 func TestReadAtMost(t *testing.T) {
+	rng := rand.New(rand.NewSource(0))
 	for _, c := range []struct {
 		input, limit  int
 		shouldSucceed bool
@@ -23,7 +24,7 @@ func TestReadAtMost(t *testing.T) {
 		{bytes.MinRead*5 + 1, bytes.MinRead * 5, false},
 	} {
 		input := make([]byte, c.input)
-		_, err := rand.Read(input)
+		_, err := rng.Read(input)
 		require.NoError(t, err)
 		result, err := ReadAtMost(bytes.NewReader(input), c.limit)
 		if c.shouldSucceed {

internal/uploadreader/upload_reader.go

@@ -11,7 +11,7 @@ import (
 // The net/http package uses a separate goroutine to upload data to a HTTP connection,
 // and it is possible for the server to return a response (typically an error) before consuming
 // the full body of the request. In that case http.Client.Do can return with an error while
-// the body is still being read — regardless of of the cancellation, if any, of http.Request.Context().
+// the body is still being read — regardless of the cancellation, if any, of http.Request.Context().
 //
 // As a result, any data used/updated by the io.Reader() provided as the request body may be
 // used/updated even after http.Client.Do returns, causing races.

manifest/oci.go

@@ -177,7 +177,7 @@ func getEncryptedMediaType(mediatype string) (string, error) {
 // an error if the mediatype does not support decryption
 func getDecryptedMediaType(mediatype string) (string, error) {
 	if !strings.HasSuffix(mediatype, "+encrypted") {
-		return "", fmt.Errorf("unsupported mediaType to decrypt %v:", mediatype)
+		return "", fmt.Errorf("unsupported mediaType to decrypt: %v", mediatype)
 	}
 
 	return strings.TrimSuffix(mediatype, "+encrypted"), nil

oci/layout/oci_src_test.go

@@ -29,7 +29,7 @@ var httpServerAddr string
 func TestMain(m *testing.M) {
 	httpServer, err := startRemoteLayerServer()
 	if err != nil {
-		println("Error starting test TLS server", err.Error())
+		fmt.Fprintf(os.Stderr, "Error starting test TLS server: %v", err.Error())
 		os.Exit(1)
 	}
 

oci/layout/oci_transport.go

@@ -100,7 +100,7 @@ func (ref ociReference) Transport() types.ImageTransport {
 // StringWithinTransport returns a string representation of the reference, which MUST be such that
 // reference.Transport().ParseReference(reference.StringWithinTransport()) returns an equivalent reference.
 // NOTE: The returned string is not promised to be equal to the original input to ParseReference;
-// e.g. default attribute values omitted by the user may be filled in in the return value, or vice versa.
+// e.g. default attribute values omitted by the user may be filled in the return value, or vice versa.
 // WARNING: Do not use the return value in the UI to describe an image, it does not contain the Transport().Name() prefix.
 func (ref ociReference) StringWithinTransport() string {
 	return fmt.Sprintf("%s:%s", ref.dir, ref.image)

openshift/openshift-copies.go

@@ -207,10 +207,7 @@ func (config *directClientConfig) ClientConfig() (*restConfig, error) {
 	if isConfigTransportTLS(*clientConfig) {
 		var err error
 		// REMOVED: Support for interactive fallback.
-		userAuthPartialConfig, err := getUserIdentificationPartialConfig(configAuthInfo)
-		if err != nil {
-			return nil, err
-		}
+		userAuthPartialConfig := getUserIdentificationPartialConfig(configAuthInfo)
 		if err = mergo.MergeWithOverwrite(clientConfig, userAuthPartialConfig); err != nil {
 			return nil, err
 		}
@@ -257,7 +254,7 @@ func getServerIdentificationPartialConfig(configAuthInfo clientcmdAuthInfo, conf
 // 2.  configAuthInfo.auth-path (this file can contain information that conflicts with #1, and we want #1 to win the priority)
 // 3.  if there is not enough information to identify the user, load try the ~/.kubernetes_auth file
 // 4.  if there is not enough information to identify the user, prompt if possible
-func getUserIdentificationPartialConfig(configAuthInfo clientcmdAuthInfo) (*restConfig, error) {
+func getUserIdentificationPartialConfig(configAuthInfo clientcmdAuthInfo) *restConfig {
 	mergedConfig := &restConfig{}
 
 	// blindly overwrite existing values based on precedence
@@ -276,7 +273,7 @@ func getUserIdentificationPartialConfig(configAuthInfo clientcmdAuthInfo) (*rest
 	}
 
 	// REMOVED: prompting for missing information.
-	return mergedConfig, nil
+	return mergedConfig
 }
 
 // ConfirmUsable is a modified copy of k8s.io/kubernetes/pkg/client/unversioned/clientcmd.DirectClientConfig.ConfirmUsable.

openshift/openshift_transport.go

@@ -89,7 +89,7 @@ func (ref openshiftReference) Transport() types.ImageTransport {
 // StringWithinTransport returns a string representation of the reference, which MUST be such that
 // reference.Transport().ParseReference(reference.StringWithinTransport()) returns an equivalent reference.
 // NOTE: The returned string is not promised to be equal to the original input to ParseReference;
-// e.g. default attribute values omitted by the user may be filled in in the return value, or vice versa.
+// e.g. default attribute values omitted by the user may be filled in the return value, or vice versa.
 // WARNING: Do not use the return value in the UI to describe an image, it does not contain the Transport().Name() prefix.
 func (ref openshiftReference) StringWithinTransport() string {
 	return reference.FamiliarString(ref.dockerReference)

ostree/ostree_transport.go

@@ -133,7 +133,7 @@ func (ref ostreeReference) Transport() types.ImageTransport {
 // StringWithinTransport returns a string representation of the reference, which MUST be such that
 // reference.Transport().ParseReference(reference.StringWithinTransport()) returns an equivalent reference.
 // NOTE: The returned string is not promised to be equal to the original input to ParseReference;
-// e.g. default attribute values omitted by the user may be filled in in the return value, or vice versa.
+// e.g. default attribute values omitted by the user may be filled in the return value, or vice versa.
 // WARNING: Do not use the return value in the UI to describe an image, it does not contain the Transport().Name() prefix.
 func (ref ostreeReference) StringWithinTransport() string {
 	return fmt.Sprintf("%s@%s", ref.image, ref.repo)

pkg/blobinfocache/memory/memory.go

@@ -20,7 +20,7 @@ type locationKey struct {
 	blobDigest digest.Digest
 }
 
-// cache implements an in-memory-only BlobInfoCache
+// cache implements an in-memory-only BlobInfoCache.
 type cache struct {
 	mutex sync.Mutex
 	// The following fields can only be accessed with mutex held.

pkg/compression/compression.go

@@ -30,7 +30,7 @@ var (
 	// Zstd compression.
 	Zstd = internal.NewAlgorithm(types.ZstdAlgorithmName, types.ZstdAlgorithmName,
 		[]byte{0x28, 0xb5, 0x2f, 0xfd}, ZstdDecompressor, zstdCompressor)
-	// Zstd:chunked compression.
+	// ZstdChunked is a Zstd compresion with chunk metadta which allows random access to individual files.
 	ZstdChunked = internal.NewAlgorithm(types.ZstdChunkedAlgorithmName, types.ZstdAlgorithmName, /* Note: InternalUnstableUndocumentedMIMEQuestionMark is not ZstdChunkedAlgorithmName */
 		nil, ZstdDecompressor, compressor.ZstdCompressor)
 

pkg/docker/config/config.go

@@ -392,17 +392,16 @@ func RemoveAuthentication(sys *types.SystemContext, key string) error {
 		if isNamespaced {
 			logrus.Debugf("Not removing credentials because namespaced keys are not supported for the credential helper: %s", helper)
 			return
-		} else {
-			err := deleteAuthFromCredHelper(helper, key)
-			if err == nil {
-				logrus.Debugf("Credentials for %q were deleted from credential helper %s", key, helper)
-				isLoggedIn = true
-				return
-			}
-			if credentials.IsErrCredentialsNotFoundMessage(err.Error()) {
-				logrus.Debugf("Not logged in to %s with credential helper %s", key, helper)
-				return
-			}
+		}
+		err := deleteAuthFromCredHelper(helper, key)
+		if err == nil {
+			logrus.Debugf("Credentials for %q were deleted from credential helper %s", key, helper)
+			isLoggedIn = true
+			return
+		}
+		if credentials.IsErrCredentialsNotFoundMessage(err.Error()) {
+			logrus.Debugf("Not logged in to %s with credential helper %s", key, helper)
+			return
 		}
 		multiErr = multierror.Append(multiErr, fmt.Errorf("removing credentials for %s from credential helper %s: %w", key, helper, err))
 	}