Merge pull request moby#50500 from thaJeztah/registry_fix_linting

registry: fix assorted linting issues

Author: thaJeztah

registry/auth.go

@@ -40,9 +40,9 @@ type staticCredentialStore struct {
 
 // NewStaticCredentialStore returns a credential store
 // which always returns the same credential values.
-func NewStaticCredentialStore(auth *registry.AuthConfig) auth.CredentialStore {
+func NewStaticCredentialStore(ac *registry.AuthConfig) auth.CredentialStore {
 	return staticCredentialStore{
-		auth: auth,
+		auth: ac,
 	}
 }
 
@@ -60,7 +60,7 @@ func (scs staticCredentialStore) RefreshToken(*url.URL, string) string {
 	return scs.auth.IdentityToken
 }
 
-func (scs staticCredentialStore) SetRefreshToken(*url.URL, string, string) {
+func (staticCredentialStore) SetRefreshToken(*url.URL, string, string) {
 }
 
 // loginV2 tries to login to the v2 registry server. The given registry
@@ -131,12 +131,15 @@ func v2AuthHTTPClient(endpoint *url.URL, authTransport http.RoundTripper, modifi
 // to just its hostname. It is used to match credentials, which may be either
 // stored as hostname or as hostname including scheme (in legacy configuration
 // files).
-func ConvertToHostname(url string) string {
-	stripped := url
-	if strings.HasPrefix(stripped, "http://") {
-		stripped = strings.TrimPrefix(stripped, "http://")
-	} else if strings.HasPrefix(stripped, "https://") {
-		stripped = strings.TrimPrefix(stripped, "https://")
+func ConvertToHostname(maybeURL string) string {
+	stripped := maybeURL
+	if scheme, remainder, ok := strings.Cut(stripped, "://"); ok {
+		switch scheme {
+		case "http", "https":
+			stripped = remainder
+		default:
+			// unknown, or no scheme; doing nothing for now, as we never did.
+		}
 	}
 	stripped, _, _ = strings.Cut(stripped, "/")
 	return stripped
@@ -175,9 +178,9 @@ func (err PingResponseError) Error() string {
 // PingV2Registry attempts to ping a v2 registry and on success return a
 // challenge manager for the supported authentication types.
 // If a response is received but cannot be interpreted, a PingResponseError will be returned.
-func PingV2Registry(endpoint *url.URL, transport http.RoundTripper) (challenge.Manager, error) {
+func PingV2Registry(endpoint *url.URL, authTransport http.RoundTripper) (challenge.Manager, error) {
 	pingClient := &http.Client{
-		Transport: transport,
+		Transport: authTransport,
 		Timeout:   15 * time.Second,
 	}
 	endpointStr := strings.TrimRight(endpoint.String(), "/") + "/v2/"

registry/config.go

@@ -168,14 +168,15 @@ skip:
 		if _, err := ValidateIndexName(r); err != nil {
 			return err
 		}
-		if strings.HasPrefix(strings.ToLower(r), "http://") {
-			log.G(context.TODO()).Warnf("insecure registry %s should not contain 'http://' and 'http://' has been removed from the insecure registry config", r)
-			r = r[7:]
-		} else if strings.HasPrefix(strings.ToLower(r), "https://") {
-			log.G(context.TODO()).Warnf("insecure registry %s should not contain 'https://' and 'https://' has been removed from the insecure registry config", r)
-			r = r[8:]
-		} else if hasScheme(r) {
-			return invalidParamf("insecure registry %s should not contain '://'", r)
+		if scheme, host, ok := strings.Cut(r, "://"); ok {
+			switch strings.ToLower(scheme) {
+			case "http", "https":
+				log.G(context.TODO()).Warnf("insecure registry %[1]s should not contain '%[2]s' and '%[2]ss' has been removed from the insecure registry config", r, scheme)
+				r = host
+			default:
+				// unsupported scheme
+				return invalidParamf("insecure registry %s should not contain '://'", r)
+			}
 		}
 		// Check if CIDR was passed to --insecure-registry
 		_, ipnet, err := net.ParseCIDR(r)
@@ -240,18 +241,18 @@ func (config *serviceConfig) isSecureIndex(indexName string) bool {
 // for mocking in unit tests.
 var lookupIP = net.LookupIP
 
-// isCIDRMatch returns true if URLHost matches an element of cidrs. URLHost is a URL.Host (`host:port` or `host`)
+// isCIDRMatch returns true if urlHost matches an element of cidrs. urlHost is a URL.Host ("host:port" or "host")
 // where the `host` part can be either a domain name or an IP address. If it is a domain name, then it will be
 // resolved to IP addresses for matching. If resolution fails, false is returned.
-func isCIDRMatch(cidrs []*registry.NetIPNet, URLHost string) bool {
+func isCIDRMatch(cidrs []*registry.NetIPNet, urlHost string) bool {
 	if len(cidrs) == 0 {
 		return false
 	}
 
-	host, _, err := net.SplitHostPort(URLHost)
+	host, _, err := net.SplitHostPort(urlHost)
 	if err != nil {
-		// Assume URLHost is a host without port and go on.
-		host = URLHost
+		// Assume urlHost is a host without port and go on.
+		host = urlHost
 	}
 
 	var addresses []net.IP

registry/errors.go

@@ -8,17 +8,13 @@ import (
 )
 
 func translateV2AuthError(err error) error {
-	switch e := err.(type) {
-	case *url.Error:
-		switch e2 := e.Err.(type) {
-		case errcode.Error:
-			switch e2.Code {
-			case errcode.ErrorCodeUnauthorized:
-				return unauthorizedErr{err}
-			}
+	var e *url.Error
+	if errors.As(err, &e) {
+		var e2 errcode.Error
+		if errors.As(e, &e2) && errors.Is(e2.Code, errcode.ErrorCodeUnauthorized) {
+			return unauthorizedErr{err}
 		}
 	}
-
 	return err
 }
 

registry/registry_mock_test.go

@@ -116,4 +116,5 @@ func TestPing(t *testing.T) {
 	}
 	assert.Equal(t, res.StatusCode, http.StatusOK, "")
 	assert.Equal(t, res.Header.Get("Server"), "docker-tests/mock")
+	_ = res.Body.Close()
 }

registry/registry_test.go

@@ -363,15 +363,6 @@ func TestNewIndexInfo(t *testing.T) {
 		},
 	}
 
-	testIndexInfo := func(t *testing.T, config *serviceConfig, expectedIndexInfos map[string]*registry.IndexInfo) {
-		for indexName, expected := range expectedIndexInfos {
-			t.Run(indexName, func(t *testing.T) {
-				actual := newIndexInfo(config, indexName)
-				assert.Check(t, is.DeepEqual(actual, expected))
-			})
-		}
-	}
-
 	expectedIndexInfos := map[string]*registry.IndexInfo{
 		IndexName: {
 			Name:     IndexName,
@@ -399,7 +390,12 @@ func TestNewIndexInfo(t *testing.T) {
 		},
 	}
 	t.Run("no mirrors", func(t *testing.T) {
-		testIndexInfo(t, emptyServiceConfig, expectedIndexInfos)
+		for indexName, expected := range expectedIndexInfos {
+			t.Run(indexName, func(t *testing.T) {
+				actual := newIndexInfo(emptyServiceConfig, indexName)
+				assert.Check(t, is.DeepEqual(actual, expected))
+			})
+		}
 	})
 
 	expectedIndexInfos = map[string]*registry.IndexInfo{
@@ -494,7 +490,12 @@ func TestNewIndexInfo(t *testing.T) {
 			InsecureRegistries: []string{"example.com"},
 		})
 		assert.NilError(t, err)
-		testIndexInfo(t, config, expectedIndexInfos)
+		for indexName, expected := range expectedIndexInfos {
+			t.Run(indexName, func(t *testing.T) {
+				actual := newIndexInfo(config, indexName)
+				assert.Check(t, is.DeepEqual(actual, expected))
+			})
+		}
 	})
 
 	expectedIndexInfos = map[string]*registry.IndexInfo{
@@ -546,7 +547,12 @@ func TestNewIndexInfo(t *testing.T) {
 			InsecureRegistries: []string{"42.42.0.0/16"},
 		})
 		assert.NilError(t, err)
-		testIndexInfo(t, config, expectedIndexInfos)
+		for indexName, expected := range expectedIndexInfos {
+			t.Run(indexName, func(t *testing.T) {
+				actual := newIndexInfo(config, indexName)
+				assert.Check(t, is.DeepEqual(actual, expected))
+			})
+		}
 	})
 }
 

registry/search_endpoint_v1.go

@@ -5,6 +5,7 @@ import (
 	"crypto/tls"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"net/http"
 	"net/url"
 	"strings"
@@ -58,7 +59,12 @@ func newV1Endpoint(ctx context.Context, index *registry.IndexInfo, headers http.
 		if endpoint.IsSecure {
 			// If registry is secure and HTTPS failed, show user the error and tell them about `--insecure-registry`
 			// in case that's what they need. DO NOT accept unknown CA certificates, and DO NOT fall back to HTTP.
-			return nil, invalidParamf("invalid registry endpoint %s: %v. If this private registry supports only HTTP or HTTPS with an unknown CA certificate, please add `--insecure-registry %s` to the daemon's arguments. In the case of HTTPS, if you have access to the registry's CA certificate, no need for the flag; simply place the CA certificate at /etc/docker/certs.d/%s/ca.crt", endpoint, err, endpoint.URL.Host, endpoint.URL.Host)
+			hint := fmt.Sprintf(
+				". If this private registry supports only HTTP or HTTPS with an unknown CA certificate, add `--insecure-registry %[1]s` to the daemon's arguments. "+
+					"In the case of HTTPS, if you have access to the registry's CA certificate, no need for the flag; place the CA certificate at /etc/docker/certs.d/%[1]s/ca.crt",
+				endpoint.URL.Host,
+			)
+			return nil, invalidParamf("invalid registry endpoint %s: %v%s", endpoint, err, hint)
 		}
 
 		// registry is insecure and HTTPS failed, fallback to HTTP.
@@ -163,9 +169,9 @@ func (e *v1Endpoint) ping(ctx context.Context) (v1PingResult, error) {
 
 // httpClient returns an HTTP client structure which uses the given transport
 // and contains the necessary headers for redirected requests
-func httpClient(transport http.RoundTripper) *http.Client {
+func httpClient(tr http.RoundTripper) *http.Client {
 	return &http.Client{
-		Transport:     transport,
+		Transport:     tr,
 		CheckRedirect: addRequiredHeadersToRedirectedRequests,
 	}
 }

registry/search_session.go

@@ -10,6 +10,7 @@ import (
 	"net/http"
 	"net/http/cookiejar"
 	"net/url"
+	"strconv"
 	"strings"
 	"sync"
 
@@ -219,7 +220,7 @@ func (r *session) searchRepositories(ctx context.Context, term string, limit int
 	if limit < 1 || limit > 100 {
 		return nil, invalidParamf("limit %d is outside the range of [1, 100]", limit)
 	}
-	u := r.indexEndpoint.String() + "search?q=" + url.QueryEscape(term) + "&n=" + url.QueryEscape(fmt.Sprintf("%d", limit))
+	u := r.indexEndpoint.String() + "search?q=" + url.QueryEscape(term) + "&n=" + url.QueryEscape(strconv.Itoa(limit))
 	log.G(ctx).WithField("url", u).Debug("searchRepositories")
 
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, http.NoBody)
@@ -236,7 +237,7 @@ func (r *session) searchRepositories(ctx context.Context, term string, limit int
 	if res.StatusCode != http.StatusOK {
 		// TODO(thaJeztah): return upstream response body for errors (see https://github.com/moby/moby/issues/27286).
 		// TODO(thaJeztah): handle other status-codes to return correct error-type
-		return nil, errUnknown{fmt.Errorf("Unexpected status code %d", res.StatusCode)}
+		return nil, errUnknown{fmt.Errorf("unexpected status code %d", res.StatusCode)}
 	}
 	result := &registry.SearchResults{}
 	err = json.NewDecoder(res.Body).Decode(result)

registry/search_test.go

@@ -16,6 +16,7 @@ import (
 )
 
 func spawnTestRegistrySession(t *testing.T) *session {
+	t.Helper()
 	authConfig := &registry.AuthConfig{}
 	endpoint, err := newV1Endpoint(context.Background(), makeIndex("/v1/"), nil)
 	if err != nil {
@@ -89,7 +90,7 @@ func TestSearchErrors(t *testing.T) {
 		expectedError     string
 	}{
 		{
-			expectedError:     "Unexpected status code 500",
+			expectedError:     "unexpected status code 500",
 			shouldReturnError: true,
 		},
 		{