Skip to content

Commit

Permalink
fix(sdk): avoid panic when parsing empty workflow (#6382)
Browse files Browse the repository at this point in the history
  • Loading branch information
sguiheux authored Dec 2, 2022
1 parent b8fd7db commit 8de3050
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 18 deletions.
36 changes: 22 additions & 14 deletions sdk/exportentities/v1/workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ type ConditionEntry struct {
LuaScript string `json:"script,omitempty" yaml:"script,omitempty"`
}

//WorkflowNodeCondition represents a condition to trigger ot not a pipeline in a workflow. Operator can be =, !=, regex
// WorkflowNodeCondition represents a condition to trigger ot not a pipeline in a workflow. Operator can be =, !=, regex
type PlainConditionEntry struct {
Variable string `json:"variable" yaml:"variable"`
Operator string `json:"operator" yaml:"operator"`
Expand All @@ -79,20 +79,23 @@ func (w Workflow) Entries() map[string]NodeEntry {
return w.Workflow
}

singleEntry := NodeEntry{
ApplicationName: w.ApplicationName,
EnvironmentName: w.EnvironmentName,
ProjectIntegrationName: w.ProjectIntegrationName,
PipelineName: w.PipelineName,
Conditions: w.Conditions,
When: w.When,
Payload: w.Payload,
Parameters: w.Parameters,
OneAtATime: w.OneAtATime,
}
return map[string]NodeEntry{
w.PipelineName: singleEntry,
if w.PipelineName != "" {
singleEntry := NodeEntry{
ApplicationName: w.ApplicationName,
EnvironmentName: w.EnvironmentName,
ProjectIntegrationName: w.ProjectIntegrationName,
PipelineName: w.PipelineName,
Conditions: w.Conditions,
When: w.When,
Payload: w.Payload,
Parameters: w.Parameters,
OneAtATime: w.OneAtATime,
}
return map[string]NodeEntry{
w.PipelineName: singleEntry,
}
}
return nil
}

func (w Workflow) CheckValidity() error {
Expand Down Expand Up @@ -209,6 +212,11 @@ func (w Workflow) GetWorkflow() (*sdk.Workflow, error) {

rand.Seed(time.Now().Unix())
entries := w.Entries()

if len(entries) == 0 {
return nil, sdk.NewErrorFrom(sdk.ErrWorkflowInvalid, "a workflow must contains at least 1 pipeline")
}

var attempt int
fakeID := rand.Int63n(5000)
// attempt is there to avoid infinite loop, but it should not happened because we check validity and dependencies earlier
Expand Down
22 changes: 20 additions & 2 deletions sdk/exportentities/v1/workflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ func TestWorkflow_GetWorkflow(t *testing.T) {
{
name: "Simple workflow with mutex should not raise an error",
fields: fields{
Version: exportentities.ActionVersion1,
Version: exportentities.WorkflowVersion1,
Name: "myworkflow",
PipelineName: "pipeline",
OneAtATime: &strue,
Expand All @@ -249,8 +249,8 @@ func TestWorkflow_GetWorkflow(t *testing.T) {
fields: fields{
Version: exportentities.ActionVersion1,
Name: "myworkflow",
PipelineName: "pipeline",
Description: "this is my description",
PipelineName: "pipeline",
PipelineHooks: []v1.HookEntry{
{
Model: "Scheduler",
Expand Down Expand Up @@ -791,6 +791,24 @@ func TestWorkflow_GetWorkflow(t *testing.T) {
},
},
},
{
name: "Workflow v2 with no nodes",
fields: fields{
Name: "myworkflow",
Version: exportentities.WorkflowVersion2,
Workflow: map[string]v1.NodeEntry{},
},
wantErr: true,
},
{
name: "Workflow v1 with no nodes",
fields: fields{
Name: "myworkflow",
Version: exportentities.WorkflowVersion1,
Workflow: map[string]v1.NodeEntry{},
},
wantErr: true,
},
}

for _, tt := range tsts {
Expand Down
8 changes: 6 additions & 2 deletions sdk/exportentities/v2/workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ type ConditionEntry struct {
LuaScript string `json:"script,omitempty" yaml:"script,omitempty"`
}

//WorkflowNodeCondition represents a condition to trigger ot not a pipeline in a workflow. Operator can be =, !=, regex
// WorkflowNodeCondition represents a condition to trigger ot not a pipeline in a workflow. Operator can be =, !=, regex
type PlainConditionEntry struct {
Variable string `json:"variable" yaml:"variable"`
Operator string `json:"operator" yaml:"operator"`
Expand Down Expand Up @@ -113,7 +113,7 @@ func (h HookEntry) IsDefault(model sdk.WorkflowHookModel) bool {

type ExportOptions func(w sdk.Workflow, exportedWorkflow *Workflow) error

//NewWorkflow creates a new exportable workflow
// NewWorkflow creates a new exportable workflow
func NewWorkflow(ctx context.Context, w sdk.Workflow, version string, opts ...ExportOptions) (Workflow, error) {
exportedWorkflow := Workflow{}
exportedWorkflow.Name = w.Name
Expand Down Expand Up @@ -433,6 +433,10 @@ func (w Workflow) GetWorkflow(ctx context.Context) (*sdk.Workflow, error) {
wf.RetentionPolicy = *w.RetentionPolicy
}

if len(w.Workflow) == 0 {
return nil, sdk.NewErrorFrom(sdk.ErrWorkflowInvalid, "a workflow must contains at least 1 pipeline")
}

r := rand.New(rand.NewSource(time.Now().Unix()))
var attempt int
fakeID := r.Int63n(5000)
Expand Down

0 comments on commit 8de3050

Please sign in to comment.