Use Injecter to send entry instead of calling GitLab API

TODO: Fix tests!

internal/artifacts/artifact_download_test.go

-package artifacts
-
-import (
-	"archive/zip"
-	"encoding/base64"
-	"encoding/json"
-	"fmt"
-	"io/ioutil"
-	"net/http"
-	"net/http/httptest"
-	"os"
-	"testing"
-
-	"gitlab.com/gitlab-org/gitlab-workhorse/internal/api"
-	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
-	"gitlab.com/gitlab-org/gitlab-workhorse/internal/testhelper"
-)
-
-func testArtifactDownloadServer(t *testing.T, archive string, entry string) *httptest.Server {
-	mux := http.NewServeMux()
-	mux.HandleFunc("/url/path", func(w http.ResponseWriter, r *http.Request) {
-		if r.Method != "GET" {
-			t.Fatal("Expected GET request")
-		}
-
-		w.Header().Set("Content-Type", "application/json")
-
-		data, err := json.Marshal(&api.Response{
-			Archive: archive,
-			Entry:   base64.StdEncoding.EncodeToString([]byte(entry)),
-		})
-		if err != nil {
-			t.Fatal(err)
-		}
-		w.Write(data)
-	})
-	return testhelper.TestServerWithHandler(nil, mux.ServeHTTP)
-}
-
-func testDownloadArtifact(t *testing.T, ts *httptest.Server) *httptest.ResponseRecorder {
-	httpRequest, err := http.NewRequest("GET", ts.URL+"/url/path", nil)
-	if err != nil {
-		t.Fatal(err)
-	}
-	response := httptest.NewRecorder()
-	apiClient := api.NewAPI(helper.URLMustParse(ts.URL), "123", nil)
-	DownloadArtifact(apiClient).ServeHTTP(response, httpRequest)
-	return response
-}
-
-func TestDownloadingFromValidArchive(t *testing.T) {
-	tempFile, err := ioutil.TempFile("", "uploads")
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer tempFile.Close()
-	defer os.Remove(tempFile.Name())
-
-	archive := zip.NewWriter(tempFile)
-	defer archive.Close()
-	fileInArchive, err := archive.Create("test.txt")
-	if err != nil {
-		t.Fatal(err)
-	}
-	fmt.Fprint(fileInArchive, "testtest")
-	archive.Close()
-
-	ts := testArtifactDownloadServer(t, tempFile.Name(), "test.txt")
-	defer ts.Close()
-
-	response := testDownloadArtifact(t, ts)
-	testhelper.AssertResponseCode(t, response, 200)
-
-	testhelper.AssertResponseHeader(t, response,
-		"Content-Type",
-		"text/plain; charset=utf-8")
-	testhelper.AssertResponseHeader(t, response,
-		"Content-Disposition",
-		"attachment; filename=\"test.txt\"")
-
-	testhelper.AssertResponseBody(t, response, "testtest")
-}
-
-func TestDownloadingNonExistingFile(t *testing.T) {
-	tempFile, err := ioutil.TempFile("", "uploads")
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer tempFile.Close()
-	defer os.Remove(tempFile.Name())
-
-	archive := zip.NewWriter(tempFile)
-	defer archive.Close()
-	archive.Close()
-
-	ts := testArtifactDownloadServer(t, tempFile.Name(), "test")
-	defer ts.Close()
-
-	response := testDownloadArtifact(t, ts)
-	testhelper.AssertResponseCode(t, response, 404)
-}
-
-func TestDownloadingFromInvalidArchive(t *testing.T) {
-	ts := testArtifactDownloadServer(t, "path/to/non/existing/file", "test")
-	defer ts.Close()
-
-	response := testDownloadArtifact(t, ts)
-	testhelper.AssertResponseCode(t, response, 404)
-}
-
-func TestIncompleteApiResponse(t *testing.T) {
-	ts := testArtifactDownloadServer(t, "", "")
-	defer ts.Close()
-
-	response := testDownloadArtifact(t, ts)
-	testhelper.AssertResponseCode(t, response, 500)
-}

internal/artifacts/artifact_download.go → internal/artifacts/entry.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"log"
 	"mime"
 	"net/http"
 	"os"
@@ -13,11 +14,40 @@ import (
 	"strings"
 	"syscall"
 
-	"gitlab.com/gitlab-org/gitlab-workhorse/internal/api"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
+	"gitlab.com/gitlab-org/gitlab-workhorse/internal/senddata"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/zipartifacts"
 )
 
+type entry struct{ senddata.Prefix }
+type entryParams struct{ Archive, Entry string }
+
+var SendEntry = &entry{"artifacts-entry:"}
+
+// Artifacts downloader doesn't support ranges when downloading a single file
+func (e *entry) Inject(w http.ResponseWriter, r *http.Request, sendData string) {
+	var params entryParams
+	if err := e.Unpack(&params, sendData); err != nil {
+		helper.Fail500(w, fmt.Errorf("SendEntry: unpack sendData: %v", err))
+		return
+	}
+
+	log.Printf("SendEntry: sending %q for %q", params.Archive, r.URL.Path)
+
+	if params.Archive == "" || params.Entry == "" {
+		helper.Fail500(w, errors.New("SendEntry: Archive or Entry is empty"))
+		return
+	}
+
+	err := unpackFileFromZip(params.Archive, params.Entry, w.Header(), w)
+
+	if os.IsNotExist(err) {
+		http.NotFound(w, r)
+	} else if err != nil {
+		helper.Fail500(w, fmt.Errorf("SendEntry: %v", err))
+	}
+}
+
 func detectFileContentType(fileName string) string {
 	contentType := mime.TypeByExtension(filepath.Ext(fileName))
 	if contentType == "" {
@@ -80,21 +110,3 @@ func waitCatFile(cmd *exec.Cmd) error {
 	return fmt.Errorf("wait for %v to finish: %v", cmd.Args, err)
 
 }
-
-// Artifacts downloader doesn't support ranges when downloading a single file
-func DownloadArtifact(myAPI *api.API) http.Handler {
-	return myAPI.PreAuthorizeHandler(func(w http.ResponseWriter, r *http.Request, a *api.Response) {
-		if a.Archive == "" || a.Entry == "" {
-			helper.Fail500(w, errors.New("DownloadArtifact: Archive or Path is empty"))
-			return
-		}
-
-		err := unpackFileFromZip(a.Archive, a.Entry, w.Header(), w)
-		if os.IsNotExist(err) {
-			http.NotFound(w, r)
-			return
-		} else if err != nil {
-			helper.Fail500(w, fmt.Errorf("DownloadArtifact: %v", err))
-		}
-	}, "")
-}

internal/upstream/routes.go

@@ -50,6 +50,7 @@ func (u *Upstream) configureRoutes() {
 		git.SendBlob,
 		git.SendDiff,
 		git.SendPatch,
+		artifacts.SendEntry,
 	)
 
 	u.Routes = []route{
@@ -60,7 +61,6 @@ func (u *Upstream) configureRoutes() {
 		route{"PUT", regexp.MustCompile(gitProjectPattern + `gitlab-lfs/objects/([0-9a-f]{64})/([0-9]+)\z`), lfs.PutStore(api, proxy)},
 
 		// CI Artifacts
-		route{"GET", regexp.MustCompile(projectPattern + `builds/[0-9]+/artifacts/file/`), contentEncodingHandler(artifacts.DownloadArtifact(api))},
 		route{"POST", regexp.MustCompile(ciAPIPattern + `v1/builds/[0-9]+/artifacts\z`), contentEncodingHandler(artifacts.UploadArtifacts(api, proxy))},
 
 		// Explicitly proxy API requests