Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement bandwidth limit for proxy-cache #20812

Merged
merged 1 commit into from
Aug 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions api/v2.0/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7340,6 +7340,10 @@ definitions:
type: string
description: 'The ID of the tag retention policy for the project'
x-nullable: true
proxy_speed_kb:
type: string
wy65701436 marked this conversation as resolved.
Show resolved Hide resolved
description: 'The bandwidth limit of proxy cache, in Kbps (kilobits per second). It limits the communication between Harbor and the upstream registry, not the client and the Harbor.'
x-nullable: true
ProjectSummary:
type: object
properties:
Expand Down
2 changes: 1 addition & 1 deletion src/controller/proxy/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@
func (c *controller) ProxyBlob(ctx context.Context, p *proModels.Project, art lib.ArtifactInfo) (int64, io.ReadCloser, error) {
remoteRepo := getRemoteRepo(art)
log.Debugf("The blob doesn't exist, proxy the request to the target server, url:%v", remoteRepo)
rHelper, err := NewRemoteHelper(ctx, p.RegistryID)
rHelper, err := NewRemoteHelper(ctx, p.RegistryID, WithSpeed(p.ProxyCacheSpeed()))

Check warning on line 267 in src/controller/proxy/controller.go

View check run for this annotation

Codecov / codecov/patch

src/controller/proxy/controller.go#L267

Added line #L267 was not covered by tests
if err != nil {
return 0, nil, err
}
Expand Down
37 changes: 37 additions & 0 deletions src/controller/proxy/options.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// 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 proxy

type Option func(*Options)

type Options struct {
// Speed is the data transfer speed for proxy cache from Harbor to upstream registry, no limit by default.
Speed int32
}

func NewOptions(opts ...Option) *Options {
o := &Options{}
for _, opt := range opts {
opt(o)
}

return o
}

func WithSpeed(speed int32) Option {
return func(o *Options) {
o.Speed = speed
}
}
33 changes: 33 additions & 0 deletions src/controller/proxy/options_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// 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 proxy

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestNewOptions(t *testing.T) {
// test default options
o := NewOptions()
assert.Equal(t, int32(0), o.Speed)

// test with options
// with speed
withSpeed := WithSpeed(1024)
o = NewOptions(withSpeed)
assert.Equal(t, int32(1024), o.Speed)
}
17 changes: 14 additions & 3 deletions src/controller/proxy/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

"github.com/docker/distribution"

"github.com/goharbor/harbor/src/lib"
"github.com/goharbor/harbor/src/pkg/reg"
"github.com/goharbor/harbor/src/pkg/reg/adapter"
"github.com/goharbor/harbor/src/pkg/reg/model"
Expand All @@ -43,13 +44,16 @@
regID int64
registry adapter.ArtifactRegistry
registryMgr reg.Manager
opts *Options
}

// NewRemoteHelper create a remote interface
func NewRemoteHelper(ctx context.Context, regID int64) (RemoteInterface, error) {
func NewRemoteHelper(ctx context.Context, regID int64, opts ...Option) (RemoteInterface, error) {

Check warning on line 51 in src/controller/proxy/remote.go

View check run for this annotation

Codecov / codecov/patch

src/controller/proxy/remote.go#L51

Added line #L51 was not covered by tests
r := &remoteHelper{
regID: regID,
registryMgr: reg.Mgr}
registryMgr: reg.Mgr,
opts: NewOptions(opts...),
}

Check warning on line 56 in src/controller/proxy/remote.go

View check run for this annotation

Codecov / codecov/patch

src/controller/proxy/remote.go#L54-L56

Added lines #L54 - L56 were not covered by tests
if err := r.init(ctx); err != nil {
return nil, err
}
Expand Down Expand Up @@ -83,7 +87,14 @@
}

func (r *remoteHelper) BlobReader(repo, dig string) (int64, io.ReadCloser, error) {
return r.registry.PullBlob(repo, dig)
sz, bReader, err := r.registry.PullBlob(repo, dig)
if err != nil {
return 0, nil, err
}
if r.opts != nil && r.opts.Speed > 0 {
bReader = lib.NewReader(bReader, r.opts.Speed)
}
return sz, bReader, err

Check warning on line 97 in src/controller/proxy/remote.go

View check run for this annotation

Codecov / codecov/patch

src/controller/proxy/remote.go#L90-L97

Added lines #L90 - L97 were not covered by tests
}

func (r *remoteHelper) Manifest(repo string, ref string) (distribution.Manifest, string, error) {
Expand Down
5 changes: 3 additions & 2 deletions src/controller/replication/transfer/image/transfer.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

common_http "github.com/goharbor/harbor/src/common/http"
trans "github.com/goharbor/harbor/src/controller/replication/transfer"
"github.com/goharbor/harbor/src/lib"
"github.com/goharbor/harbor/src/lib/log"
"github.com/goharbor/harbor/src/pkg/reg/adapter"
"github.com/goharbor/harbor/src/pkg/reg/model"
Expand Down Expand Up @@ -380,7 +381,7 @@
return err
}
if speed > 0 {
data = trans.NewReader(data, speed)
data = lib.NewReader(data, speed)

Check warning on line 384 in src/controller/replication/transfer/image/transfer.go

View check run for this annotation

Codecov / codecov/patch

src/controller/replication/transfer/image/transfer.go#L384

Added line #L384 was not covered by tests
}
defer data.Close()
// get size 0 from PullBlob, use size from distribution.Descriptor instead.
Expand Down Expand Up @@ -435,7 +436,7 @@
}

if speed > 0 {
data = trans.NewReader(data, speed)
data = lib.NewReader(data, speed)

Check warning on line 439 in src/controller/replication/transfer/image/transfer.go

View check run for this annotation

Codecov / codecov/patch

src/controller/replication/transfer/image/transfer.go#L439

Added line #L439 was not covered by tests
}
// failureEnd will only be used for adjusting content range when issue happened during push the chunk.
var failureEnd int64
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package transfer
package lib

import (
"fmt"
Expand Down
1 change: 1 addition & 0 deletions src/pkg/project/models/pro_meta.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,5 @@ const (
ProMetaAutoScan = "auto_scan"
ProMetaReuseSysCVEAllowlist = "reuse_sys_cve_allowlist"
ProMetaAutoSBOMGen = "auto_sbom_generation"
ProMetaProxySpeed = "proxy_speed_kb"
)
13 changes: 13 additions & 0 deletions src/pkg/project/models/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,19 @@
return isTrue(auto)
}

// ProxyCacheSpeed ...
func (p *Project) ProxyCacheSpeed() int32 {
speed, exist := p.GetMetadata(ProMetaProxySpeed)
if !exist {
return 0
}
speedInt, err := strconv.ParseInt(speed, 10, 32)
if err != nil {
return 0
}
return int32(speedInt)

Check warning on line 169 in src/pkg/project/models/project.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/project/models/project.go#L160-L169

Added lines #L160 - L169 were not covered by tests
Fixed Show fixed Hide fixed
}

// FilterByPublic returns orm.QuerySeter with public filter
func (p *Project) FilterByPublic(_ context.Context, qs orm.QuerySeter, _ string, value interface{}) orm.QuerySeter {
subQuery := `SELECT project_id FROM project_metadata WHERE name = 'public' AND value = '%s'`
Expand Down
10 changes: 5 additions & 5 deletions src/server/middleware/repoproxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@

func handleBlob(w http.ResponseWriter, r *http.Request, next http.Handler) error {
ctx := r.Context()
art, p, proxyCtl, err := preCheck(ctx)
art, p, proxyCtl, err := preCheck(ctx, true)

Check warning on line 63 in src/server/middleware/repoproxy/proxy.go

View check run for this annotation

Codecov / codecov/patch

src/server/middleware/repoproxy/proxy.go#L63

Added line #L63 was not covered by tests
if err != nil {
return err
}
Expand Down Expand Up @@ -96,14 +96,14 @@
return nil
}

func preCheck(ctx context.Context) (art lib.ArtifactInfo, p *proModels.Project, ctl proxy.Controller, err error) {
func preCheck(ctx context.Context, withProjectMetadata bool) (art lib.ArtifactInfo, p *proModels.Project, ctl proxy.Controller, err error) {

Check warning on line 99 in src/server/middleware/repoproxy/proxy.go

View check run for this annotation

Codecov / codecov/patch

src/server/middleware/repoproxy/proxy.go#L99

Added line #L99 was not covered by tests
none := lib.ArtifactInfo{}
art = lib.GetArtifactInfo(ctx)
if art == none {
return none, nil, nil, errors.New("artifactinfo is not found").WithCode(errors.NotFoundCode)
}
ctl = proxy.ControllerInstance()
p, err = project.Ctl.GetByName(ctx, art.ProjectName, project.Metadata(false))
p, err = project.Ctl.GetByName(ctx, art.ProjectName, project.Metadata(withProjectMetadata))

Check warning on line 106 in src/server/middleware/repoproxy/proxy.go

View check run for this annotation

Codecov / codecov/patch

src/server/middleware/repoproxy/proxy.go#L106

Added line #L106 was not covered by tests
return
}

Expand Down Expand Up @@ -155,7 +155,7 @@

func handleManifest(w http.ResponseWriter, r *http.Request, next http.Handler) error {
ctx := r.Context()
art, p, proxyCtl, err := preCheck(ctx)
art, p, proxyCtl, err := preCheck(ctx, true)

Check warning on line 158 in src/server/middleware/repoproxy/proxy.go

View check run for this annotation

Codecov / codecov/patch

src/server/middleware/repoproxy/proxy.go#L158

Added line #L158 was not covered by tests
if err != nil {
return err
}
Expand All @@ -174,7 +174,7 @@
next.ServeHTTP(w, r)
return nil
}
remote, err := proxy.NewRemoteHelper(r.Context(), p.RegistryID)
remote, err := proxy.NewRemoteHelper(r.Context(), p.RegistryID, proxy.WithSpeed(p.ProxyCacheSpeed()))

Check warning on line 177 in src/server/middleware/repoproxy/proxy.go

View check run for this annotation

Codecov / codecov/patch

src/server/middleware/repoproxy/proxy.go#L177

Added line #L177 was not covered by tests
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions src/server/middleware/repoproxy/tag.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
return middleware.New(func(w http.ResponseWriter, r *http.Request, next http.Handler) {
ctx := r.Context()

art, p, _, err := preCheck(ctx)
art, p, _, err := preCheck(ctx, false)

Check warning on line 38 in src/server/middleware/repoproxy/tag.go

View check run for this annotation

Codecov / codecov/patch

src/server/middleware/repoproxy/tag.go#L38

Added line #L38 was not covered by tests
if err != nil {
libhttp.SendError(w, err)
return
Expand Down Expand Up @@ -69,7 +69,7 @@
util.SendListTagsResponse(w, r, tags)
}()

remote, err := proxy.NewRemoteHelper(ctx, p.RegistryID)
remote, err := proxy.NewRemoteHelper(ctx, p.RegistryID, proxy.WithSpeed(p.ProxyCacheSpeed()))

Check warning on line 72 in src/server/middleware/repoproxy/tag.go

View check run for this annotation

Codecov / codecov/patch

src/server/middleware/repoproxy/tag.go#L72

Added line #L72 was not covered by tests
if err != nil {
logger.Warningf("failed to get remote interface, error: %v, fallback to local tags", err)
return
Expand Down
17 changes: 17 additions & 0 deletions src/server/v2.0/handler/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,11 @@
}
}

// ignore metadata.proxy_speed_kb for non-proxy-cache project
if req.RegistryID == nil {
req.Metadata.ProxySpeedKb = nil
}

Check warning on line 165 in src/server/v2.0/handler/project.go

View check run for this annotation

Codecov / codecov/patch

src/server/v2.0/handler/project.go#L163-L165

Added lines #L163 - L165 were not covered by tests

// ignore enable_content_trust metadata for proxy cache project
// see https://github.com/goharbor/harbor/issues/12940 to get more info
if req.RegistryID != nil {
Expand Down Expand Up @@ -551,6 +556,11 @@
}
}

// ignore metadata.proxy_speed_kb for non-proxy-cache project
if params.Project.Metadata != nil && !p.IsProxy() {
params.Project.Metadata.ProxySpeedKb = nil
}

Check warning on line 562 in src/server/v2.0/handler/project.go

View check run for this annotation

Codecov / codecov/patch

src/server/v2.0/handler/project.go#L560-L562

Added lines #L560 - L562 were not covered by tests

yanji09 marked this conversation as resolved.
Show resolved Hide resolved
// ignore enable_content_trust metadata for proxy cache project
// see https://github.com/goharbor/harbor/issues/12940 to get more info
if params.Project.Metadata != nil && p.IsProxy() {
Expand Down Expand Up @@ -792,6 +802,13 @@
if !permitted {
return errors.BadRequestError(fmt.Errorf("unsupported registry type %s", string(registry.Type)))
}

// validate metadata.proxy_speed_kb. It should be an int32
if ps := req.Metadata.ProxySpeedKb; ps != nil {
if _, err := strconv.ParseInt(*ps, 10, 32); err != nil {
return errors.BadRequestError(nil).WithMessage(fmt.Sprintf("metadata.proxy_speed_kb should by an int32, but got: '%s', err: %s", *ps, err))
}

Check warning on line 810 in src/server/v2.0/handler/project.go

View check run for this annotation

Codecov / codecov/patch

src/server/v2.0/handler/project.go#L807-L810

Added lines #L807 - L810 were not covered by tests
}
}

if req.StorageLimit != nil {
Expand Down
6 changes: 6 additions & 0 deletions src/server/v2.0/handler/project_metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,12 @@
return nil, errors.New(nil).WithCode(errors.BadRequestCode).WithMessage("invalid value: %s", value)
}
metas[proModels.ProMetaSeverity] = strings.ToLower(severity.String())
case proModels.ProMetaProxySpeed:
v, err := strconv.ParseInt(value, 10, 32)
zyyw marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, errors.New(nil).WithCode(errors.BadRequestCode).WithMessage("invalid value: %s", value)
}
metas[proModels.ProMetaProxySpeed] = strconv.FormatInt(v, 10)

Check warning on line 163 in src/server/v2.0/handler/project_metadata.go

View check run for this annotation

Codecov / codecov/patch

src/server/v2.0/handler/project_metadata.go#L158-L163

Added lines #L158 - L163 were not covered by tests
default:
return nil, errors.New(nil).WithCode(errors.BadRequestCode).WithMessage("invalid key: %s", key)
}
Expand Down
Loading