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 <yshengwen@vmware.com>

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 {
+		if err := lib.ValidateQuotaLimit(value); err != nil {
-			return fmt.Errorf("invalid value for resource %s", resource)
+			return err
 		}
 	}
 

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() {
 
 	{

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
+}

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)
+			}
+		}
+	}
+}

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)

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"
 )