From d03f0dcf2d6a420e66b486988c3dd00feaeb02d1 Mon Sep 17 00:00:00 2001 From: "stonezdj(Daojun Zhang)" Date: Mon, 20 Feb 2023 15:09:21 +0800 Subject: [PATCH] Skip to update pull time and pull count for scanner robot account (#17807) Add prefix for scanner robot account Fixes #14638 Signed-off-by: stonezdj # Conflicts: # api/v2.0/swagger.yaml # src/common/const.go # src/lib/config/metadata/metadatalist.go --- api/v2.0/swagger.yaml | 8 +++ make/photon/prepare/templates/core/env.jinja | 1 + make/photon/prepare/utils/core.py | 1 + src/common/const.go | 4 ++ .../event/handler/internal/artifact.go | 23 +++++++ .../event/handler/internal/artifact_test.go | 60 ++++++++++++++++++- src/controller/scan/base_controller.go | 3 +- src/controller/scan/base_controller_test.go | 2 +- src/lib/config/metadata/metadatalist.go | 4 +- src/lib/config/systemconfig.go | 8 +++ src/lib/config/userconfig.go | 6 ++ 11 files changed, 115 insertions(+), 5 deletions(-) diff --git a/api/v2.0/swagger.yaml b/api/v2.0/swagger.yaml index 8bd354ced..b30ebf582 100644 --- a/api/v2.0/swagger.yaml +++ b/api/v2.0/swagger.yaml @@ -8737,6 +8737,9 @@ definitions: skip_audit_log_database: $ref: '#/definitions/BoolConfigItem' description: Whether skip the audit log in database + scanner_skip_update_pulltime: + $ref: '#/definitions/BoolConfigItem' + description: Whether or not to skip update the pull time for scanner scan_all_policy: type: object properties: @@ -9016,6 +9019,11 @@ definitions: description: The session timeout for harbor, in minutes. x-omitempty: true x-isnullable: true + scanner_skip_update_pulltime: + type: boolean + description: Whether or not to skip update pull time for scanner + x-omitempty: true + x-isnullable: true StringConfigItem: type: object properties: diff --git a/make/photon/prepare/templates/core/env.jinja b/make/photon/prepare/templates/core/env.jinja index 49c46298c..28f38d43f 100644 --- a/make/photon/prepare/templates/core/env.jinja +++ b/make/photon/prepare/templates/core/env.jinja @@ -38,6 +38,7 @@ REGISTRY_CONTROLLER_URL={{registry_controller_url}} REGISTRY_CREDENTIAL_USERNAME={{registry_username}} REGISTRY_CREDENTIAL_PASSWORD={{registry_password}} CSRF_KEY={{csrf_key}} +ROBOT_SCANNER_NAME_PREFIX={{scan_robot_prefix}} PERMITTED_REGISTRY_TYPES_FOR_PROXY_CACHE=docker-hub,harbor,azure-acr,aws-ecr,google-gcr,quay,docker-registry,github-ghcr,jfrog-artifactory HTTP_PROXY={{core_http_proxy}} diff --git a/make/photon/prepare/utils/core.py b/make/photon/prepare/utils/core.py index 137c4b24f..1cc98b350 100644 --- a/make/photon/prepare/utils/core.py +++ b/make/photon/prepare/utils/core.py @@ -24,6 +24,7 @@ def prepare_core(config_dict, with_notary, with_trivy): with_notary=with_notary, with_trivy=with_trivy, csrf_key=generate_random_string(32), + scan_robot_prefix=generate_random_string(8), **config_dict) render_jinja( diff --git a/src/common/const.go b/src/common/const.go index cdb3d809b..25ad6efad 100755 --- a/src/common/const.go +++ b/src/common/const.go @@ -142,6 +142,8 @@ const ( RobotPrefix = "robot$" // System admin defined the robot name prefix. RobotNamePrefix = "robot_name_prefix" + // Scanner robot name prefix + RobotScannerNamePrefix = "robot_scanner_name_prefix" // Use this prefix to index user who tries to login with web hook token. AuthProxyUserNamePrefix = "tokenreview$" CoreConfigPath = "/api/v2.0/internalconfig" @@ -214,6 +216,8 @@ const ( SkipAuditLogDatabase = "skip_audit_log_database" // MaxAuditRetentionHour allowed in audit log purge MaxAuditRetentionHour = 240000 + // ScannerSkipUpdatePullTime + ScannerSkipUpdatePullTime = "scanner_skip_update_pulltime" // SessionTimeout defines the web session timeout SessionTimeout = "session_timeout" diff --git a/src/controller/event/handler/internal/artifact.go b/src/controller/event/handler/internal/artifact.go index cfc29354d..284934d5e 100644 --- a/src/controller/event/handler/internal/artifact.go +++ b/src/controller/event/handler/internal/artifact.go @@ -99,6 +99,9 @@ func (a *Handler) IsStateful() bool { } func (a *Handler) onPull(ctx context.Context, event *event.ArtifactEvent) error { + if config.ScannerSkipUpdatePullTime(ctx) && isScannerUser(ctx, event) { + return nil + } // if duration is equal to 0 or negative, keep original sync mode. if asyncFlushDuration <= 0 { var tagName string @@ -240,3 +243,23 @@ func (a *Handler) onPush(ctx context.Context, event *event.ArtifactEvent) error return nil } + +// isScannerUser check if the current user is a scanner user by its prefix +// usually a scanner user should be named like `robot$+--` +// verify it by the prefix `robot$+` +func isScannerUser(ctx context.Context, event *event.ArtifactEvent) bool { + if len(event.Operator) == 0 { + return false + } + robotPrefix := config.RobotPrefix(ctx) + scannerPrefix := config.ScannerRobotPrefix(ctx) + prefix := fmt.Sprintf("%s%s+%s", robotPrefix, parseProjectName(event.Repository), scannerPrefix) + return strings.HasPrefix(event.Operator, prefix) +} + +func parseProjectName(repoName string) string { + if strings.Contains(repoName, "/") { + return strings.Split(repoName, "/")[0] + } + return "" +} diff --git a/src/controller/event/handler/internal/artifact_test.go b/src/controller/event/handler/internal/artifact_test.go index 07887e22b..1a2a56af7 100644 --- a/src/controller/event/handler/internal/artifact_test.go +++ b/src/controller/event/handler/internal/artifact_test.go @@ -24,22 +24,28 @@ import ( common_dao "github.com/goharbor/harbor/src/common/dao" "github.com/goharbor/harbor/src/controller/event" + "github.com/goharbor/harbor/src/controller/scanner" "github.com/goharbor/harbor/src/lib/config" "github.com/goharbor/harbor/src/lib/orm" "github.com/goharbor/harbor/src/pkg" "github.com/goharbor/harbor/src/pkg/artifact" _ "github.com/goharbor/harbor/src/pkg/config/db" + "github.com/goharbor/harbor/src/pkg/project" "github.com/goharbor/harbor/src/pkg/repository/model" "github.com/goharbor/harbor/src/pkg/tag" tagmodel "github.com/goharbor/harbor/src/pkg/tag/model/tag" + scannerCtlMock "github.com/goharbor/harbor/src/testing/controller/scanner" + projectMock "github.com/goharbor/harbor/src/testing/pkg/project" ) // ArtifactHandlerTestSuite is test suite for artifact handler. type ArtifactHandlerTestSuite struct { suite.Suite - ctx context.Context - handler *Handler + ctx context.Context + handler *Handler + projectManager project.Manager + scannerCtl scanner.Controller } // TestArtifactHandler tests ArtifactHandler. @@ -53,6 +59,8 @@ func (suite *ArtifactHandlerTestSuite) SetupSuite() { config.Init() suite.handler = &Handler{} suite.ctx = orm.NewContext(context.TODO(), beegoorm.NewOrm()) + suite.projectManager = &projectMock.Manager{} + suite.scannerCtl = &scannerCtlMock.Controller{} // mock artifact _, err := pkg.ArtifactMgr.Create(suite.ctx, &artifact.Artifact{ID: 1, RepositoryID: 1}) @@ -143,3 +151,51 @@ func (suite *ArtifactHandlerTestSuite) TestOnPull() { return int64(2) == repository.PullCount }, 3*asyncFlushDuration, asyncFlushDuration/2, "wait for pull_count async update") } + +func (suite *ArtifactHandlerTestSuite) TestIsScannerUser() { + type args struct { + prefix string + event *event.ArtifactEvent + } + tests := []struct { + name string + args args + want bool + }{ + {"normal_true", args{"robot$", &event.ArtifactEvent{Operator: "robot$library+scanner+Trivy-2e6240a1-f3be-11ec-8fba-0242ac1e0009", Repository: "library/nginx"}}, true}, + {"no_scanner_prefix_false", args{"robot$", &event.ArtifactEvent{Operator: "robot$library+Trivy-2e6240a1-f3be-11ec-8fba-0242ac1e0009", Repository: "library/nginx"}}, false}, + {"operator_empty", args{"robot$", &event.ArtifactEvent{Operator: "", Repository: "library/nginx"}}, false}, + {"normal_user", args{"robot$", &event.ArtifactEvent{Operator: "Trivy_sample", Repository: "library/nginx"}}, false}, + {"normal_user_with_robotname", args{"robot$", &event.ArtifactEvent{Operator: "robot_Trivy", Repository: "library/nginx"}}, false}, + } + + for _, tt := range tests { + suite.Run(tt.name, func() { + if got := isScannerUser(suite.ctx, tt.args.event); got != tt.want { + suite.Errorf(nil, "isScannerUser() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_parseProjectName(t *testing.T) { + type args struct { + repoName string + } + tests := []struct { + name string + args args + want string + }{ + {"normal repo name", args{"library/nginx"}, "library"}, + {"three levels of repository", args{"library/nginx/nginx"}, "library"}, + {"repo name without project name", args{"nginx"}, ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := parseProjectName(tt.args.repoName); got != tt.want { + t.Errorf("parseProjectName() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/src/controller/scan/base_controller.go b/src/controller/scan/base_controller.go index dccea957f..81bee0178 100644 --- a/src/controller/scan/base_controller.go +++ b/src/controller/scan/base_controller.go @@ -847,10 +847,11 @@ func (bc *basicController) makeRobotAccount(ctx context.Context, projectID int64 } projectName := strings.Split(repository, "/")[0] + scannerPrefix := config.ScannerRobotPrefix(ctx) robotReq := &robot.Robot{ Robot: model.Robot{ - Name: fmt.Sprintf("%s-%s", registration.Name, UUID), + Name: fmt.Sprintf("%s-%s-%s", scannerPrefix, registration.Name, UUID), Description: "for scan", ProjectID: projectID, }, diff --git a/src/controller/scan/base_controller_test.go b/src/controller/scan/base_controller_test.go index 392d54638..84aab98d9 100644 --- a/src/controller/scan/base_controller_test.go +++ b/src/controller/scan/base_controller_test.go @@ -184,7 +184,7 @@ func (suite *ControllerTestSuite) SetupSuite() { rc := &robottesting.Controller{} - rname := fmt.Sprintf("%s-%s", suite.registration.Name, "the-uuid-123") + rname := fmt.Sprintf("%s-%s-%s", config.ScannerRobotPrefix(context.TODO()), suite.registration.Name, "the-uuid-123") conf := map[string]interface{}{ common.RobotTokenDuration: "30", diff --git a/src/lib/config/metadata/metadatalist.go b/src/lib/config/metadata/metadatalist.go index 7b2b45950..612c181de 100644 --- a/src/lib/config/metadata/metadatalist.go +++ b/src/lib/config/metadata/metadatalist.go @@ -148,7 +148,8 @@ var ( {Name: common.WithNotary, Scope: SystemScope, Group: BasicGroup, EnvKey: "WITH_NOTARY", DefaultValue: "false", ItemType: &BoolType{}, Editable: true}, // the unit of expiration is days {Name: common.RobotTokenDuration, Scope: UserScope, Group: BasicGroup, EnvKey: "ROBOT_TOKEN_DURATION", DefaultValue: "30", ItemType: &IntType{}, Editable: true, Description: `The robot account token duration in days`}, - {Name: common.RobotNamePrefix, Scope: UserScope, Group: BasicGroup, EnvKey: "ROBOT_NAME_PREFIX", DefaultValue: "robot$", ItemType: &StringType{}, Editable: true, Description: `The rebot account name prefix`}, + {Name: common.RobotNamePrefix, Scope: UserScope, Group: BasicGroup, EnvKey: "ROBOT_NAME_PREFIX", DefaultValue: "robot$", ItemType: &StringType{}, Editable: true, Description: `The robot account name prefix`}, + {Name: common.RobotScannerNamePrefix, Scope: SystemScope, Group: BasicGroup, EnvKey: "ROBOT_SCANNER_NAME_PREFIX", DefaultValue: "scanner", ItemType: &StringType{}, Editable: true, Description: `The scanner robot account name prefix`}, {Name: common.NotificationEnable, Scope: UserScope, Group: BasicGroup, EnvKey: "NOTIFICATION_ENABLE", DefaultValue: "true", ItemType: &BoolType{}, Editable: true, Description: `Enable notification`}, {Name: common.MetricEnable, Scope: SystemScope, Group: BasicGroup, EnvKey: "METRIC_ENABLE", DefaultValue: "false", ItemType: &BoolType{}, Editable: true}, @@ -185,6 +186,7 @@ var ( {Name: common.AuditLogForwardEndpoint, Scope: UserScope, Group: BasicGroup, EnvKey: "AUDIT_LOG_FORWARD_ENDPOINT", DefaultValue: "", ItemType: &StringType{}, Editable: false, Description: `The endpoint to forward the audit log.`}, {Name: common.SkipAuditLogDatabase, Scope: UserScope, Group: BasicGroup, EnvKey: "SKIP_LOG_AUDIT_DATABASE", DefaultValue: "false", ItemType: &BoolType{}, Editable: false, Description: `The option to skip audit log in database`}, + {Name: common.ScannerSkipUpdatePullTime, Scope: UserScope, Group: BasicGroup, EnvKey: "SCANNER_SKIP_UPDATE_PULL_TIME", DefaultValue: "false", ItemType: &BoolType{}, Editable: false, Description: `The option to skip update pull time for scanner`}, {Name: common.SessionTimeout, Scope: UserScope, Group: BasicGroup, EnvKey: "SESSION_TIMEOUT", DefaultValue: "60", ItemType: &Int64Type{}, Editable: true, Description: `The session timeout in minutes`}, } diff --git a/src/lib/config/systemconfig.go b/src/lib/config/systemconfig.go index bb122eeea..727280523 100644 --- a/src/lib/config/systemconfig.go +++ b/src/lib/config/systemconfig.go @@ -253,6 +253,14 @@ func CacheExpireHours() int { return hours } +// ScannerRobotPrefix returns the scanner of robot account prefix. +func ScannerRobotPrefix(ctx context.Context) string { + if DefaultMgr() != nil { + return DefaultMgr().Get(ctx, common.RobotScannerNamePrefix).GetString() + } + return os.Getenv("ROBOT_SCANNER_NAME_PREFIX") +} + // Database returns database settings func Database() (*models.Database, error) { database := &models.Database{} diff --git a/src/lib/config/userconfig.go b/src/lib/config/userconfig.go index b7a38c3d7..12c91d4f1 100644 --- a/src/lib/config/userconfig.go +++ b/src/lib/config/userconfig.go @@ -249,3 +249,9 @@ func AuditLogForwardEndpoint(ctx context.Context) string { func SkipAuditLogDatabase(ctx context.Context) bool { return DefaultMgr().Get(ctx, common.SkipAuditLogDatabase).GetBool() } + +// ScannerSkipUpdatePullTime returns the scanner skip update pull time setting +func ScannerSkipUpdatePullTime(ctx context.Context) bool { + log.Infof("skip_update_pull_time:%v", DefaultMgr().Get(ctx, common.ScannerSkipUpdatePullTime).GetBool()) + return DefaultMgr().Get(ctx, common.ScannerSkipUpdatePullTime).GetBool() +}