Lint: fixes for upload artifacts package

workhorse/internal/upload/artifacts_store_test.go

+// Package upload contains tests for artifact storage functionality.
 package upload
 
 import (
@@ -21,6 +22,10 @@ import (
 	"gitlab.com/gitlab-org/gitlab/workhorse/internal/upload/destination/objectstore/test"
 )
 
+const (
+	putURL = "/url/put"
+)
+
 func createTestZipArchive(t *testing.T) (data []byte, md5Hash string) {
 	var buffer bytes.Buffer
 	archive := zip.NewWriter(&buffer)
@@ -67,7 +72,7 @@ func TestUploadHandlerSendingToExternalStorage(t *testing.T) {
 
 	storeServerCalled := 0
 	storeServerMux := http.NewServeMux()
-	storeServerMux.HandleFunc("/url/put", func(w http.ResponseWriter, r *http.Request) {
+	storeServerMux.HandleFunc(putURL, func(w http.ResponseWriter, r *http.Request) {
 		require.Equal(t, "PUT", r.Method)
 
 		receivedData, err := io.ReadAll(r.Body)
@@ -103,7 +108,7 @@ func TestUploadHandlerSendingToExternalStorage(t *testing.T) {
 			name: "ObjectStore Upload",
 			preauth: &api.Response{
 				RemoteObject: api.RemoteObject{
-					StoreURL: storeServer.URL + "/url/put" + qs,
+					StoreURL: storeServer.URL + putURL + qs,
 					ID:       "store-id",
 					GetURL:   storeServer.URL + "/store-id",
 				},
@@ -177,7 +182,7 @@ func TestUploadHandlerSendingToExternalStorageAndItReturnsAnError(t *testing.T)
 	putCalledTimes := 0
 
 	storeServerMux := http.NewServeMux()
-	storeServerMux.HandleFunc("/url/put", func(w http.ResponseWriter, r *http.Request) {
+	storeServerMux.HandleFunc(putURL, func(w http.ResponseWriter, r *http.Request) {
 		putCalledTimes++
 		require.Equal(t, "PUT", r.Method)
 		w.WriteHeader(510)
@@ -192,7 +197,7 @@ func TestUploadHandlerSendingToExternalStorageAndItReturnsAnError(t *testing.T)
 
 	authResponse := &api.Response{
 		RemoteObject: api.RemoteObject{
-			StoreURL: storeServer.URL + "/url/put",
+			StoreURL: storeServer.URL + putURL,
 			ID:       "store-id",
 		},
 	}
@@ -208,7 +213,7 @@ func TestUploadHandlerSendingToExternalStorageAndItReturnsAnError(t *testing.T)
 func TestUploadHandlerSendingToExternalStorageAndSupportRequestTimeout(t *testing.T) {
 	shutdown := make(chan struct{})
 	storeServerMux := http.NewServeMux()
-	storeServerMux.HandleFunc("/url/put", func(w http.ResponseWriter, r *http.Request) {
+	storeServerMux.HandleFunc(putURL, func(w http.ResponseWriter, r *http.Request) {
 		<-shutdown
 	})
 
@@ -224,7 +229,7 @@ func TestUploadHandlerSendingToExternalStorageAndSupportRequestTimeout(t *testin
 
 	authResponse := &api.Response{
 		RemoteObject: api.RemoteObject{
-			StoreURL: storeServer.URL + "/url/put",
+			StoreURL: storeServer.URL + putURL,
 			ID:       "store-id",
 			Timeout:  0.1,
 		},

workhorse/internal/upload/artifacts_upload_test.go

@@ -77,11 +77,9 @@ func testArtifactsUploadServer(t *testing.T, authResponse *api.Response, bodyPro
 				t.Fatal("Expected file to be readable")
 				return
 			}
-		} else {
-			if r.FormValue("file.remote_url") == "" {
-				t.Fatal("Expected file to be remote accessible")
-				return
-			}
+		} else if r.FormValue("file.remote_url") == "" {
+			t.Fatal("Expected file to be remote accessible")
+			return
 		}
 
 		if r.FormValue("metadata.path") != "" {
@@ -107,7 +105,6 @@ func testArtifactsUploadServer(t *testing.T, authResponse *api.Response, bodyPro
 			}
 
 			w.Header().Set(MetadataHeaderKey, MetadataHeaderPresent)
-
 		} else {
 			w.Header().Set(MetadataHeaderKey, MetadataHeaderMissing)
 		}
@@ -204,7 +201,7 @@ func TestUploadHandlerAddingMetadata(t *testing.T) {
 					require.NoError(t, err)
 
 					rewrittenFields := token.Claims.(*MultipartClaims).RewrittenFields
-					require.Equal(t, 2, len(rewrittenFields))
+					require.Len(t, rewrittenFields, 2)
 
 					require.Contains(t, rewrittenFields, "file")
 					require.Contains(t, rewrittenFields, "metadata")
@@ -235,7 +232,7 @@ func TestUploadHandlerTarArtifact(t *testing.T) {
 			require.NoError(t, err)
 
 			rewrittenFields := token.Claims.(*MultipartClaims).RewrittenFields
-			require.Equal(t, 1, len(rewrittenFields))
+			require.Len(t, rewrittenFields, 1)
 
 			require.Contains(t, rewrittenFields, "file")
 			require.Contains(t, r.PostForm, "file.gitlab-workhorse-upload")

workhorse/internal/upload/artifacts_uploader.go

@@ -34,7 +34,7 @@ const (
 var zipSubcommandsErrorsCounter = promauto.NewCounterVec(
 	prometheus.CounterOpts{
 		Name: "gitlab_workhorse_zip_subcommand_errors_total",
-		Help: "Errors comming from subcommands used for processing ZIP archives",
+		Help: "Errors coming from subcommands used for processing ZIP archives",
 	}, []string{"error"})
 
 type artifactsUploadProcessor struct {
@@ -80,12 +80,20 @@ func (a *artifactsUploadProcessor) generateMetadataFromZip(ctx context.Context,
 	if err != nil {
 		return nil, err
 	}
-	defer zipMdOut.Close()
+	defer func() {
+		if err = zipMdOut.Close(); err != nil {
+			log.ContextLogger(ctx).WithError(err).Error("Failed to close zip-metadata stdout")
+		}
+	}()
 
-	if err := zipMd.Start(); err != nil {
+	if err = zipMd.Start(); err != nil {
 		return nil, err
 	}
-	defer command.KillProcessGroup(zipMd)
+	defer func() {
+		if err = command.KillProcessGroup(zipMd); err != nil {
+			log.ContextLogger(ctx).WithError(err).Error("Failed to kill zip-metadata process group")
+		}
+	}()
 
 	fh, err := destination.Upload(ctx, zipMdOut, -1, "metadata.gz", metaOpts)
 	if err != nil {
@@ -146,7 +154,9 @@ func (a *artifactsUploadProcessor) ProcessFile(ctx context.Context, formName str
 		}
 
 		for k, v := range fields {
-			writer.WriteField(k, v)
+			if err := writer.WriteField(k, v); err != nil {
+				return fmt.Errorf("write metadata field error: %v", err)
+			}
 		}
 
 		a.Track("metadata", metadata.LocalPath)

workhorse/internal/upload/body_uploader.go

+// Package upload provides middleware for handling request bodies and uploading them to a destination.
 package upload
 
 import (

workhorse/internal/upload/body_uploader_test.go

@@ -29,6 +29,7 @@ func TestRequestBody(t *testing.T) {
 	body := strings.NewReader(fileContent)
 
 	resp := testUpload(&rails{}, &alwaysLocalPreparer{}, echoProxy(t, fileLen), body)
+	defer resp.Body.Close()
 	require.Equal(t, http.StatusOK, resp.StatusCode)
 
 	uploadEcho, err := io.ReadAll(resp.Body)
@@ -41,6 +42,7 @@ func TestRequestBodyCustomPreparer(t *testing.T) {
 	body := strings.NewReader(fileContent)
 
 	resp := testUpload(&rails{}, &alwaysLocalPreparer{}, echoProxy(t, fileLen), body)
+	defer resp.Body.Close()
 	require.Equal(t, http.StatusOK, resp.StatusCode)
 
 	uploadEcho, err := io.ReadAll(resp.Body)
@@ -73,6 +75,7 @@ func testNoProxyInvocation(t *testing.T, expectedStatus int, auth PreAuthorizer,
 	})
 
 	resp := testUpload(auth, preparer, proxy, nil)
+	defer resp.Body.Close()
 	require.Equal(t, expectedStatus, resp.StatusCode)
 }
 
@@ -128,7 +131,7 @@ func echoProxy(t *testing.T, expectedBodyLength int) http.Handler {
 		uploaded, err := os.Open(path)
 		require.NoError(t, err, "File not uploaded")
 
-		//sending back the file for testing purpose
+		// sending back the file for testing purpose
 		io.Copy(w, uploaded)
 	})
 }