From 58040f45621be5b393896eae255604c2c92e91ed Mon Sep 17 00:00:00 2001 From: Ramkumar Chinchani Date: Thu, 30 Jan 2020 23:59:36 -0800 Subject: [PATCH] check: add unit tests to cover the new code, fix linter errors --- pkg/api/errors.go | 7 +- pkg/api/routes.go | 114 +++++++++++++++++++++------------ pkg/compliance/v1_0_0/check.go | 70 +++++++++++++++++++- pkg/storage/storage.go | 1 + pkg/storage/storage_test.go | 10 +++ 5 files changed, 157 insertions(+), 45 deletions(-) diff --git a/pkg/api/errors.go b/pkg/api/errors.go index a6b12819..b97bf11d 100644 --- a/pkg/api/errors.go +++ b/pkg/api/errors.go @@ -54,10 +54,11 @@ func (e ErrorCode) String() string { DENIED: "DENIED", UNSUPPORTED: "UNSUPPORTED", } + return m[e] } -func NewError(code ErrorCode, detail ...interface{}) Error { +func NewError(code ErrorCode, detail ...interface{}) Error { //nolint (interfacer) var errMap = map[ErrorCode]Error{ BLOB_UNKNOWN: { Message: "blob unknown to registry", @@ -166,9 +167,11 @@ func NewError(code ErrorCode, detail ...interface{}) Error { func NewErrorList(errors ...Error) ErrorList { el := make([]*Error, 0) + er := Error{} for _, e := range errors { - el = append(el, &e) + er = e + el = append(el, &er) } return ErrorList{el} diff --git a/pkg/api/routes.go b/pkg/api/routes.go index 88114b14..d973d7a8 100644 --- a/pkg/api/routes.go +++ b/pkg/api/routes.go @@ -243,10 +243,12 @@ func (rh *RouteHandler) CheckManifest(w http.ResponseWriter, r *http.Request) { if err != nil { switch err { case errors.ErrManifestNotFound: - WriteJSON(w, http.StatusNotFound, NewErrorList(NewError(MANIFEST_UNKNOWN, map[string]string{"reference": reference}))) + WriteJSON(w, http.StatusNotFound, + NewErrorList(NewError(MANIFEST_UNKNOWN, map[string]string{"reference": reference}))) default: rh.c.Log.Error().Err(err).Msg("unexpected error") - WriteJSON(w, http.StatusInternalServerError, NewErrorList(NewError(MANIFEST_INVALID, map[string]string{"reference": reference}))) + WriteJSON(w, http.StatusInternalServerError, + NewErrorList(NewError(MANIFEST_INVALID, map[string]string{"reference": reference}))) } return @@ -293,11 +295,14 @@ func (rh *RouteHandler) GetManifest(w http.ResponseWriter, r *http.Request) { if err != nil { switch err { case errors.ErrRepoNotFound: - WriteJSON(w, http.StatusNotFound, NewErrorList(NewError(NAME_UNKNOWN, map[string]string{"name": name}))) + WriteJSON(w, http.StatusNotFound, + NewErrorList(NewError(NAME_UNKNOWN, map[string]string{"name": name}))) case errors.ErrRepoBadVersion: - WriteJSON(w, http.StatusNotFound, NewErrorList(NewError(NAME_UNKNOWN, map[string]string{"name": name}))) + WriteJSON(w, http.StatusNotFound, + NewErrorList(NewError(NAME_UNKNOWN, map[string]string{"name": name}))) case errors.ErrManifestNotFound: - WriteJSON(w, http.StatusNotFound, NewErrorList(NewError(MANIFEST_UNKNOWN, map[string]string{"reference": reference}))) + WriteJSON(w, http.StatusNotFound, + NewErrorList(NewError(MANIFEST_UNKNOWN, map[string]string{"reference": reference}))) default: rh.c.Log.Error().Err(err).Msg("unexpected error") w.WriteHeader(http.StatusInternalServerError) @@ -356,13 +361,17 @@ func (rh *RouteHandler) UpdateManifest(w http.ResponseWriter, r *http.Request) { if err != nil { switch err { case errors.ErrRepoNotFound: - WriteJSON(w, http.StatusNotFound, NewErrorList(NewError(NAME_UNKNOWN, map[string]string{"name": name}))) + WriteJSON(w, http.StatusNotFound, + NewErrorList(NewError(NAME_UNKNOWN, map[string]string{"name": name}))) case errors.ErrManifestNotFound: - WriteJSON(w, http.StatusNotFound, NewErrorList(NewError(MANIFEST_UNKNOWN, map[string]string{"reference": reference}))) + WriteJSON(w, http.StatusNotFound, + NewErrorList(NewError(MANIFEST_UNKNOWN, map[string]string{"reference": reference}))) case errors.ErrBadManifest: - WriteJSON(w, http.StatusBadRequest, NewErrorList(NewError(MANIFEST_INVALID, map[string]string{"reference": reference}))) + WriteJSON(w, http.StatusBadRequest, + NewErrorList(NewError(MANIFEST_INVALID, map[string]string{"reference": reference}))) case errors.ErrBlobNotFound: - WriteJSON(w, http.StatusBadRequest, NewErrorList(NewError(BLOB_UNKNOWN, map[string]string{"blob": digest}))) + WriteJSON(w, http.StatusBadRequest, + NewErrorList(NewError(BLOB_UNKNOWN, map[string]string{"blob": digest}))) default: rh.c.Log.Error().Err(err).Msg("unexpected error") w.WriteHeader(http.StatusInternalServerError) @@ -404,9 +413,11 @@ func (rh *RouteHandler) DeleteManifest(w http.ResponseWriter, r *http.Request) { if err != nil { switch err { case errors.ErrRepoNotFound: - WriteJSON(w, http.StatusNotFound, NewErrorList(NewError(NAME_UNKNOWN, map[string]string{"name": name}))) + WriteJSON(w, http.StatusNotFound, + NewErrorList(NewError(NAME_UNKNOWN, map[string]string{"name": name}))) case errors.ErrManifestNotFound: - WriteJSON(w, http.StatusNotFound, NewErrorList(NewError(MANIFEST_UNKNOWN, map[string]string{"reference": reference}))) + WriteJSON(w, http.StatusNotFound, + NewErrorList(NewError(MANIFEST_UNKNOWN, map[string]string{"reference": reference}))) default: rh.c.Log.Error().Err(err).Msg("unexpected error") w.WriteHeader(http.StatusInternalServerError) @@ -587,6 +598,18 @@ func (rh *RouteHandler) CreateBlobUpload(w http.ResponseWriter, r *http.Request) return } + // blob mounts not allowed since we don't have access control yet, and this + // may be a uncommon use case, but remain compliant + if _, ok := r.URL.Query()["mount"]; ok { + w.WriteHeader(http.StatusMethodNotAllowed) + return + } + + if _, ok := r.URL.Query()["from"]; ok { + w.WriteHeader(http.StatusMethodNotAllowed) + return + } + // a full blob upload if "digest" is present digests, ok := r.URL.Query()["digest"] if ok { @@ -607,16 +630,18 @@ func (rh *RouteHandler) CreateBlobUpload(w http.ResponseWriter, r *http.Request) rh.c.Log.Info().Int64("r.ContentLength", r.ContentLength).Msg("DEBUG") var contentLength int64 + var err error - if contentLength, err = strconv.ParseInt(r.Header.Get("Content-Length"), 10, 64); err != nil { + if contentLength, err = strconv.ParseInt(r.Header.Get("Content-Length"), 10, 64); err != nil || contentLength <= 0 { rh.c.Log.Warn().Str("actual", r.Header.Get("Content-Length")).Msg("invalid content length") - w.WriteHeader(http.StatusBadRequest) + WriteJSON(w, http.StatusBadRequest, + NewErrorList(NewError(BLOB_UPLOAD_INVALID, map[string]string{"digest": digest}))) return } - size, err := rh.c.ImageStore.FullBlobUpload(name, r.Body, digest) + sessionID, size, err := rh.c.ImageStore.FullBlobUpload(name, r.Body, digest) if err != nil { rh.c.Log.Error().Err(err).Int64("actual", size).Int64("expected", contentLength).Msg("failed full upload") w.WriteHeader(http.StatusInternalServerError) @@ -631,19 +656,10 @@ func (rh *RouteHandler) CreateBlobUpload(w http.ResponseWriter, r *http.Request) return } + w.Header().Set("Location", fmt.Sprintf("/v2/%s/blobs/%s", name, digest)) + w.Header().Set(BlobUploadUUID, sessionID) w.WriteHeader(http.StatusCreated) - return - } - // blob mounts not allowed since we don't have access control yet, and this - // may be a uncommon use case, but remain compliant - if _, ok := r.URL.Query()["mount"]; ok { - w.WriteHeader(http.StatusMethodNotAllowed) - return - } - - if _, ok := r.URL.Query()["from"]; ok { - w.WriteHeader(http.StatusMethodNotAllowed) return } @@ -697,13 +713,17 @@ func (rh *RouteHandler) GetBlobUpload(w http.ResponseWriter, r *http.Request) { if err != nil { switch err { case errors.ErrBadUploadRange: - WriteJSON(w, http.StatusBadRequest, NewErrorList(NewError(BLOB_UPLOAD_INVALID, map[string]string{"session_id": sessionID}))) + WriteJSON(w, http.StatusBadRequest, + NewErrorList(NewError(BLOB_UPLOAD_INVALID, map[string]string{"session_id": sessionID}))) case errors.ErrBadBlobDigest: - WriteJSON(w, http.StatusBadRequest, NewErrorList(NewError(BLOB_UPLOAD_INVALID, map[string]string{"session_id": sessionID}))) + WriteJSON(w, http.StatusBadRequest, + NewErrorList(NewError(BLOB_UPLOAD_INVALID, map[string]string{"session_id": sessionID}))) case errors.ErrRepoNotFound: - WriteJSON(w, http.StatusNotFound, NewErrorList(NewError(NAME_UNKNOWN, map[string]string{"name": name}))) + WriteJSON(w, http.StatusNotFound, + NewErrorList(NewError(NAME_UNKNOWN, map[string]string{"name": name}))) case errors.ErrUploadNotFound: - WriteJSON(w, http.StatusNotFound, NewErrorList(NewError(BLOB_UPLOAD_UNKNOWN, map[string]string{"session_id": sessionID}))) + WriteJSON(w, http.StatusNotFound, + NewErrorList(NewError(BLOB_UPLOAD_UNKNOWN, map[string]string{"session_id": sessionID}))) default: rh.c.Log.Error().Err(err).Msg("unexpected error") w.WriteHeader(http.StatusInternalServerError) @@ -794,11 +814,14 @@ func (rh *RouteHandler) PatchBlobUpload(w http.ResponseWriter, r *http.Request) if err != nil { switch err { case errors.ErrBadUploadRange: - WriteJSON(w, http.StatusRequestedRangeNotSatisfiable, NewErrorList(NewError(BLOB_UPLOAD_INVALID, map[string]string{"session_id": sessionID}))) + WriteJSON(w, http.StatusRequestedRangeNotSatisfiable, + NewErrorList(NewError(BLOB_UPLOAD_INVALID, map[string]string{"session_id": sessionID}))) case errors.ErrRepoNotFound: - WriteJSON(w, http.StatusNotFound, NewErrorList(NewError(NAME_UNKNOWN, map[string]string{"name": name}))) + WriteJSON(w, http.StatusNotFound, + NewErrorList(NewError(NAME_UNKNOWN, map[string]string{"name": name}))) case errors.ErrUploadNotFound: - WriteJSON(w, http.StatusNotFound, NewErrorList(NewError(BLOB_UPLOAD_UNKNOWN, map[string]string{"session_id": sessionID}))) + WriteJSON(w, http.StatusNotFound, + NewErrorList(NewError(BLOB_UPLOAD_UNKNOWN, map[string]string{"session_id": sessionID}))) default: rh.c.Log.Error().Err(err).Msg("unexpected error") w.WriteHeader(http.StatusInternalServerError) @@ -900,11 +923,14 @@ func (rh *RouteHandler) UpdateBlobUpload(w http.ResponseWriter, r *http.Request) if err != nil { switch err { case errors.ErrBadUploadRange: - WriteJSON(w, http.StatusBadRequest, NewErrorList(NewError(BLOB_UPLOAD_INVALID, map[string]string{"session_id": sessionID}))) + WriteJSON(w, http.StatusBadRequest, + NewErrorList(NewError(BLOB_UPLOAD_INVALID, map[string]string{"session_id": sessionID}))) case errors.ErrRepoNotFound: - WriteJSON(w, http.StatusNotFound, NewErrorList(NewError(NAME_UNKNOWN, map[string]string{"name": name}))) + WriteJSON(w, http.StatusNotFound, + NewErrorList(NewError(NAME_UNKNOWN, map[string]string{"name": name}))) case errors.ErrUploadNotFound: - WriteJSON(w, http.StatusNotFound, NewErrorList(NewError(BLOB_UPLOAD_UNKNOWN, map[string]string{"session_id": sessionID}))) + WriteJSON(w, http.StatusNotFound, + NewErrorList(NewError(BLOB_UPLOAD_UNKNOWN, map[string]string{"session_id": sessionID}))) default: rh.c.Log.Error().Err(err).Msg("unexpected error") w.WriteHeader(http.StatusInternalServerError) @@ -923,13 +949,17 @@ finish: if err := rh.c.ImageStore.FinishBlobUpload(name, sessionID, r.Body, digest); err != nil { switch err { case errors.ErrBadBlobDigest: - WriteJSON(w, http.StatusBadRequest, NewErrorList(NewError(DIGEST_INVALID, map[string]string{"digest": digest}))) + WriteJSON(w, http.StatusBadRequest, + NewErrorList(NewError(DIGEST_INVALID, map[string]string{"digest": digest}))) case errors.ErrBadUploadRange: - WriteJSON(w, http.StatusBadRequest, NewErrorList(NewError(BLOB_UPLOAD_INVALID, map[string]string{"session_id": sessionID}))) + WriteJSON(w, http.StatusBadRequest, + NewErrorList(NewError(BLOB_UPLOAD_INVALID, map[string]string{"session_id": sessionID}))) case errors.ErrRepoNotFound: - WriteJSON(w, http.StatusNotFound, NewErrorList(NewError(NAME_UNKNOWN, map[string]string{"name": name}))) + WriteJSON(w, http.StatusNotFound, + NewErrorList(NewError(NAME_UNKNOWN, map[string]string{"name": name}))) case errors.ErrUploadNotFound: - WriteJSON(w, http.StatusNotFound, NewErrorList(NewError(BLOB_UPLOAD_UNKNOWN, map[string]string{"session_id": sessionID}))) + WriteJSON(w, http.StatusNotFound, + NewErrorList(NewError(BLOB_UPLOAD_UNKNOWN, map[string]string{"session_id": sessionID}))) default: rh.c.Log.Error().Err(err).Msg("unexpected error") w.WriteHeader(http.StatusInternalServerError) @@ -973,9 +1003,11 @@ func (rh *RouteHandler) DeleteBlobUpload(w http.ResponseWriter, r *http.Request) if err := rh.c.ImageStore.DeleteBlobUpload(name, sessionID); err != nil { switch err { case errors.ErrRepoNotFound: - WriteJSON(w, http.StatusNotFound, NewErrorList(NewError(NAME_UNKNOWN, map[string]string{"name": name}))) + WriteJSON(w, http.StatusNotFound, + NewErrorList(NewError(NAME_UNKNOWN, map[string]string{"name": name}))) case errors.ErrUploadNotFound: - WriteJSON(w, http.StatusNotFound, NewErrorList(NewError(BLOB_UPLOAD_UNKNOWN, map[string]string{"session_id": sessionID}))) + WriteJSON(w, http.StatusNotFound, + NewErrorList(NewError(BLOB_UPLOAD_UNKNOWN, map[string]string{"session_id": sessionID}))) default: rh.c.Log.Error().Err(err).Msg("unexpected error") w.WriteHeader(http.StatusInternalServerError) diff --git a/pkg/compliance/v1_0_0/check.go b/pkg/compliance/v1_0_0/check.go index 05a4d80f..4d7be43b 100644 --- a/pkg/compliance/v1_0_0/check.go +++ b/pkg/compliance/v1_0_0/check.go @@ -169,6 +169,44 @@ func CheckWorkflows(t *testing.T, config *compliance.Config) { So(resp.StatusCode(), ShouldEqual, 200) }) + Convey("Monolithic blob upload with body", func() { + Print("\nMonolithic blob upload") + // create content + content := []byte("this is a blob") + digest := godigest.FromBytes(content) + So(digest, ShouldNotBeNil) + // setting invalid URL params should fail + resp, err := resty.R(). + SetQueryParam("digest", digest.String()). + SetQueryParam("from", digest.String()). + SetHeader("Content-Type", "application/octet-stream"). + SetBody(content). + Post(baseURL + "/v2/repo2/blobs/uploads/") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, 405) + // setting a "?digest=<>" but without body should fail + resp, err = resty.R(). + SetQueryParam("digest", digest.String()). + SetHeader("Content-Type", "application/octet-stream"). + Post(baseURL + "/v2/repo2/blobs/uploads/") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, 400) + // set a "?digest=<>" + resp, err = resty.R(). + SetQueryParam("digest", digest.String()). + SetHeader("Content-Type", "application/octet-stream"). + SetBody(content). + Post(baseURL + "/v2/repo2/blobs/uploads/") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, 201) + loc := Location(baseURL, resp) + So(loc, ShouldNotBeEmpty) + // blob reference should be accessible + resp, err = resty.R().Get(loc) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, 200) + }) + Convey("Monolithic blob upload with multiple name components", func() { Print("\nMonolithic blob upload with multiple name components") resp, err := resty.R().Post(baseURL + "/v2/repo10/repo20/repo30/blobs/uploads/") @@ -258,7 +296,7 @@ func CheckWorkflows(t *testing.T, config *compliance.Config) { resp, err = resty.R().SetHeader("Content-Type", "application/octet-stream"). SetHeader("Content-Range", contentRange).SetBody(chunk1).Patch(loc) So(err, ShouldBeNil) - So(resp.StatusCode(), ShouldEqual, 400) + So(resp.StatusCode(), ShouldEqual, 416) So(resp.String(), ShouldNotBeEmpty) chunk2 := []byte("this is the second chunk") @@ -326,7 +364,7 @@ func CheckWorkflows(t *testing.T, config *compliance.Config) { resp, err = resty.R().SetHeader("Content-Type", "application/octet-stream"). SetHeader("Content-Range", contentRange).SetBody(chunk1).Patch(loc) So(err, ShouldBeNil) - So(resp.StatusCode(), ShouldEqual, 400) + So(resp.StatusCode(), ShouldEqual, 416) So(resp.String(), ShouldNotBeEmpty) chunk2 := []byte("this is the second chunk") @@ -450,6 +488,23 @@ func CheckWorkflows(t *testing.T, config *compliance.Config) { So(d, ShouldNotBeEmpty) So(d, ShouldEqual, digest.String()) + content = []byte("this is a blob") + digest = godigest.FromBytes(content) + So(digest, ShouldNotBeNil) + // create a manifest with same blob but a different tag + m = ispec.Manifest{Layers: []ispec.Descriptor{{Digest: digest}}} + content, err = json.Marshal(m) + So(err, ShouldBeNil) + digest = godigest.FromBytes(content) + So(digest, ShouldNotBeNil) + resp, err = resty.R().SetHeader("Content-Type", "application/vnd.oci.image.manifest.v1+json"). + SetBody(content).Put(baseURL + "/v2/repo7/manifests/test:2.0") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, 201) + d = resp.Header().Get(api.DistContentDigestKey) + So(d, ShouldNotBeEmpty) + So(d, ShouldEqual, digest.String()) + // check/get by tag resp, err = resty.R().Head(baseURL + "/v2/repo7/manifests/test:1.0") So(err, ShouldBeNil) @@ -475,6 +530,10 @@ func CheckWorkflows(t *testing.T, config *compliance.Config) { resp, err = resty.R().Delete(baseURL + "/v2/repo7/manifests/" + digest.String()) So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, 202) + // delete manifest by digest + resp, err = resty.R().Delete(baseURL + "/v2/repo7/manifests/" + digest.String()) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, 202) // delete again should fail resp, err = resty.R().Delete(baseURL + "/v2/repo7/manifests/" + digest.String()) So(err, ShouldBeNil) @@ -488,6 +547,13 @@ func CheckWorkflows(t *testing.T, config *compliance.Config) { So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, 404) So(resp.Body(), ShouldNotBeEmpty) + resp, err = resty.R().Head(baseURL + "/v2/repo7/manifests/test:2.0") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, 404) + resp, err = resty.R().Get(baseURL + "/v2/repo7/manifests/test:2.0") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, 404) + So(resp.Body(), ShouldNotBeEmpty) // check/get by reference resp, err = resty.R().Head(baseURL + "/v2/repo7/manifests/" + digest.String()) So(err, ShouldBeNil) diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index dc226317..2ee742ac 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -727,6 +727,7 @@ func (is *ImageStore) FullBlobUpload(repo string, body io.Reader, digest string) digester := sha256.New() mw := io.MultiWriter(f, digester) n, err := io.Copy(mw, body) + if err != nil { return "", -1, err } diff --git a/pkg/storage/storage_test.go b/pkg/storage/storage_test.go index 69342c27..71497e48 100644 --- a/pkg/storage/storage_test.go +++ b/pkg/storage/storage_test.go @@ -58,6 +58,16 @@ func TestAPIs(t *testing.T) { So(v, ShouldBeEmpty) }) + Convey("Full blob upload", func() { + body := []byte("this is a blob") + buf := bytes.NewBuffer(body) + d := godigest.FromBytes(body) + u, n, err := il.FullBlobUpload("test", buf, d.String()) + So(err, ShouldBeNil) + So(n, ShouldEqual, len(body)) + So(u, ShouldNotBeEmpty) + }) + Convey("New blob upload", func() { v, err := il.NewBlobUpload("test") So(err, ShouldBeNil)