diff --git a/src/core/middlewares/middlewares.go b/src/core/middlewares/middlewares.go index b94362fee..6edaf986e 100644 --- a/src/core/middlewares/middlewares.go +++ b/src/core/middlewares/middlewares.go @@ -15,8 +15,11 @@ package middlewares import ( + "net/http" + "regexp" + "github.com/astaxie/beego" - "github.com/docker/distribution/reference" + "github.com/goharbor/harbor/src/pkg/distribution" "github.com/goharbor/harbor/src/server/middleware" "github.com/goharbor/harbor/src/server/middleware/csrf" "github.com/goharbor/harbor/src/server/middleware/log" @@ -27,19 +30,22 @@ import ( "github.com/goharbor/harbor/src/server/middleware/security" "github.com/goharbor/harbor/src/server/middleware/session" "github.com/goharbor/harbor/src/server/middleware/transaction" - "net/http" - "regexp" ) var ( match = regexp.MustCompile numericRegexp = match(`[0-9]+`) - blobURLRe = match("^/v2/(" + reference.NameRegexp.String() + ")/blobs/" + reference.DigestRegexp.String()) - - // fetchBlobAPISkipper skip transaction middleware for fetch blob API - // because transaction use the ResponseBuffer for the response which will degrade the performance for fetch blob - fetchBlobAPISkipper = middleware.MethodAndPathSkipper(http.MethodGet, blobURLRe) + // dbTxSkippers skip the transaction middleware for GET Blob, PATCH Blob Upload and PUT Blob Upload APIs + // because the APIs may take a long time to run, enable the transaction middleware in them will hold the database connections + // until the API finished, this behavior may eat all the database connections. + // There are no database writing operations in the GET Blob and PATCH Blob APIs, so skip the transaction middleware is all ok. + // For the PUT Blob Upload API, we will make a transaction manually to write blob info to the database when put blob upload successfully. + dbTxSkippers = []middleware.Skipper{ + middleware.MethodAndPathSkipper(http.MethodGet, distribution.BlobURLRegexp), + middleware.MethodAndPathSkipper(http.MethodPatch, distribution.BlobUploadURLRegexp), + middleware.MethodAndPathSkipper(http.MethodPut, distribution.BlobUploadURLRegexp), + } // readonlySkippers skip the post request when harbor sets to readonly. readonlySkippers = []middleware.Skipper{ @@ -65,11 +71,10 @@ func MiddleWares() []beego.MiddleWare { log.Middleware(), session.Middleware(), csrf.Middleware(), + orm.Middleware(), + notification.Middleware(), // notification must ahead of transaction ensure the DB transaction execution complete + transaction.Middleware(dbTxSkippers...), security.Middleware(), readonly.Middleware(readonlySkippers...), - orm.Middleware(), - // notification must ahead of transaction ensure the DB transaction execution complete - notification.Middleware(), - transaction.Middleware(fetchBlobAPISkipper), } } diff --git a/src/core/middlewares/middlewares_test.go b/src/core/middlewares/middlewares_test.go index 036cdae1e..355dcd1a0 100644 --- a/src/core/middlewares/middlewares_test.go +++ b/src/core/middlewares/middlewares_test.go @@ -20,28 +20,6 @@ import ( "testing" ) -func Test_fetchBlobAPISkipper(t *testing.T) { - type args struct { - r *http.Request - } - tests := []struct { - name string - args args - want bool - }{ - {"fetch blob", args{httptest.NewRequest(http.MethodGet, "/v2/library/photon/blobs/sha256:6e0447537050cf871f9ab6a3fec5715f9c6fff5212f6666993f1fc46b1f717a3", nil)}, true}, - {"delete blob", args{httptest.NewRequest(http.MethodDelete, "/v2/library/photon/blobs/sha256:6e0447537050cf871f9ab6a3fec5715f9c6fff5212f6666993f1fc46b1f717a3", nil)}, false}, - {"get manifest", args{httptest.NewRequest(http.MethodDelete, "/v2/library/photon/manifests/sha256:6e0447537050cf871f9ab6a3fec5715f9c6fff5212f6666993f1fc46b1f717a3", nil)}, false}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := fetchBlobAPISkipper(tt.args.r); got != tt.want { - t.Errorf("fetchBlobAPISkipper() = %v, want %v", got, tt.want) - } - }) - } -} - func Test_readonlySkipper(t *testing.T) { type args struct { r *http.Request diff --git a/src/pkg/distribution/distribution.go b/src/pkg/distribution/distribution.go index 17413b168..1817942fd 100644 --- a/src/pkg/distribution/distribution.go +++ b/src/pkg/distribution/distribution.go @@ -47,8 +47,12 @@ var ( var ( name = fmt.Sprintf("(?P%s)", ref.NameRegexp) reference = fmt.Sprintf("(?P((%s)|(%s)))", ref.DigestRegexp, ref.TagRegexp) + dgt = fmt.Sprintf("(?P%s)", ref.DigestRegexp) sessionID = "(?P[a-zA-Z0-9-_.=]+)" + // BlobURLRegexp regexp which match blob url + BlobURLRegexp = regexp.MustCompile(`^/v2/` + name + `/blobs/` + dgt) + // BlobUploadURLRegexp regexp which match blob upload url BlobUploadURLRegexp = regexp.MustCompile(`^/v2/` + name + `/blobs/uploads/` + sessionID) diff --git a/src/server/middleware/blob/put_blob_upload.go b/src/server/middleware/blob/put_blob_upload.go index 618645e38..4e630e3a2 100644 --- a/src/server/middleware/blob/put_blob_upload.go +++ b/src/server/middleware/blob/put_blob_upload.go @@ -15,11 +15,14 @@ package blob import ( - "github.com/goharbor/harbor/src/lib/log" - "github.com/goharbor/harbor/src/pkg/distribution" - "github.com/goharbor/harbor/src/server/middleware" + "context" "net/http" "strconv" + + "github.com/goharbor/harbor/src/lib/log" + "github.com/goharbor/harbor/src/lib/orm" + "github.com/goharbor/harbor/src/pkg/distribution" + "github.com/goharbor/harbor/src/server/middleware" ) // PutBlobUploadMiddleware middleware is to update the blob status according to the different situation before the request passed into proxy(distribution). @@ -42,36 +45,40 @@ func PutBlobUploadMiddleware() func(http.Handler) http.Handler { ctx := r.Context() - logger := log.G(ctx).WithFields(log.Fields{"middleware": "blob"}) + h := func(ctx context.Context) error { + logger := log.G(ctx).WithFields(log.Fields{"middleware": "blob"}) - size, err := strconv.ParseInt(r.Header.Get("Content-Length"), 10, 64) - if err != nil || size == 0 { - size, err = blobController.GetAcceptedBlobSize(distribution.ParseSessionID(r.URL.Path)) - } - if err != nil { - logger.Errorf("get blob size failed, error: %v", err) - return err + size, err := strconv.ParseInt(r.Header.Get("Content-Length"), 10, 64) + if err != nil || size == 0 { + size, err = blobController.GetAcceptedBlobSize(distribution.ParseSessionID(r.URL.Path)) + } + if err != nil { + logger.Errorf("get blob size failed, error: %v", err) + return err + } + + p, err := projectController.GetByName(ctx, distribution.ParseProjectName(r.URL.Path)) + if err != nil { + logger.Errorf("get project failed, error: %v", err) + return err + } + + digest := w.Header().Get("Docker-Content-Digest") + blobID, err := blobController.Ensure(ctx, digest, "application/octet-stream", size) + if err != nil { + logger.Errorf("ensure blob %s failed, error: %v", digest, err) + return err + } + + if err := blobController.AssociateWithProjectByID(ctx, blobID, p.ProjectID); err != nil { + logger.Errorf("associate blob %s with project %s failed, error: %v", digest, p.Name, err) + return err + } + + return nil } - p, err := projectController.GetByName(ctx, distribution.ParseProjectName(r.URL.Path)) - if err != nil { - logger.Errorf("get project failed, error: %v", err) - return err - } - - digest := w.Header().Get("Docker-Content-Digest") - blobID, err := blobController.Ensure(ctx, digest, "application/octet-stream", size) - if err != nil { - logger.Errorf("ensure blob %s failed, error: %v", digest, err) - return err - } - - if err := blobController.AssociateWithProjectByID(ctx, blobID, p.ProjectID); err != nil { - logger.Errorf("associate blob %s with project %s failed, error: %v", digest, p.Name, err) - return err - } - - return nil + return orm.WithTransaction(h)(ctx) }) return middleware.Chain(before, after)