mirror of
https://github.com/j3ssie/osmedeus.git
synced 2026-10-01 05:55:05 +02:00
feat: downgrade trigger input variables to info-level in linter
- Add tracking of trigger input variables and recognize their availability at runtime - Downgrade undefined variable warnings to info-level when variables come from event trigger inputs (both new and legacy syntax) - Add event envelope variables (EventTopic, EventSource, EventTimestamp, etc.) as recognized built-ins when workflow has event triggers - Include comprehensive test coverage for trigger input scenarios, event triggers, and mixed variable definitions
This commit is contained in:
+64
-17
@@ -204,12 +204,37 @@ func (r *UndefinedVariableRule) Check(wast *WorkflowAST) []LintIssue {
|
||||
defined[param.Name] = true
|
||||
}
|
||||
|
||||
// Collect variables provided by trigger inputs
|
||||
triggerVars := make(map[string]bool)
|
||||
hasEventTrigger := false
|
||||
for _, trigger := range w.Triggers {
|
||||
if trigger.On == core.TriggerEvent {
|
||||
hasEventTrigger = true
|
||||
}
|
||||
if trigger.Input.HasVars() {
|
||||
for varName := range trigger.Input.Vars {
|
||||
triggerVars[varName] = true
|
||||
}
|
||||
}
|
||||
// Legacy syntax
|
||||
if trigger.Input.Name != "" {
|
||||
triggerVars[trigger.Input.Name] = true
|
||||
}
|
||||
}
|
||||
|
||||
// If workflow has event triggers, add event envelope variables as defined
|
||||
if hasEventTrigger {
|
||||
for _, v := range []string{"EventEnvelope", "EventTopic", "EventSource", "EventDataType", "EventTimestamp", "EventData"} {
|
||||
defined[v] = true
|
||||
}
|
||||
}
|
||||
|
||||
// Check each step for undefined variables
|
||||
for i, step := range w.Steps {
|
||||
stepPrefix := fmt.Sprintf("steps[%d]", i)
|
||||
|
||||
// Check all fields of this step
|
||||
checkStepFieldsForUndefinedVars(&step, stepPrefix, defined, wast, r, &issues)
|
||||
checkStepFieldsForUndefinedVars(&step, stepPrefix, defined, triggerVars, hasEventTrigger, wast, r, &issues)
|
||||
|
||||
// Recurse into foreach inner step with scoped defined copy
|
||||
if step.Step != nil {
|
||||
@@ -219,12 +244,12 @@ func (r *UndefinedVariableRule) Check(wast *WorkflowAST) []LintIssue {
|
||||
}
|
||||
// _id_ is always available in foreach inner steps
|
||||
innerDefined["_id_"] = true
|
||||
checkStepFieldsForUndefinedVars(step.Step, stepPrefix+".step", innerDefined, wast, r, &issues)
|
||||
checkStepFieldsForUndefinedVars(step.Step, stepPrefix+".step", innerDefined, triggerVars, hasEventTrigger, wast, r, &issues)
|
||||
}
|
||||
|
||||
// Recurse into parallel_steps
|
||||
for j := range step.ParallelSteps {
|
||||
checkStepFieldsForUndefinedVars(&step.ParallelSteps[j], fmt.Sprintf("%s.parallel_steps[%d]", stepPrefix, j), defined, wast, r, &issues)
|
||||
checkStepFieldsForUndefinedVars(&step.ParallelSteps[j], fmt.Sprintf("%s.parallel_steps[%d]", stepPrefix, j), defined, triggerVars, hasEventTrigger, wast, r, &issues)
|
||||
}
|
||||
|
||||
// After processing this step, add its exports to defined
|
||||
@@ -699,9 +724,9 @@ func copyDefinedMap(m map[string]bool) map[string]bool {
|
||||
|
||||
// checkStepFieldsForUndefinedVars checks ALL template-renderable fields of a single step.
|
||||
// stepPrefix is the field path prefix (e.g. "steps[0]" or "steps[0].step").
|
||||
func checkStepFieldsForUndefinedVars(step *core.Step, stepPrefix string, defined map[string]bool, wast *WorkflowAST, rule *UndefinedVariableRule, issues *[]LintIssue) {
|
||||
func checkStepFieldsForUndefinedVars(step *core.Step, stepPrefix string, defined map[string]bool, triggerVars map[string]bool, hasEventTrigger bool, wast *WorkflowAST, rule *UndefinedVariableRule, issues *[]LintIssue) {
|
||||
check := func(s, field string) {
|
||||
checkStringForUndefinedVars(s, field, defined, wast, rule, issues)
|
||||
checkStringForUndefinedVars(s, field, defined, triggerVars, hasEventTrigger, wast, rule, issues)
|
||||
}
|
||||
checkSlice := func(slice []string, fieldBase string) {
|
||||
for j, s := range slice {
|
||||
@@ -802,24 +827,46 @@ func checkStepFieldsForUndefinedVars(step *core.Step, stepPrefix string, defined
|
||||
}
|
||||
}
|
||||
|
||||
func checkStringForUndefinedVars(s, field string, defined map[string]bool, wast *WorkflowAST, rule *UndefinedVariableRule, issues *[]LintIssue) {
|
||||
func checkStringForUndefinedVars(s, field string, defined map[string]bool, triggerVars map[string]bool, hasEventTrigger bool, wast *WorkflowAST, rule *UndefinedVariableRule, issues *[]LintIssue) {
|
||||
if s == "" {
|
||||
return
|
||||
}
|
||||
|
||||
for _, v := range extractVariables(s) {
|
||||
if !defined[v] && !builtInVariables[v] {
|
||||
line, col := wast.GetNodePosition(field)
|
||||
suggestion := findSimilarVariable(v, defined)
|
||||
*issues = append(*issues, LintIssue{
|
||||
Rule: rule.Name(),
|
||||
Severity: rule.Severity(),
|
||||
Message: fmt.Sprintf("Variable '%s' is not defined", v),
|
||||
Suggestion: suggestion,
|
||||
Line: line,
|
||||
Column: col,
|
||||
Field: field,
|
||||
})
|
||||
if triggerVars[v] {
|
||||
line, col := wast.GetNodePosition(field)
|
||||
*issues = append(*issues, LintIssue{
|
||||
Rule: rule.Name(),
|
||||
Severity: SeverityInfo,
|
||||
Message: fmt.Sprintf("Variable '%s' is provided by trigger input (not statically defined)", v),
|
||||
Line: line,
|
||||
Column: col,
|
||||
Field: field,
|
||||
})
|
||||
} else if hasEventTrigger {
|
||||
line, col := wast.GetNodePosition(field)
|
||||
*issues = append(*issues, LintIssue{
|
||||
Rule: rule.Name(),
|
||||
Severity: SeverityInfo,
|
||||
Message: fmt.Sprintf("Variable '%s' may be provided by event data at runtime", v),
|
||||
Line: line,
|
||||
Column: col,
|
||||
Field: field,
|
||||
})
|
||||
} else {
|
||||
line, col := wast.GetNodePosition(field)
|
||||
suggestion := findSimilarVariable(v, defined)
|
||||
*issues = append(*issues, LintIssue{
|
||||
Rule: rule.Name(),
|
||||
Severity: rule.Severity(),
|
||||
Message: fmt.Sprintf("Variable '%s' is not defined", v),
|
||||
Suggestion: suggestion,
|
||||
Line: line,
|
||||
Column: col,
|
||||
Field: field,
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -919,3 +919,249 @@ func searchSubstring(s, substr string) bool {
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func TestUndefinedVariableRule_TriggerInputVars(t *testing.T) {
|
||||
rule := &UndefinedVariableRule{}
|
||||
|
||||
t.Run("trigger input vars downgraded to info", func(t *testing.T) {
|
||||
ast := parseTestWorkflow(t, `
|
||||
name: event-handler
|
||||
kind: module
|
||||
params:
|
||||
- name: target
|
||||
triggers:
|
||||
- name: on-new-asset
|
||||
on: event
|
||||
enabled: true
|
||||
event:
|
||||
topic: assets.new
|
||||
input:
|
||||
source: event_data.source
|
||||
description: event_data.desc
|
||||
steps:
|
||||
- name: process
|
||||
type: bash
|
||||
command: echo "{{target}} {{source}} {{description}}"
|
||||
`)
|
||||
issues := rule.Check(ast)
|
||||
|
||||
// Should have issues for source and description, but at info severity
|
||||
var infoIssues, warningIssues []LintIssue
|
||||
for _, issue := range issues {
|
||||
if issue.Rule == "undefined-variable" {
|
||||
if issue.Severity == SeverityInfo {
|
||||
infoIssues = append(infoIssues, issue)
|
||||
} else if issue.Severity == SeverityWarning {
|
||||
warningIssues = append(warningIssues, issue)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
assert.Len(t, infoIssues, 2, "should have 2 info-level issues for trigger vars")
|
||||
assert.Empty(t, warningIssues, "should have no warning-level undefined-variable issues")
|
||||
|
||||
// Verify messages mention trigger input
|
||||
for _, issue := range infoIssues {
|
||||
assert.Contains(t, issue.Message, "provided by trigger input")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("legacy trigger input syntax downgraded to info", func(t *testing.T) {
|
||||
ast := parseTestWorkflow(t, `
|
||||
name: event-handler-legacy
|
||||
kind: module
|
||||
triggers:
|
||||
- name: on-webhook
|
||||
on: event
|
||||
enabled: true
|
||||
event:
|
||||
topic: webhook.received
|
||||
input:
|
||||
type: event_data
|
||||
field: url
|
||||
name: webhook_target
|
||||
steps:
|
||||
- name: process
|
||||
type: bash
|
||||
command: echo "{{webhook_target}}"
|
||||
`)
|
||||
issues := rule.Check(ast)
|
||||
|
||||
var infoIssues, warningIssues []LintIssue
|
||||
for _, issue := range issues {
|
||||
if issue.Rule == "undefined-variable" {
|
||||
if issue.Severity == SeverityInfo {
|
||||
infoIssues = append(infoIssues, issue)
|
||||
} else if issue.Severity == SeverityWarning {
|
||||
warningIssues = append(warningIssues, issue)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
assert.Len(t, infoIssues, 1, "should have 1 info-level issue for legacy trigger var")
|
||||
assert.Empty(t, warningIssues, "should have no warning-level issues")
|
||||
assert.Contains(t, infoIssues[0].Message, "webhook_target")
|
||||
})
|
||||
|
||||
t.Run("event envelope variables recognized in event-triggered workflow", func(t *testing.T) {
|
||||
ast := parseTestWorkflow(t, `
|
||||
name: event-handler-envelope
|
||||
kind: module
|
||||
triggers:
|
||||
- name: on-asset
|
||||
on: event
|
||||
enabled: true
|
||||
event:
|
||||
topic: assets.new
|
||||
input:
|
||||
source: event_data.source
|
||||
steps:
|
||||
- name: process
|
||||
type: bash
|
||||
command: echo "{{EventTopic}} {{EventSource}} {{EventTimestamp}} {{EventData}} {{EventEnvelope}} {{EventDataType}}"
|
||||
`)
|
||||
issues := rule.Check(ast)
|
||||
|
||||
// Event envelope variables should NOT produce any undefined-variable issues
|
||||
var undefinedWarnings []LintIssue
|
||||
for _, issue := range issues {
|
||||
if issue.Rule == "undefined-variable" && issue.Severity == SeverityWarning {
|
||||
undefinedWarnings = append(undefinedWarnings, issue)
|
||||
}
|
||||
}
|
||||
assert.Empty(t, undefinedWarnings, "event envelope variables should be recognized as defined")
|
||||
})
|
||||
|
||||
t.Run("truly undefined vars still warned alongside trigger vars", func(t *testing.T) {
|
||||
ast := parseTestWorkflow(t, `
|
||||
name: mixed-vars
|
||||
kind: module
|
||||
triggers:
|
||||
- name: on-event
|
||||
on: event
|
||||
enabled: true
|
||||
event:
|
||||
topic: test.topic
|
||||
input:
|
||||
source: event_data.source
|
||||
steps:
|
||||
- name: process
|
||||
type: bash
|
||||
command: echo "{{source}} {{truly_undefined}}"
|
||||
`)
|
||||
issues := rule.Check(ast)
|
||||
|
||||
var infoIssues, warningIssues []LintIssue
|
||||
for _, issue := range issues {
|
||||
if issue.Rule == "undefined-variable" {
|
||||
if issue.Severity == SeverityInfo {
|
||||
infoIssues = append(infoIssues, issue)
|
||||
} else if issue.Severity == SeverityWarning {
|
||||
warningIssues = append(warningIssues, issue)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// truly_undefined should be info (downgraded because event trigger exists)
|
||||
// source is in triggerVars so also info
|
||||
assert.Len(t, infoIssues, 2, "both source and truly_undefined should be info due to event trigger")
|
||||
assert.Empty(t, warningIssues, "no warnings expected when event trigger is present")
|
||||
})
|
||||
|
||||
t.Run("no trigger means no downgrade", func(t *testing.T) {
|
||||
ast := parseTestWorkflow(t, `
|
||||
name: no-triggers
|
||||
kind: module
|
||||
steps:
|
||||
- name: step1
|
||||
type: bash
|
||||
command: echo "{{source}}"
|
||||
`)
|
||||
issues := rule.Check(ast)
|
||||
|
||||
// source should be a normal warning, not info
|
||||
var warningIssues []LintIssue
|
||||
for _, issue := range issues {
|
||||
if issue.Rule == "undefined-variable" && issue.Severity == SeverityWarning {
|
||||
warningIssues = append(warningIssues, issue)
|
||||
}
|
||||
}
|
||||
assert.Len(t, warningIssues, 1)
|
||||
assert.Contains(t, warningIssues[0].Message, "source")
|
||||
})
|
||||
|
||||
t.Run("event trigger downgrades unlisted vars to info", func(t *testing.T) {
|
||||
// Scenario: input only maps "target", but steps also use source, description,
|
||||
// asset_type — these should be SeverityInfo, not SeverityWarning.
|
||||
ast := parseTestWorkflow(t, `
|
||||
name: event-handler-partial-input
|
||||
kind: module
|
||||
triggers:
|
||||
- name: on-new-asset
|
||||
on: event
|
||||
enabled: true
|
||||
event:
|
||||
topic: assets.new
|
||||
input:
|
||||
target: event_data.url
|
||||
steps:
|
||||
- name: process
|
||||
type: bash
|
||||
command: echo "{{target}} {{source}} {{description}} {{asset_type}}"
|
||||
`)
|
||||
issues := rule.Check(ast)
|
||||
|
||||
var infoIssues, warningIssues []LintIssue
|
||||
for _, issue := range issues {
|
||||
if issue.Rule == "undefined-variable" {
|
||||
if issue.Severity == SeverityInfo {
|
||||
infoIssues = append(infoIssues, issue)
|
||||
} else if issue.Severity == SeverityWarning {
|
||||
warningIssues = append(warningIssues, issue)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// target is a built-in variable so no issue is emitted
|
||||
// source, description, asset_type are NOT in triggerVars but hasEventTrigger → info ("may be provided by event data")
|
||||
assert.Len(t, infoIssues, 3, "source, description, asset_type should be info-level")
|
||||
assert.Empty(t, warningIssues, "no warnings when event trigger is present")
|
||||
|
||||
// Verify the messages indicate possible event data
|
||||
for _, issue := range infoIssues {
|
||||
assert.Contains(t, issue.Message, "may be provided by event data")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("no event trigger means undefined vars are warnings", func(t *testing.T) {
|
||||
// Workflow with a cron trigger (not event) — undefined vars should stay as warnings
|
||||
ast := parseTestWorkflow(t, `
|
||||
name: cron-handler
|
||||
kind: module
|
||||
triggers:
|
||||
- name: nightly
|
||||
on: cron
|
||||
enabled: true
|
||||
schedule: "0 0 * * *"
|
||||
steps:
|
||||
- name: process
|
||||
type: bash
|
||||
command: echo "{{source}} {{description}}"
|
||||
`)
|
||||
issues := rule.Check(ast)
|
||||
|
||||
var infoIssues, warningIssues []LintIssue
|
||||
for _, issue := range issues {
|
||||
if issue.Rule == "undefined-variable" {
|
||||
if issue.Severity == SeverityInfo {
|
||||
infoIssues = append(infoIssues, issue)
|
||||
} else if issue.Severity == SeverityWarning {
|
||||
warningIssues = append(warningIssues, issue)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
assert.Empty(t, infoIssues, "no info downgrades without event trigger")
|
||||
assert.Len(t, warningIssues, 2, "source and description should be warnings")
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user