Skip to content

Commit

Permalink
Update purge audit to purge both audit_log_ext and audit_log (#21608)
Browse files Browse the repository at this point in the history
Fix integration issue with UI

Signed-off-by: stonezdj <[email protected]>
  • Loading branch information
stonezdj authored Feb 18, 2025
1 parent add0b60 commit 4565907
Show file tree
Hide file tree
Showing 16 changed files with 85 additions and 50 deletions.
12 changes: 10 additions & 2 deletions api/v2.0/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4486,7 +4486,7 @@ paths:
description: |
The purge job's schedule, it is a json object. |
The sample format is |
{"parameters":{"audit_retention_hour":168,"dry_run":true, "include_operations":"create,delete,pull"},"schedule":{"type":"Hourly","cron":"0 0 * * * *"}} |
{"parameters":{"audit_retention_hour":168,"dry_run":true,"include_event_types":"create_artifact,delete_artifact,pull_artifact"},"schedule":{"type":"Hourly","cron":"0 0 * * * *"}} |
the include_operation should be a comma separated string, e.g. create,delete,pull, if it is empty, no operation will be purged.
tags:
- purge
Expand Down Expand Up @@ -4516,7 +4516,7 @@ paths:
description: |
The purge job's schedule, it is a json object. |
The sample format is |
{"parameters":{"audit_retention_hour":168,"dry_run":true, "include_operations":"create,delete,pull"},"schedule":{"type":"Hourly","cron":"0 0 * * * *"}} |
{"parameters":{"audit_retention_hour":168,"dry_run":true,"include_event_types":"create_artifact,delete_artifact,pull_artifact"},"schedule":{"type":"Hourly","cron":"0 0 * * * *"}} |
the include_operation should be a comma separated string, e.g. create,delete,pull, if it is empty, no operation will be purged.
tags:
- purge
Expand Down Expand Up @@ -9101,6 +9101,9 @@ definitions:
banner_message:
$ref: '#/definitions/StringConfigItem'
description: The banner message for the UI.It is the stringified result of the banner message object
disabled_audit_log_event_types:
$ref: '#/definitions/StringConfigItem'
description: The audit log event types to skip to log in database
Configurations:
type: object
properties:
Expand Down Expand Up @@ -9379,6 +9382,11 @@ definitions:
description: The banner message for the UI.It is the stringified result of the banner message object
x-omitempty: true
x-isnullable: true
disabled_audit_log_event_types:
type: string
description: the list to disable log audit event types.
x-omitempty: true
x-isnullable: true
StringConfigItem:
type: object
properties:
Expand Down
6 changes: 3 additions & 3 deletions src/common/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ const (
// 24h.
DefaultCacheExpireHours = 24

PurgeAuditIncludeOperations = "include_operations"
PurgeAuditIncludeEventTypes = "include_event_types"
PurgeAuditDryRun = "dry_run"
PurgeAuditRetentionHour = "audit_retention_hour"
// AuditLogForwardEndpoint indicate to forward the audit log to an endpoint
Expand All @@ -220,8 +220,8 @@ const (
// ScannerSkipUpdatePullTime
ScannerSkipUpdatePullTime = "scanner_skip_update_pulltime"

// AuditLogEventsDisabled
AuditLogEventsDisabled = "audit_log_events_disabled"
// AuditLogEventsDisabled ...
AuditLogEventsDisabled = "disabled_audit_log_event_types"

// SessionTimeout defines the web session timeout
SessionTimeout = "session_timeout"
Expand Down
2 changes: 1 addition & 1 deletion src/controller/purge/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func (c *controller) Start(ctx context.Context, policy JobPolicy, trigger string

para[common.PurgeAuditDryRun] = policy.DryRun
para[common.PurgeAuditRetentionHour] = policy.RetentionHour
para[common.PurgeAuditIncludeOperations] = policy.IncludeOperations
para[common.PurgeAuditIncludeEventTypes] = policy.IncludeEventTypes

execID, err := c.exeMgr.Create(ctx, job.PurgeAuditVendorType, -1, trigger, para)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion src/controller/purge/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ type JobPolicy struct {
Trigger *Trigger `json:"trigger"`
DryRun bool `json:"dryrun"`
RetentionHour int `json:"retention_hour"`
IncludeOperations string `json:"include_operations"`
IncludeEventTypes string `json:"include_event_types"`
ExtraAttrs map[string]interface{} `json:"extra_attrs"`
}

Expand Down
24 changes: 17 additions & 7 deletions src/jobservice/job/impl/purge/purge.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,16 @@ import (
"github.com/goharbor/harbor/src/common"
"github.com/goharbor/harbor/src/jobservice/job"
"github.com/goharbor/harbor/src/pkg/audit"
"github.com/goharbor/harbor/src/pkg/auditext"
)

// Job defines the purge job
type Job struct {
retentionHour int
includeOperations []string
includeEventTypes []string
dryRun bool
auditMgr audit.Manager
auditExtMgr auditext.Manager
}

// MaxFails is implementation of same method in Interface.
Expand Down Expand Up @@ -72,18 +74,19 @@ func (j *Job) parseParams(params job.Parameters) {
}
}

j.includeOperations = []string{}
operations, exist := params[common.PurgeAuditIncludeOperations]
j.includeEventTypes = []string{}
operations, exist := params[common.PurgeAuditIncludeEventTypes]
if exist {
if includeOps, ok := operations.(string); ok {
if len(includeOps) > 0 {
j.includeOperations = strings.Split(includeOps, ",")
j.includeEventTypes = strings.Split(includeOps, ",")
}
}
}
// UT will use the mock mgr
if os.Getenv("UTTEST") != "true" {
j.auditMgr = audit.Mgr
j.auditExtMgr = auditext.Mgr
}
}

Expand All @@ -106,17 +109,24 @@ func (j *Job) Run(ctx job.Context, params job.Parameters) error {
if j.retentionHour > common.MaxAuditRetentionHour {
j.retentionHour = common.MaxAuditRetentionHour
}
n, err := j.auditMgr.Purge(ormCtx, j.retentionHour, j.includeOperations, j.dryRun)
n, err := j.auditMgr.Purge(ormCtx, j.retentionHour, j.includeEventTypes, j.dryRun)
if err != nil {
logger.Errorf("failed to purge audit log, error: %v", err)
return err
}
logger.Infof("Purge operation parameter, retention_hour=%v, include_operations:%v, dry_run:%v",
j.retentionHour, j.includeOperations, j.dryRun)
n2, err2 := j.auditExtMgr.Purge(ormCtx, j.retentionHour, j.includeEventTypes, j.dryRun)
if err2 != nil {
logger.Errorf("failed to purge audit log ext, error: %v", err2)
return err2
}
logger.Infof("Purge operation parameter, retention_hour:%v, include_event_types:%v, dry_run:%v",
j.retentionHour, j.includeEventTypes, j.dryRun)
if j.dryRun {
logger.Infof("[DRYRUN]Purged %d rows of audit logs", n)
logger.Infof("[DRYRUN]Purged %d rows of audit log exts", n2)
} else {
logger.Infof("Purged %d rows of audit logs", n)
logger.Infof("Purged %d rows of audit log exts", n2)
}

// Successfully exit
Expand Down
16 changes: 10 additions & 6 deletions src/jobservice/job/impl/purge/purge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
mockjobservice "github.com/goharbor/harbor/src/testing/jobservice"
"github.com/goharbor/harbor/src/testing/mock"
mockAudit "github.com/goharbor/harbor/src/testing/pkg/audit"
mockauditext "github.com/goharbor/harbor/src/testing/pkg/auditext"
)

type PurgeJobTestSuite struct {
Expand All @@ -51,14 +52,14 @@ func (suite *PurgeJobTestSuite) TestParseParams() {
j.parseParams(param)
suite.Require().Equal(true, j.dryRun)
suite.Require().Equal(128, j.retentionHour)
suite.Require().Equal([]string{}, j.includeOperations)
suite.Require().Equal([]string{}, j.includeEventTypes)

j2 := &Job{}
param2 := job.Parameters{common.PurgeAuditRetentionHour: 24, common.PurgeAuditDryRun: false, common.PurgeAuditIncludeOperations: "Delete,Create,Pull"}
param2 := job.Parameters{common.PurgeAuditRetentionHour: 24, common.PurgeAuditDryRun: false, common.PurgeAuditIncludeEventTypes: "delete_artifact,create_artifact,pull_artifact"}
j2.parseParams(param2)
suite.Require().Equal(false, j2.dryRun)
suite.Require().Equal(24, j2.retentionHour)
suite.Require().Equal([]string{"Delete", "Create", "Pull"}, j2.includeOperations)
suite.Require().Equal([]string{"delete_artifact", "create_artifact", "pull_artifact"}, j2.includeEventTypes)
}

func (suite *PurgeJobTestSuite) TestRun() {
Expand All @@ -67,14 +68,16 @@ func (suite *PurgeJobTestSuite) TestRun() {
ctx.On("GetLogger").Return(logger)
ctx.On("OPCommand").Return(job.NilCommand, true)
auditManager := &mockAudit.Manager{}
auditExtManager := &mockauditext.Manager{}
auditManager.On("Purge", mock.Anything, 128, []string{}, true).Return(int64(100), nil)
j := &Job{auditMgr: auditManager}
auditExtManager.On("Purge", mock.Anything, 128, []string{}, true).Return(int64(100), nil)
j := &Job{auditMgr: auditManager, auditExtMgr: auditExtManager}
param := job.Parameters{common.PurgeAuditRetentionHour: 128, common.PurgeAuditDryRun: true}
ret := j.Run(ctx, param)
suite.Require().Nil(ret)

auditManager.On("Purge", mock.Anything, 24, []string{}, false).Return(int64(0), fmt.Errorf("failed to connect database"))
j2 := &Job{auditMgr: auditManager}
j2 := &Job{auditMgr: auditManager, auditExtMgr: auditExtManager}
param2 := job.Parameters{common.PurgeAuditRetentionHour: 24, common.PurgeAuditDryRun: false}
ret2 := j2.Run(ctx, param2)
suite.Require().NotNil(ret2)
Expand All @@ -85,7 +88,8 @@ func (suite *PurgeJobTestSuite) TestStop() {
ctx.On("GetLogger").Return(logger)
ctx.On("OPCommand").Return(job.StopCommand, true)
auditManager := &mockAudit.Manager{}
j := &Job{auditMgr: auditManager}
auditExtManager := &mockauditext.Manager{}
j := &Job{auditMgr: auditManager, auditExtMgr: auditExtManager}
suite.True(j.shouldStop(ctx))
}

Expand Down
2 changes: 2 additions & 0 deletions src/pkg/auditext/dao/dao.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,8 @@ func permitEventTypes(includeEventTypes []string) []string {
event := strings.ToLower(e)
if utils.StringInSlice(event, model.EventTypes) {
filterEvents = append(filterEvents, e)
} else if event == model.OtherEvents { // include all other events
filterEvents = append(filterEvents, model.OtherEventTypes...)
}
}
return filterEvents
Expand Down
5 changes: 5 additions & 0 deletions src/pkg/auditext/dao/dao_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,4 +185,9 @@ func TestPermitEventTypes(t *testing.T) {
t.Errorf("permitEventTypes failed")
}

// test other event types
otherEventTypes := permitEventTypes([]string{"create_artifact", "delete_artifact", "pull_artifact", "other_events"})
if len(otherEventTypes) != len(model.EventTypes) {
t.Errorf("permitOtherEventTypes failed, it should include all event types")
}
}
2 changes: 1 addition & 1 deletion src/pkg/auditext/event/basic.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ type Resolver struct {
SucceedCodes []int
// SensitiveAttributes is the attributes that need to be redacted
SensitiveAttributes []string
// HasResourceName indicates if the resource has name, if true, need to resolve the resource name before delete
// ShouldResolveName indicates if the resource name should be resolved before delete, if true, need to resolve the resource name before delete
ShouldResolveName bool
// IDToNameFunc is used to resolve the resource name from resource id
IDToNameFunc ResolveIDToNameFunc
Expand Down
7 changes: 6 additions & 1 deletion src/pkg/auditext/model/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
beego_orm "github.com/beego/beego/v2/client/orm"
)

const OtherEvents = "other_events"

func init() {
beego_orm.RegisterModel(&AuditLogExt{})
}
Expand All @@ -43,7 +45,7 @@ func (a *AuditLogExt) TableName() string {
return "audit_log_ext"
}

// EventTypes defines the types of audit log event
// EventTypes defines the types of audit log event, new event types should be added at the end of the list
var EventTypes = []string{
"create_artifact",
"delete_artifact",
Expand All @@ -60,3 +62,6 @@ var EventTypes = []string{
"delete_robot",
"update_configure",
}

// OtherEventTypes defines the types of other audit log event types excludes previous EventTypes: create_artifact, delete_artifact, pull_artifact
var OtherEventTypes = EventTypes[3:]
12 changes: 6 additions & 6 deletions src/server/v2.0/handler/purge.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ func verifyCreateRequest(params purge.CreatePurgeScheduleParams) error {
if _, err := retentionHour(params.Schedule.Parameters); err != nil {
return err
}
if _, exist := params.Schedule.Parameters[common.PurgeAuditIncludeOperations]; !exist {
return errors.BadRequestError(fmt.Errorf("include_operations should provide"))
if _, exist := params.Schedule.Parameters[common.PurgeAuditIncludeEventTypes]; !exist {
return errors.BadRequestError(fmt.Errorf("include_event_types should provide"))
}
return nil
}
Expand Down Expand Up @@ -119,8 +119,8 @@ func (p *purgeAPI) kick(ctx context.Context, vendorType string, scheType string,
if dryRun, ok := parameters[common.PurgeAuditDryRun].(bool); ok {
policy.DryRun = dryRun
}
if includeOperations, ok := parameters[common.PurgeAuditIncludeOperations].(string); ok {
policy.IncludeOperations = includeOperations
if includeEventTypes, ok := parameters[common.PurgeAuditIncludeEventTypes].(string); ok {
policy.IncludeEventTypes = includeEventTypes
}
retHour, err := retentionHour(parameters)
if err != nil {
Expand Down Expand Up @@ -312,8 +312,8 @@ func verifyUpdateRequest(params purge.UpdatePurgeScheduleParams) error {
if _, err := retentionHour(params.Schedule.Parameters); err != nil {
return err
}
if _, exist := params.Schedule.Parameters[common.PurgeAuditIncludeOperations]; !exist {
return errors.BadRequestError(fmt.Errorf("include_operations should provide"))
if _, exist := params.Schedule.Parameters[common.PurgeAuditIncludeEventTypes]; !exist {
return errors.BadRequestError(fmt.Errorf("include_event_types should provide"))
}
return nil
}
Expand Down
17 changes: 9 additions & 8 deletions src/server/v2.0/handler/purge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@ import (
"fmt"
"testing"

"github.com/stretchr/testify/assert"

"github.com/goharbor/harbor/src/common"
"github.com/goharbor/harbor/src/server/v2.0/models"
"github.com/goharbor/harbor/src/server/v2.0/restapi/operations/purge"
"github.com/stretchr/testify/assert"
)

func Test_verifyUpdateRequest(t *testing.T) {
Expand All @@ -33,9 +34,9 @@ func Test_verifyUpdateRequest(t *testing.T) {
args args
wantErr bool
}{
{"normal", args{purge.UpdatePurgeScheduleParams{Schedule: &models.Schedule{Schedule: &models.ScheduleObj{}, Parameters: map[string]interface{}{common.PurgeAuditRetentionHour: "168", common.PurgeAuditIncludeOperations: "pull"}}}}, false},
{"missing_schedule", args{purge.UpdatePurgeScheduleParams{Schedule: &models.Schedule{Parameters: map[string]interface{}{common.PurgeAuditRetentionHour: "168", common.PurgeAuditIncludeOperations: "pull"}}}}, true},
{"missing_retention_hour", args{purge.UpdatePurgeScheduleParams{Schedule: &models.Schedule{Schedule: &models.ScheduleObj{}, Parameters: map[string]interface{}{common.PurgeAuditIncludeOperations: "pull"}}}}, true},
{"normal", args{purge.UpdatePurgeScheduleParams{Schedule: &models.Schedule{Schedule: &models.ScheduleObj{}, Parameters: map[string]interface{}{common.PurgeAuditRetentionHour: "168", common.PurgeAuditIncludeEventTypes: "pull_artifact"}}}}, false},
{"missing_schedule", args{purge.UpdatePurgeScheduleParams{Schedule: &models.Schedule{Parameters: map[string]interface{}{common.PurgeAuditRetentionHour: "168", common.PurgeAuditIncludeEventTypes: "pull_artifact"}}}}, true},
{"missing_retention_hour", args{purge.UpdatePurgeScheduleParams{Schedule: &models.Schedule{Schedule: &models.ScheduleObj{}, Parameters: map[string]interface{}{common.PurgeAuditIncludeEventTypes: "pull_artifact"}}}}, true},
{"missing_operations", args{purge.UpdatePurgeScheduleParams{Schedule: &models.Schedule{Schedule: &models.ScheduleObj{}, Parameters: map[string]interface{}{common.PurgeAuditRetentionHour: "168"}}}}, true},
}
for _, tt := range tests {
Expand All @@ -56,10 +57,10 @@ func Test_verifyCreateRequest(t *testing.T) {
args args
wantErr bool
}{
{"normal", args{purge.CreatePurgeScheduleParams{Schedule: &models.Schedule{Schedule: &models.ScheduleObj{}, Parameters: map[string]interface{}{common.PurgeAuditRetentionHour: "168", common.PurgeAuditIncludeOperations: "pull"}}}}, false},
{"missing_schedule", args{purge.CreatePurgeScheduleParams{Schedule: &models.Schedule{Parameters: map[string]interface{}{common.PurgeAuditRetentionHour: "168", common.PurgeAuditIncludeOperations: "pull"}}}}, true},
{"missing_retention_hour", args{purge.CreatePurgeScheduleParams{Schedule: &models.Schedule{Schedule: &models.ScheduleObj{}, Parameters: map[string]interface{}{common.PurgeAuditIncludeOperations: "pull"}}}}, true},
{"missing_operations", args{purge.CreatePurgeScheduleParams{Schedule: &models.Schedule{Schedule: &models.ScheduleObj{}, Parameters: map[string]interface{}{common.PurgeAuditRetentionHour: "168"}}}}, true},
{"normal", args{purge.CreatePurgeScheduleParams{Schedule: &models.Schedule{Schedule: &models.ScheduleObj{}, Parameters: map[string]interface{}{common.PurgeAuditRetentionHour: "168", common.PurgeAuditIncludeEventTypes: "pull_artifact"}}}}, false},
{"missing_schedule", args{purge.CreatePurgeScheduleParams{Schedule: &models.Schedule{Parameters: map[string]interface{}{common.PurgeAuditRetentionHour: "168", common.PurgeAuditIncludeEventTypes: "pull_artifact"}}}}, true},
{"missing_retention_hour", args{purge.CreatePurgeScheduleParams{Schedule: &models.Schedule{Schedule: &models.ScheduleObj{}, Parameters: map[string]interface{}{common.PurgeAuditIncludeEventTypes: "pull_artifact"}}}}, true},
{"missing_event_types", args{purge.CreatePurgeScheduleParams{Schedule: &models.Schedule{Schedule: &models.ScheduleObj{}, Parameters: map[string]interface{}{common.PurgeAuditRetentionHour: "168"}}}}, true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
8 changes: 4 additions & 4 deletions tests/apitests/python/library/purge.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ class Purge(base.Base):
def __init__(self):
super(Purge, self).__init__(api_type="purge")

def create_purge_schedule(self, type, cron, dry_run=True, audit_retention_hour=24, include_operations="create,delete,pull", expect_status_code=201, expect_response_body=None, **kwargs):
def create_purge_schedule(self, type, cron, dry_run=True, audit_retention_hour=24, include_event_types="create_artifact,delete_artifact,pull_artifact", expect_status_code=201, expect_response_body=None, **kwargs):
scheduleObj = v2_swagger_client.ScheduleObj(type=type)
if cron is not None:
scheduleObj.cron = cron
parameters = {
"audit_retention_hour": audit_retention_hour,
"include_operations": include_operations,
"include_event_types": include_event_types,
"dry_run": dry_run
}
schedule = v2_swagger_client.Schedule(schedule=scheduleObj, parameters=parameters)
Expand All @@ -30,11 +30,11 @@ def create_purge_schedule(self, type, cron, dry_run=True, audit_retention_hour=2
return
base._assert_status_code(expect_status_code, status_code)

def update_purge_schedule(self, type, cron, audit_retention_hour=24, include_operations="create,delete,pull", expect_status_code=200, expect_response_body=None, **kwargs):
def update_purge_schedule(self, type, cron, audit_retention_hour=24, include_event_types="create_artifact,delete_artifact,pull_artifact", expect_status_code=200, expect_response_body=None, **kwargs):
scheduleObj = v2_swagger_client.ScheduleObj(type=type, cron=cron)
parameters = {
"audit_retention_hour": audit_retention_hour,
"include_operations": include_operations,
"include_event_types": include_event_types,
"dry_run": False
}
schedule = v2_swagger_client.Schedule(schedule=scheduleObj, parameters=parameters)
Expand Down
Loading

0 comments on commit 4565907

Please sign in to comment.