Repository: txtdirect/txtdirect
Pull request #343 (2020-04-30 13:53:49)

[dockerv2]: Use new container regex and add new parser
**What this PR does / why we need it**:
Switches the regex to support all *pull* requests and adds a new parser for rec.To in dockerv2.

The previous regex and parser didn't support blobs requests for specific+tag endpoints.
Example edge case:
```
to=: http://127.0.0.1:5000/txtdirect/txtdirect:e2e
Request Path:  /v2/txtdirect/blobs/sha256:c98a4e389d58a7a9996f8c685569b35703cdcf10d517e7027dcb8092434677bb
Generated URI:  http://127.0.0.1:5000/v2/txtdirect/txtdirect/blobs/e2e
```

<!--
**Which issue this PR fixes** *(optional - uncomment and add issue)*:
fixes #0
-->

**Special notes for your reviewer**:

**Release note**:
<!--
Optional one line note for this specific change, that can be used in a release-note or changelog.
-->
```release-note

```



Patch:
diff --git a/dockerv2.go b/dockerv2.go
index 7bffca2..b68b49b 100644
--- a/dockerv2.go
+++ b/dockerv2.go
@@ -29,6 +29,14 @@ type Dockerv2 struct {
 	req *http.Request
 	c   Config
 	rec record
+
+	image Image
+}
+
+type Image struct {
+	Registry string
+	Image    string
+	Tag      string
 }
 
 // NewDockerv2 returns a fresh instance of Dockerv2 struct
@@ -43,7 +51,7 @@ func NewDockerv2(w http.ResponseWriter, r *http.Request, rec record, c Config) *
 
 var dockerRegexes = map[string]*regexp.Regexp{
 	"v2":        regexp.MustCompile("^\\/?v2\\/?$"),
-	"container": regexp.MustCompile("v2\\/(([\\w\\d-]+\\/?)+)\\/(tags|manifests|_catalog|blobs)"),
+	"container": regexp.MustCompile("/v2/(.*?)\\/\\s*(blobs|manifests)/(.*)"),
 }
 
 // Redirect handles the requests for "dockerv2" type
@@ -64,7 +72,7 @@ func (d *Dockerv2) Redirect() error {
 		return err
 	}
 	if path != "/" {
-		uri, err := createDockerv2URI(d.rec.To, path)
+		uri, err := d.ParseReference()
 		if err != nil {
 			return err
 		}
@@ -79,28 +87,47 @@ func (d *Dockerv2) Redirect() error {
 	return nil
 }
 
-func createDockerv2URI(to string, path string) (string, error) {
-	uri, err := url.Parse(to)
+func (d *Dockerv2) ParseRecordReference() error {
+	uri, err := url.Parse(d.rec.To)
 	if err != nil {
-		return "", err
+		return fmt.Errorf("Couldn't parse the record endpoint: %s", err.Error())
+	}
+
+	if uri.Path == "" {
+		uri.Path = "/"
+	}
+
+	path := strings.Split(uri.Path, ":")
+	if len(path) == 2 {
+		d.image.Tag = path[1]
 	}
 
-	if uri.Path == "/" || uri.Path == "" {
-		uri.Path = path
-		return uri.String(), nil
+	d.image.Registry = fmt.Sprintf("%s://%s", uri.Scheme, uri.Host)
+
+	d.image.Image = path[0][1:]
+
+	return nil
+}
+
+func (d *Dockerv2) ParseReference() (string, error) {
+	path := d.req.URL.Path
+	if err := d.ParseRecordReference(); err != nil {
+		return "", fmt.Errorf("Couldn't parse the image reference: %s", err.Error())
 	}
 
-	// Replace container's path in docker's request with what's inside rec.To
-	containerPath := dockerRegexes["container"].FindAllStringSubmatch(path, -1)[0][1] // [0][1]: The second item in first group is always container path
-	containerAndVersion := strings.Split(uri.Path, ":")                               // First item in slice is container and second item is version
-	uri.Path = strings.Replace(path, containerPath, containerAndVersion[0][1:], -1)
+	matches := dockerRegexes["container"].FindAllStringSubmatch(path, -1)[0]
+
+	// Replace image tag on manifests requests
+	if matches[2] == "manifests" {
+		if d.image.Tag != "" {
+			path = strings.Replace(path, matches[3], d.image.Tag, -1)
+		}
+	}
 
-	// Replace the version number in docker's request with what's inside rec.To
-	if len(containerAndVersion) == 2 {
-		pathSlice := strings.Split(uri.Path, "/")
-		pathSlice[len(pathSlice)-1] = containerAndVersion[1]
-		uri.Path = strings.Join(pathSlice, "/")
+	// Replace image name and namepsace
+	if d.image.Image != "" {
+		path = strings.Replace(path, matches[1], d.image.Image, -1)
 	}
 
-	return uri.String(), nil
+	return fmt.Sprintf("%s%s", d.image.Registry, path), nil
 }


Test patch:
diff --git a/dockerv2_test.go b/dockerv2_test.go
index d3d7fac..ec178e9 100644
--- a/dockerv2_test.go
+++ b/dockerv2_test.go
@@ -14,113 +14,159 @@ limitations under the License.
 package txtdirect
 
 import (
-	"fmt"
+	"net/http"
 	"net/http/httptest"
-	"strings"
 	"testing"
 )
 
-func Test_generateDockerv2URI(t *testing.T) {
+func TestDockerv2_ParseRecordReference(t *testing.T) {
 	tests := []struct {
-		path     string
 		rec      record
-		expected string
+		expected Image
 	}{
 		{
-			"/v2/",
-			record{
-				To:   "https://gcr.io/seetheprogress/txtdirect:latest",
-				Code: 302,
+			rec: record{
+				To: "https://example.com",
+			},
+			expected: Image{
+				Registry: "https://example.com",
 			},
-			"OK",
 		},
 		{
-			"/v2/random/container/tags/latest",
-			record{
-				To:   "https://gcr.io/",
-				Code: 302,
+			rec: record{
+				To: "https://example.com/txtdirect/txtdirect",
+			},
+			expected: Image{
+				Registry: "https://example.com",
+				Image:    "txtdirect/txtdirect",
 			},
-			"https://gcr.io/v2/random/container/tags/latest",
 		},
 		{
-			"/v2/random/container/tags/v2.0.0",
-			record{
-				To:   "https://gcr.io/",
-				Code: 302,
+			rec: record{
+				To: "https://example.com/txtdirect",
+			},
+			expected: Image{
+				Registry: "https://example.com",
+				Image:    "txtdirect",
 			},
-			"https://gcr.io/v2/random/container/tags/v2.0.0",
 		},
 		{
-			"/v2/testing/container/tags/v3.0.0",
-			record{
-				To:   "https://gcr.io/testing/container:v2.0.0",
-				Code: 302,
+			rec: record{
+				To: "https://example.com/txtdirect/txtdirect:v0.0.1",
+			},
+			expected: Image{
+				Registry: "https://example.com",
+				Image:    "txtdirect/txtdirect",
+				Tag:      "v0.0.1",
 			},
-			"https://gcr.io/v2/testing/container/tags/v2.0.0",
 		},
 		{
-			"/v2/testing/container/tags/v2.0.0",
-			record{
-				To:   "https://gcr.io/testing/container",
-				Code: 302,
+			rec: record{
+				To: "https://example.com/txtdirect:v0.0.1",
+			},
+			expected: Image{
+				Registry: "https://example.com",
+				Image:    "txtdirect",
+				Tag:      "v0.0.1",
 			},
-			"https://gcr.io/v2/testing/container/tags/v2.0.0",
 		},
+	}
+	for _, test := range tests {
+		resp := httptest.NewRecorder()
+		docker := NewDockerv2(resp, &http.Request{}, test.rec, Config{})
+
+		if err := docker.ParseRecordReference(); err != nil {
+			t.Errorf("Unexpected error happened: %s", err)
+		}
+		if docker.image != test.expected {
+			t.Errorf("Expected %+v, got %+v", test.expected, docker.image)
+		}
+	}
+}
+
+func TestDockerv2_ParseReference(t *testing.T) {
+	tests := []struct {
+		rec      record
+		uri      string
+		expected string
+	}{
 		{
-			"/v2/random/container/tags/latest",
-			record{
-				To:   "https://gcr.io/testing/container:v2.0.0",
-				Code: 302,
+			rec: record{
+				To: "https://example.com",
 			},
-			"https://gcr.io/v2/testing/container/tags/v2.0.0",
+			uri:      "https://example.test/v2/txtdirect/manifests/latest",
+			expected: "https://example.com/v2/txtdirect/manifests/latest",
+		},
+		{
+			rec: record{
+				To: "https://example.com",
+			},
+			uri:      "https://example.test/v2/txtdirect/manifests/v0.0.1",
+			expected: "https://example.com/v2/txtdirect/manifests/v0.0.1",
+		},
+		{
+			rec: record{
+				To: "https://example.com",
+			},
+			uri:      "https://example.test/v2/txtdirect/blobs/sha256:000000",
+			expected: "https://example.com/v2/txtdirect/blobs/sha256:000000",
 		},
-
 		{
-			"/v2/random/container/tags/v2.0.0",
-			record{
-				To:   "https://gcr.io/testing/container",
-				Code: 302,
+			rec: record{
+				To: "https://example.com/txtdirect/txtdirect",
 			},
-			"https://gcr.io/v2/testing/container/tags/v2.0.0",
+			uri:      "https://example.test/v2/txtdirect/txtdirect/manifests/latest",
+			expected: "https://example.com/v2/txtdirect/txtdirect/manifests/latest",
 		},
 		{
-			"/v2/random/container/_catalog",
-			record{
-				To:   "https://gcr.io/",
-				Code: 302,
+			rec: record{
+				To: "https://example.com/txtdirect/txtdirect",
 			},
-			"https://gcr.io/v2/random/container/_catalog",
+			uri:      "https://example.test/v2/txtdirect/txtdirect/blobs/sha256:000000",
+			expected: "https://example.com/v2/txtdirect/txtdirect/blobs/sha256:000000",
 		},
 		{
-			"/random/path",
-			record{
-				To:      "https://gcr.io/",
-				Code:    302,
-				Website: "https://fallback.test",
+			rec: record{
+				To: "https://example.com/txtdirect",
 			},
-			"https://fallback.test",
+			uri:      "https://example.test/v2/txtdirect/txtdirect/manifests/latest",
+			expected: "https://example.com/v2/txtdirect/manifests/latest",
 		},
 		{
-			"",
-			record{
-				To:   "https://gcr.io/",
-				Code: 302,
-				Root: "https://fallback.test",
+			rec: record{
+				To: "https://example.com/txtdirect",
 			},
-			"https://fallback.test",
+			uri:      "https://example.test/v2/txtdirect/txtdirect/blobs/sha256:000000",
+			expected: "https://example.com/v2/txtdirect/blobs/sha256:000000",
+		},
+		{
+			rec: record{
+				To: "https://example.com/txtdirect/txtdirect:v0.0.1",
+			},
+			uri:      "https://example.test/v2/txtdirect/manifests/latest",
+			expected: "https://example.com/v2/txtdirect/txtdirect/manifests/v0.0.1",
+		},
+		{
+			rec: record{
+				To: "https://example.com/txtdirect:v0.0.1",
+			},
+			uri:      "https://example.test/v2/txtdirect/txtdirect/blobs/sha256:000000",
+			expected: "https://example.com/v2/txtdirect/blobs/sha256:000000",
 		},
 	}
 	for _, test := range tests {
-		req := httptest.NewRequest("GET", fmt.Sprintf("https://example.com%s", test.path), nil)
+		req := httptest.NewRequest("GET", test.uri, nil)
 		resp := httptest.NewRecorder()
-		req = test.rec.addToContext(req)
 		docker := NewDockerv2(resp, req, test.rec, Config{})
 
-		if err := docker.Redirect(); err != nil {
+		uri, err := docker.ParseReference()
+
+		if err != nil {
 			t.Errorf("Unexpected error happened: %s", err)
 		}
-		if !strings.Contains(resp.Body.String(), test.expected) {
-			t.Errorf("Expected %s, got %s:", test.expected, resp.Body.String())
+
+		if uri != test.expected {
+			t.Errorf("Expected %s for dockerv2 redirect URI, got %s", test.expected, uri)
 		}
 	}
 }