Skip to content

Commit dbe19fd

Browse files
ehl-jfehl-jf
authored
Better validation of FilterCriteria (#15)
1 parent 5c98bd4 commit dbe19fd

File tree

7 files changed

+187
-36
lines changed

7 files changed

+187
-36
lines changed

‎commands/common/manifest.go‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,36 @@ func DecryptManifestSecrets(mf *model.Manifest, withPassword ...string) error {
145145
return nil
146146
}
147147

148+
func ValidateFilterCriteria(c *model.FilterCriteria, actionMeta *model.ActionMetadata) error {
149+
if !actionMeta.MandatoryFilter {
150+
return nil
151+
}
152+
153+
if actionMeta.FilterType == model.FilterTypeSchedule {
154+
if c.ArtifactFilterCriteria != nil {
155+
return invalidManifestErr("scheduled event cannot have artifact filter criteria")
156+
}
157+
if err := ValidateScheduleCriteria(c.Schedule); err != nil {
158+
return fmt.Errorf("manifest validation failed: %w", err)
159+
}
160+
}
161+
162+
if actionMeta.FilterType == model.FilterTypeRepo {
163+
if c.Schedule != nil {
164+
return invalidManifestErr("repository events cannot have schedule criteria")
165+
}
166+
if c.ArtifactFilterCriteria == nil {
167+
return invalidManifestErr("missing artifact filter criteria")
168+
}
169+
hasAnyRepo := c.ArtifactFilterCriteria.AnyFederated || c.ArtifactFilterCriteria.AnyLocal || c.ArtifactFilterCriteria.AnyRemote
170+
if len(c.ArtifactFilterCriteria.RepoKeys) == 0 && !hasAnyRepo {
171+
return invalidManifestErr("at least one repository key must be provided")
172+
}
173+
}
174+
175+
return nil
176+
}
177+
148178
var cronParser = cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow)
149179

150180
func ValidateScheduleCriteria(c *model.ScheduleFilterCriteria) error {

‎commands/common/manifest_test.go‎

Lines changed: 118 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ var manifestSample = &model.Manifest{
3030
"hidden2": "hidden2.value",
3131
},
3232
FilterCriteria: model.FilterCriteria{
33-
ArtifactFilterCriteria: model.ArtifactFilterCriteria{
33+
ArtifactFilterCriteria: &model.ArtifactFilterCriteria{
3434
RepoKeys: []string{
3535
"my-repo-local",
3636
},
@@ -293,6 +293,123 @@ func TestManifest_DecryptSecrets(t *testing.T) {
293293
}
294294
}
295295

296+
func TestManifest_ValidateFilterCriteria(t *testing.T) {
297+
tests := []struct {
298+
name string
299+
criteria *model.FilterCriteria
300+
actionMeta *model.ActionMetadata
301+
wantErr error
302+
}{
303+
{
304+
name: "no mandatory filters",
305+
actionMeta: &model.ActionMetadata{
306+
FilterType: model.FilterTypeSchedule,
307+
MandatoryFilter: false,
308+
},
309+
criteria: &model.FilterCriteria{
310+
Schedule: &model.ScheduleFilterCriteria{},
311+
ArtifactFilterCriteria: &model.ArtifactFilterCriteria{},
312+
},
313+
},
314+
{
315+
name: "valid schedule criteria",
316+
actionMeta: &model.ActionMetadata{
317+
FilterType: model.FilterTypeSchedule,
318+
MandatoryFilter: true,
319+
},
320+
criteria: &model.FilterCriteria{
321+
Schedule: &model.ScheduleFilterCriteria{
322+
Cron: "0 1 1 * *",
323+
Timezone: "UTC",
324+
},
325+
},
326+
},
327+
{
328+
name: "valid artifact filter criteria",
329+
actionMeta: &model.ActionMetadata{
330+
FilterType: model.FilterTypeRepo,
331+
MandatoryFilter: true,
332+
},
333+
criteria: &model.FilterCriteria{
334+
ArtifactFilterCriteria: &model.ArtifactFilterCriteria{
335+
RepoKeys: []string{"my-repo-local"},
336+
},
337+
},
338+
},
339+
{
340+
name: "valid with any local",
341+
actionMeta: &model.ActionMetadata{
342+
FilterType: model.FilterTypeRepo,
343+
MandatoryFilter: true,
344+
},
345+
criteria: &model.FilterCriteria{
346+
ArtifactFilterCriteria: &model.ArtifactFilterCriteria{
347+
AnyLocal: true,
348+
},
349+
},
350+
},
351+
{
352+
name: "valid with any federated",
353+
actionMeta: &model.ActionMetadata{
354+
FilterType: model.FilterTypeRepo,
355+
MandatoryFilter: true,
356+
},
357+
criteria: &model.FilterCriteria{
358+
ArtifactFilterCriteria: &model.ArtifactFilterCriteria{
359+
AnyFederated: true,
360+
},
361+
},
362+
},
363+
{
364+
name: "valid with any local",
365+
actionMeta: &model.ActionMetadata{
366+
FilterType: model.FilterTypeRepo,
367+
MandatoryFilter: true,
368+
},
369+
criteria: &model.FilterCriteria{
370+
ArtifactFilterCriteria: &model.ArtifactFilterCriteria{
371+
AnyRemote: true,
372+
},
373+
},
374+
},
375+
{
376+
name: "only schedule for scheduled action",
377+
actionMeta: &model.ActionMetadata{
378+
FilterType: model.FilterTypeSchedule,
379+
MandatoryFilter: true,
380+
},
381+
criteria: &model.FilterCriteria{
382+
Schedule: &model.ScheduleFilterCriteria{},
383+
ArtifactFilterCriteria: &model.ArtifactFilterCriteria{},
384+
},
385+
wantErr: invalidManifestErr("scheduled event cannot have artifact filter criteria"),
386+
},
387+
{
388+
name: "only artifactFilterCriteria for non-scheduled action",
389+
actionMeta: &model.ActionMetadata{
390+
FilterType: model.FilterTypeRepo,
391+
MandatoryFilter: true,
392+
},
393+
criteria: &model.FilterCriteria{
394+
Schedule: &model.ScheduleFilterCriteria{},
395+
ArtifactFilterCriteria: &model.ArtifactFilterCriteria{},
396+
},
397+
wantErr: invalidManifestErr("repository events cannot have schedule criteria"),
398+
},
399+
}
400+
401+
for _, tt := range tests {
402+
t.Run(tt.name, func(t *testing.T) {
403+
err := ValidateFilterCriteria(tt.criteria, tt.actionMeta)
404+
if tt.wantErr == nil {
405+
assert.NoError(t, err)
406+
} else {
407+
assert.EqualError(t, err, tt.wantErr.Error())
408+
}
409+
})
410+
}
411+
}
412+
296413
func TestManifest_ValidateScheduleCriteria(t *testing.T) {
297414
tests := []struct {
298415
name string

‎commands/deploy_cmd.go‎

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,15 @@ import (
1515
)
1616

1717
type deployRequest struct {
18-
Key string `json:"key"`
19-
Description string `json:"description"`
20-
Enabled bool `json:"enabled"`
21-
Debug bool `json:"debug"`
22-
SourceCode string `json:"sourceCode"`
23-
Action string `json:"action"`
24-
FilterCriteria model.FilterCriteria `json:"filterCriteria,omitempty"`
25-
Secrets []*model.Secret `json:"secrets"`
26-
ProjectKey string `json:"projectKey"`
18+
Key string `json:"key"`
19+
Description string `json:"description"`
20+
Enabled bool `json:"enabled"`
21+
Debug bool `json:"debug"`
22+
SourceCode string `json:"sourceCode"`
23+
Action string `json:"action"`
24+
FilterCriteria *model.FilterCriteria `json:"filterCriteria,omitempty"`
25+
Secrets []*model.Secret `json:"secrets"`
26+
ProjectKey string `json:"projectKey"`
2727
}
2828

2929
func GetDeployCommand() components.Command {
@@ -61,10 +61,8 @@ func GetDeployCommand() components.Command {
6161
return err
6262
}
6363

64-
if actionMeta.MandatoryFilter && actionMeta.FilterType == model.FilterTypeSchedule {
65-
if err = common.ValidateScheduleCriteria(&manifest.FilterCriteria.Schedule); err != nil {
66-
return fmt.Errorf("manifest validation failed: %w", err)
67-
}
64+
if err = common.ValidateFilterCriteria(&manifest.FilterCriteria, actionMeta); err != nil {
65+
return err
6866
}
6967

7068
if !c.GetBoolFlagValue(model.FlagNoSecrets) {
@@ -73,18 +71,18 @@ func GetDeployCommand() components.Command {
7371
}
7472
}
7573

76-
return runDeployCommand(c, manifest, server.GetUrl(), server.GetAccessToken())
74+
return runDeployCommand(c, manifest, actionMeta, server.GetUrl(), server.GetAccessToken())
7775
},
7876
}
7977
}
8078

81-
func runDeployCommand(ctx *components.Context, manifest *model.Manifest, serverUrl string, token string) error {
79+
func runDeployCommand(ctx *components.Context, manifest *model.Manifest, actionMeta *model.ActionMetadata, serverUrl string, token string) error {
8280
existingWorker, err := common.FetchWorkerDetails(ctx, serverUrl, token, manifest.Name, manifest.ProjectKey)
8381
if err != nil {
8482
return err
8583
}
8684

87-
body, err := prepareDeployRequest(ctx, manifest, existingWorker)
85+
body, err := prepareDeployRequest(ctx, manifest, actionMeta, existingWorker)
8886
if err != nil {
8987
return err
9088
}
@@ -126,7 +124,7 @@ func runDeployCommand(ctx *components.Context, manifest *model.Manifest, serverU
126124
return err
127125
}
128126

129-
func prepareDeployRequest(ctx *components.Context, manifest *model.Manifest, existingWorker *model.WorkerDetails) (*deployRequest, error) {
127+
func prepareDeployRequest(ctx *components.Context, manifest *model.Manifest, actionMeta *model.ActionMetadata, existingWorker *model.WorkerDetails) (*deployRequest, error) {
130128
sourceCode, err := common.ReadSourceCode(manifest)
131129
if err != nil {
132130
return nil, err
@@ -140,15 +138,18 @@ func prepareDeployRequest(ctx *components.Context, manifest *model.Manifest, exi
140138
}
141139

142140
payload := &deployRequest{
143-
Key: manifest.Name,
144-
Action: manifest.Action,
145-
Description: manifest.Description,
146-
Enabled: manifest.Enabled,
147-
Debug: manifest.Debug,
148-
FilterCriteria: manifest.FilterCriteria,
149-
SourceCode: sourceCode,
150-
Secrets: secrets,
151-
ProjectKey: manifest.ProjectKey,
141+
Key: manifest.Name,
142+
Action: manifest.Action,
143+
Description: manifest.Description,
144+
Enabled: manifest.Enabled,
145+
Debug: manifest.Debug,
146+
SourceCode: sourceCode,
147+
Secrets: secrets,
148+
ProjectKey: manifest.ProjectKey,
149+
}
150+
151+
if actionMeta.MandatoryFilter {
152+
payload.FilterCriteria = &manifest.FilterCriteria
152153
}
153154

154155
return payload, nil

‎commands/deploy_cmd_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ func TestDeployCommand(t *testing.T) {
115115
workerName: "wk-3",
116116
serverBehavior: common.NewServerStub(t).WithGetOneEndpoint(),
117117
patchManifest: func(mf *model.Manifest) {
118-
mf.FilterCriteria.Schedule = model.ScheduleFilterCriteria{
118+
mf.FilterCriteria.Schedule = &model.ScheduleFilterCriteria{
119119
Cron: "1h",
120120
Timezone: "Asia/New_York",
121121
}
@@ -231,8 +231,8 @@ func getExpectedDeployRequestForAction(
231231
require.NoError(t, err)
232232

233233
if actionMeta.MandatoryFilter && actionMeta.FilterType == model.FilterTypeRepo {
234-
r.FilterCriteria = model.FilterCriteria{
235-
ArtifactFilterCriteria: model.ArtifactFilterCriteria{
234+
r.FilterCriteria = &model.FilterCriteria{
235+
ArtifactFilterCriteria: &model.ArtifactFilterCriteria{
236236
RepoKeys: []string{"example-repo-local"},
237237
},
238238
}

‎commands/edit_schedule_cmd.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,12 @@ func (c *editScheduleCommand) run() error {
5050
return fmt.Errorf("the worker is not a SCHEDULED_EVENT worker")
5151
}
5252

53-
newCriteria := model.ScheduleFilterCriteria{
53+
newCriteria := &model.ScheduleFilterCriteria{
5454
Cron: c.ctx.GetStringFlagValue(flagScheduleCron),
5555
Timezone: c.ctx.GetStringFlagValue(flagScheduleTimezone),
5656
}
5757

58-
if err = common.ValidateScheduleCriteria(&newCriteria); err != nil {
58+
if err = common.ValidateScheduleCriteria(newCriteria); err != nil {
5959
return fmt.Errorf("invalid schedule provided: %w", err)
6060
}
6161

‎model/manifest.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
package model
22

33
type ArtifactFilterCriteria struct {
4-
RepoKeys []string `json:"repoKeys,omitempty"`
4+
RepoKeys []string `json:"repoKeys,omitempty"`
5+
AnyLocal bool `json:"anyLocal,omitempty"`
6+
AnyFederated bool `json:"anyFederated,omitempty"`
7+
AnyRemote bool `json:"anyRemote,omitempty"`
58
}
69

710
type ScheduleFilterCriteria struct {
@@ -10,8 +13,8 @@ type ScheduleFilterCriteria struct {
1013
}
1114

1215
type FilterCriteria struct {
13-
ArtifactFilterCriteria ArtifactFilterCriteria `json:"artifactFilterCriteria,omitempty"`
14-
Schedule ScheduleFilterCriteria `json:"schedule,omitempty"`
16+
ArtifactFilterCriteria *ArtifactFilterCriteria `json:"artifactFilterCriteria,omitempty"`
17+
Schedule *ScheduleFilterCriteria `json:"schedule,omitempty"`
1518
}
1619

1720
type Secrets map[string]string

‎test/commands/list_cmd_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func TestListCommand(t *testing.T) {
5454
SourceCode: `export default async function() { return { "status": "OK" } }`,
5555
Action: "BEFORE_DOWNLOAD",
5656
FilterCriteria: model.FilterCriteria{
57-
ArtifactFilterCriteria: model.ArtifactFilterCriteria{
57+
ArtifactFilterCriteria: &model.ArtifactFilterCriteria{
5858
RepoKeys: []string{"example-repo-local"},
5959
},
6060
},

0 commit comments

Comments
 (0)