Skip to content

Commit

Permalink
fix: add storage_limit check (add ValidateQuotaLimit as a general met…
Browse files Browse the repository at this point in the history
…hod to validate quota limit value)

Signed-off-by: Shengwen Yu <yshengwen@vmware.com>
  • Loading branch information
zyyw committed Aug 8, 2023
1 parent bd34ad5 commit 2722954
Show file tree
Hide file tree
Showing 6 changed files with 157 additions and 2 deletions.
5 changes: 3 additions & 2 deletions src/controller/quota/driver/project/project.go
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
}

Expand Down
48 changes: 48 additions & 0 deletions src/controller/quota/driver/project/project_test.go
Expand Up @@ -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() {

{
Expand Down
35 changes: 35 additions & 0 deletions 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
}
51 changes: 51 additions & 0 deletions 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)
}
}
}
}
16 changes: 16 additions & 0 deletions src/pkg/config/manager.go
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}

Check warning on line 201 in src/pkg/config/manager.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/config/manager.go#L195-L201

Added lines #L195 - L201 were not covered by tests

if err := lib.ValidateQuotaLimit(storagePerProject); err != nil {
return err
}

Check warning on line 205 in src/pkg/config/manager.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/config/manager.go#L203-L205

Added lines #L203 - L205 were not covered by tests
}

_, err := metadata.NewCfgValue(key, strVal)
if err != nil {
return errors.Wrap(err, "item name "+key)
Expand Down
4 changes: 4 additions & 0 deletions src/pkg/quota/types/resources.go
Expand Up @@ -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"
)
Expand Down

0 comments on commit 2722954

Please sign in to comment.