diff options
author | Jaime Martinez <jmartinez@gitlab.com> | 2021-12-14 06:49:17 +0300 |
---|---|---|
committer | Jaime Martinez <jmartinez@gitlab.com> | 2021-12-14 06:49:17 +0300 |
commit | 0c6aa86b4e87146ee122ff9e02c1e010b769ce30 (patch) | |
tree | d59be4c096ef833e1875543732642101e0c4a904 | |
parent | 753ed6a16616776b9956ba253959d8fdd604340e (diff) | |
parent | e3f274f1b8eb69fcc57a89b89197560903fe1144 (diff) |
Merge branch 'fix/sha-lookups' into 'master'
test: add sha256 to mocked lookups and fix tests
See merge request gitlab-org/gitlab-pages!634
-rw-r--r-- | internal/domain/domain_test.go | 2 | ||||
-rw-r--r-- | internal/logging/logging_test.go | 1 | ||||
-rw-r--r-- | internal/serving/disk/zip/serving_test.go | 19 | ||||
-rw-r--r-- | internal/source/gitlab/client/testdata/group-first.gitlab.io.json | 6 | ||||
-rw-r--r-- | internal/source/gitlab/client/testdata/test.gitlab.io.json | 9 | ||||
-rw-r--r-- | internal/vfs/zip/vfs.go | 26 | ||||
-rw-r--r-- | shared/lookups/new-source-test.gitlab.io.json | 3 | ||||
-rw-r--r-- | shared/lookups/zip-malformed.gitlab.io.json | 3 | ||||
-rw-r--r-- | shared/lookups/zip-not-found.gitlab.io.json | 3 | ||||
-rw-r--r-- | shared/lookups/zip.gitlab.io.json | 3 | ||||
-rw-r--r-- | test/acceptance/testdata/api_responses.go | 19 |
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, }, }, }, |