From 48bf7f69f881815fb39d2f570a9dc5928fcb655c Mon Sep 17 00:00:00 2001 From: Alexei Dodon Date: Wed, 13 Sep 2023 10:28:14 +0300 Subject: [PATCH] refactor: Reduce zb binary size (#1783) Signed-off-by: Alexei Dodon --- .github/workflows/compare-binary-size.yml | 73 ++++++++++++++++++++++ .github/workflows/golangci-lint.yaml | 3 - .github/workflows/zot-minimal-size.yml | 40 ------------ cmd/zb/helper.go | 76 ++++++++++++++++++----- pkg/api/controller_test.go | 35 ++++++----- pkg/api/routes.go | 9 +-- pkg/cli/image_cmd_test.go | 7 ++- pkg/common/model.go | 5 ++ pkg/compliance/v1_0_0/check.go | 31 ++++----- pkg/extensions/lint/lint_test.go | 5 +- pkg/extensions/sync/sync_test.go | 9 +-- pkg/log/log_test.go | 7 ++- pkg/test/common.go | 24 +------ pkg/test/common/utils.go | 25 ++++++++ swagger/docs.go | 16 ++--- swagger/swagger.json | 16 ++--- swagger/swagger.yaml | 14 ++--- 17 files changed, 243 insertions(+), 152 deletions(-) create mode 100644 .github/workflows/compare-binary-size.yml delete mode 100644 .github/workflows/zot-minimal-size.yml create mode 100644 pkg/test/common/utils.go diff --git a/.github/workflows/compare-binary-size.yml b/.github/workflows/compare-binary-size.yml new file mode 100644 index 00000000..e39782c3 --- /dev/null +++ b/.github/workflows/compare-binary-size.yml @@ -0,0 +1,73 @@ +name: "Check binary size increase" +on: + pull_request: + branches: [main] + +permissions: read-all + +jobs: + compare-binary-sizes: + runs-on: ubuntu-latest + name: compare-with-main + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-go@v4 + with: + cache: false + go-version: 1.20.x + - name: Checkout zot (main branch) + run: | + mkdir -p $GITHUB_WORKSPACE/zot_main + git clone https://github.com/project-zot/zot zot_main/ + - name: Check if zot-minimal binary increased with more than 1% + run: | + echo "Building zot-minimal and check size" + cd $GITHUB_WORKSPACE + make binary-minimal + BINSIZE=$(stat -c%s "bin/zot-linux-amd64-minimal") + + echo "Building zot-minimal on main branch and check size" + cd zot_main + make binary-minimal + BINSIZE_MAIN=$(stat -c%s "bin/zot-linux-amd64-minimal") + + echo "PR binary size: $BINSIZE Bytes" + echo "main branch binary size: $BINSIZE_MAIN Bytes" + [[ $BINSIZE -eq $BINSIZE_MAIN ]] && echo "zot-minimal binary size is not affected by PR" && exit 0 + + if [[ $BINSIZE -gt $BINSIZE_MAIN ]]; then \ + PERCENTAGE=$(echo "scale=2; (($BINSIZE-$BINSIZE_MAIN)*100)/$BINSIZE_MAIN" | bc); \ + echo "zot minimal binary increased by $PERCENTAGE% comparing with main"; \ + if ((`bc <<< "$PERCENTAGE>=1.0"`)); then exit 1; fi; \ + else \ + PERCENTAGE=$(echo "scale=2; (($BINSIZE_MAIN-$BINSIZE)*100)/$BINSIZE_MAIN" | bc); \ + echo "zot minimal binary decreased by $PERCENTAGE% comparing with main"; \ + fi + - if: always() + name: Check if zb binary increased with more than 1% + run: | + echo "Building zb and check size" + cd $GITHUB_WORKSPACE + make bench + BINSIZE=$(stat -c%s "bin/zb-linux-amd64") + + echo "Building zb on main branch and check size" + cd zot_main + make bench + BINSIZE_MAIN=$(stat -c%s "bin/zb-linux-amd64") + + echo "PR binary size: $BINSIZE Bytes" + echo "main branch binary size: $BINSIZE_MAIN Bytes" + [[ $BINSIZE -eq $BINSIZE_MAIN ]] && echo "zb binary size is not affected by PR" && exit 0 + + if [[ $BINSIZE -gt $BINSIZE_MAIN ]]; then \ + PERCENTAGE=$(echo "scale=2; (($BINSIZE-$BINSIZE_MAIN)*100)/$BINSIZE_MAIN" | bc); \ + echo "zb binary increased by $PERCENTAGE% comparing with main"; \ + if ((`bc <<< "$PERCENTAGE>=1.0"`)); then exit 1; fi; \ + else \ + PERCENTAGE=$(echo "scale=2; (($BINSIZE_MAIN-$BINSIZE)*100)/$BINSIZE_MAIN" | bc); \ + echo "zb binary decreased by $PERCENTAGE% comparing with main"; \ + fi + + + diff --git a/.github/workflows/golangci-lint.yaml b/.github/workflows/golangci-lint.yaml index add68051..78031a1c 100644 --- a/.github/workflows/golangci-lint.yaml +++ b/.github/workflows/golangci-lint.yaml @@ -36,9 +36,6 @@ jobs: # Optional: show only new issues if it's a pull request. The default value is `false`. # only-new-issues: true - # Optional: if set to true then the action will use pre-installed Go. - skip-go-installation: true - # Optional: if set to true then the action don't cache or restore ~/go/pkg. # skip-pkg-cache: true diff --git a/.github/workflows/zot-minimal-size.yml b/.github/workflows/zot-minimal-size.yml deleted file mode 100644 index 0c5ff892..00000000 --- a/.github/workflows/zot-minimal-size.yml +++ /dev/null @@ -1,40 +0,0 @@ -name: "zot minimal binary size" -on: - pull_request: - branches: [main] - -permissions: read-all - -jobs: - zot-minimal-size: - runs-on: ubuntu-latest - name: compare-binary-size - steps: - - uses: actions/checkout@v3 - - uses: actions/setup-go@v4 - with: - cache: false - go-version: 1.20.x - - name: Check if zot-minimal binary increased with more than 1% - run: | - echo "Building zot-minimal and check size" - cd $GITHUB_WORKSPACE - make binary-minimal - BINSIZE=$(stat -c%s "bin/zot-linux-amd64-minimal") - - echo "Building zot-minimal on main branch and check size" - mkdir -p zot_main - git clone https://github.com/project-zot/zot zot_main/ - cd zot_main - make binary-minimal - BINSIZE_MAIN=$(stat -c%s "bin/zot-linux-amd64-minimal") - cd $GITHUB_WORKSPACE && rm -rf zot_main - - [[ $BINSIZE -gt $BINSIZE_MAIN ]] || exit 0 - echo "PR changes increased size of zot-minimal binary" - echo "PR binary size: $BINSIZE Bytes" - echo "main branch binary size: $BINSIZE_MAIN Bytes" - - PERCENTACE=$(echo "scale=2; (($BINSIZE-$BINSIZE_MAIN)*100)/$BINSIZE_MAIN" | bc) - if ((`bc <<< "$PERCENTACE>=1.0"`)); then echo "zot minimal binary increased by $PERCENTACE% comparing with main"; exit 1; fi - diff --git a/cmd/zb/helper.go b/cmd/zb/helper.go index 19c9b765..ba24cfd9 100644 --- a/cmd/zb/helper.go +++ b/cmd/zb/helper.go @@ -2,10 +2,12 @@ package main import ( "bytes" + crand "crypto/rand" "encoding/json" "fmt" "io" "log" + "math/big" "math/rand" "net/http" "os" @@ -14,13 +16,14 @@ import ( "time" "github.com/google/uuid" + godigest "github.com/opencontainers/go-digest" imeta "github.com/opencontainers/image-spec/specs-go" ispec "github.com/opencontainers/image-spec/specs-go/v1" "gopkg.in/resty.v1" zerr "zotregistry.io/zot/errors" - "zotregistry.io/zot/pkg/api" - "zotregistry.io/zot/pkg/test" + "zotregistry.io/zot/pkg/common" + testc "zotregistry.io/zot/pkg/test/common" ) func makeHTTPGetRequest(url string, resultPtr interface{}, client *resty.Client) error { @@ -62,7 +65,7 @@ func makeHTTPDeleteRequest(url string, client *resty.Client) error { func deleteTestRepo(repos []string, url string, client *resty.Client) error { for _, repo := range repos { - var tags api.ImageTags + var tags common.ImageTags // get tags err := makeHTTPGetRequest(fmt.Sprintf("%s/v2/%s/tags/list", url, repo), &tags, client) @@ -342,7 +345,7 @@ func pushMonolithImage(workdir, url, trepo string, repos []string, config testCo resp.StatusCode(), string(resp.Body())) //nolint: goerr113 } - loc := test.Location(url, resp) + loc := testc.Location(url, resp) var size int @@ -396,8 +399,8 @@ func pushMonolithImage(workdir, url, trepo string, repos []string, config testCo resp.StatusCode(), string(resp.Body())) } - loc = test.Location(url, resp) - cblob, cdigest := test.GetRandomImageConfig() + loc = testc.Location(url, resp) + cblob, cdigest := getRandomImageConfig() resp, err = client.R(). SetContentLength(true). SetHeader("Content-Length", fmt.Sprintf("%d", len(cblob))). @@ -524,7 +527,7 @@ func pushMonolithAndCollect(workdir, url, trepo string, count int, return } - loc := test.Location(url, resp) + loc := testc.Location(url, resp) var size int @@ -592,8 +595,8 @@ func pushMonolithAndCollect(workdir, url, trepo string, count int, return } - loc = test.Location(url, resp) - cblob, cdigest := test.GetRandomImageConfig() + loc = testc.Location(url, resp) + cblob, cdigest := getRandomImageConfig() resp, err = client.R(). SetContentLength(true). SetHeader("Content-Length", fmt.Sprintf("%d", len(cblob))). @@ -728,7 +731,7 @@ func pushChunkAndCollect(workdir, url, trepo string, count int, return } - loc := test.Location(url, resp) + loc := testc.Location(url, resp) var size int @@ -766,7 +769,7 @@ func pushChunkAndCollect(workdir, url, trepo string, count int, return } - loc = test.Location(url, resp) + loc = testc.Location(url, resp) // request specific check statusCode = resp.StatusCode() @@ -820,8 +823,8 @@ func pushChunkAndCollect(workdir, url, trepo string, count int, return } - loc = test.Location(url, resp) - cblob, cdigest := test.GetRandomImageConfig() + loc = testc.Location(url, resp) + cblob, cdigest := getRandomImageConfig() resp, err = client.R(). SetContentLength(true). SetHeader("Content-Type", "application/octet-stream"). @@ -857,7 +860,7 @@ func pushChunkAndCollect(workdir, url, trepo string, count int, return } - loc = test.Location(url, resp) + loc = testc.Location(url, resp) // request specific check statusCode = resp.StatusCode() @@ -1017,3 +1020,48 @@ func loadOrStore(statusRequests *sync.Map, key string, value int) int { //nolint return intValue } + +// TO DO: replace with pkg/test/images when available. +func getRandomImageConfig() ([]byte, godigest.Digest) { + const maxLen = 16 + + randomAuthor := randomString(maxLen) + + config := ispec.Image{ + Platform: ispec.Platform{ + Architecture: "amd64", + OS: "linux", + }, + RootFS: ispec.RootFS{ + Type: "layers", + DiffIDs: []godigest.Digest{}, + }, + Author: randomAuthor, + } + + configBlobContent, err := json.MarshalIndent(&config, "", "\t") + if err != nil { + log.Fatal(err) + } + + configBlobDigestRaw := godigest.FromBytes(configBlobContent) + + return configBlobContent, configBlobDigestRaw +} + +func randomString(n int) string { + const letters = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz-" + + ret := make([]byte, n) + + for count := 0; count < n; count++ { + num, err := crand.Int(crand.Reader, big.NewInt(int64(len(letters)))) + if err != nil { + panic(err) + } + + ret[count] = letters[num.Int64()] + } + + return string(ret) +} diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go index 4ab83d42..64457b0b 100644 --- a/pkg/api/controller_test.go +++ b/pkg/api/controller_test.go @@ -61,6 +61,7 @@ import ( "zotregistry.io/zot/pkg/storage" storageConstants "zotregistry.io/zot/pkg/storage/constants" "zotregistry.io/zot/pkg/test" + testc "zotregistry.io/zot/pkg/test/common" "zotregistry.io/zot/pkg/test/inject" "zotregistry.io/zot/pkg/test/mocks" ) @@ -3788,7 +3789,7 @@ func TestAuthorizationWithOnlyAnonymousPolicy(t *testing.T) { So(err, ShouldBeNil) So(resp, ShouldNotBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) - loc = test.Location(baseURL, resp) + loc = testc.Location(baseURL, resp) // uploading blob should get 201 resp, err = resty.R().SetHeader("Content-Length", fmt.Sprintf("%d", len(cblob))). @@ -3833,7 +3834,7 @@ func TestAuthorizationWithOnlyAnonymousPolicy(t *testing.T) { So(err, ShouldBeNil) So(resp, ShouldNotBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) - loc = test.Location(baseURL, resp) + loc = testc.Location(baseURL, resp) // uploading blob should get 201 resp, err = resty.R(). SetHeader("Content-Length", fmt.Sprintf("%d", len(updateBlob))). @@ -4494,7 +4495,7 @@ func TestCrossRepoMount(t *testing.T) { Post(baseURL + "/v2/zot-y-test/blobs/uploads/") So(err, ShouldBeNil) So(postResponse.StatusCode(), ShouldEqual, http.StatusAccepted) - So(test.Location(baseURL, postResponse), ShouldStartWith, fmt.Sprintf("%s%s/zot-y-test/%s/%s", + So(testc.Location(baseURL, postResponse), ShouldStartWith, fmt.Sprintf("%s%s/zot-y-test/%s/%s", baseURL, constants.RoutePrefix, constants.Blobs, constants.Uploads)) // Use correct request @@ -4505,7 +4506,7 @@ func TestCrossRepoMount(t *testing.T) { Post(baseURL + "/v2/zot-c-test/blobs/uploads/") So(err, ShouldBeNil) So(postResponse.StatusCode(), ShouldEqual, http.StatusAccepted) - So(test.Location(baseURL, postResponse), ShouldStartWith, fmt.Sprintf("%s%s/zot-c-test/%s/%s", + So(testc.Location(baseURL, postResponse), ShouldStartWith, fmt.Sprintf("%s%s/zot-c-test/%s/%s", baseURL, constants.RoutePrefix, constants.Blobs, constants.Uploads)) // Send same request again @@ -4578,7 +4579,7 @@ func TestCrossRepoMount(t *testing.T) { Post(baseURL + "/v2/zot-mount-test/blobs/uploads/") So(err, ShouldBeNil) So(postResponse.StatusCode(), ShouldEqual, http.StatusCreated) - So(test.Location(baseURL, postResponse), ShouldEqual, fmt.Sprintf("%s%s/zot-mount-test/%s/%s:%s", + So(testc.Location(baseURL, postResponse), ShouldEqual, fmt.Sprintf("%s%s/zot-mount-test/%s/%s:%s", baseURL, constants.RoutePrefix, constants.Blobs, godigest.SHA256, blob)) // Check os.SameFile here @@ -4603,7 +4604,7 @@ func TestCrossRepoMount(t *testing.T) { Post(baseURL + "/v2/zot-mount1-test/blobs/uploads/") So(err, ShouldBeNil) So(postResponse.StatusCode(), ShouldEqual, http.StatusCreated) - So(test.Location(baseURL, postResponse), ShouldEqual, fmt.Sprintf("%s%s/zot-mount1-test/%s/%s:%s", + So(testc.Location(baseURL, postResponse), ShouldEqual, fmt.Sprintf("%s%s/zot-mount1-test/%s/%s:%s", baseURL, constants.RoutePrefix, constants.Blobs, godigest.SHA256, blob)) linkPath = path.Join(ctlr.Config.Storage.RootDirectory, "zot-mount1-test", "blobs/sha256", dgst.Encoded()) @@ -5556,7 +5557,7 @@ func TestArtifactReferences(t *testing.T) { resp, err = resty.R().Post(baseURL + fmt.Sprintf("/v2/%s/blobs/uploads/", repoName)) So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) - loc := test.Location(baseURL, resp) + loc := testc.Location(baseURL, resp) cblob, cdigest := test.GetEmptyImageConfig() resp, err = resty.R(). @@ -5653,7 +5654,7 @@ func TestArtifactReferences(t *testing.T) { resp, err = resty.R().Post(baseURL + fmt.Sprintf("/v2/%s/blobs/uploads/", repoName)) So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) - loc := test.Location(baseURL, resp) + loc := testc.Location(baseURL, resp) cblob = []byte("{}") cdigest = godigest.FromBytes(cblob) So(cdigest, ShouldNotBeNil) @@ -6514,7 +6515,7 @@ func TestListingTags(t *testing.T) { So(resp, ShouldNotBeNil) So(resp.StatusCode, ShouldEqual, http.StatusOK) - var tags api.ImageTags + var tags common.ImageTags err := json.NewDecoder(resp.Body).Decode(&tags) So(err, ShouldBeNil) So(tags.Tags, ShouldEqual, testCase.expectedTags) @@ -6886,7 +6887,7 @@ func TestManifestImageIndex(t *testing.T) { So(resp, ShouldNotBeNil) So(resp.StatusCode, ShouldEqual, http.StatusOK) - var tags api.ImageTags + var tags common.ImageTags err = json.NewDecoder(resp.Body).Decode(&tags) So(err, ShouldBeNil) So(len(tags.Tags), ShouldEqual, 3) @@ -7216,7 +7217,7 @@ func TestPullRange(t *testing.T) { resp, err := resty.R().Post(baseURL + "/v2/index/blobs/uploads/") So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) - loc := test.Location(baseURL, resp) + loc := testc.Location(baseURL, resp) So(loc, ShouldNotBeEmpty) // since we are not specifying any prefix i.e provided in config while starting server, @@ -7379,7 +7380,7 @@ func TestInjectInterruptedImageManifest(t *testing.T) { resp, err := resty.R().Post(baseURL + "/v2/repotest/blobs/uploads/") So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) - loc := test.Location(baseURL, resp) + loc := testc.Location(baseURL, resp) So(loc, ShouldNotBeEmpty) // since we are not specifying any prefix i.e provided in config while starting server, @@ -7407,7 +7408,7 @@ func TestInjectInterruptedImageManifest(t *testing.T) { resp, err = resty.R().Post(baseURL + "/v2/repotest/blobs/uploads/") So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) - loc = test.Location(baseURL, resp) + loc = testc.Location(baseURL, resp) cblob, cdigest := test.GetRandomImageConfig() resp, err = resty.R(). @@ -7488,7 +7489,7 @@ func TestInjectTooManyOpenFiles(t *testing.T) { resp, err := resty.R().Post(baseURL + "/v2/repotest/blobs/uploads/") So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) - loc := test.Location(baseURL, resp) + loc := testc.Location(baseURL, resp) So(loc, ShouldNotBeEmpty) // since we are not specifying any prefix i.e provided in config while starting server, @@ -7540,7 +7541,7 @@ func TestInjectTooManyOpenFiles(t *testing.T) { resp, err = resty.R().Post(baseURL + "/v2/repotest/blobs/uploads/") So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) - loc = test.Location(baseURL, resp) + loc = testc.Location(baseURL, resp) cblob, cdigest := test.GetRandomImageConfig() resp, err = resty.R(). @@ -9466,7 +9467,7 @@ func RunAuthorizationTests(t *testing.T, client *resty.Client, baseURL string, c So(err, ShouldBeNil) So(resp, ShouldNotBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) - loc = test.Location(baseURL, resp) + loc = testc.Location(baseURL, resp) // uploading blob should get 201 resp, err = client.R(). @@ -9486,7 +9487,7 @@ func RunAuthorizationTests(t *testing.T, client *resty.Client, baseURL string, c So(err, ShouldBeNil) So(resp, ShouldNotBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) - loc = test.Location(baseURL, resp) + loc = testc.Location(baseURL, resp) // uploading blob should get 201 resp, err = client.R(). diff --git a/pkg/api/routes.go b/pkg/api/routes.go index fabfa698..cd749a6e 100644 --- a/pkg/api/routes.go +++ b/pkg/api/routes.go @@ -261,11 +261,6 @@ func (rh *RouteHandler) CheckVersionSupport(response http.ResponseWriter, reques zcommon.WriteData(response, http.StatusOK, "application/json", []byte{}) } -type ImageTags struct { - Name string `json:"name"` - Tags []string `json:"tags"` -} - // ListTags godoc // @Summary List image tags // @Description List all image tags in a repository @@ -275,7 +270,7 @@ type ImageTags struct { // @Param name path string true "test" // @Param n query integer true "limit entries for pagination" // @Param last query string true "last tag value for pagination" -// @Success 200 {object} api.ImageTags +// @Success 200 {object} common.ImageTags // @Failure 404 {string} string "not found" // @Failure 400 {string} string "bad request". func (rh *RouteHandler) ListTags(response http.ResponseWriter, request *http.Request) { @@ -373,7 +368,7 @@ func (rh *RouteHandler) ListTags(response http.ResponseWriter, request *http.Req } } - pTags := ImageTags{Name: name} + pTags := zcommon.ImageTags{Name: name} if paginate && numTags == 0 { pTags.Tags = []string{} diff --git a/pkg/cli/image_cmd_test.go b/pkg/cli/image_cmd_test.go index 2e5ea52b..f5f1930b 100644 --- a/pkg/cli/image_cmd_test.go +++ b/pkg/cli/image_cmd_test.go @@ -33,6 +33,7 @@ import ( extconf "zotregistry.io/zot/pkg/extensions/config" zlog "zotregistry.io/zot/pkg/log" "zotregistry.io/zot/pkg/test" + testc "zotregistry.io/zot/pkg/test/common" ) func TestSearchImageCmd(t *testing.T) { @@ -1941,7 +1942,7 @@ func uploadTestMultiarch(baseURL string) { func uploadManifest(url string) error { // create and upload a blob/layer resp, _ := resty.R().Post(url + "/v2/repo7/blobs/uploads/") - loc := test.Location(url, resp) + loc := testc.Location(url, resp) content := []byte("this is a blob5") digest := godigest.FromBytes(content) @@ -1973,7 +1974,7 @@ func uploadManifest(url string) error { // upload image config blob resp, _ = resty.R().Post(url + "/v2/repo7/blobs/uploads/") - loc = test.Location(url, resp) + loc = testc.Location(url, resp) _, _ = resty.R(). SetContentLength(true). @@ -2079,7 +2080,7 @@ func uploadManifestDerivedBase(url string) error { // upload image config blob resp, _ := resty.R().Post(url + "/v2/repo7/blobs/uploads/") - loc := test.Location(url, resp) + loc := testc.Location(url, resp) _, _ = resty.R(). SetContentLength(true). diff --git a/pkg/common/model.go b/pkg/common/model.go index cbb949fa..cacfc1f5 100644 --- a/pkg/common/model.go +++ b/pkg/common/model.go @@ -254,3 +254,8 @@ type BookmarkedReposResponse struct { BookmarkedRepos `json:"data"` Errors []ErrorGQL `json:"errors"` } + +type ImageTags struct { + Name string `json:"name"` + Tags []string `json:"tags"` +} diff --git a/pkg/compliance/v1_0_0/check.go b/pkg/compliance/v1_0_0/check.go index bac6079c..a2d25af6 100644 --- a/pkg/compliance/v1_0_0/check.go +++ b/pkg/compliance/v1_0_0/check.go @@ -23,6 +23,7 @@ import ( "zotregistry.io/zot/pkg/api/constants" "zotregistry.io/zot/pkg/compliance" "zotregistry.io/zot/pkg/test" + testc "zotregistry.io/zot/pkg/test/common" ) func CheckWorkflows(t *testing.T, config *compliance.Config) { @@ -118,7 +119,7 @@ func CheckWorkflows(t *testing.T, config *compliance.Config) { resp, err := resty.R().Post(baseURL + "/v2/repo2/blobs/uploads/") So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) - loc := test.Location(baseURL, resp) + loc := testc.Location(baseURL, resp) So(loc, ShouldNotBeEmpty) resp, err = resty.R().Get(loc) @@ -154,7 +155,7 @@ func CheckWorkflows(t *testing.T, config *compliance.Config) { SetHeader("Content-Type", "application/octet-stream").SetBody(content).Put(loc) So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusCreated) - blobLoc := test.Location(baseURL, resp) + blobLoc := testc.Location(baseURL, resp) So(blobLoc, ShouldNotBeEmpty) So(resp.Header().Get("Content-Length"), ShouldEqual, "0") So(resp.Header().Get(constants.DistContentDigestKey), ShouldNotBeEmpty) @@ -198,7 +199,7 @@ func CheckWorkflows(t *testing.T, config *compliance.Config) { Post(baseURL + "/v2/repo2/blobs/uploads/") So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusCreated) - loc := test.Location(baseURL, resp) + loc := testc.Location(baseURL, resp) So(loc, ShouldNotBeEmpty) // blob reference should be accessible resp, err = resty.R().Get(loc) @@ -211,7 +212,7 @@ func CheckWorkflows(t *testing.T, config *compliance.Config) { resp, err := resty.R().Post(baseURL + "/v2/repo10/repo20/repo30/blobs/uploads/") So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) - loc := test.Location(baseURL, resp) + loc := testc.Location(baseURL, resp) So(loc, ShouldNotBeEmpty) resp, err = resty.R().Get(loc) @@ -247,7 +248,7 @@ func CheckWorkflows(t *testing.T, config *compliance.Config) { SetHeader("Content-Type", "application/octet-stream").SetBody(content).Put(loc) So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusCreated) - blobLoc := test.Location(baseURL, resp) + blobLoc := testc.Location(baseURL, resp) So(blobLoc, ShouldNotBeEmpty) So(resp.Header().Get("Content-Length"), ShouldEqual, "0") So(resp.Header().Get(constants.DistContentDigestKey), ShouldNotBeEmpty) @@ -266,7 +267,7 @@ func CheckWorkflows(t *testing.T, config *compliance.Config) { resp, err := resty.R().Post(baseURL + "/v2/repo3/blobs/uploads/") So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) - loc := test.Location(baseURL, resp) + loc := testc.Location(baseURL, resp) So(loc, ShouldNotBeEmpty) var buf bytes.Buffer @@ -313,7 +314,7 @@ func CheckWorkflows(t *testing.T, config *compliance.Config) { SetHeader("Content-Type", "application/octet-stream").SetBody(chunk2).Put(loc) So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusCreated) - blobLoc := test.Location(baseURL, resp) + blobLoc := testc.Location(baseURL, resp) So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusCreated) So(blobLoc, ShouldNotBeEmpty) @@ -334,7 +335,7 @@ func CheckWorkflows(t *testing.T, config *compliance.Config) { resp, err := resty.R().Post(baseURL + "/v2/repo40/repo50/repo60/blobs/uploads/") So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) - loc := test.Location(baseURL, resp) + loc := testc.Location(baseURL, resp) So(loc, ShouldNotBeEmpty) var buf bytes.Buffer @@ -381,7 +382,7 @@ func CheckWorkflows(t *testing.T, config *compliance.Config) { SetHeader("Content-Type", "application/octet-stream").SetBody(chunk2).Put(loc) So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusCreated) - blobLoc := test.Location(baseURL, resp) + blobLoc := testc.Location(baseURL, resp) So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusCreated) So(blobLoc, ShouldNotBeEmpty) @@ -403,7 +404,7 @@ func CheckWorkflows(t *testing.T, config *compliance.Config) { resp, err := resty.R().Post(baseURL + "/v2/repo4/blobs/uploads/") So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) - loc := test.Location(baseURL, resp) + loc := testc.Location(baseURL, resp) So(loc, ShouldNotBeEmpty) // delete this upload @@ -418,7 +419,7 @@ func CheckWorkflows(t *testing.T, config *compliance.Config) { resp, err := resty.R().Post(baseURL + "/v2/repo5/blobs/uploads/") So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) - loc := test.Location(baseURL, resp) + loc := testc.Location(baseURL, resp) So(loc, ShouldNotBeEmpty) content := []byte("this is a blob4") @@ -429,7 +430,7 @@ func CheckWorkflows(t *testing.T, config *compliance.Config) { SetHeader("Content-Type", "application/octet-stream").SetBody(content).Put(loc) So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusCreated) - blobLoc := test.Location(baseURL, resp) + blobLoc := testc.Location(baseURL, resp) So(blobLoc, ShouldNotBeEmpty) So(resp.Header().Get(constants.DistContentDigestKey), ShouldNotBeEmpty) @@ -454,7 +455,7 @@ func CheckWorkflows(t *testing.T, config *compliance.Config) { resp, err := resty.R().Post(baseURL + "/v2/repo7/blobs/uploads/") So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) - loc := test.Location(baseURL, resp) + loc := testc.Location(baseURL, resp) So(loc, ShouldNotBeEmpty) // since we are not specifying any prefix i.e provided in config while starting server, @@ -666,7 +667,7 @@ func CheckWorkflows(t *testing.T, config *compliance.Config) { resp, err := resty.R().Post(baseURL + "/v2/firsttest/first/blobs/uploads/") So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) - firstloc := test.Location(baseURL, resp) + firstloc := testc.Location(baseURL, resp) So(firstloc, ShouldNotBeEmpty) resp, err = resty.R().Get(firstloc) @@ -681,7 +682,7 @@ func CheckWorkflows(t *testing.T, config *compliance.Config) { resp, err = resty.R().Post(baseURL + "/v2/secondtest/second/blobs/uploads/") So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) - secondloc := test.Location(baseURL, resp) + secondloc := testc.Location(baseURL, resp) So(secondloc, ShouldNotBeEmpty) resp, err = resty.R().Get(secondloc) diff --git a/pkg/extensions/lint/lint_test.go b/pkg/extensions/lint/lint_test.go index 240cafaf..2e361f21 100644 --- a/pkg/extensions/lint/lint_test.go +++ b/pkg/extensions/lint/lint_test.go @@ -24,6 +24,7 @@ import ( "zotregistry.io/zot/pkg/log" "zotregistry.io/zot/pkg/storage/local" "zotregistry.io/zot/pkg/test" + testc "zotregistry.io/zot/pkg/test/common" ) const ( @@ -257,7 +258,7 @@ func TestVerifyMandatoryAnnotations(t *testing.T) { resp, err = resty.R(). Post(fmt.Sprintf("%s/v2/zot-test/blobs/uploads/", baseURL)) So(err, ShouldBeNil) - loc := test.Location(baseURL, resp) + loc := testc.Location(baseURL, resp) _, err = resty.R(). SetContentLength(true). @@ -344,7 +345,7 @@ func TestVerifyMandatoryAnnotations(t *testing.T) { _, err = resty.R(). Post(fmt.Sprintf("%s/v2/zot-test/blobs/uploads/", baseURL)) So(err, ShouldBeNil) - loc := test.Location(baseURL, resp) + loc := testc.Location(baseURL, resp) _, err = resty.R(). SetContentLength(true). diff --git a/pkg/extensions/sync/sync_test.go b/pkg/extensions/sync/sync_test.go index 644ab0ca..66e74125 100644 --- a/pkg/extensions/sync/sync_test.go +++ b/pkg/extensions/sync/sync_test.go @@ -48,6 +48,7 @@ import ( mTypes "zotregistry.io/zot/pkg/meta/types" storageConstants "zotregistry.io/zot/pkg/storage/constants" "zotregistry.io/zot/pkg/test" + testc "zotregistry.io/zot/pkg/test/common" "zotregistry.io/zot/pkg/test/mocks" ) @@ -6329,7 +6330,7 @@ func pushRepo(url, repoName string) godigest.Digest { panic(err) } - loc := test.Location(url, resp) + loc := testc.Location(url, resp) _, err = resty.R().Get(loc) if err != nil { @@ -6356,7 +6357,7 @@ func pushRepo(url, repoName string) godigest.Digest { panic(fmt.Errorf("invalid status code: %d %w", resp.StatusCode(), errBadStatus)) } - loc = test.Location(url, resp) + loc = testc.Location(url, resp) cblob, cdigest := ispec.DescriptorEmptyJSON.Data, ispec.DescriptorEmptyJSON.Digest resp, err = resty.R(). @@ -6385,7 +6386,7 @@ func pushRepo(url, repoName string) godigest.Digest { panic(fmt.Errorf("invalid status code: %d %w", resp.StatusCode(), errBadStatus)) } - loc = test.Location(url, resp) + loc = testc.Location(url, resp) cblob, cdigest = test.GetRandomImageConfig() resp, err = resty.R(). @@ -6553,7 +6554,7 @@ func pushBlob(url string, repoName string, buf []byte) godigest.Digest { panic(fmt.Errorf("invalid status code: %d %w", resp.StatusCode(), errBadStatus)) } - loc := test.Location(url, resp) + loc := testc.Location(url, resp) digest := godigest.FromBytes(buf) resp, err = resty.R(). diff --git a/pkg/log/log_test.go b/pkg/log/log_test.go index 5a981735..82e71111 100644 --- a/pkg/log/log_test.go +++ b/pkg/log/log_test.go @@ -25,6 +25,7 @@ import ( "zotregistry.io/zot/pkg/api/constants" "zotregistry.io/zot/pkg/log" . "zotregistry.io/zot/pkg/test" + testc "zotregistry.io/zot/pkg/test/common" ) const ( @@ -127,7 +128,7 @@ func TestAuditLogMessages(t *testing.T) { resp, err := resty.R().SetBasicAuth(username, passphrase).Post(baseURL + path) So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) - loc := Location(baseURL, resp) + loc := testc.Location(baseURL, resp) So(loc, ShouldNotBeEmpty) location := resp.Header().Get("Location") So(location, ShouldNotBeEmpty) @@ -163,7 +164,7 @@ func TestAuditLogMessages(t *testing.T) { SetHeader("Content-Type", "application/octet-stream").SetBody(content).Put(loc) So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusCreated) - blobLoc := Location(baseURL, resp) + blobLoc := testc.Location(baseURL, resp) So(blobLoc, ShouldNotBeEmpty) So(resp.Header().Get(constants.DistContentDigestKey), ShouldNotBeEmpty) @@ -223,7 +224,7 @@ func TestAuditLogMessages(t *testing.T) { resp, err := resty.R().SetBasicAuth(username, passphrase).Post(baseURL + path) So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) - loc := Location(baseURL, resp) + loc := testc.Location(baseURL, resp) So(loc, ShouldNotBeEmpty) location := resp.Header().Get("Location") So(location, ShouldNotBeEmpty) diff --git a/pkg/test/common.go b/pkg/test/common.go index 5e99f858..34571dde 100644 --- a/pkg/test/common.go +++ b/pkg/test/common.go @@ -18,7 +18,6 @@ import ( mathRand "math/rand" "net" "net/http" - "net/url" "os" "path" "path/filepath" @@ -55,6 +54,7 @@ import ( storageCommon "zotregistry.io/zot/pkg/storage/common" "zotregistry.io/zot/pkg/storage/local" stypes "zotregistry.io/zot/pkg/storage/types" + testc "zotregistry.io/zot/pkg/test/common" "zotregistry.io/zot/pkg/test/inject" "zotregistry.io/zot/pkg/test/mocks" ) @@ -158,24 +158,6 @@ func MakeHtpasswdFileFromString(fileContent string) string { return htpasswdFile.Name() } -func Location(baseURL string, resp *resty.Response) string { - // For some API responses, the Location header is set and is supposed to - // indicate an opaque value. However, it is not clear if this value is an - // absolute URL (https://server:port/v2/...) or just a path (/v2/...) - // zot implements the latter as per the spec, but some registries appear to - // return the former - this needs to be clarified - loc := resp.Header().Get("Location") - - uloc, err := url.Parse(loc) - if err != nil { - return "" - } - - path := uloc.Path - - return baseURL + path -} - func CopyFiles(sourceDir, destDir string) error { sourceMeta, err := os.Stat(sourceDir) if err != nil { @@ -954,7 +936,7 @@ func UploadImage(img Image, baseURL, repo, ref string) error { return ErrPostBlob } - loc := Location(baseURL, resp) + loc := testc.Location(baseURL, resp) // uploading blob should get 201 resp, err = resty.R(). @@ -1633,7 +1615,7 @@ func UploadImageWithBasicAuth(img Image, baseURL, repo, ref, user, password stri return ErrPostBlob } - loc := Location(baseURL, resp) + loc := testc.Location(baseURL, resp) // uploading blob should get 201 resp, err = resty.R(). diff --git a/pkg/test/common/utils.go b/pkg/test/common/utils.go new file mode 100644 index 00000000..86f97cf3 --- /dev/null +++ b/pkg/test/common/utils.go @@ -0,0 +1,25 @@ +package common + +import ( + "net/url" + + "gopkg.in/resty.v1" +) + +func Location(baseURL string, resp *resty.Response) string { + // For some API responses, the Location header is set and is supposed to + // indicate an opaque value. However, it is not clear if this value is an + // absolute URL (https://server:port/v2/...) or just a path (/v2/...) + // zot implements the latter as per the spec, but some registries appear to + // return the former - this needs to be clarified + loc := resp.Header().Get("Location") + + uloc, err := url.Parse(loc) + if err != nil { + return "" + } + + path := uloc.Path + + return baseURL + path +} diff --git a/swagger/docs.go b/swagger/docs.go index 6c325a88..a53de872 100644 --- a/swagger/docs.go +++ b/swagger/docs.go @@ -1146,7 +1146,7 @@ const docTemplate = `{ "200": { "description": "OK", "schema": { - "$ref": "#/definitions/api.ImageTags" + "$ref": "#/definitions/common.ImageTags" } }, "400": { @@ -1280,13 +1280,10 @@ const docTemplate = `{ } } }, - "api.ImageTags": { + "api.RepositoryList": { "type": "object", "properties": { - "name": { - "type": "string" - }, - "tags": { + "repositories": { "type": "array", "items": { "type": "string" @@ -1294,10 +1291,13 @@ const docTemplate = `{ } } }, - "api.RepositoryList": { + "common.ImageTags": { "type": "object", "properties": { - "repositories": { + "name": { + "type": "string" + }, + "tags": { "type": "array", "items": { "type": "string" diff --git a/swagger/swagger.json b/swagger/swagger.json index f3f02786..f8a7d0ed 100644 --- a/swagger/swagger.json +++ b/swagger/swagger.json @@ -1137,7 +1137,7 @@ "200": { "description": "OK", "schema": { - "$ref": "#/definitions/api.ImageTags" + "$ref": "#/definitions/common.ImageTags" } }, "400": { @@ -1271,13 +1271,10 @@ } } }, - "api.ImageTags": { + "api.RepositoryList": { "type": "object", "properties": { - "name": { - "type": "string" - }, - "tags": { + "repositories": { "type": "array", "items": { "type": "string" @@ -1285,10 +1282,13 @@ } } }, - "api.RepositoryList": { + "common.ImageTags": { "type": "object", "properties": { - "repositories": { + "name": { + "type": "string" + }, + "tags": { "type": "array", "items": { "type": "string" diff --git a/swagger/swagger.yaml b/swagger/swagger.yaml index cbe46dc1..e97f9d70 100644 --- a/swagger/swagger.yaml +++ b/swagger/swagger.yaml @@ -83,18 +83,18 @@ definitions: manifest forming an association between the image manifest and the other manifest. type: object - api.ImageTags: + api.RepositoryList: properties: - name: - type: string - tags: + repositories: items: type: string type: array type: object - api.RepositoryList: + common.ImageTags: properties: - repositories: + name: + type: string + tags: items: type: string type: array @@ -994,7 +994,7 @@ paths: "200": description: OK schema: - $ref: '#/definitions/api.ImageTags' + $ref: '#/definitions/common.ImageTags' "400": description: bad request". schema: