Upstream/harbor
1
0
Fork 0
mirror of https://github.com/goharbor/harbor.git synced 2024-06-28 15:55:04 +02:00
Code Issues Projects Releases Wiki Activity

fix(preheat): fix npe issues

Browse Source 
- fix npe issue in create/update policy
- fix issue of missing schedule job id in the preheat policy

Signed-off-by: Steven Zou <szou@vmware.com>

- increase the client timeout
This commit is contained in:
Steven Zou 2020-07-29 18:16:30 +08:00
parent 5b1b94f25c
commit 507d792655
5 changed files with 120 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

75
src/controller/p2p/preheat/controller.go
View File

@ -2,6 +2,7 @@ package preheat
import (
"context"
"encoding/json"
"time"
"github.com/goharbor/harbor/src/lib/errors"
@ -212,10 +213,19 @@ type TriggerParam struct {
// CreatePolicy creates the policy.
func (c *controller) CreatePolicy(ctx context.Context, schema *policyModels.Schema) (id int64, err error) {
if schema != nil {
now := time.Now()
schema.CreatedAt = now
schema.UpdatedTime = now
if schema == nil {
return 0, errors.New("nil policy schema provided")
}
// Update timestamps
now := time.Now()
schema.CreatedAt = now
schema.UpdatedTime = now
// Get full model of policy schema
_, err = policy.ParsePolicy(schema)
if err != nil {
return 0, err
}
id, err = c.pManager.Create(ctx, schema)
@ -223,6 +233,8 @@ func (c *controller) CreatePolicy(ctx context.Context, schema *policyModels.Sche
return
}
schema.ID = id
if schema.Trigger != nil &&
schema.Trigger.Type == policyModels.TriggerTypeScheduled &&
len(schema.Trigger.Settings.Cron) > 0 {
@ -232,13 +244,15 @@ func (c *controller) CreatePolicy(ctx context.Context, schema *policyModels.Sche
return 0, err
}
schema.ID = id
err = c.pManager.Update(ctx, schema, "trigger")
if err = decodeSchema(schema); err == nil {
err = c.pManager.Update(ctx, schema, "trigger")
}
if err != nil {
errUnsch := c.scheduler.UnSchedule(ctx, schema.Trigger.Settings.JobID)
if errUnsch != nil {
return 0, errUnsch
if e := c.scheduler.UnSchedule(ctx, schema.Trigger.Settings.JobID); e != nil {
return 0, errors.Wrap(e, err.Error())
}
return 0, err
}
}
@ -258,13 +272,27 @@ func (c *controller) GetPolicyByName(ctx context.Context, projectID int64, name
// UpdatePolicy updates the policy.
func (c *controller) UpdatePolicy(ctx context.Context, schema *policyModels.Schema, props ...string) error {
if schema != nil {
schema.UpdatedTime = time.Now()
if schema == nil {
return errors.New("nil policy schema provided")
}
// Get policy cache
s0, err := c.pManager.Get(ctx, schema.ID)
if err != nil {
return err
}
// Double check trigger
if s0.Trigger == nil {
return errors.Errorf("missing trigger settings in preheat policy %s", s0.Name)
}
// Get full model of updating policy
_, err = policy.ParsePolicy(schema)
if err != nil {
return err
}
var cron = schema.Trigger.Settings.Cron
var oldJobID = s0.Trigger.Settings.JobID
var needUn bool
@ -309,8 +337,16 @@ func (c *controller) UpdatePolicy(ctx context.Context, schema *policyModels.Sche
return err
}
schema.Trigger.Settings.JobID = jobid
if err := decodeSchema(schema); err != nil {
// Possible
// TODO: Refactor
return err // whether update or not has been not important as the jon reference will be lost
}
}
// Update timestamp
schema.UpdatedTime = time.Now()
return c.pManager.Update(ctx, schema, props...)
}
@ -369,3 +405,20 @@ func (c *controller) CheckHealth(ctx context.Context, instance *providerModels.I
return nil
}
// decodeSchema decodes the trigger object to JSON string
func decodeSchema(schema *policyModels.Schema) error {
if schema.Trigger == nil {
// do nothing
return nil
}
b, err := json.Marshal(schema.Trigger)
if err != nil {
return err
}
schema.TriggerStr = string(b)
return nil
}

33
src/controller/p2p/preheat/controllor_test.go
View File

@ -3,6 +3,7 @@ package preheat
import (
"context"
"errors"
"fmt"
"net/http"
"net/http/httptest"
"testing"
@ -198,8 +199,11 @@ func (s *preheatSuite) TestCountPolicy() {
}
func (s *preheatSuite) TestCreatePolicy() {
policy := &policy.Schema{Name: "test", Trigger: &policy.Trigger{Type: policy.TriggerTypeScheduled}}
policy.Trigger.Settings.Cron = "* * * * */1"
policy := &policy.Schema{
Name: "test",
FiltersStr: `[{"type":"repository","value":"harbor*"},{"type":"tag","value":"2*"}]`,
TriggerStr: fmt.Sprintf(`{"type":"%s", "trigger_setting":{"cron":"* * * * */1"}}`, policy.TriggerTypeScheduled),
}
s.fakeScheduler.On("Schedule", s.ctx, mock.Anything, mock.Anything, mock.Anything).Return(int64(1), nil)
s.fakePolicyMgr.On("Create", s.ctx, policy).Return(int64(1), nil)
s.fakePolicyMgr.On("Update", s.ctx, mock.Anything, mock.Anything).Return(nil)
@ -229,18 +233,37 @@ func (s *preheatSuite) TestUpdatePolicy() {
var p0 = &policy.Schema{Name: "test", Trigger: &policy.Trigger{Type: policy.TriggerTypeScheduled}}
p0.Trigger.Settings.JobID = 1
p0.Trigger.Settings.Cron = "* * * * */1"
p0.Filters = []*policy.Filter{
{
Type: policy.FilterTypeRepository,
Value: "harbor*",
},
{
Type: policy.FilterTypeTag,
Value: "2*",
},
}
s.fakePolicyMgr.On("Get", s.ctx, int64(1)).Return(p0, nil)
// need change to schedule
p1 := &policy.Schema{ID: 1, Name: "test", Trigger: &policy.Trigger{Type: policy.TriggerTypeManual}}
p1 := &policy.Schema{
ID: 1,
Name: "test",
FiltersStr: `[{"type":"repository","value":"harbor*"},{"type":"tag","value":"2*"}]`,
TriggerStr: fmt.Sprintf(`{"type":"%s", "trigger_setting":{}}`, policy.TriggerTypeManual),
}
s.fakePolicyMgr.On("Update", s.ctx, p1, mock.Anything).Return(nil)
err := s.controller.UpdatePolicy(s.ctx, p1, "")
s.NoError(err)
s.False(p1.UpdatedTime.IsZero())
// need update schedule
p2 := &policy.Schema{ID: 1, Name: "test", Trigger: &policy.Trigger{Type: policy.TriggerTypeScheduled}}
p2.Trigger.Settings.Cron = "* * * * */2"
p2 := &policy.Schema{
ID: 1,
Name: "test",
FiltersStr: `[{"type":"repository","value":"harbor*"},{"type":"tag","value":"2*"}]`,
TriggerStr: fmt.Sprintf(`{"type":"%s", "trigger_setting":{"cron":"* * * * */2"}}`, policy.TriggerTypeScheduled),
}
s.fakePolicyMgr.On("Update", s.ctx, p2, mock.Anything).Return(nil)
err = s.controller.UpdatePolicy(s.ctx, p2, "")
s.NoError(err)

16
src/pkg/p2p/preheat/policy/manager.go
View File

@ -81,7 +81,7 @@ func (m *manager) Get(ctx context.Context, id int64) (schema *policy.Schema, err
return nil, err
}
return parsePolicy(schema)
return ParsePolicy(schema)
}
// Get the policy schema by name
@ -91,7 +91,7 @@ func (m *manager) GetByName(ctx context.Context, projectID int64, name string) (
return nil, err
}
return parsePolicy(schema)
return ParsePolicy(schema)
}
// Delete the policy schema by id
@ -107,7 +107,7 @@ func (m *manager) ListPolicies(ctx context.Context, query *q.Query) (schemas []*
}
for i := range schemas {
schema, err := parsePolicy(schemas[i])
schema, err := ParsePolicy(schemas[i])
if err != nil {
return nil, err
}
@ -132,8 +132,8 @@ func (m *manager) ListPoliciesByProject(ctx context.Context, project int64, quer
return m.ListPolicies(ctx, query)
}
// parsePolicy parse policy model.
func parsePolicy(schema *policy.Schema) (*policy.Schema, error) {
// ParsePolicy parses persisting data to policy model.
func ParsePolicy(schema *policy.Schema) (*policy.Schema, error) {
if schema == nil {
return nil, errors.New("policy schema can not be nil")
}
@ -157,8 +157,9 @@ func parsePolicy(schema *policy.Schema) (*policy.Schema, error) {
// parseFilters parse filterStr to filter.
func parseFilters(filterStr string) ([]*policy.Filter, error) {
// Filters are required
if len(filterStr) == 0 {
return nil, nil
return nil, errors.New("missing filters in preheat policy schema")
}
var filters []*policy.Filter
@ -188,8 +189,9 @@ func parseFilters(filterStr string) ([]*policy.Filter, error) {
// parseTrigger parse triggerStr to trigger.
func parseTrigger(triggerStr string) (*policy.Trigger, error) {
// trigger must be existing, at least is a "manual" trigger.
if len(triggerStr) == 0 {
return nil, nil
return nil, errors.New("missing trigger settings in preheat policy schema")
}
trigger := &policy.Trigger{}

24
src/pkg/p2p/preheat/policy/manager_test.go
View File

@ -16,6 +16,7 @@ package policy
import (
"context"
"fmt"
"testing"
"github.com/goharbor/harbor/src/lib/q"
@ -115,15 +116,26 @@ func (m *managerTestSuite) TestUpdate() {
// TestGet tests Get method.
func (m *managerTestSuite) TestGet() {
m.dao.On("Get").Return(&policy.Schema{}, nil)
m.dao.On("Get").Return(&policy.Schema{
ID: 1,
Name: "mgr-policy",
FiltersStr: `[{"type":"repository","value":"harbor*"},{"type":"tag","value":"2*"}]`,
TriggerStr: fmt.Sprintf(`{"type":"%s", "trigger_setting":{"cron":"* * * * */1"}}`, policy.TriggerTypeScheduled),
}, nil)
_, err := m.mgr.Get(nil, 1)
m.Require().Nil(err)
}
// TestGetByName tests Get method.
func (m *managerTestSuite) TestGetByName() {
m.dao.On("GetByName").Return(&policy.Schema{}, nil)
_, err := m.mgr.Get(nil, 1)
m.dao.On("GetByName").Return(&policy.Schema{
ID: 1,
ProjectID: 1,
Name: "mgr-policy",
FiltersStr: `[{"type":"repository","value":"harbor*"},{"type":"tag","value":"2*"}]`,
TriggerStr: fmt.Sprintf(`{"type":"%s", "trigger_setting":{"cron":"* * * * */1"}}`, policy.TriggerTypeScheduled),
}, nil)
_, err := m.mgr.GetByName(nil, 1, "mgr-policy")
m.Require().Nil(err)
}
@ -151,11 +163,11 @@ func (m *managerTestSuite) TestListPoliciesByProject() {
// TestParsePolicy tests parsePolicy.
func (m *managerTestSuite) TestParsePolicy() {
schema := &policy.Schema{FiltersStr: "invalid"}
_, err := parsePolicy(schema)
_, err := ParsePolicy(schema)
m.Require().Error(err)
schema = &policy.Schema{TriggerStr: "invalid"}
_, err = parsePolicy(schema)
_, err = ParsePolicy(schema)
m.Require().Error(err)
schema = &policy.Schema{
@ -187,7 +199,7 @@ func (m *managerTestSuite) TestParsePolicy() {
}
}`,
}
schema, err = parsePolicy(schema)
schema, err = ParsePolicy(schema)
m.Require().NoError(err)
m.Require().NotNil(schema.Trigger)
m.Require().Equal("0 0 2 1 * ? *", schema.Trigger.Settings.Cron)

2
src/pkg/p2p/preheat/provider/client/http_client.go
View File

@ -16,7 +16,7 @@ import (
)
const (
clientTimeout = 10 * time.Second
clientTimeout = 30 * time.Second
maxIdleConnections = 20
idleConnectionTimeout = 30 * time.Second
tlsHandshakeTimeout = 30 * time.Second