Merge branch 'fix/sha-lookups' into 'master'

test: add sha256 to mocked lookups and fix tests

See merge request gitlab-org/gitlab-pages!634

11 files changed, 46 insertions, 48 deletions

diff --git a/internal/domain/domain_test.go b/internal/domain/domain_test.go
index 5a95b4ec..d76e10e8 100644
--- a/internal/domain/domain_test.go
+++ b/internal/domain/domain_test.go
@@ -50,6 +50,7 @@ func TestIsHTTPSOnly(t *testing.T) {
 				&stubbedResolver{
 					project: &serving.LookupPath{
 						Path:        "group/project/public",
+						SHA256:      "foo",
 						IsHTTPSOnly: true,
 					},
 				}),
@@ -62,6 +63,7 @@ func TestIsHTTPSOnly(t *testing.T) {
 				&stubbedResolver{
 					project: &serving.LookupPath{
 						Path:        "group/project/public",
+						SHA256:      "foo",
 						IsHTTPSOnly: false,
 					},
 				}),
diff --git a/internal/logging/logging_test.go b/internal/logging/logging_test.go
index fe157e88..d56f6777 100644
--- a/internal/logging/logging_test.go
+++ b/internal/logging/logging_test.go
@@ -30,6 +30,7 @@ func TestGetExtraLogFields(t *testing.T) {
 			ServingType: "file",
 			ProjectID:   100,
 			Prefix:      "/prefix",
+			SHA256:      "foo",
 		}
 	}})
 
diff --git a/internal/serving/disk/zip/serving_test.go b/internal/serving/disk/zip/serving_test.go
index f16bbada..fefc8cbe 100644
--- a/internal/serving/disk/zip/serving_test.go
+++ b/internal/serving/disk/zip/serving_test.go
@@ -1,6 +1,7 @@
 package zip
 
 import (
+	"crypto/sha256"
 	"io"
 	"net/http"
 	"net/http/httptest"
@@ -27,7 +28,6 @@ func TestZip_ServeFileHTTP(t *testing.T) {
 
 	tests := map[string]struct {
 		vfsPath        string
-		sha256         string
 		path           string
 		expectedStatus int
 		expectedBody   string
@@ -35,21 +35,18 @@ func TestZip_ServeFileHTTP(t *testing.T) {
 	}{
 		"accessing /index.html": {
 			vfsPath:        httpURL,
-			sha256:         "d6b318b399cfe9a1c8483e49847ee49a2676d8cfd6df57ec64d971ad03640a75",
 			path:           "/index.html",
 			expectedStatus: http.StatusOK,
 			expectedBody:   "zip.gitlab.io/project/index.html\n",
 		},
 		"accessing /index.html from disk": {
 			vfsPath:        fileURL,
-			sha256:         "15c5438164ec67bb2225f68d7d7a2e0b608035264e5275b7e3302641aa25a528",
 			path:           "/index.html",
 			expectedStatus: http.StatusOK,
 			expectedBody:   "zip.gitlab.io/project/index.html\n",
 		},
 		"accessing /": {
 			vfsPath:        httpURL,
-			sha256:         "d6b318b399cfe9a1c8483e49847ee49a2676d8cfd6df57ec64d971ad03640a75",
 			path:           "/",
 			expectedStatus: http.StatusOK,
 			expectedBody:   "zip.gitlab.io/project/index.html\n",
@@ -72,30 +69,25 @@ func TestZip_ServeFileHTTP(t *testing.T) {
 		},
 		"accessing / from disk": {
 			vfsPath:        fileURL,
-			sha256:         "15c5438164ec67bb2225f68d7d7a2e0b608035264e5275b7e3302641aa25a528",
 			path:           "/",
 			expectedStatus: http.StatusOK,
 			expectedBody:   "zip.gitlab.io/project/index.html\n",
 		},
 		"accessing without /": {
 			vfsPath:        httpURL,
-			sha256:         "d6b318b399cfe9a1c8483e49847ee49a2676d8cfd6df57ec64d971ad03640a75",
 			path:           "",
 			expectedStatus: http.StatusFound,
 			expectedBody:   `<a href="//zip.gitlab.io/zip/">Found</a>.`,
 		},
 		"accessing without / from disk": {
 			vfsPath:        fileURL,
-			sha256:         "15c5438164ec67bb2225f68d7d7a2e0b608035264e5275b7e3302641aa25a528",
 			path:           "",
 			expectedStatus: http.StatusFound,
 			expectedBody:   `<a href="//zip.gitlab.io/zip/">Found</a>.`,
 		},
 		"accessing archive that is 404": {
 			vfsPath: testServerURL + "/invalid.zip",
-			// the sha is needed or we would get a 500
-			sha256: "foo",
-			path:   "/index.html",
+			path:    "/index.html",
 			// we expect the status to not be set
 			expectedStatus: 0,
 		},
@@ -129,17 +121,20 @@ func TestZip_ServeFileHTTP(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			w := httptest.NewRecorder()
 			w.Code = 0 // ensure that code is not set, and it is being set by handler
-			r := httptest.NewRequest("GET", "http://zip.gitlab.io/zip"+test.path, nil)
+			r := httptest.NewRequest(http.MethodGet, "http://zip.gitlab.io/zip"+test.path, nil)
 
 			r.Header = test.extraHeaders
 
+			sha := sha256.Sum256([]byte(test.vfsPath))
+			etag := string(sha[:])
+
 			handler := serving.Handler{
 				Writer:  w,
 				Request: r,
 				LookupPath: &serving.LookupPath{
 					Prefix: "/zip/",
 					Path:   test.vfsPath,
-					SHA256: test.sha256,
+					SHA256: etag,
 				},
 				SubPath: test.path,
 			}
diff --git a/internal/source/gitlab/client/testdata/group-first.gitlab.io.json b/internal/source/gitlab/client/testdata/group-first.gitlab.io.json
index 2f055654..3ac675b6 100644
--- a/internal/source/gitlab/client/testdata/group-first.gitlab.io.json
+++ b/internal/source/gitlab/client/testdata/group-first.gitlab.io.json
@@ -9,7 +9,8 @@
         "project_id": 1,
         "source": {
           "path": "some/path/group/",
-          "type": "file"
+          "type": "file",
+          "sha256": "6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b"
         }
       },
       {
@@ -19,7 +20,8 @@
         "project_id": 2,
         "source": {
           "path": "some/path/to/project-2/",
-          "type": "file"
+          "type": "file",
+          "sha256": "d4735e3a265e16eee03f59718b9b5d03019c07d8b6c51f90da3a666eec13ab35"
         }
       }
     ]
diff --git a/internal/source/gitlab/client/testdata/test.gitlab.io.json b/internal/source/gitlab/client/testdata/test.gitlab.io.json
index e3430119..c094e52a 100644
--- a/internal/source/gitlab/client/testdata/test.gitlab.io.json
+++ b/internal/source/gitlab/client/testdata/test.gitlab.io.json
@@ -9,7 +9,8 @@
             "project_id": 123,
             "source": {
                 "path": "some/path/to/project/",
-                "type": "file"
+                "type": "file",
+                "sha256": "a665a45920422f9d417e4867efdc4fb8a04a1f3fff1fa07e998e86f7f7a27ae3"
             }
         },
         {
@@ -19,7 +20,8 @@
             "project_id": 124,
             "source": {
                 "path": "some/path/to/project-2/",
-                "type": "file"
+                "type": "file",
+                "sha256": "6affdae3b3c1aa6aa7689e9b6a7b3225a636aa1ac0025f490cca1285ceaf1487"
             }
         },
         {
@@ -29,7 +31,8 @@
             "project_id": 125,
             "source": {
                 "path": "some/path/to/project-3/",
-                "type": "file"
+                "type": "file",
+                "sha256": "0f8ef3377b30fc47f96b48247f463a726a802f62f3faa03d56403751d2f66c67"
             }
         }
     ]
diff --git a/internal/vfs/zip/vfs.go b/internal/vfs/zip/vfs.go
index c28ec017..d0608c81 100644
--- a/internal/vfs/zip/vfs.go
+++ b/internal/vfs/zip/vfs.go
@@ -5,7 +5,6 @@ import (
 	"errors"
 	"io/fs"
 	"net/http"
-	"net/url"
 	"sync"
 	"time"
 
@@ -33,7 +32,8 @@ const (
 )
 
 var (
-	errAlreadyCached = errors.New("archive already cached")
+	errAlreadyCached   = errors.New("archive already cached")
+	errMissingCacheKey = errors.New("missing cache key")
 )
 
 type lruCache interface {
@@ -144,20 +144,6 @@ func (zfs *zipVFS) resetCache() {
 	})
 }
 
-func keyFromPath(path string) (string, error) {
-	// We assume that our URL is https://.../artifacts.zip?content-sign=aaa
-	// our caching key is `https://.../artifacts.zip`
-	// TODO: replace caching key with file_sha256
-	// https://gitlab.com/gitlab-org/gitlab-pages/-/issues/489
-	key, err := url.Parse(path)
-	if err != nil {
-		return "", err
-	}
-	key.RawQuery = ""
-	key.Fragment = ""
-	return key.String(), nil
-}
-
 // Root opens an archive given a URL path and returns an instance of zipArchive
 // that implements the vfs.VFS interface.
 // To avoid using locks, the findOrOpenArchive function runs inside of a for
@@ -166,14 +152,8 @@ func keyFromPath(path string) (string, error) {
 // to try and find the cached archive or return if there's an error, for example
 // if the context is canceled.
 func (zfs *zipVFS) Root(ctx context.Context, path string, cacheKey string) (vfs.Root, error) {
-	// TODO: update acceptance tests mocked response to return a cacheKey
 	if cacheKey == "" {
-		k, err := keyFromPath(path)
-		if err != nil {
-			return nil, err
-		}
-
-		cacheKey = k
+		return nil, errMissingCacheKey
 	}
 
 	// we do it in loop to not use any additional locks
diff --git a/shared/lookups/new-source-test.gitlab.io.json b/shared/lookups/new-source-test.gitlab.io.json
index f84fde35..ce98cc02 100644
--- a/shared/lookups/new-source-test.gitlab.io.json
+++ b/shared/lookups/new-source-test.gitlab.io.json
@@ -9,7 +9,8 @@
             "project_id": 123,
             "source": {
                 "path": "group/new-source-test.gitlab.io/public/",
-                "type": "file"
+                "type": "file",
+                "sha256": "0da779bb0f6de46082e32a7d59e52392e02c87fda3cfeb7a629c5591646443bb"
             }
         }
     ]
diff --git a/shared/lookups/zip-malformed.gitlab.io.json b/shared/lookups/zip-malformed.gitlab.io.json
index 8c0185da..358e2e7b 100644
--- a/shared/lookups/zip-malformed.gitlab.io.json
+++ b/shared/lookups/zip-malformed.gitlab.io.json
@@ -9,7 +9,8 @@
             "project_id": 123,
             "source": {
                 "path": "http://127.0.0.1:38001/malformed.zip",
-                "type": "zip"
+                "type": "zip",
+                "sha256": "74961508d735a7af35864fc0a955990bda2ca63916737117f19b878a42e53beb"
             }
         }
     ]
diff --git a/shared/lookups/zip-not-found.gitlab.io.json b/shared/lookups/zip-not-found.gitlab.io.json
index 514b8ff2..abee3dfb 100644
--- a/shared/lookups/zip-not-found.gitlab.io.json
+++ b/shared/lookups/zip-not-found.gitlab.io.json
@@ -9,7 +9,8 @@
             "project_id": 123,
             "source": {
                 "path": "http://127.0.0.1:38001/not-found.zip",
-                "type": "zip"
+                "type": "zip",
+                "sha256": "84d0a9f3e5430a2b849b8240359578a6a0710ee34e4ba77b240b1e5142d8d4be"
             }
         }
     ]
diff --git a/shared/lookups/zip.gitlab.io.json b/shared/lookups/zip.gitlab.io.json
index 0549adc8..a4097414 100644
--- a/shared/lookups/zip.gitlab.io.json
+++ b/shared/lookups/zip.gitlab.io.json
@@ -9,7 +9,8 @@
             "project_id": 123,
             "source": {
                 "path": "http://127.0.0.1:38001/public.zip",
-                "type": "zip"
+                "type": "zip",
+                "sha256": "a8085b818beaf93ad5319592acb5f8eb3fcada67f5eb025c83b5470b72e585fc"
             }
         }
     ]
diff --git a/test/acceptance/testdata/api_responses.go b/test/acceptance/testdata/api_responses.go
index 10121f40..5061dcaf 100644
--- a/test/acceptance/testdata/api_responses.go
+++ b/test/acceptance/testdata/api_responses.go
@@ -1,6 +1,7 @@
 package testdata
 
 import (
+	"crypto/sha256"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -162,6 +163,10 @@ func generateVirtualDomainFromDir(dir, rootDomain string, perPrefixConfig map[st
 				cfg = projectConfig{}
 			}
 
+			sourcePath := fmt.Sprintf("file://%s", wd+"/"+dir+project)
+			sum := sha256.Sum256([]byte(sourcePath))
+			sha := string(sum[:])
+
 			lookupPath := api.LookupPath{
 				ProjectID:     cfg.projectID,
 				AccessControl: cfg.accessControl,
@@ -169,8 +174,9 @@ func generateVirtualDomainFromDir(dir, rootDomain string, perPrefixConfig map[st
 				// gitlab.Resolve logic expects prefix to have ending slash
 				Prefix: ensureEndingSlash(prefix),
 				Source: api.Source{
-					Type: "zip",
-					Path: fmt.Sprintf("file://%s", wd+"/"+dir+project),
+					Type:   "zip",
+					Path:   sourcePath,
+					SHA256: sha,
 				},
 			}
 
@@ -196,6 +202,10 @@ func customDomain(config projectConfig) responseFn {
 	return func(t *testing.T, wd string) api.VirtualDomain {
 		t.Helper()
 
+		sourcePath := fmt.Sprintf("file://%s/%s/public.zip", wd, config.pathOnDisk)
+		sum := sha256.Sum256([]byte(sourcePath))
+		sha := string(sum[:])
+
 		return api.VirtualDomain{
 			Certificate: "",
 			Key:         "",
@@ -209,8 +219,9 @@ func customDomain(config projectConfig) responseFn {
 					// see internal/serving/disk/ for details
 					Prefix: "/",
 					Source: api.Source{
-						Type: "zip",
-						Path: fmt.Sprintf("file://%s/%s/public.zip", wd, config.pathOnDisk),
+						Type:   "zip",
+						SHA256: sha,
+						Path:   sourcePath,
 					},
 				},
 			},