diff --git a/config.go b/config.go index 354a4195e..1c6f83382 100644 --- a/config.go +++ b/config.go @@ -51,6 +51,11 @@ type PathConfig struct { Ignore IgnorePatterns `yaml:"ignore"` } +type TimeoutMinutesConfig struct { + Required bool `yaml:"required"` + MaxMinutes float64 `yaml:"max"` +} + // Config is configuration of actionlint. This struct instance is parsed from "actionlint.yaml" // file usually put in ".github" directory. type Config struct { @@ -67,6 +72,10 @@ type Config struct { // Paths is a "paths" mapping in the configuration file. The keys are glob patterns to match file paths. // And the values are corresponding configurations applied to the file paths. Paths map[string]PathConfig `yaml:"paths"` + // TimeoutMinutes is an object where Required set true requires a timeout-minutes value to be present. + // Optionally, the MaxMinutes field can be set to enforce an upper limit on the timeout-minutes value. + // This is used in timeout-check rule. + TimeoutMinutes TimeoutMinutesConfig `yaml:"timeout-minutes"` } // PathConfigs returns a list of all PathConfig values matching to the given file path. The path must @@ -153,6 +162,13 @@ config-variables: null paths: # .github/workflows/**/*.yml: # ignore: [] + +# timeout-minutes is an object where 'required' set true requires a timeout-minutes value to be present. +# Optionally, the 'max' field can be set to enforce an upper limit on the timeout-minutes value. +# This is used in timeout-check rule. +timeout-minutes: + required: false + max: 60 `) if err := os.WriteFile(path, b, 0644); err != nil { return fmt.Errorf("could not write default configuration file at %q: %w", path, err) diff --git a/docs/checks.md b/docs/checks.md index ca9ed6626..4b9c613a9 100644 --- a/docs/checks.md +++ b/docs/checks.md @@ -2923,6 +2923,40 @@ directly via command line arguments. Note that `steps` in Composite action's metadata is not checked at this point. It will be supported in the future. + + +## Timeout Check + +Example input: + +```yaml +# This section is referred to generate the output and the playground link +on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - run: echo hello +``` + +Output: + +``` +test.yaml:4:3: You must have a timeout-minutes set to avoid overspend. [timeout-check] + | +4 | test: + | ^~~~~ +``` + + + + +This check is only run when set in the configuration file. As below, you can set required to true to enforce a value. If you wish to be prescriptive about the maximum value allowed, you may set the `max` to an appropriate number of minutes, else you can leave `max` unset. +``` +timeout-minutes: + required: true + max: 60 +``` --- [Installation](install.md) | [Usage](usage.md) | [Configuration](config.md) | [Go API](api.md) | [References](reference.md) diff --git a/docs/config.md b/docs/config.md index 2a2fdc47f..b7dd4a791 100644 --- a/docs/config.md +++ b/docs/config.md @@ -42,6 +42,13 @@ paths: ignore: # Ignore errors from the old runner check. This may be useful for (outdated) self-hosted runner environment. - 'the runner of ".+" action is too old to run on GitHub Actions' + +# timeout-minutes is an object where 'required' set true requires a timeout-minutes value to be present. +# Optionally, the 'max' field can be set to enforce an upper limit on the timeout-minutes value. +# This is used in timeout-check rule. +timeout-minutes: + required: true + max: 60 ``` - `self-hosted-runner`: Configuration for your self-hosted runner environment. @@ -57,6 +64,10 @@ paths: - `ignore`: The configuration to ignore (filter) the errors by the error messages. This is an array of regular expressions. When one of the patterns matches the error message, the error will be ignored. It's similar to the `-ignore` command line option. +- `timeout-minutes`: Configures enforcement for ensuring the timeout-minutes field is set at Job level + - `required`: true or false + - `max`: Maximum minutes that is acceptable for the value + ## Generate the initial configuration diff --git a/linter.go b/linter.go index e4a38d501..1db20b04f 100644 --- a/linter.go +++ b/linter.go @@ -565,6 +565,7 @@ func (l *Linter) check( NewRuleID(), NewRuleGlob(), NewRulePermissions(), + NewRuleTimeoutCheck(), NewRuleWorkflowCall(path, localReusableWorkflows), NewRuleExpression(localActions, localReusableWorkflows), NewRuleDeprecatedCommands(), diff --git a/rule_timeout_check.go b/rule_timeout_check.go new file mode 100644 index 000000000..f0a1c0fc5 --- /dev/null +++ b/rule_timeout_check.go @@ -0,0 +1,44 @@ +package actionlint + +type RuleTimeoutCheck struct { + RuleBase +} + +func NewRuleTimeoutCheck() *RuleTimeoutCheck { + return &RuleTimeoutCheck{ + RuleBase: RuleBase{ + name: "timeout-check", + desc: "Checks that timeout-minutes is set per job, with optional max limit", + }, + } +} + +func (rule *RuleTimeoutCheck) VisitJobPre(n *Job) error { + if rule.config == nil || !rule.Config().TimeoutMinutes.Required { + // No need to check anything + return nil + } + + if n.Steps == nil { + // This must be using a reusable workflow which does not support timeout-minutes + return nil + } + + if n.TimeoutMinutes == nil { + rule.Error( + n.Pos, + "You must have a timeout-minutes set to avoid overspend.", + ) + } + + if n.TimeoutMinutes != nil && + rule.config.TimeoutMinutes.MaxMinutes != 0 && + n.TimeoutMinutes.Value > rule.config.TimeoutMinutes.MaxMinutes { + rule.Errorf( + n.Pos, + "Your timeout-minutes is greater than %d minutes.", + int(rule.config.TimeoutMinutes.MaxMinutes), + ) + } + return nil +} diff --git a/rule_timeout_check_test.go b/rule_timeout_check_test.go new file mode 100644 index 000000000..dd6b3a128 --- /dev/null +++ b/rule_timeout_check_test.go @@ -0,0 +1,94 @@ +package actionlint + +import ( + "testing" +) + +func TestTimeoutChecks(t *testing.T) { + enforceField := NewRuleTimeoutCheck() + enforceField.SetConfig(&Config{ + TimeoutMinutes: TimeoutMinutesConfig{ + Required: true, + }, + }) + + enforceFieldValue := NewRuleTimeoutCheck() + enforceFieldValue.SetConfig(&Config{ + TimeoutMinutes: TimeoutMinutesConfig{ + Required: true, + MaxMinutes: 30, + }, + }) + + noTimeout := &Job{ + Pos: &Pos{Line: 1, Col: 1}, + Name: &String{Value: "no-timeout"}, + Steps: []*Step{ + { + Name: &String{Value: "do-something"}, + Exec: &ExecAction{Uses: &String{Value: "actions/checkout@v4"}}, + }, + }, + } + + highTimeout := &Job{ + Pos: &Pos{Line: 1, Col: 1}, + Name: &String{Value: "high-timeout"}, + Steps: []*Step{ + { + Name: &String{Value: "do-something"}, + Exec: &ExecAction{Uses: &String{Value: "actions/checkout@v4"}}, + }, + }, + TimeoutMinutes: &Float{Value: 45}, + } + goodTimeout := &Job{ + Pos: &Pos{Line: 1, Col: 1}, + Name: &String{Value: "good-timeout"}, + Steps: []*Step{ + { + Name: &String{Value: "do-something"}, + Exec: &ExecAction{Uses: &String{Value: "actions/checkout@v4"}}, + }, + }, + TimeoutMinutes: &Float{Value: 15}, + } + + reusableWorkflow := &Job{ + Pos: &Pos{Line: 1, Col: 1}, + Name: &String{Value: "reusable-workflow"}, + } + + tests := []struct { + name string + config *RuleTimeoutCheck + job *Job + expect []*Error + }{ + {"no timeout fail", enforceField, noTimeout, []*Error{{Message: "You must have a timeout-minutes set to avoid overspend.", Line: 1, Column: 1}}}, + {"no timeout pass", enforceField, highTimeout, []*Error{}}, + {"high timeout fail", enforceFieldValue, highTimeout, []*Error{{Message: "Your timeout-minutes is greater than 30 minutes.", Line: 1, Column: 1}}}, + {"good timeout value pass", enforceFieldValue, goodTimeout, []*Error{}}, + {"reusable workflow pass", enforceField, reusableWorkflow, []*Error{}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.config.errs = nil + tt.config.VisitJobPre(tt.job) + errs := tt.config.Errs() + + if len(errs) != len(tt.expect) { + t.Fatalf("expected %d errors but got %d: %+v", len(tt.expect), len(errs), errs) + } + for i, e := range errs { + if e.Message != tt.expect[i].Message { + t.Errorf("expected error message %q but got %q", tt.expect[i].Message, e.Message) + } + if e.Line != tt.expect[i].Line || e.Column != tt.expect[i].Column { + t.Errorf("expected error position %d:%d but got %d:%d", tt.expect[i].Line, tt.expect[i].Column, e.Line, e.Column) + } + } + }) + } +} diff --git a/testdata/format/test.sarif b/testdata/format/test.sarif index cab475761..3ffb4f1d5 100644 --- a/testdata/format/test.sarif +++ b/testdata/format/test.sarif @@ -234,6 +234,21 @@ }, "helpUri": "https://github.com/rhysd/actionlint/blob/main/docs/checks.md" }, + { + "id": "timeout-check", + "name": "TimeoutCheck", + "defaultConfiguration": { + "level": "error" + }, + "properties": { + "description": "Checks that timeout-minutes is set per job, with optional max limit", + "queryURI": "https://github.com/rhysd/actionlint/blob/main/docs/checks.md" + }, + "fullDescription": { + "text": "Checks that timeout-minutes is set per job, with optional max limit" + }, + "helpUri": "https://github.com/rhysd/actionlint/blob/main/docs/checks.md" + }, { "id": "workflow-call", "name": "WorkflowCall",