Merge branch '83-artifact-proxy-changes' into 'master'

Rework the artifacts proxy to operate within Pages group domains Closes #83 See merge request gitlab-org/gitlab-pages!46
author: Kamil Trzciński <ayufan@ayufan.eu> 2017-10-02 16:49:55 +0300
committer: Kamil Trzciński <ayufan@ayufan.eu> 2017-10-02 16:49:55 +0300
commit: 26a29a445dcd363f7cd8854f98fcc4a62ad69d44 (patch)
tree: 36a0dad27af2230c494af83110637d409c46a670
parent: b322a578e5cfa144f5d0c5be3f43b2a0c9d8d42e (diff)
parent: e8e40691069f1475ee7798a584352a80e892b635 (diff)
4 files changed, 191 insertions, 212 deletions
diff --git a/acceptance_test.go b/acceptance_test.go
index d6fc1400..f4e7e007 100644
--- a/acceptance_test.go
+++ b/acceptance_test.go
@@ -212,22 +212,25 @@ func TestStatusPage(t *testing.T) {
 	assert.Equal(t, http.StatusOK, rsp.StatusCode)
 }
 
-func TestProxyRequest(t *testing.T) {
+func TestArtifactProxyRequest(t *testing.T) {
 	skipUnlessEnabled(t)
 	content := "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>"
 	contentLength := int64(len(content))
 	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		switch r.URL.Path {
-		case "/projects/1/jobs/2/artifacts/delayed_200.html":
+		switch r.URL.RawPath {
+		case "/api/v4/projects/group%2Fproject/jobs/1/artifacts/delayed_200.html":
 			time.Sleep(2 * time.Second)
-		case "/projects/1/jobs/2/artifacts/200.html":
+			fallthrough
+		case "/api/v4/projects/group%2Fproject/jobs/1/artifacts/200.html",
+			"/api/v4/projects/group%2Fsubgroup%2Fproject/jobs/1/artifacts/200.html":
 			w.Header().Set("Content-Type", "text/html; charset=utf-8")
 			fmt.Fprint(w, content)
-		case "/projects/1/jobs/2/artifacts/500.html":
+		case "/api/v4/projects/group%2Fproject/jobs/1/artifacts/500.html":
 			w.Header().Set("Content-Type", "text/html; charset=utf-8")
 			w.WriteHeader(http.StatusInternalServerError)
 			fmt.Fprint(w, content)
 		default:
+			t.Logf("Unexpected r.URL.RawPath: %q", r.URL.RawPath)
 			w.Header().Set("Content-Type", "text/html; charset=utf-8")
 			w.WriteHeader(http.StatusNotFound)
 			fmt.Fprint(w, content)
@@ -235,6 +238,7 @@ func TestProxyRequest(t *testing.T) {
 	}))
 	defer testServer.Close()
 	cases := []struct {
+		Host         string
 		Path         string
 		Status       int
 		BinaryOption string
@@ -245,7 +249,8 @@ func TestProxyRequest(t *testing.T) {
 		Description  string
 	}{
 		{
-			"200.html",
+			"group.gitlab-example.com",
+			"/-/project/-/jobs/1/artifacts/200.html",
 			http.StatusOK,
 			"",
 			content,
@@ -255,7 +260,19 @@ func TestProxyRequest(t *testing.T) {
 			"basic proxied request",
 		},
 		{
-			"delayed_200.html",
+			"group.gitlab-example.com",
+			"/-/subgroup/project/-/jobs/1/artifacts/200.html",
+			http.StatusOK,
+			"",
+			content,
+			contentLength,
+			"max-age=3600",
+			"text/html; charset=utf-8",
+			"basic proxied request for subgroup",
+		},
+		{
+			"group.gitlab-example.com",
+			"/-/project/-/jobs/1/artifacts/delayed_200.html",
 			http.StatusBadGateway,
 			"-artifacts-server-timeout=1",
 			"",
@@ -265,7 +282,8 @@ func TestProxyRequest(t *testing.T) {
 			"502 error while attempting to proxy",
 		},
 		{
-			"404.html",
+			"group.gitlab-example.com",
+			"/-/project/-/jobs/1/artifacts/404.html",
 			http.StatusNotFound,
 			"",
 			"",
@@ -275,7 +293,8 @@ func TestProxyRequest(t *testing.T) {
 			"Proxying 404 from server",
 		},
 		{
-			"500.html",
+			"group.gitlab-example.com",
+			"/-/project/-/jobs/1/artifacts/500.html",
 			http.StatusInternalServerError,
 			"",
 			"",
@@ -288,9 +307,9 @@ func TestProxyRequest(t *testing.T) {
 
 	for _, c := range cases {
 		t.Run(fmt.Sprintf("Proxy Request Test: %s", c.Description), func(t *testing.T) {
-			teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-artifacts-server="+testServer.URL, c.BinaryOption)
+			teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-artifacts-server="+testServer.URL+"/api/v4", c.BinaryOption)
 			defer teardown()
-			resp, err := GetPageFromListener(t, httpListener, "artifact~1~2.gitlab-example.com", c.Path)
+			resp, err := GetPageFromListener(t, httpListener, c.Host, c.Path)
 			require.NoError(t, err)
 			defer resp.Body.Close()
 			assert.Equal(t, c.Status, resp.StatusCode)
diff --git a/helpers_test.go b/helpers_test.go
index c1d1bf34..9f372224 100644
--- a/helpers_test.go
+++ b/helpers_test.go
@@ -10,6 +10,7 @@ import (
 	"net/http"
 	"os"
 	"os/exec"
+	"strings"
 	"testing"
 	"time"
 
@@ -113,6 +114,8 @@ func (l ListenSpec) URL(suffix string) string {
 		scheme = "https"
 	}
 
+	suffix = strings.TrimPrefix(suffix, "/")
+
 	return fmt.Sprintf("%s://%s/%s", scheme, l.JoinHostPort(), suffix)
 }
 
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index bcb525ac..100ec90d 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -5,6 +5,7 @@ import (
 	"io"
 	"net/http"
 	"net/url"
+	"path"
 	"regexp"
 	"strconv"
 	"strings"
@@ -14,58 +15,68 @@ import (
 )
 
 const (
-	baseURL             = "/projects/%s/jobs/%s/artifacts"
-	hostPatternTemplate = `(?i)\Aartifact~(\d+)~(\d+)\.%s\z`
-	minStatusCode       = 200
-	maxStatusCode       = 299
+	// Format a non-/-suffixed URL, an escaped project full_path, a job ID and
+	// a /-prefixed file path into an URL string
+	apiURLTemplate = "%s/projects/%s/jobs/%s/artifacts%s"
+
+	minStatusCode = 200
+	maxStatusCode = 299
+)
+
+var (
+	// Captures subgroup + project, job ID and artifacts path
+	pathExtractor = regexp.MustCompile(`(?i)\A/-/(.*)/-/jobs/(\d+)/artifacts(/[^?]*)\z`)
 )
 
-// Artifact is a struct that is made up of a url.URL, http.Client, and
-// regexp.Regexp that is used to proxy requests where applicable.
+// Artifact proxies requests for artifact files to the GitLab artifacts API
 type Artifact struct {
-	server  string
-	client  *http.Client
-	pattern *regexp.Regexp
+	server string
+	suffix string
+	client *http.Client
 }
 
 // New when provided the arguments defined herein, returns a pointer to an
 // Artifact that is used to proxy requests.
-func New(s string, timeout int, pagesDomain string) *Artifact {
+func New(server string, timeoutSeconds int, pagesDomain string) *Artifact {
 	return &Artifact{
-		server:  s,
-		client:  &http.Client{Timeout: time.Second * time.Duration(timeout)},
-		pattern: hostPatternGen(pagesDomain),
+		server: strings.TrimRight(server, "/"),
+		suffix: "." + strings.ToLower(pagesDomain),
+		client: &http.Client{Timeout: time.Second * time.Duration(timeoutSeconds)},
 	}
-
 }
 
 // TryMakeRequest will attempt to proxy a request and write it to the argument
 // http.ResponseWriter, ultimately returning a bool that indicates if the
 // http.ResponseWriter has been written to in any capacity.
 func (a *Artifact) TryMakeRequest(host string, w http.ResponseWriter, r *http.Request) bool {
-	if a == nil || a.server == "" {
+	if a == nil || a.server == "" || host == "" {
 		return false
 	}
 
-	reqURL, ok := a.buildURL(host, r.URL.Path)
+	reqURL, ok := a.BuildURL(host, r.URL.Path)
 	if !ok {
 		return false
 	}
 
+	a.makeRequest(w, reqURL)
+	return true
+}
+
+func (a *Artifact) makeRequest(w http.ResponseWriter, reqURL *url.URL) {
 	resp, err := a.client.Get(reqURL.String())
 	if err != nil {
 		httperrors.Serve502(w)
-		return true
+		return
 	}
 
 	if resp.StatusCode == http.StatusNotFound {
 		httperrors.Serve404(w)
-		return true
+		return
 	}
 
 	if resp.StatusCode == http.StatusInternalServerError {
 		httperrors.Serve500(w)
-		return true
+		return
 	}
 
 	// we only cache responses within the 2xx series response codes
@@ -77,44 +88,43 @@ func (a *Artifact) TryMakeRequest(host string, w http.ResponseWriter, r *http.Re
 	w.Header().Set("Content-Length", strconv.FormatInt(resp.ContentLength, 10))
 	w.WriteHeader(resp.StatusCode)
 	io.Copy(w, resp.Body)
-	return true
+	return
 }
 
-// buildURL returns a pointer to a url.URL for where the request should be
+// BuildURL returns a pointer to a url.URL for where the request should be
 // proxied to. The returned bool will indicate if there is some sort of issue
 // with the url while it is being generated.
-func (a *Artifact) buildURL(host, path string) (*url.URL, bool) {
-	ids := a.pattern.FindAllStringSubmatch(host, -1)
-	if len(ids) != 1 || len(ids[0]) != 3 {
+//
+// The URL is generated from the host (which contains the top-level group and
+// ends with the pagesDomain) and the path (which contains any subgroups, the
+// project, a job ID and a path
+// for the artifact file we want to download)
+func (a *Artifact) BuildURL(host, requestPath string) (*url.URL, bool) {
+	if !strings.HasSuffix(strings.ToLower(host), a.suffix) {
 		return nil, false
 	}
 
-	strippedIds := ids[0][1:3]
-	body := fmt.Sprintf(baseURL, strippedIds[0], strippedIds[1])
-	ourPath := a.server
-	if strings.HasSuffix(ourPath, "/") {
-		ourPath = ourPath[0:len(ourPath)-1] + body
-	} else {
-		ourPath = ourPath + body
+	topGroup := host[0 : len(host)-len(a.suffix)]
+
+	parts := pathExtractor.FindAllStringSubmatch(requestPath, 1)
+	if len(parts) != 1 || len(parts[0]) != 4 {
+		return nil, false
 	}
 
-	if len(path) == 0 || strings.HasPrefix(path, "/") {
-		ourPath = ourPath + path
-	} else {
-		ourPath = ourPath + "/" + path
+	restOfPath := strings.TrimLeft(strings.TrimRight(parts[0][1], "/"), "/")
+	if len(restOfPath) == 0 {
+		return nil, false
 	}
 
-	u, err := url.Parse(ourPath)
+	jobID := parts[0][2]
+	artifactPath := parts[0][3]
+
+	projectID := url.PathEscape(path.Join(topGroup, restOfPath))
+	generated := fmt.Sprintf(apiURLTemplate, a.server, projectID, jobID, artifactPath)
+
+	u, err := url.Parse(generated)
 	if err != nil {
 		return nil, false
 	}
 	return u, true
 }
-
-// hostPatternGen returns a pointer to a regexp.Regexp that is made up of
-// the constant hostPatternTemplate and the argument which represents the pages domain.
-// This is used to ensure that the requested page meets not only the hostPatternTemplate
-// requirements, but is suffixed with the proper pagesDomain.
-func hostPatternGen(pagesDomain string) *regexp.Regexp {
-	return regexp.MustCompile(fmt.Sprintf(hostPatternTemplate, regexp.QuoteMeta(pagesDomain)))
-}
diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go
index ad2285f6..62962513 100644
--- a/internal/artifact/artifact_test.go
+++ b/internal/artifact/artifact_test.go
@@ -1,15 +1,15 @@
-package artifact
+package artifact_test
 
 import (
 	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
-	"regexp"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/assert"
+
+	"gitlab.com/gitlab-org/gitlab-pages/internal/artifact"
 )
 
 func TestTryMakeRequest(t *testing.T) {
@@ -17,17 +17,20 @@ func TestTryMakeRequest(t *testing.T) {
 	contentType := "text/html; charset=utf-8"
 	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", contentType)
-		switch r.URL.Path {
-		case "/projects/1/jobs/2/artifacts/200.html":
+		switch r.URL.RawPath {
+		case "/projects/group%2Fsubgroup%2Fproject/jobs/1/artifacts/200.html":
 			w.WriteHeader(http.StatusOK)
-		case "/projects/1/jobs/2/artifacts/max-caching.html":
+		case "/projects/group%2Fsubgroup%2Fproject/jobs/1/artifacts/max-caching.html":
 			w.WriteHeader(http.StatusIMUsed)
-		case "/projects/1/jobs/2/artifacts/non-caching.html":
+		case "/projects/group%2Fsubgroup%2Fproject/jobs/1/artifacts/non-caching.html":
 			w.WriteHeader(http.StatusTeapot)
-		case "/projects/1/jobs/2/artifacts/500.html":
+		case "/projects/group%2Fsubgroup%2Fproject/jobs/1/artifacts/500.html":
 			w.WriteHeader(http.StatusInternalServerError)
-		case "/projects/1/jobs/2/artifacts/404.html":
+		case "/projects/group%2Fsubgroup%2Fgroup%2Fproject/jobs/1/artifacts/404.html":
 			w.WriteHeader(http.StatusNotFound)
+		default:
+			t.Log("Surprising r.URL.RawPath", r.URL.RawPath)
+			w.WriteHeader(999)
 		}
 		fmt.Fprint(w, content)
 	}))
@@ -72,22 +75,21 @@ func TestTryMakeRequest(t *testing.T) {
 	}
 
 	for _, c := range cases {
-		result := httptest.NewRecorder()
-		reqURL, err := url.Parse(c.Path)
-		assert.NoError(t, err)
-		r := &http.Request{URL: reqURL}
-		art := &Artifact{
-			server:  testServer.URL,
-			client:  &http.Client{Timeout: time.Second * time.Duration(1)},
-			pattern: regexp.MustCompile(fmt.Sprintf(hostPatternTemplate, "gitlab-example.io")),
-		}
+		t.Run(c.Description, func(t *testing.T) {
+			result := httptest.NewRecorder()
+			reqURL, err := url.Parse("/-/subgroup/project/-/jobs/1/artifacts" + c.Path)
+			assert.NoError(t, err)
+			r := &http.Request{URL: reqURL}
+			art := artifact.New(testServer.URL, 1, "gitlab-example.io")
+
+			assert.True(t, art.TryMakeRequest("group.gitlab-example.io", result, r))
+			assert.Equal(t, c.Status, result.Code)
+			assert.Equal(t, c.ContentType, result.Header().Get("Content-Type"))
+			assert.Equal(t, c.Length, result.Header().Get("Content-Length"))
+			assert.Equal(t, c.CacheControl, result.Header().Get("Cache-Control"))
+			assert.Equal(t, c.Content, string(result.Body.Bytes()))
 
-		assert.True(t, art.TryMakeRequest("artifact~1~2.gitlab-example.io", result, r))
-		assert.Equal(t, c.ContentType, result.Header().Get("Content-Type"))
-		assert.Equal(t, c.Length, result.Header().Get("Content-Length"))
-		assert.Equal(t, c.CacheControl, result.Header().Get("Cache-Control"))
-		assert.Equal(t, c.Content, string(result.Body.Bytes()))
-		assert.Equal(t, c.Status, result.Code)
+		})
 	}
 }
 
@@ -103,201 +105,146 @@ func TestBuildURL(t *testing.T) {
 	}{
 		{
 			"https://gitlab.com/api/v4",
-			"artifact~1~2.gitlab.io",
-			"/path/to/file.txt",
-			"https://gitlab.com/api/v4/projects/1/jobs/2/artifacts/path/to/file.txt",
+			"group.gitlab.io",
+			"/-/project/-/jobs/1/artifacts/",
+			`https://gitlab.com/api/v4/projects/group%2Fproject/jobs/1/artifacts/`,
 			"gitlab.io",
 			true,
-			"basic case",
+			"Basic case",
 		},
 		{
 			"https://gitlab.com/api/v4/",
-			"artifact~1~2.gitlab.io",
-			"/path/to/file.txt",
-			"https://gitlab.com/api/v4/projects/1/jobs/2/artifacts/path/to/file.txt",
-			"gitlab.io",
-			true,
-			"basic case 2",
-		},
-		{
-			"https://gitlab.com/api/v4",
-			"artifact~1~2.gitlab.io",
-			"path/to/file.txt",
-			"https://gitlab.com/api/v4/projects/1/jobs/2/artifacts/path/to/file.txt",
+			"group.gitlab.io",
+			"/-/project/-/jobs/1/artifacts/",
+			"https://gitlab.com/api/v4/projects/group%2Fproject/jobs/1/artifacts/",
 			"gitlab.io",
 			true,
-			"basic case 3",
+			"API URL has trailing slash",
 		},
 		{
 			"https://gitlab.com/api/v4/",
-			"artifact~1~2.gitlab.io",
-			"path/to/file.txt",
-			"https://gitlab.com/api/v4/projects/1/jobs/2/artifacts/path/to/file.txt",
+			"GROUP.GITLAB.IO",
+			"/-/SUBGROUP/PROJECT/-/JOBS/1/ARTIFACTS/PATH/TO/FILE.txt",
+			"https://gitlab.com/api/v4/projects/GROUP%2FSUBGROUP%2FPROJECT/jobs/1/artifacts/PATH/TO/FILE.txt",
 			"gitlab.io",
 			true,
-			"basic case 4",
+			"Uppercase names",
 		},
 		{
 			"https://gitlab.com/api/v4",
-			"artifact~1~2.gitlab.io",
+			"group.gitlab.io",
+			"/-/project/-/jobs/1foo1/artifacts/",
 			"",
-			"https://gitlab.com/api/v4/projects/1/jobs/2/artifacts",
 			"gitlab.io",
-			true,
-			"basic case 5",
+			false,
+			"Job ID has letters",
 		},
 		{
-			"https://gitlab.com/api/v4/",
-			"artifact~1~2.gitlab.io",
+			"https://gitlab.com/api/v4",
+			"group.gitlab.io",
+			"/-/project/-/jobs/1$1/artifacts/",
 			"",
-			"https://gitlab.com/api/v4/projects/1/jobs/2/artifacts",
 			"gitlab.io",
-			true,
-			"basic case 6",
+			false,
+			"Job ID has special characters",
 		},
 		{
 			"https://gitlab.com/api/v4",
-			"artifact~1~2.gitlab.io",
-			"/",
-			"https://gitlab.com/api/v4/projects/1/jobs/2/artifacts/",
-			"gitlab.io",
-			true,
-			"basic case 7",
-		},
-		{
-			"https://gitlab.com/api/v4/",
-			"artifact~1~2.gitlab.io",
-			"/",
-			"https://gitlab.com/api/v4/projects/1/jobs/2/artifacts/",
+			"group.gitlab.io",
+			"/-/project/-/jobs/1/artifacts/path/to/file.txt",
+			"https://gitlab.com/api/v4/projects/group%2Fproject/jobs/1/artifacts/path/to/file.txt",
 			"gitlab.io",
 			true,
-			"basic case 8",
+			"Artifact in subdirectory",
 		},
 		{
 			"https://gitlab.com/api/v4",
-			"artifact~100000~200000.gitlab.io",
-			"/file.txt",
-			"https://gitlab.com/api/v4/projects/100000/jobs/200000/artifacts/file.txt",
+			"group.gitlab.io",
+			"/-/subgroup1/sub.group2/project/-/jobs/1/artifacts/path/to/file.txt",
+			"https://gitlab.com/api/v4/projects/group%2Fsubgroup1%2Fsub.group2%2Fproject/jobs/1/artifacts/path/to/file.txt",
 			"gitlab.io",
 			true,
-			"expanded case",
-		},
-		{
-			"https://gitlab.com/api/v4/",
-			"artifact~1~2.gitlab.io",
-			"/file.txt",
-			"https://gitlab.com/api/v4/projects/1/jobs/2/artifacts/file.txt",
-			"gitlab.io",
-			true,
-			"server with tailing slash",
+			"Basic subgroup case",
 		},
 		{
 			"https://gitlab.com/api/v4",
-			"artifact~A~B.gitlab.io",
-			"/index.html",
-			"",
-			"example.com",
-			false,
-			"non matching domain and request",
-		},
-		{
-			"",
-			"artifact~A~B.gitlab.io",
-			"",
-			"",
-			"",
-			false,
-			"un-parseable Host",
-		},
-	}
-
-	for _, c := range cases {
-		a := &Artifact{server: c.RawServer, pattern: regexp.MustCompile(fmt.Sprintf(hostPatternTemplate, c.PagesDomain))}
-		u, ok := a.buildURL(c.Host, c.Path)
-		assert.Equal(t, c.Ok, ok, c.Description)
-		if c.Ok {
-			assert.Equal(t, c.Expected, u.String(), c.Description)
-		}
-	}
-}
-
-func TestMatchHostGen(t *testing.T) {
-	cases := []struct {
-		URLHost     string
-		PagesDomain string
-		Expected    bool
-		Description string
-	}{
-		{
-			"artifact~1~2.gitlab.io",
+			"group.gitlab.io",
+			"/-//project/-/jobs/1/artifacts/",
+			"https://gitlab.com/api/v4/projects/group%2Fproject/jobs/1/artifacts/",
 			"gitlab.io",
 			true,
-			"basic case",
+			"Leading / in remainder of project path",
 		},
 		{
-			"ARTIFACT~1~2.gitlab.io",
+			"https://gitlab.com/api/v4",
+			"group.gitlab.io",
+			"/-/subgroup/project//-/jobs/1/artifacts/",
+			"https://gitlab.com/api/v4/projects/group%2Fsubgroup%2Fproject/jobs/1/artifacts/",
 			"gitlab.io",
 			true,
-			"capital letters case",
+			"Trailing / in remainder of project path",
 		},
 		{
-			"ARTIFACT~11234~2908908.gitlab.io",
+			"https://gitlab.com/api/v4",
+			"group.gitlab.io",
+			"/-//subgroup/project//-/jobs/1/artifacts/",
+			"https://gitlab.com/api/v4/projects/group%2Fsubgroup%2Fproject/jobs/1/artifacts/",
 			"gitlab.io",
 			true,
-			"additional capital letters case",
+			"Leading and trailing /",
 		},
 		{
-			"artifact~10000~20000.gitlab.io",
+			"https://gitlab.com/api/v4",
+			"group.name.gitlab.io",
+			"/-/subgroup/project/-/jobs/1/artifacts/",
+			"https://gitlab.com/api/v4/projects/group.name%2Fsubgroup%2Fproject/jobs/1/artifacts/",
 			"gitlab.io",
 			true,
-			"expanded case",
+			"Toplevel group has period",
 		},
 		{
-			"artifact~86753095555~55550935768.gitlab.io",
+			"https://gitlab.com/api/v4",
+			"gitlab.io.gitlab.io",
+			"/-/project/-/jobs/1/artifacts/",
+			"https://gitlab.com/api/v4/projects/gitlab.io%2Fproject/jobs/1/artifacts/",
 			"gitlab.io",
 			true,
-			"large number case",
-		},
-		{
-			"artifact~one~two.gitlab.io",
-			"gitlab.io",
-			false,
-			"letters rather than numbers",
+			"Toplevel group matches pages domain",
 		},
 		{
-			"artifact~One111~tWo222.gitlab.io",
-			"gitlab.io",
-			false,
-			"Mixture of alphanumeric",
-		},
-		{
-			"artifact~!@#$%~%$#@!.gitlab.io",
-			"gitlab.io",
-			false,
-			"special characters",
-		},
-		{
-			"artifact~1.gitlab.io",
-			"gitlab.io",
-			false,
-			"not enough ids",
-		},
-		{
-			"artifact~1~2~34444~1~4.gitlab.io",
+			"https://gitlab.com/api/v4",
+			"group.gitlab.io",
+			"/-/project/-/jobs/1/artifacts",
+			"",
 			"gitlab.io",
 			false,
-			"too many ids",
+			"No artifact specified",
 		},
 		{
-			"artifact~1~2.gitlab.io",
-			"otherhost.io",
+			"https://gitlab.com/api/v4",
+			"group.gitlab.io",
+			"/index.html",
+			"",
+			"example.com",
 			false,
-			"different domain / suffix",
+			"non matching domain and request",
 		},
 	}
 
 	for _, c := range cases {
-		reg := hostPatternGen(c.PagesDomain)
-		assert.Equal(t, c.Expected, reg.MatchString(c.URLHost), c.Description)
+		t.Run(c.Description, func(t *testing.T) {
+			a := artifact.New(c.RawServer, 1, c.PagesDomain)
+			u, ok := a.BuildURL(c.Host, c.Path)
+
+			msg := c.Description + " - generated URL: "
+			if u != nil {
+				msg = msg + u.String()
+			}
+
+			assertOk := assert.Equal(t, c.Ok, ok, msg)
+			if assertOk && c.Ok {
+				assert.Equal(t, c.Expected, u.String(), c.Description)
+			}
+		})
 	}
 }
author	Kamil Trzciński <ayufan@ayufan.eu>	2017-10-02 16:49:55 +0300
committer	Kamil Trzciński <ayufan@ayufan.eu>	2017-10-02 16:49:55 +0300
commit	26a29a445dcd363f7cd8854f98fcc4a62ad69d44 (patch)
tree	36a0dad27af2230c494af83110637d409c46a670
parent	b322a578e5cfa144f5d0c5be3f43b2a0c9d8d42e (diff)
parent	e8e40691069f1475ee7798a584352a80e892b635 (diff)