Skip to content

Commit b2792ba

Browse files
committed
feat: Regex filters for P2P preheat (phase 3)
The preheat policy filters bypassed the selector registry: buildFilter reached straight for the doublestar selector, so a repository or tag pattern could only be a glob, while the retention, immutability and replication filters can already be regular expressions. A preheat filter now carries an optional "kind", the pattern engine of its value, with the same literals the other filters use. The field is additive and omitted when it is the doublestar default, so a policy stored before this keeps decoding and behaving exactly as it did. Only the repository and tag filters take a kind. The label filter matches label names exactly, and signature and vulnerability carry a bool and an int, so a kind there is rejected rather than silently ignored. The policy schema validates the filters at write time, which is the create and update path of the controller, and returns a 400 for an unknown kind, a kind on a filter type that has no pattern, or an expression that does not compile. The filter builder keeps its own check as a backstop for a policy that was stored by an older version or written past the API. The engine is picked per pattern input in the preheat policy dialog, next to the repositories and the tags, and the payload carries the kind only when it is not the default. A regex is stored verbatim: the portal wraps a comma separated doublestar list in braces, which a regex uses for quantifiers. Signed-off-by: Vadim Bauer <vb@container-registry.com>
1 parent 06a99e9 commit b2792ba

22 files changed

Lines changed: 722 additions & 27 deletions

src/controller/p2p/preheat/controller_test.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/stretchr/testify/suite"
1414

1515
"github.com/goharbor/harbor/src/lib/config"
16+
harborerrors "github.com/goharbor/harbor/src/lib/errors"
1617
"github.com/goharbor/harbor/src/lib/orm"
1718
"github.com/goharbor/harbor/src/lib/q"
1819
"github.com/goharbor/harbor/src/pkg/p2p/preheat/models/policy"
@@ -253,6 +254,73 @@ func (s *preheatSuite) TestCreatePolicy() {
253254
s.False(policy.UpdatedTime.IsZero())
254255
}
255256

257+
func (s *preheatSuite) TestPolicyFilterKindValidation() {
258+
cases := []struct {
259+
name string
260+
filtersStr string
261+
wantErr bool
262+
}{
263+
{
264+
name: "no kind",
265+
filtersStr: `[{"type":"repository","value":"harbor*"},{"type":"tag","value":"2*"}]`,
266+
},
267+
{
268+
name: "regex kind",
269+
filtersStr: `[{"type":"repository","value":"harbor.*","kind":"regex"},{"type":"tag","value":"2.*","kind":"regex"}]`,
270+
},
271+
{
272+
name: "unknown kind",
273+
filtersStr: `[{"type":"repository","value":"harbor*","kind":"glob"}]`,
274+
wantErr: true,
275+
},
276+
{
277+
name: "invalid regex",
278+
filtersStr: `[{"type":"repository","value":"foo)|(?:bar","kind":"regex"}]`,
279+
wantErr: true,
280+
},
281+
{
282+
name: "kind on a label filter",
283+
filtersStr: `[{"type":"label","value":"prod","kind":"regex"}]`,
284+
wantErr: true,
285+
},
286+
}
287+
288+
stored := &policy.Schema{ID: 99, Name: "test-filter-kind", Trigger: &policy.Trigger{Type: policy.TriggerTypeManual}}
289+
s.fakePolicyMgr.On("Get", s.ctx, int64(99)).Return(stored, nil)
290+
291+
for _, c := range cases {
292+
s.Run(c.name, func() {
293+
p := &policy.Schema{
294+
ID: 99,
295+
Name: "test-filter-kind",
296+
FiltersStr: c.filtersStr,
297+
TriggerStr: fmt.Sprintf(`{"type":"%s", "trigger_setting":{}}`, policy.TriggerTypeManual),
298+
}
299+
s.fakePolicyMgr.On("Create", s.ctx, p).Return(int64(99), nil).Maybe()
300+
s.fakePolicyMgr.On("Update", s.ctx, p, mock.Anything).Return(nil).Maybe()
301+
302+
_, createErr := s.controller.CreatePolicy(s.ctx, p)
303+
updateErr := s.controller.UpdatePolicy(s.ctx, p, "")
304+
if !c.wantErr {
305+
s.NoError(createErr)
306+
s.NoError(updateErr)
307+
// the policy of this case is a distinct pointer, so the recorded calls
308+
// of the other tests in the suite never satisfy these
309+
s.fakePolicyMgr.AssertCalled(s.T(), "Create", s.ctx, p)
310+
s.fakePolicyMgr.AssertCalled(s.T(), "Update", s.ctx, p, mock.Anything)
311+
return
312+
}
313+
s.Error(createErr)
314+
s.True(harborerrors.IsErr(createErr, harborerrors.BadRequestCode), "create error is a bad request: %v", createErr)
315+
s.Error(updateErr)
316+
s.True(harborerrors.IsErr(updateErr, harborerrors.BadRequestCode), "update error is a bad request: %v", updateErr)
317+
// validation runs before persistence, so a rejected policy is never stored
318+
s.fakePolicyMgr.AssertNotCalled(s.T(), "Create", s.ctx, p)
319+
s.fakePolicyMgr.AssertNotCalled(s.T(), "Update", s.ctx, p, mock.Anything)
320+
})
321+
}
322+
}
323+
256324
func (s *preheatSuite) TestGetPolicy() {
257325
s.fakePolicyMgr.On("Get", s.ctx, int64(1)).Return(&policy.Schema{Name: "test"}, nil)
258326
p, err := s.controller.GetPolicy(s.ctx, 1)

src/pkg/p2p/preheat/models/policy/policy.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222

2323
"github.com/goharbor/harbor/src/common/utils"
2424
"github.com/goharbor/harbor/src/lib/errors"
25+
"github.com/goharbor/harbor/src/lib/pattern"
2526
"github.com/goharbor/harbor/src/lib/q"
2627
)
2728

@@ -48,6 +49,12 @@ const (
4849
// FilterTypeLabel represents the label filter type
4950
FilterTypeLabel FilterType = "label"
5051

52+
// FilterKindDoublestar interprets the filter value as a doublestar pattern, the default
53+
FilterKindDoublestar = pattern.KindDoublestar
54+
// FilterKindRegex interprets the filter value as a regular expression matching the whole
55+
// value, the same engine the retention, immutability and replication filters use
56+
FilterKindRegex = pattern.KindRegex
57+
5158
// TriggerTypeManual represents the manual trigger type
5259
TriggerTypeManual TriggerType = "manual"
5360
// TriggerTypeScheduled represents the scheduled trigger type
@@ -103,6 +110,50 @@ type FilterType = string
103110
type Filter struct {
104111
Type FilterType `json:"type"`
105112
Value any `json:"value"`
113+
// Kind selects the pattern engine used for Value, empty means FilterKindDoublestar.
114+
// Only the repository and tag filters carry a pattern, the others are exact or numeric.
115+
Kind string `json:"kind,omitempty"`
116+
}
117+
118+
// supportsKind reports whether the filter type evaluates its value as a pattern
119+
func (f *Filter) supportsKind() bool {
120+
return f.Type == FilterTypeRepository || f.Type == FilterTypeTag
121+
}
122+
123+
// Validate checks the pattern engine selection of the filter. The value type checks stay
124+
// in the filter builder, which is also reached by the policies stored before this existed.
125+
func (f *Filter) Validate() error {
126+
if f.Kind == "" {
127+
return nil
128+
}
129+
130+
if f.Kind != FilterKindDoublestar && f.Kind != FilterKindRegex {
131+
return errors.New(nil).WithCode(errors.BadRequestCode).
132+
WithMessagef("invalid filter kind: %s", f.Kind)
133+
}
134+
135+
if !f.supportsKind() {
136+
return errors.New(nil).WithCode(errors.BadRequestCode).
137+
WithMessagef("only the %s and %s filters support kind, got: %s",
138+
FilterTypeRepository, FilterTypeTag, f.Type)
139+
}
140+
141+
if f.Kind != FilterKindRegex {
142+
return nil
143+
}
144+
145+
value, ok := f.Value.(string)
146+
if !ok {
147+
return errors.New(nil).WithCode(errors.BadRequestCode).
148+
WithMessagef("the value of the %s filter isn't a string", f.Type)
149+
}
150+
151+
if err := pattern.ValidateRegex(value); err != nil {
152+
return errors.New(nil).WithCode(errors.BadRequestCode).
153+
WithMessagef("invalid regex filter value %q: %v", value, err)
154+
}
155+
156+
return nil
106157
}
107158

108159
// TriggerType represents the type of trigger.
@@ -120,6 +171,15 @@ type Trigger struct {
120171

121172
// ValidatePreheatPolicy validate preheat policy
122173
func (s *Schema) ValidatePreheatPolicy() error {
174+
for _, filter := range s.Filters {
175+
if filter == nil {
176+
continue
177+
}
178+
if err := filter.Validate(); err != nil {
179+
return err
180+
}
181+
}
182+
123183
// currently only validate cron string of preheat policy
124184
if s.Trigger != nil && s.Trigger.Type == TriggerTypeScheduled && len(s.Trigger.Settings.Cron) > 0 {
125185
if err := utils.ValidateCronString(s.Trigger.Settings.Cron); err != nil {

src/pkg/p2p/preheat/models/policy/policy_test.go

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,12 @@
1515
package policy
1616

1717
import (
18+
"strings"
1819
"testing"
1920

2021
"github.com/stretchr/testify/suite"
22+
23+
"github.com/goharbor/harbor/src/lib/errors"
2124
)
2225

2326
// PolicyTestSuite is a test suite for policy schema.
@@ -66,6 +69,124 @@ func (p *PolicyTestSuite) TestValidatePreheatPolicy() {
6669
p.NoError(p.schema.ValidatePreheatPolicy())
6770
}
6871

72+
// TestValidateFilterKind tests the pattern engine validation of the policy filters
73+
func (p *PolicyTestSuite) TestValidateFilterKind() {
74+
cases := []struct {
75+
name string
76+
filter *Filter
77+
wantErr bool
78+
}{
79+
{
80+
name: "absent kind",
81+
filter: &Filter{Type: FilterTypeRepository, Value: "**"},
82+
},
83+
{
84+
name: "explicit doublestar kind",
85+
filter: &Filter{Type: FilterTypeTag, Value: "prod*", Kind: FilterKindDoublestar},
86+
},
87+
{
88+
name: "regex repository",
89+
filter: &Filter{Type: FilterTypeRepository, Value: "library/.*", Kind: FilterKindRegex},
90+
},
91+
{
92+
name: "regex tag",
93+
filter: &Filter{Type: FilterTypeTag, Value: `v\d+\.\d+`, Kind: FilterKindRegex},
94+
},
95+
{
96+
name: "empty regex pattern",
97+
filter: &Filter{Type: FilterTypeTag, Value: "", Kind: FilterKindRegex},
98+
},
99+
{
100+
name: "unknown kind",
101+
filter: &Filter{Type: FilterTypeTag, Value: "**", Kind: "glob"},
102+
wantErr: true,
103+
},
104+
{
105+
name: "kind on a label filter",
106+
filter: &Filter{Type: FilterTypeLabel, Value: "prod", Kind: FilterKindRegex},
107+
wantErr: true,
108+
},
109+
{
110+
name: "kind on a signature filter",
111+
filter: &Filter{Type: FilterTypeSignature, Value: true, Kind: FilterKindDoublestar},
112+
wantErr: true,
113+
},
114+
{
115+
name: "kind on a vulnerability filter",
116+
filter: &Filter{Type: FilterTypeVulnerability, Value: 3, Kind: FilterKindRegex},
117+
wantErr: true,
118+
},
119+
{
120+
name: "invalid regex",
121+
filter: &Filter{Type: FilterTypeTag, Value: "[", Kind: FilterKindRegex},
122+
wantErr: true,
123+
},
124+
{
125+
name: "regex escaping the anchoring",
126+
filter: &Filter{Type: FilterTypeTag, Value: "foo)|(?:bar", Kind: FilterKindRegex},
127+
wantErr: true,
128+
},
129+
{
130+
name: "regex longer than the pattern limit",
131+
filter: &Filter{Type: FilterTypeTag, Value: strings.Repeat("a", 513), Kind: FilterKindRegex},
132+
wantErr: true,
133+
},
134+
{
135+
name: "regex at the pattern limit",
136+
filter: &Filter{Type: FilterTypeTag, Value: strings.Repeat("a", 512), Kind: FilterKindRegex},
137+
},
138+
{
139+
name: "regex value that isn't a string",
140+
filter: &Filter{Type: FilterTypeTag, Value: 100, Kind: FilterKindRegex},
141+
wantErr: true,
142+
},
143+
}
144+
145+
for _, c := range cases {
146+
p.Run(c.name, func() {
147+
s := &Schema{Filters: []*Filter{c.filter}, Trigger: &Trigger{Type: TriggerTypeManual}}
148+
err := s.ValidatePreheatPolicy()
149+
if !c.wantErr {
150+
p.NoError(err)
151+
return
152+
}
153+
p.Error(err)
154+
p.True(errors.IsErr(err, errors.BadRequestCode), "error is a bad request: %v", err)
155+
})
156+
}
157+
}
158+
159+
// TestFilterKindRoundTrip tests that the kind survives an encode/decode cycle and that a
160+
// policy stored without a kind keeps decoding
161+
func (p *PolicyTestSuite) TestFilterKindRoundTrip() {
162+
s := &Schema{
163+
Filters: []*Filter{
164+
{Type: FilterTypeRepository, Value: "library/.*", Kind: FilterKindRegex},
165+
{Type: FilterTypeTag, Value: "**"},
166+
},
167+
Trigger: &Trigger{Type: TriggerTypeManual},
168+
}
169+
p.NoError(s.Encode())
170+
p.Equal(`[{"type":"repository","value":"library/.*","kind":"regex"},{"type":"tag","value":"**"}]`, s.FiltersStr)
171+
172+
// a policy stored before the kind existed decodes into the doublestar default
173+
stored := &Schema{
174+
FiltersStr: `[{"type":"repository","value":"**"},{"type":"tag","value":"**"},{"type":"label","value":"test"}]`,
175+
TriggerStr: `{"type":"manual","trigger_setting":{"cron":""}}`,
176+
}
177+
p.NoError(stored.Decode())
178+
p.Len(stored.Filters, 3)
179+
for _, f := range stored.Filters {
180+
p.Empty(f.Kind)
181+
}
182+
p.NoError(stored.ValidatePreheatPolicy())
183+
184+
decoded := &Schema{FiltersStr: s.FiltersStr, TriggerStr: `{"type":"manual","trigger_setting":{"cron":""}}`}
185+
p.NoError(decoded.Decode())
186+
p.Equal(FilterKindRegex, decoded.Filters[0].Kind)
187+
p.Empty(decoded.Filters[1].Kind)
188+
}
189+
69190
// TestDecode tests decode.
70191
func (p *PolicyTestSuite) TestDecode() {
71192
s := &Schema{

src/pkg/p2p/preheat/policy/filter.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"github.com/goharbor/harbor/src/lib/selector"
2424
"github.com/goharbor/harbor/src/lib/selector/selectors/doublestar"
2525
"github.com/goharbor/harbor/src/lib/selector/selectors/label"
26+
regexpselector "github.com/goharbor/harbor/src/lib/selector/selectors/regexp"
2627
"github.com/goharbor/harbor/src/lib/selector/selectors/severity"
2728
"github.com/goharbor/harbor/src/lib/selector/selectors/signature"
2829
"github.com/goharbor/harbor/src/pkg/p2p/preheat/models/policy"
@@ -152,6 +153,12 @@ func buildFilter(f *policy.Filter) (selector.Selector, error) {
152153
return nil, errors.Errorf("pattern value is missing for filter: %s", f.Type)
153154
}
154155

156+
// Backstop for the write time validation: a policy stored by an older version, or
157+
// written past the API, still has to fail here instead of building a broken selector.
158+
if err := f.Validate(); err != nil {
159+
return nil, err
160+
}
161+
155162
// Current value type
156163
cvt := reflect.TypeOf(f.Value).Name()
157164

@@ -176,8 +183,14 @@ func buildFilter(f *policy.Filter) (selector.Selector, error) {
176183
// Build selectors
177184
switch f.Type {
178185
case policy.FilterTypeRepository:
186+
if f.Kind == policy.FilterKindRegex {
187+
return regexpselector.New(regexpselector.RepoMatches, f.Value, ""), nil
188+
}
179189
return doublestar.New(doublestar.RepoMatches, f.Value, ""), nil
180190
case policy.FilterTypeTag:
191+
if f.Kind == policy.FilterKindRegex {
192+
return regexpselector.New(regexpselector.Matches, f.Value, ""), nil
193+
}
181194
return doublestar.New(doublestar.Matches, f.Value, ""), nil
182195
case policy.FilterTypeLabel:
183196
return label.New(label.With, f.Value, ""), nil

0 commit comments

Comments
 (0)