Skip to content

Commit

Permalink
[KS-136] Disallow non-trigger steps with no dependent ref (#12742)
Browse files Browse the repository at this point in the history
  • Loading branch information
cedric-cordenier committed Apr 8, 2024
1 parent 14bd537 commit aca8f7d
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 0 deletions.
1 change: 1 addition & 0 deletions core/services/workflows/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,7 @@ func NewEngine(cfg Config) (engine *Engine, err error) {
// - that there are no step `ref` called `trigger` as this is reserved for any triggers
// - that there are no duplicate `ref`s
// - that the `ref` for any triggers is empty -- and filled in with `trigger`
// - that the resulting graph is strongly connected (i.e. no disjointed subgraphs exist)
// - etc.

workflow, err := Parse(cfg.Spec)
Expand Down
5 changes: 5 additions & 0 deletions core/services/workflows/models.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package workflows

import (
"errors"
"fmt"

"github.com/dominikbraun/graph"
Expand Down Expand Up @@ -182,6 +183,10 @@ func Parse(yamlWorkflow string) (*workflow, error) {
}
step.dependencies = refs

if stepRef != keywordTrigger && len(refs) == 0 {
return nil, errors.New("all non-trigger steps must have a dependent ref")
}

for _, r := range refs {
innerErr = g.AddEdge(r, step.Ref)
if innerErr != nil {
Expand Down
25 changes: 25 additions & 0 deletions core/services/workflows/models_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,31 @@ targets:
"a-target": {},
},
},
{
name: "non-trigger step with no dependent refs",
yaml: `
triggers:
- type: "a-trigger"
- type: "a-second-trigger"
actions:
- type: "an-action"
ref: "an-action"
inputs:
hello: "world"
consensus:
- type: "a-consensus"
ref: "a-consensus"
inputs:
trigger_output: $(trigger.outputs)
action_output: $(an-action.outputs)
targets:
- type: "a-target"
ref: "a-target"
inputs:
consensus_output: $(a-consensus.outputs)
`,
errMsg: "all non-trigger steps must have a dependent ref",
},
}

for _, tc := range testCases {
Expand Down

0 comments on commit aca8f7d

Please sign in to comment.