Merge pull request #360 from sapcc/fix-gocritic

cmd/api/gui_redirect.go

@@ -82,9 +82,9 @@ func (g *guiRedirecter) tryRedirectToGUI(w http.ResponseWriter, r *http.Request)
 		if policy.Matches(ip, repo.Name, auth.AnonymousUserIdentity.UserName()) {
 			// do the redirect
 			s := g.urlStr
-			s = strings.Replace(s, "%AUTH_TENANT_ID%", account.AuthTenantID, -1)
-			s = strings.Replace(s, "%ACCOUNT_NAME%", account.Name, -1)
-			s = strings.Replace(s, "%REPO_NAME%", repo.Name, -1)
+			s = strings.ReplaceAll(s, "%AUTH_TENANT_ID%", account.AuthTenantID)
+			s = strings.ReplaceAll(s, "%ACCOUNT_NAME%", account.Name)
+			s = strings.ReplaceAll(s, "%REPO_NAME%", repo.Name)
 			w.Header().Set("Location", s)
 			w.WriteHeader(http.StatusFound)
 			return

cmd/healthmonitor/main.go

@@ -211,11 +211,12 @@ func (j *healthMonitorJob) ReportHealthcheckResult(w http.ResponseWriter, r *htt
 	lastResult := j.LastResult
 	j.LastResultLock.RUnlock()
 
-	if lastResult == nil {
+	switch {
+	case lastResult == nil:
 		http.Error(w, "still starting up", http.StatusServiceUnavailable)
-	} else if *lastResult {
+	case *lastResult:
 		w.WriteHeader(http.StatusNoContent)
-	} else {
+	default:
 		http.Error(w, "healthcheck failed", http.StatusInternalServerError)
 	}
 }

internal/api/auth/api_test.go

@@ -790,7 +790,7 @@ func TestAnycastAndDomainRemappedTokens(t *testing.T) {
 					}
 					req.ExpectBody = expectedContents
 				} else {
-					msg := strings.Replace(c.ErrorMessage, "%SERVICE%", domainPrefix+c.Service, -1)
+					msg := strings.ReplaceAll(c.ErrorMessage, "%SERVICE%", domainPrefix+c.Service)
 					req.ExpectStatus = http.StatusBadRequest
 					req.ExpectBody = assert.JSONObject{"details": msg}
 				}

internal/api/registry/blobs.go

@@ -91,18 +91,19 @@ func (a *API) handleGetOrHeadBlob(w http.ResponseWriter, r *http.Request) {
 		responseWasWritten, err := a.processor().ReplicateBlob(r.Context(), *blob, *account, *repo, w)
 
 		if err != nil {
-			if responseWasWritten {
+			switch {
+			case responseWasWritten:
 				// we cannot write to `w` if br.Execute() wrote a response there already
 				logg.Error("while trying to replicate blob %s in %s/%s: %s",
 					blob.Digest, account.Name, repo.Name, err.Error())
-			} else if errors.Is(err, processor.ErrConcurrentReplication) {
+			case errors.Is(err, processor.ErrConcurrentReplication):
 				// special handling for GET during ongoing replication (429 Too Many
 				// Requests is not a perfect match, but it's my best guess for getting
 				// clients to automatically retry the request after a few seconds)
 				w.Header().Set("Retry-After", "10")
 				msg := "currently replicating on a different worker, please retry in a few seconds"
 				keppel.ErrTooManyRequests.With(msg).WriteAsRegistryV2ResponseTo(w, r)
-			} else {
+			default:
 				respondWithError(w, r, err)
 			}
 		} else if !responseWasWritten {

internal/api/registry/catalog_test.go

@@ -133,7 +133,7 @@ func testNonEmptyCatalog(t *testing.T, s test.Setup) {
 				expectedPage = expectedPage[:length]
 				lastRepoName := expectedPage[len(expectedPage)-1]
 				expectedHeaders["Link"] = fmt.Sprintf(`</v2/_catalog?last=%s&n=%d>; rel="next"`,
-					strings.Replace(lastRepoName, "/", "%2F", -1), length,
+					strings.ReplaceAll(lastRepoName, "/", "%2F"), length,
 				)
 			}
 

internal/api/registry/shared_test.go

@@ -45,12 +45,12 @@ const authTenantID = "test1authtenant"
 func testWithPrimary(t *testing.T, setupOptions []test.SetupOption, action func(test.Setup)) {
 	test.WithRoundTripper(func(tt *test.RoundTripper) {
 		for _, withAnycast := range []bool{false, true} {
-			options := append(setupOptions,
+			setupOptions = append(setupOptions,
 				test.WithAnycast(withAnycast),
 				test.WithAccount(keppel.Account{Name: "test1", AuthTenantID: authTenantID}),
 				test.WithQuotas,
 			)
-			s := test.NewSetup(t, options...)
+			s := test.NewSetup(t, setupOptions...)
 			currentlyWithAnycast = withAnycast
 
 			// run the tests for this scenario

internal/api/registry/tags_test.go

@@ -108,7 +108,7 @@ func TestListTags(t *testing.T) {
 					expectedPage = expectedPage[:length]
 					lastRepoName := expectedPage[len(expectedPage)-1]
 					expectedHeaders["Link"] = fmt.Sprintf(`</v2/test1/foo/tags/list?last=%s&n=%d>; rel="next"`,
-						strings.Replace(lastRepoName, "/", "%2F", -1), length,
+						strings.ReplaceAll(lastRepoName, "/", "%2F"), length,
 					)
 				}
 

internal/auth/filter.go

@@ -55,11 +55,12 @@ func filterAuthorized(ir IncomingRequest, uid keppel.UserIdentity, audience Audi
 			}
 
 		case "keppel_api":
-			if scope.Contains(PeerAPIScope) && uid.UserType() == keppel.PeerUser {
+			switch {
+			case scope.Contains(PeerAPIScope) && uid.UserType() == keppel.PeerUser:
 				filtered.Actions = PeerAPIScope.Actions
-			} else if scope.Contains(InfoAPIScope) && uid.UserType() != keppel.AnonymousUser {
+			case scope.Contains(InfoAPIScope) && uid.UserType() != keppel.AnonymousUser:
 				filtered.Actions = InfoAPIScope.Actions
-			} else {
+			default:
 				filtered.Actions = nil
 			}
 
@@ -70,14 +71,15 @@ func filterAuthorized(ir IncomingRequest, uid keppel.UserIdentity, audience Audi
 			}
 
 		case "keppel_auth_tenant":
-			if audience.AccountName != "" {
+			switch {
+			case audience.AccountName != "":
 				// this type of scope is only used by the Keppel API, which does not
 				// allow domain-remapping anyway
 				filtered.Actions = nil
-			} else if audience.IsAnycast {
+			case audience.IsAnycast:
 				// defense in depth: any APIs requiring auth-tenant-level permission are not anycastable anyway
 				filtered.Actions = nil
-			} else {
+			default:
 				filtered.Actions = filterAuthTenantActions(scope.ResourceName, scope.Actions, uid)
 			}
 

internal/auth/request.go

@@ -141,12 +141,13 @@ func (ir IncomingRequest) Authorize(ctx context.Context, cfg keppel.Configuratio
 			return nil, rerr
 		}
 		if uid == nil {
-			if authHeader == "keppel" {
+			switch {
+			case authHeader == "keppel":
 				// do not fallback if we were explicitly instructed to only use driver auth
 				return nil, keppel.ErrUnauthorized.With("no credentials found in request")
-			} else if ir.NoImplicitAnonymous {
+			case ir.NoImplicitAnonymous:
 				return nil, keppel.ErrUnauthorized.With("no bearer token found in request headers").WithHeader("Www-Authenticate", ir.buildAuthChallenge(cfg, audience, ""))
-			} else {
+			default:
 				uid = AnonymousUserIdentity
 				allowChallenge = true
 			}

internal/keppel/errors.go

@@ -60,10 +60,8 @@ const (
 func (c RegistryV2ErrorCode) With(msg string, args ...any) *RegistryV2Error {
 	if msg == "" {
 		msg = apiErrorMessages[c]
-	} else {
-		if len(args) > 0 {
-			msg = fmt.Sprintf(msg, args...)
-		}
+	} else if len(args) > 0 {
+		msg = fmt.Sprintf(msg, args...)
 	}
 	return &RegistryV2Error{
 		Code:    c,

internal/models/imageref.go

@@ -83,7 +83,8 @@ func ParseImageReference(input string) (ImageReference, string, error) {
 	}
 
 	var ref ImageReference
-	if strings.Contains(imageURL.Path, "@") {
+	switch {
+	case strings.Contains(imageURL.Path, "@"):
 		// input references a digest
 		pathParts := ImageReferenceRx.FindStringSubmatch(imageURL.Path)
 		parsedDigest, err := digest.Parse(pathParts[len(pathParts)-1])
@@ -95,15 +96,15 @@ func ParseImageReference(input string) (ImageReference, string, error) {
 			RepoName:  strings.TrimPrefix(pathParts[1], "/"),
 			Reference: ManifestReference{Digest: parsedDigest},
 		}
-	} else if strings.Contains(imageURL.Path, ":") {
+	case strings.Contains(imageURL.Path, ":"):
 		// input references a tag name
 		pathParts := strings.SplitN(imageURL.Path, ":", 2)
 		ref = ImageReference{
 			Host:      imageURL.Host,
 			RepoName:  strings.TrimPrefix(pathParts[0], "/"),
 			Reference: ManifestReference{Tag: pathParts[1]},
 		}
-	} else {
+	default:
 		// input references no tag or digest - use default tag
 		ref = ImageReference{
 			Host:      imageURL.Host,