Fix replication bugs (#7470)

1. Only return the event based trigger for local Harbor
2. Valid the trigger pattern and cron string when creating/updating policies
3. Set the schema as "http" if it isn't specified when creating/updating registries

Signed-off-by: Wenkai Yin <yinw@vmware.com>

src/core/api/registry.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"net/http"
 	"strconv"
+	"strings"
 
 	common_http "github.com/goharbor/harbor/src/common/http"
 	"github.com/goharbor/harbor/src/common/utils"
@@ -206,6 +207,10 @@ func (t *RegistryAPI) Post() {
 		t.SendConflictError(fmt.Errorf("name '%s' is already used", r.Name))
 		return
 	}
+	i := strings.Index(r.URL, "://")
+	if i == -1 {
+		r.URL = fmt.Sprintf("http://%s", r.URL)
+	}
 
 	status, err := registry.CheckHealthStatus(r)
 	if err != nil {
@@ -411,6 +416,10 @@ func (t *RegistryAPI) GetInfo() {
 		t.SendInternalServerError(fmt.Errorf("failed to get registry info %d: %v", id, err))
 		return
 	}
+	// currently, only the local Harbor registry supports the event based trigger, append it here
+	if id == 0 {
+		info.SupportedTriggers = append(info.SupportedTriggers, model.TriggerTypeEventBased)
+	}
 	t.WriteJSONData(process(info))
 }
 

src/replication/adapter/harbor/adapter.go

@@ -113,7 +113,6 @@ func (a *adapter) Info() (*model.RegistryInfo, error) {
 		SupportedTriggers: []model.TriggerType{
 			model.TriggerTypeManual,
 			model.TriggerTypeScheduled,
-			model.TriggerTypeEventBased,
 		},
 	}
 

src/replication/adapter/harbor/adapter_test.go

@@ -43,7 +43,7 @@ func TestInfo(t *testing.T) {
 	require.Nil(t, err)
 	assert.Equal(t, model.RegistryTypeHarbor, info.Type)
 	assert.Equal(t, 2, len(info.SupportedResourceFilters))
-	assert.Equal(t, 3, len(info.SupportedTriggers))
+	assert.Equal(t, 2, len(info.SupportedTriggers))
 	assert.Equal(t, 2, len(info.SupportedResourceTypes))
 	assert.Equal(t, model.ResourceTypeImage, info.SupportedResourceTypes[0])
 	assert.Equal(t, model.ResourceTypeChart, info.SupportedResourceTypes[1])
@@ -67,7 +67,7 @@ func TestInfo(t *testing.T) {
 	require.Nil(t, err)
 	assert.Equal(t, model.RegistryTypeHarbor, info.Type)
 	assert.Equal(t, 2, len(info.SupportedResourceFilters))
-	assert.Equal(t, 3, len(info.SupportedTriggers))
+	assert.Equal(t, 2, len(info.SupportedTriggers))
 	assert.Equal(t, 1, len(info.SupportedResourceTypes))
 	assert.Equal(t, model.ResourceTypeImage, info.SupportedResourceTypes[0])
 	server.Close()

src/replication/model/policy.go

@@ -20,6 +20,7 @@ import (
 
 	"github.com/astaxie/beego/validation"
 	"github.com/goharbor/harbor/src/common/models"
+	"github.com/robfig/cron"
 )
 
 // const definition
@@ -88,10 +89,20 @@ func (p *Policy) Valid(v *validation.Validation) {
 
 	// valid the filters
 	for _, filter := range p.Filters {
-		if filter.Type != FilterTypeResource &&
+		value, ok := filter.Value.(string)
-			filter.Type != FilterTypeName &&
+		if !ok {
-			filter.Type != FilterTypeTag &&
+			v.SetError("filters", "the type of filter value isn't string")
-			filter.Type != FilterTypeLabel {
+			break
+		}
+		switch filter.Type {
+		case FilterTypeResource:
+			rt := ResourceType(value)
+			if !(rt == ResourceTypeImage || rt == ResourceTypeChart) {
+				v.SetError("filters", fmt.Sprintf("invalid resource filter: %s", value))
+				break
+			}
+		case FilterTypeName, FilterTypeTag, FilterTypeLabel:
+		default:
 			v.SetError("filters", "invalid filter type")
 			break
 		}
@@ -99,15 +110,20 @@ func (p *Policy) Valid(v *validation.Validation) {
 
 	// valid trigger
 	if p.Trigger != nil {
-		if p.Trigger.Type != TriggerTypeManual &&
+		switch p.Trigger.Type {
-			p.Trigger.Type != TriggerTypeScheduled &&
+		case TriggerTypeManual, TriggerTypeEventBased:
-			p.Trigger.Type != TriggerTypeEventBased {
+		case TriggerTypeScheduled:
+			if p.Trigger.Settings == nil || len(p.Trigger.Settings.Cron) == 0 {
+				v.SetError("trigger", fmt.Sprintf("the cron string cannot be empty when the trigger type is %s", TriggerTypeScheduled))
+			} else {
+				_, err := cron.Parse(p.Trigger.Settings.Cron)
+				if err != nil {
+					v.SetError("trigger", fmt.Sprintf("invalid cron string for scheduled trigger: %s", p.Trigger.Settings.Cron))
+				}
+			}
+		default:
 			v.SetError("trigger", "invalid trigger type")
 		}
-		if p.Trigger.Type == TriggerTypeScheduled &&
-			(p.Trigger.Settings == nil || len(p.Trigger.Settings.Cron) == 0) {
-			v.SetError("trigger", fmt.Sprintf("the cron string cannot be empty when the trigger type is %s", TriggerTypeScheduled))
-		}
 	}
 }
 

src/replication/model/policy_test.go

@@ -15,6 +15,7 @@
 package model
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/astaxie/beego/validation"
@@ -69,6 +70,48 @@ func TestValidOfPolicy(t *testing.T) {
 			},
 			pass: false,
 		},
+		// invalid filter
+		{
+			policy: &Policy{
+				Name: "policy01",
+				SrcRegistry: &Registry{
+					ID: 0,
+				},
+				DestRegistry: &Registry{
+					ID: 1,
+				},
+				Filters: []*Filter{
+					{
+						Type:  FilterTypeResource,
+						Value: "invalid_resource_type",
+					},
+				},
+			},
+			pass: false,
+		},
+		// invalid filter
+		{
+			policy: &Policy{
+				Name: "policy01",
+				SrcRegistry: &Registry{
+					ID: 0,
+				},
+				DestRegistry: &Registry{
+					ID: 1,
+				},
+				Filters: []*Filter{
+					{
+						Type:  FilterTypeResource,
+						Value: ResourceTypeImage,
+					},
+					{
+						Type:  FilterTypeName,
+						Value: "a[",
+					},
+				},
+			},
+			pass: false,
+		},
 		// invalid trigger
 		{
 			policy: &Policy{
@@ -113,6 +156,35 @@ func TestValidOfPolicy(t *testing.T) {
 			},
 			pass: false,
 		},
+		// invalid cron
+		{
+			policy: &Policy{
+				Name: "policy01",
+				SrcRegistry: &Registry{
+					ID: 0,
+				},
+				DestRegistry: &Registry{
+					ID: 1,
+				},
+				Filters: []*Filter{
+					{
+						Type:  FilterTypeResource,
+						Value: "image",
+					},
+					{
+						Type:  FilterTypeName,
+						Value: "library/**",
+					},
+				},
+				Trigger: &Trigger{
+					Type: TriggerTypeScheduled,
+					Settings: &TriggerSettings{
+						Cron: "* * *",
+					},
+				},
+			},
+			pass: false,
+		},
 		// pass
 		{
 			policy: &Policy{
@@ -124,15 +196,19 @@ func TestValidOfPolicy(t *testing.T) {
 					ID: 1,
 				},
 				Filters: []*Filter{
+					{
+						Type:  FilterTypeResource,
+						Value: "image",
+					},
 					{
 						Type:  FilterTypeName,
-						Value: "library",
+						Value: "library/**",
 					},
 				},
 				Trigger: &Trigger{
 					Type: TriggerTypeScheduled,
 					Settings: &TriggerSettings{
-						Cron: "* * *",
+						Cron: "* * * * * *",
 					},
 				},
 			},
@@ -140,7 +216,8 @@ func TestValidOfPolicy(t *testing.T) {
 		},
 	}
 
-	for _, c := range cases {
+	for i, c := range cases {
+		fmt.Printf("running case %d ...\n", i)
 		v := &validation.Validation{}
 		c.policy.Valid(v)
 		assert.Equal(t, c.pass, len(v.Errors) == 0)

src/replication/util/util.go

@@ -18,6 +18,8 @@ import (
 	"net/http"
 	"strings"
 
+	"github.com/goharbor/harbor/src/common/utils/log"
+
 	"github.com/bmatcuk/doublestar"
 	"github.com/goharbor/harbor/src/common/utils/registry"
 )
@@ -27,7 +29,12 @@ func Match(pattern, str string) (bool, error) {
 	if len(pattern) == 0 {
 		return true, nil
 	}
-	return doublestar.Match(pattern, str)
+	match, err := doublestar.Match(pattern, str)
+	if err == doublestar.ErrBadPattern {
+		log.Warningf("failed to match the string %s against pattern %s: %v", str, pattern, err)
+		return false, nil
+	}
+	return match, err
 }
 
 // IsSpecificRepositoryName if the name not contains any char of "*?[{\\]}^,",

src/replication/util/util_test.go

@@ -67,6 +67,11 @@ func TestMatch(t *testing.T) {
 			str:     "1.01",
 			match:   false,
 		},
+		{
+			pattern: "a[",
+			str:     "aaa",
+			match:   false,
+		},
 	}
 	for _, c := range cases {
 		match, err := Match(c.pattern, c.str)