Skip to content

Commit d283a31

Browse files
KN4CK3RKN4CK3R
authored
Check quota limits for container uploads (#22450)
The test coverage has revealed that container packages were not checked against the quota limits.
1 parent 2052a9e commit d283a31

File tree

5 files changed

+106
-31
lines changed

5 files changed

+106
-31
lines changed

‎routers/api/packages/container/blob.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,18 @@ var uploadVersionMutex sync.Mutex
2626

2727
// saveAsPackageBlob creates a package blob from an upload
2828
// The uploaded blob gets stored in a special upload version to link them to the package/image
29-
func saveAsPackageBlob(hsr packages_module.HashedSizeReader, pi *packages_service.PackageInfo) (*packages_model.PackageBlob, error) {
29+
func saveAsPackageBlob(hsr packages_module.HashedSizeReader, pci *packages_service.PackageCreationInfo) (*packages_model.PackageBlob, error) {
30+
if err := packages_service.CheckSizeQuotaExceeded(db.DefaultContext, pci.Creator, pci.Owner, packages_model.TypeContainer, hsr.Size()); err != nil {
31+
return nil, err
32+
}
33+
3034
pb := packages_service.NewPackageBlob(hsr)
3135

3236
exists := false
3337

3438
contentStore := packages_module.NewContentStore()
3539

36-
uploadVersion, err := getOrCreateUploadVersion(pi)
40+
uploadVersion, err := getOrCreateUploadVersion(&pci.PackageInfo)
3741
if err != nil {
3842
return nil, err
3943
}

‎routers/api/packages/container/container.go

+38-5
Original file line numberDiff line numberDiff line change
@@ -227,8 +227,22 @@ func InitiateUploadBlob(ctx *context.Context) {
227227
return
228228
}
229229

230-
if _, err := saveAsPackageBlob(buf, &packages_service.PackageInfo{Owner: ctx.Package.Owner, Name: image}); err != nil {
231-
apiError(ctx, http.StatusInternalServerError, err)
230+
if _, err := saveAsPackageBlob(
231+
buf,
232+
&packages_service.PackageCreationInfo{
233+
PackageInfo: packages_service.PackageInfo{
234+
Owner: ctx.Package.Owner,
235+
Name: image,
236+
},
237+
Creator: ctx.Doer,
238+
},
239+
); err != nil {
240+
switch err {
241+
case packages_service.ErrQuotaTotalCount, packages_service.ErrQuotaTypeSize, packages_service.ErrQuotaTotalSize:
242+
apiError(ctx, http.StatusForbidden, err)
243+
default:
244+
apiError(ctx, http.StatusInternalServerError, err)
245+
}
232246
return
233247
}
234248

@@ -358,8 +372,22 @@ func EndUploadBlob(ctx *context.Context) {
358372
return
359373
}
360374

361-
if _, err := saveAsPackageBlob(uploader, &packages_service.PackageInfo{Owner: ctx.Package.Owner, Name: image}); err != nil {
362-
apiError(ctx, http.StatusInternalServerError, err)
375+
if _, err := saveAsPackageBlob(
376+
uploader,
377+
&packages_service.PackageCreationInfo{
378+
PackageInfo: packages_service.PackageInfo{
379+
Owner: ctx.Package.Owner,
380+
Name: image,
381+
},
382+
Creator: ctx.Doer,
383+
},
384+
); err != nil {
385+
switch err {
386+
case packages_service.ErrQuotaTotalCount, packages_service.ErrQuotaTypeSize, packages_service.ErrQuotaTotalSize:
387+
apiError(ctx, http.StatusForbidden, err)
388+
default:
389+
apiError(ctx, http.StatusInternalServerError, err)
390+
}
363391
return
364392
}
365393

@@ -526,7 +554,12 @@ func UploadManifest(ctx *context.Context) {
526554
} else if errors.Is(err, container_model.ErrContainerBlobNotExist) {
527555
apiErrorDefined(ctx, errBlobUnknown)
528556
} else {
529-
apiError(ctx, http.StatusInternalServerError, err)
557+
switch err {
558+
case packages_service.ErrQuotaTotalCount, packages_service.ErrQuotaTypeSize, packages_service.ErrQuotaTotalSize:
559+
apiError(ctx, http.StatusForbidden, err)
560+
default:
561+
apiError(ctx, http.StatusInternalServerError, err)
562+
}
530563
}
531564
return
532565
}

‎routers/api/packages/container/manifest.go

+4
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,10 @@ func createPackageAndVersion(ctx context.Context, mci *manifestCreationInfo, met
327327
}
328328
}
329329

330+
if err := packages_service.CheckCountQuotaExceeded(ctx, mci.Creator, mci.Owner); err != nil {
331+
return nil, err
332+
}
333+
330334
if mci.IsTagged {
331335
if _, err := packages_model.InsertProperty(ctx, packages_model.PropertyTypeVersion, pv.ID, container_module.PropertyManifestTagged, ""); err != nil {
332336
log.Error("Error setting package version property: %v", err)

‎services/packages/packages.go

+8-4
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ func createPackageAndVersion(ctx context.Context, pvci *PackageCreationInfo, all
173173
}
174174

175175
if versionCreated {
176-
if err := checkCountQuotaExceeded(ctx, pvci.Creator, pvci.Owner); err != nil {
176+
if err := CheckCountQuotaExceeded(ctx, pvci.Creator, pvci.Owner); err != nil {
177177
return nil, false, err
178178
}
179179

@@ -240,7 +240,7 @@ func NewPackageBlob(hsr packages_module.HashedSizeReader) *packages_model.Packag
240240
func addFileToPackageVersion(ctx context.Context, pv *packages_model.PackageVersion, pvi *PackageInfo, pfci *PackageFileCreationInfo) (*packages_model.PackageFile, *packages_model.PackageBlob, bool, error) {
241241
log.Trace("Adding package file: %v, %s", pv.ID, pfci.Filename)
242242

243-
if err := checkSizeQuotaExceeded(ctx, pfci.Creator, pvi.Owner, pvi.PackageType, pfci.Data.Size()); err != nil {
243+
if err := CheckSizeQuotaExceeded(ctx, pfci.Creator, pvi.Owner, pvi.PackageType, pfci.Data.Size()); err != nil {
244244
return nil, nil, false, err
245245
}
246246

@@ -302,7 +302,9 @@ func addFileToPackageVersion(ctx context.Context, pv *packages_model.PackageVers
302302
return pf, pb, !exists, nil
303303
}
304304

305-
func checkCountQuotaExceeded(ctx context.Context, doer, owner *user_model.User) error {
305+
// CheckCountQuotaExceeded checks if the owner has more than the allowed packages
306+
// The check is skipped if the doer is an admin.
307+
func CheckCountQuotaExceeded(ctx context.Context, doer, owner *user_model.User) error {
306308
if doer.IsAdmin {
307309
return nil
308310
}
@@ -324,7 +326,9 @@ func checkCountQuotaExceeded(ctx context.Context, doer, owner *user_model.User)
324326
return nil
325327
}
326328

327-
func checkSizeQuotaExceeded(ctx context.Context, doer, owner *user_model.User, packageType packages_model.Type, uploadSize int64) error {
329+
// CheckSizeQuotaExceeded checks if the upload size is bigger than the allowed size
330+
// The check is skipped if the doer is an admin.
331+
func CheckSizeQuotaExceeded(ctx context.Context, doer, owner *user_model.User, packageType packages_model.Type, uploadSize int64) error {
328332
if doer.IsAdmin {
329333
return nil
330334
}

‎tests/integration/api_packages_test.go

+50-20
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@ package integration
55

66
import (
77
"bytes"
8+
"crypto/sha256"
89
"fmt"
910
"net/http"
11+
"strings"
1012
"testing"
1113
"time"
1214

@@ -171,34 +173,62 @@ func TestPackageAccess(t *testing.T) {
171173
func TestPackageQuota(t *testing.T) {
172174
defer tests.PrepareTestEnv(t)()
173175

174-
limitTotalOwnerCount, limitTotalOwnerSize, limitSizeGeneric := setting.Packages.LimitTotalOwnerCount, setting.Packages.LimitTotalOwnerSize, setting.Packages.LimitSizeGeneric
176+
limitTotalOwnerCount, limitTotalOwnerSize := setting.Packages.LimitTotalOwnerCount, setting.Packages.LimitTotalOwnerSize
175177

178+
// Exceeded quota result in StatusForbidden for normal users but admins are always allowed to upload.
176179
admin := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
177180
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 10})
178181

179-
uploadPackage := func(doer *user_model.User, version string, expectedStatus int) {
180-
url := fmt.Sprintf("/api/packages/%s/generic/test-package/%s/file.bin", user.Name, version)
181-
req := NewRequestWithBody(t, "PUT", url, bytes.NewReader([]byte{1}))
182-
AddBasicAuthHeader(req, doer.Name)
183-
MakeRequest(t, req, expectedStatus)
184-
}
182+
t.Run("Common", func(t *testing.T) {
183+
defer tests.PrintCurrentTest(t)()
185184

186-
// Exceeded quota result in StatusForbidden for normal users but admins are always allowed to upload.
185+
limitSizeGeneric := setting.Packages.LimitSizeGeneric
186+
187+
uploadPackage := func(doer *user_model.User, version string, expectedStatus int) {
188+
url := fmt.Sprintf("/api/packages/%s/generic/test-package/%s/file.bin", user.Name, version)
189+
req := NewRequestWithBody(t, "PUT", url, bytes.NewReader([]byte{1}))
190+
AddBasicAuthHeader(req, doer.Name)
191+
MakeRequest(t, req, expectedStatus)
192+
}
187193

188-
setting.Packages.LimitTotalOwnerCount = 0
189-
uploadPackage(user, "1.0", http.StatusForbidden)
190-
uploadPackage(admin, "1.0", http.StatusCreated)
191-
setting.Packages.LimitTotalOwnerCount = limitTotalOwnerCount
194+
setting.Packages.LimitTotalOwnerCount = 0
195+
uploadPackage(user, "1.0", http.StatusForbidden)
196+
uploadPackage(admin, "1.0", http.StatusCreated)
197+
setting.Packages.LimitTotalOwnerCount = limitTotalOwnerCount
192198

193-
setting.Packages.LimitTotalOwnerSize = 0
194-
uploadPackage(user, "1.1", http.StatusForbidden)
195-
uploadPackage(admin, "1.1", http.StatusCreated)
196-
setting.Packages.LimitTotalOwnerSize = limitTotalOwnerSize
199+
setting.Packages.LimitTotalOwnerSize = 0
200+
uploadPackage(user, "1.1", http.StatusForbidden)
201+
uploadPackage(admin, "1.1", http.StatusCreated)
202+
setting.Packages.LimitTotalOwnerSize = limitTotalOwnerSize
197203

198-
setting.Packages.LimitSizeGeneric = 0
199-
uploadPackage(user, "1.2", http.StatusForbidden)
200-
uploadPackage(admin, "1.2", http.StatusCreated)
201-
setting.Packages.LimitSizeGeneric = limitSizeGeneric
204+
setting.Packages.LimitSizeGeneric = 0
205+
uploadPackage(user, "1.2", http.StatusForbidden)
206+
uploadPackage(admin, "1.2", http.StatusCreated)
207+
setting.Packages.LimitSizeGeneric = limitSizeGeneric
208+
})
209+
210+
t.Run("Container", func(t *testing.T) {
211+
defer tests.PrintCurrentTest(t)()
212+
213+
limitSizeContainer := setting.Packages.LimitSizeContainer
214+
215+
uploadBlob := func(doer *user_model.User, data string, expectedStatus int) {
216+
url := fmt.Sprintf("/v2/%s/quota-test/blobs/uploads?digest=sha256:%x", user.Name, sha256.Sum256([]byte(data)))
217+
req := NewRequestWithBody(t, "POST", url, strings.NewReader(data))
218+
AddBasicAuthHeader(req, doer.Name)
219+
MakeRequest(t, req, expectedStatus)
220+
}
221+
222+
setting.Packages.LimitTotalOwnerSize = 0
223+
uploadBlob(user, "2", http.StatusForbidden)
224+
uploadBlob(admin, "2", http.StatusCreated)
225+
setting.Packages.LimitTotalOwnerSize = limitTotalOwnerSize
226+
227+
setting.Packages.LimitSizeContainer = 0
228+
uploadBlob(user, "3", http.StatusForbidden)
229+
uploadBlob(admin, "3", http.StatusCreated)
230+
setting.Packages.LimitSizeContainer = limitSizeContainer
231+
})
202232
}
203233

204234
func TestPackageCleanup(t *testing.T) {

0 commit comments

Comments
 (0)