From 90de9092ced8ab938d90f7600f82d2bf9b66cc0e Mon Sep 17 00:00:00 2001 From: Shengwen YU Date: Wed, 9 Aug 2023 09:37:07 +0800 Subject: [PATCH] fix: add storage_limit check (#19095) fix: add storage_limit check (add ValidateQuotaLimit as a general method to validate quota limit value) Signed-off-by: Shengwen Yu --- .../quota/driver/project/project.go | 5 +- .../quota/driver/project/project_test.go | 48 +++++++++++++++++ src/lib/quota_storage_limit.go | 35 +++++++++++++ src/lib/quota_storage_limit_test.go | 51 +++++++++++++++++++ src/pkg/config/manager.go | 16 ++++++ src/pkg/quota/types/resources.go | 4 ++ 6 files changed, 157 insertions(+), 2 deletions(-) create mode 100644 src/lib/quota_storage_limit.go create mode 100644 src/lib/quota_storage_limit_test.go diff --git a/src/controller/quota/driver/project/project.go b/src/controller/quota/driver/project/project.go index 34f65d6de..637cdfda9 100644 --- a/src/controller/quota/driver/project/project.go +++ b/src/controller/quota/driver/project/project.go @@ -23,6 +23,7 @@ import ( "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/controller/blob" + "github.com/goharbor/harbor/src/lib" "github.com/goharbor/harbor/src/lib/config" "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/pkg/config/db" @@ -91,8 +92,8 @@ func (d *driver) Validate(hardLimits types.ResourceList) error { return fmt.Errorf("resource %s not support", resource) } - if value <= 0 && value != types.UNLIMITED { - return fmt.Errorf("invalid value for resource %s", resource) + if err := lib.ValidateQuotaLimit(value); err != nil { + return err } } diff --git a/src/controller/quota/driver/project/project_test.go b/src/controller/quota/driver/project/project_test.go index 70ad3dd05..f0c4ee13c 100644 --- a/src/controller/quota/driver/project/project_test.go +++ b/src/controller/quota/driver/project/project_test.go @@ -44,6 +44,54 @@ func (suite *DriverTestSuite) SetupTest() { } } +func (suite *DriverTestSuite) TestValidate() { + testCases := []struct { + description string + input types.ResourceList + hasErr bool + }{ + { + description: "quota limit is 0", + input: map[types.ResourceName]int64{types.ResourceStorage: 0}, + hasErr: true, + }, + { + description: "quota limit is -1", + input: map[types.ResourceName]int64{types.ResourceStorage: -1}, + hasErr: false, + }, + { + description: "quota limit is -2", + input: map[types.ResourceName]int64{types.ResourceStorage: -2}, + hasErr: true, + }, + { + description: "quota limit is types.MaxLimitedValue", + input: map[types.ResourceName]int64{types.ResourceStorage: int64(types.MaxLimitedValue)}, + hasErr: false, + }, + { + description: "quota limit is types.MaxLimitedValue + 1", + input: map[types.ResourceName]int64{types.ResourceStorage: int64(types.MaxLimitedValue + 1)}, + hasErr: true, + }, + { + description: "quota limit is 12345", + input: map[types.ResourceName]int64{types.ResourceStorage: int64(12345)}, + hasErr: false, + }, + } + + for _, tc := range testCases { + gotErr := suite.d.Validate(tc.input) + if tc.hasErr { + suite.Errorf(gotErr, "test case: %s", tc.description) + } else { + suite.NoErrorf(gotErr, "test case: %s", tc.description) + } + } +} + func (suite *DriverTestSuite) TestCalculateUsage() { { diff --git a/src/lib/quota_storage_limit.go b/src/lib/quota_storage_limit.go new file mode 100644 index 000000000..0aa686604 --- /dev/null +++ b/src/lib/quota_storage_limit.go @@ -0,0 +1,35 @@ +// Copyright Project Harbor Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package lib + +import ( + "fmt" + + "github.com/goharbor/harbor/src/pkg/quota/types" +) + +func ValidateQuotaLimit(storageLimit int64) error { + if storageLimit <= 0 { + if storageLimit != types.UNLIMITED { + return fmt.Errorf("invalid non-positive value for quota limit, value=%v", storageLimit) + } + } else { + // storageLimit > 0, there is a max capacity of limited storage + if uint64(storageLimit) > types.MaxLimitedValue { + return fmt.Errorf("exceeded 1024TB, which is 1125899906842624 Bytes, value=%v", storageLimit) + } + } + return nil +} diff --git a/src/lib/quota_storage_limit_test.go b/src/lib/quota_storage_limit_test.go new file mode 100644 index 000000000..b242dde33 --- /dev/null +++ b/src/lib/quota_storage_limit_test.go @@ -0,0 +1,51 @@ +package lib + +import "testing" + +func TestValidateQuotaLimit(t *testing.T) { + testCases := []struct { + description string + storageLimit int64 + hasError bool + }{ + { + description: "storage limit is -2", + storageLimit: -2, + hasError: true, + }, + { + description: "storage limit is -1", + storageLimit: -1, + hasError: false, + }, + { + description: "storage limit is 0", + storageLimit: 0, + hasError: true, + }, + { + description: "storage limit is 1125899906842624", + storageLimit: 1125899906842624, + hasError: false, + }, + { + description: "storage limit is 1125899906842625", + storageLimit: 1125899906842625, + hasError: true, + }, + } + + for _, tc := range testCases { + gotErr := ValidateQuotaLimit(tc.storageLimit) + if tc.hasError { + if gotErr == nil { + t.Errorf("test case: %s, it expects error, while got error is nil", tc.description) + } + } else { + // tc.hasError == false + if gotErr != nil { + t.Errorf("test case: %s, it doesn't expect error, while got error is not nil, gotErr=%v", tc.description, gotErr) + } + } + } +} diff --git a/src/pkg/config/manager.go b/src/pkg/config/manager.go index 0ce905cdb..dd623a899 100644 --- a/src/pkg/config/manager.go +++ b/src/pkg/config/manager.go @@ -18,10 +18,12 @@ import ( "context" "fmt" "os" + "strconv" "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/utils" + "github.com/goharbor/harbor/src/lib" "github.com/goharbor/harbor/src/lib/config/metadata" "github.com/goharbor/harbor/src/lib/errors" "github.com/goharbor/harbor/src/lib/log" @@ -188,7 +190,21 @@ func (c *CfgManager) ValidateCfg(ctx context.Context, cfgs map[string]interface{ if item.Scope == metadata.SystemScope { return fmt.Errorf("system config items cannot be updated, item: %v", key) } + strVal := utils.GetStrValueOfAnyType(value) + + // check storage per project before setting it + if key == common.StoragePerProject { + storagePerProject, err := strconv.ParseInt(strVal, 10, 64) + if err != nil { + return fmt.Errorf("cannot parse string value(%v) to int64", strVal) + } + + if err := lib.ValidateQuotaLimit(storagePerProject); err != nil { + return err + } + } + _, err := metadata.NewCfgValue(key, strVal) if err != nil { return errors.Wrap(err, "item name "+key) diff --git a/src/pkg/quota/types/resources.go b/src/pkg/quota/types/resources.go index 306bf9059..dc0ef51be 100644 --- a/src/pkg/quota/types/resources.go +++ b/src/pkg/quota/types/resources.go @@ -25,6 +25,10 @@ const ( // UNLIMITED unlimited resource value UNLIMITED = -1 + // MaxLimitedValue the max capacity of limited storage, in Bytes + // 1125899906842624 Bytes = 1024 TB + MaxLimitedValue = uint64(1125899906842624) + // ResourceStorage storage size, in bytes ResourceStorage ResourceName = "storage" )