Merge branch 'artifacts-entry' into 'master'

Use Injecter to send entry instead of calling GitLab API

Implement https://gitlab.com/gitlab-org/gitlab-ce/issues/19224

I failed to update `internal/artifacts/artifact_download_test.go` :( Still trying how to fix that and `main_test.go`

See merge request !53

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 from %q for %q", params.Entry, 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/artifacts/artifact_download_test.go → internal/artifacts/entry_test.go

@@ -3,7 +3,6 @@ package artifacts
 import (
 	"archive/zip"
 	"encoding/base64"
-	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"net/http"
@@ -11,40 +10,29 @@ import (
 	"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 {
+func testEntryServer(t *testing.T, archive string, entry string) *httptest.ResponseRecorder {
 	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")
+		encodedEntry := base64.StdEncoding.EncodeToString([]byte(entry))
+		jsonParams := fmt.Sprintf(`{"Archive":"%s","Entry":"%s"}`, archive, encodedEntry)
+		data := base64.URLEncoding.EncodeToString([]byte(jsonParams))
 
-		data, err := json.Marshal(&api.Response{
-			Archive: archive,
-			Entry:   base64.StdEncoding.EncodeToString([]byte(entry)),
-		})
-		if err != nil {
-			t.Fatal(err)
-		}
-		w.Write(data)
+		SendEntry.Inject(w, r, 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)
+	httpRequest, err := http.NewRequest("GET", "/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)
+	mux.ServeHTTP(response, httpRequest)
 	return response
 }
 
@@ -65,10 +53,8 @@ func TestDownloadingFromValidArchive(t *testing.T) {
 	fmt.Fprint(fileInArchive, "testtest")
 	archive.Close()
 
-	ts := testArtifactDownloadServer(t, tempFile.Name(), "test.txt")
-	defer ts.Close()
+	response := testEntryServer(t, tempFile.Name(), "test.txt")
 
-	response := testDownloadArtifact(t, ts)
 	testhelper.AssertResponseCode(t, response, 200)
 
 	testhelper.AssertResponseHeader(t, response,
@@ -93,25 +79,16 @@ func TestDownloadingNonExistingFile(t *testing.T) {
 	defer archive.Close()
 	archive.Close()
 
-	ts := testArtifactDownloadServer(t, tempFile.Name(), "test")
-	defer ts.Close()
-
-	response := testDownloadArtifact(t, ts)
+	response := testEntryServer(t, tempFile.Name(), "test")
 	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)
+	response := testEntryServer(t, "path/to/non/existing/file", "test")
 	testhelper.AssertResponseCode(t, response, 404)
 }
 
 func TestIncompleteApiResponse(t *testing.T) {
-	ts := testArtifactDownloadServer(t, "", "")
-	defer ts.Close()
-
-	response := testDownloadArtifact(t, ts)
+	response := testEntryServer(t, "", "")
 	testhelper.AssertResponseCode(t, response, 500)
 }

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

main_test.go

@@ -552,9 +552,9 @@ func TestArtifactsGetSingleFile(t *testing.T) {
 	resourcePath := `/namespace/project/builds/123/artifacts/file/` + fileName
 	ts := testhelper.TestServerWithHandler(regexp.MustCompile(`\A`+resourcePath+`\z`), func(w http.ResponseWriter, r *http.Request) {
 		encodedFilename := base64.StdEncoding.EncodeToString([]byte(fileName))
-		if _, err := fmt.Fprintf(w, `{"Archive":"%s","Entry":"%s"}`, archivePath, encodedFilename); err != nil {
-			t.Fatal(err)
-		}
+		jsonParams := fmt.Sprintf(`{"Archive":"%s","Entry":"%s"}`, archivePath, encodedFilename)
+		data := base64.URLEncoding.EncodeToString([]byte(jsonParams))
+		w.Header().Set("Gitlab-Workhorse-Send-Data", "artifacts-entry:"+data)
 		return
 	})
 	defer ts.Close()