fix: schema validation for job if functions (#2446)

* fix: schema validation for job if functions * Add Tests * Update pkg/schema/schema.go Co-authored-by: Josh Soref <[email protected]> * Update pkg/schema/schema.go --------- Co-authored-by: Josh Soref <[email protected]>
nektos · Sep 10, 2024 · 4143017 · 4143017
1 parent d8b6f61
commit 4143017
Show file tree

Hide file tree

Showing 2 changed files with 117 additions and 7 deletions.
diff --git a/pkg/schema/schema.go b/pkg/schema/schema.go
@@ -5,7 +5,9 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"math"
 	"regexp"
+	"strconv"
 	"strings"
 
 	"github.com/rhysd/actionlint"
@@ -18,6 +20,8 @@ var workflowSchema string
 //go:embed action_schema.json
 var actionSchema string
 
+var functions = regexp.MustCompile(`^([a-zA-Z0-9_]+)\(([0-9]+),([0-9]+|MAX)\)$`)
+
 type Schema struct {
 	Definitions map[string]Definition
 }
@@ -138,10 +142,10 @@ func (s *Node) checkSingleExpression(exprNode actionlint.ExprNode) error {
 			for _, v := range *funcs {
 				if strings.EqualFold(funcCallNode.Callee, v.name) {
 					if v.min > len(funcCallNode.Args) {
-						err = errors.Join(err, fmt.Errorf("Missing parameters for %s expected > %v got %v", funcCallNode.Callee, v.min, len(funcCallNode.Args)))
+						err = errors.Join(err, fmt.Errorf("Missing parameters for %s expected >= %v got %v", funcCallNode.Callee, v.min, len(funcCallNode.Args)))
 					}
 					if v.max < len(funcCallNode.Args) {
-						err = errors.Join(err, fmt.Errorf("To many parameters for %s expected < %v got %v", funcCallNode.Callee, v.max, len(funcCallNode.Args)))
+						err = errors.Join(err, fmt.Errorf("Too many parameters for %s expected <= %v got %v", funcCallNode.Callee, v.max, len(funcCallNode.Args)))
 					}
 					return
 				}
@@ -174,11 +178,22 @@ func (s *Node) GetFunctions() *[]FunctionInfo {
 		if i == -1 {
 			continue
 		}
-		fun := FunctionInfo{
-			name: v[:i],
-		}
-		if n, err := fmt.Sscanf(v[i:], "(%d,%d)", &fun.min, &fun.max); n == 2 && err == nil {
-			*funcs = append(*funcs, fun)
+		smatch := functions.FindStringSubmatch(v)
+		if len(smatch) > 0 {
+			functionName := smatch[1]
+			minParameters, _ := strconv.ParseInt(smatch[2], 10, 32)
+			maxParametersRaw := smatch[3]
+			var maxParameters int64
+			if strings.EqualFold(maxParametersRaw, "MAX") {
+				maxParameters = math.MaxInt32
+			} else {
+				maxParameters, _ = strconv.ParseInt(maxParametersRaw, 10, 32)
+			}
+			*funcs = append(*funcs, FunctionInfo{
+				name: functionName,
+				min:  int(minParameters),
+				max:  int(maxParameters),
+			})
 		}
 	}
 	return funcs
@@ -220,6 +235,9 @@ func AddFunction(funcs *[]FunctionInfo, s string, i1, i2 int) {
 }
 
 func (s *Node) UnmarshalYAML(node *yaml.Node) error {
+	if node != nil && node.Kind == yaml.DocumentNode {
+		return s.UnmarshalYAML(node.Content[0])
+	}
 	def := s.Schema.GetDefinition(s.Definition)
 	if s.Context == nil {
 		s.Context = def.Context

diff --git a/pkg/schema/schema_test.go b/pkg/schema/schema_test.go
@@ -0,0 +1,92 @@
+package schema
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"gopkg.in/yaml.v3"
+)
+
+func TestAdditionalFunctions(t *testing.T) {
+	var node yaml.Node
+	err := yaml.Unmarshal([]byte(`
+on: push
+jobs:
+  job-with-condition:
+    runs-on: self-hosted
+    if: success() || success('joba', 'jobb') || failure() || failure('joba', 'jobb') || always() || cancelled()
+    steps:
+    - run: exit 0
+`), &node)
+	if !assert.NoError(t, err) {
+		return
+	}
+	err = (&Node{
+		Definition: "workflow-root-strict",
+		Schema:     GetWorkflowSchema(),
+	}).UnmarshalYAML(&node)
+	assert.NoError(t, err)
+}
+
+func TestAdditionalFunctionsFailure(t *testing.T) {
+	var node yaml.Node
+	err := yaml.Unmarshal([]byte(`
+on: push
+jobs:
+  job-with-condition:
+    runs-on: self-hosted
+    if: success() || success('joba', 'jobb') || failure() || failure('joba', 'jobb') || always('error')
+    steps:
+    - run: exit 0
+`), &node)
+	if !assert.NoError(t, err) {
+		return
+	}
+	err = (&Node{
+		Definition: "workflow-root-strict",
+		Schema:     GetWorkflowSchema(),
+	}).UnmarshalYAML(&node)
+	assert.Error(t, err)
+}
+
+func TestAdditionalFunctionsSteps(t *testing.T) {
+	var node yaml.Node
+	err := yaml.Unmarshal([]byte(`
+on: push
+jobs:
+  job-with-condition:
+    runs-on: self-hosted
+    steps:
+    - run: exit 0
+      if: success() || failure() || always()
+`), &node)
+	if !assert.NoError(t, err) {
+		return
+	}
+	err = (&Node{
+		Definition: "workflow-root-strict",
+		Schema:     GetWorkflowSchema(),
+	}).UnmarshalYAML(&node)
+	assert.NoError(t, err)
+}
+
+func TestAdditionalFunctionsStepsExprSyntax(t *testing.T) {
+	var node yaml.Node
+	err := yaml.Unmarshal([]byte(`
+on: push
+jobs:
+  job-with-condition:
+    runs-on: self-hosted
+    steps:
+    - run: exit 0
+      if: ${{ success() || failure() || always() }}
+`), &node)
+	if !assert.NoError(t, err) {
+		return
+	}
+	err = (&Node{
+		Definition: "workflow-root-strict",
+		Schema:     GetWorkflowSchema(),
+	}).UnmarshalYAML(&node)
+	assert.NoError(t, err)
+}