From 5243827912c9681539df8be1498785c78d8b6c2b Mon Sep 17 00:00:00 2001 From: rhysd Date: Wed, 13 Mar 2024 21:13:22 +0900 Subject: [PATCH] check keys in `runs` in action.yml --- action_metadata.go | 24 ++++++++ action_metadata_test.go | 58 ++++++++++++------ rule_action.go | 61 +++++++++++++++++-- .../projects/local_action_invalid_runners.out | 8 +-- 4 files changed, 122 insertions(+), 29 deletions(-) diff --git a/action_metadata.go b/action_metadata.go index 94ae4dfdd..bdf25095d 100644 --- a/action_metadata.go +++ b/action_metadata.go @@ -95,6 +95,30 @@ func (inputs *ActionMetadataOutputs) UnmarshalYAML(n *yaml.Node) error { type ActionMetadataRuns struct { // Using is `using` configuration of action.yaml. It defines what runner is used for the action. Using string `yaml:"using"` + // Main is `main` configuration of action.yaml for JavaScript action. + Main string `yaml:"main"` + // Pre is `pre` configuration of action.yaml for JavaScript action. + Pre string `yaml:"pre"` + // PreIf is `pre-if` configuration of action.yaml for JavaScript action. + PreIf string `yaml:"pre-if"` + // Post is `post` configuration of action.yaml for JavaScript action. + Post string `yaml:"post"` + // PostIf is `post-if` configuration of action.yaml for JavaScript action. + PostIf string `yaml:"post-if"` + // Steps is `steps` configuration of action.yaml for Composite action. + Steps []yaml.Node `yaml:"steps"` + // Image is `image` of action.yaml for Docker action. + Image string `yaml:"image"` + // PreEntrypoint is `pre-entrypoint` of action.yaml for Docker action. + PreEntrypoint string `yaml:"pre-entrypoint"` + // Entrypoint is `entrypoint` of action.yaml for Docker action. + Entrypoint string `yaml:"entrypoint"` + // PostEntrypoint is `post-entrypoint` of action.yaml for Docker action. + PostEntrypoint string `yaml:"post-entrypoint"` + // Args is `args` of action.yaml for Docker action. + Args []*yaml.Node `yaml:"args"` + // Env is `env` of action.yaml for Docker action. + Env map[string]*yaml.Node `yaml:"env"` } // ActionMetadata represents structure of action.yaml. diff --git a/action_metadata_test.go b/action_metadata_test.go index 2cd89ed9b..990b71c0c 100644 --- a/action_metadata_test.go +++ b/action_metadata_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "gopkg.in/yaml.v3" ) @@ -24,6 +25,7 @@ func testGetWantedActionMetadata() *ActionMetadata { }, Runs: ActionMetadataRuns{ Using: "node20", + Main: "index.js", }, } return want @@ -103,25 +105,43 @@ func TestLocalActionsFindMetadata(t *testing.T) { } }) - for _, using := range []string{"docker", "composite"} { - a, u := "./"+using, using - t.Run(a, func(t *testing.T) { - want := &ActionMetadata{ - Name: "My action", - Runs: ActionMetadataRuns{ - Using: u, - }, - } - have, cached, err := c.FindMetadata(a) - if err != nil { - t.Fatal(err) - } - testCachedFlag(t, cached, false) - if !cmp.Equal(want, have) { - t.Fatal(cmp.Diff(want, have)) - } - }) - } + t.Run("./docker", func(t *testing.T) { + want := &ActionMetadata{ + Name: "My action", + Runs: ActionMetadataRuns{ + Using: "docker", + Image: "Dockerfile", + }, + } + have, cached, err := c.FindMetadata("./docker") + if err != nil { + t.Fatal(err) + } + testCachedFlag(t, cached, false) + if !cmp.Equal(want, have) { + t.Fatal(cmp.Diff(want, have)) + } + }) + + t.Run("./composite", func(t *testing.T) { + want := &ActionMetadata{ + Name: "My action", + Runs: ActionMetadataRuns{ + Using: "composite", + }, + } + have, cached, err := c.FindMetadata("./composite") + if err != nil { + t.Fatal(err) + } + testCachedFlag(t, cached, false) + if !cmp.Equal(want, have, cmpopts.IgnoreFields(ActionMetadataRuns{}, "Steps")) { + t.Fatal(cmp.Diff(want, have)) + } + if have.Runs.Steps == nil { + t.Fatal(`"steps" was not decoded`, have.Runs) + } + }) } func TestLocalActionsFindConcurrently(t *testing.T) { diff --git a/rule_action.go b/rule_action.go index c3f87af0d..4a6bd4d5c 100644 --- a/rule_action.go +++ b/rule_action.go @@ -110,25 +110,70 @@ func (rule *RuleAction) invalidActionFormat(pos *Pos, spec string, why string) { } func (rule *RuleAction) invalidRunnerName(pos *Pos, name, action, path string) { - rule.Errorf(pos, "invalid runner name %q at runs.using in the local action %q defined at %q. see https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#runs to know valid runner names", name, action, path) + rule.Errorf(pos, "invalid runner name %q at runs.using in local action %q defined at %q. see https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#runs to know valid runner names", name, action, path) +} + +func (rule *RuleAction) missingRunnerProp(pos *Pos, prop, ty, action, path string) { + rule.Errorf(pos, `%q is required at "runs" section for %s action in local action %q defined at %q`, prop, ty, action, path) +} + +func (rule *RuleAction) invalidRunnerProp(pos *Pos, prop, ty, action, path string) { + rule.Errorf(pos, `%q is not allowed at "runs" section for %s action in local action %q defined at %q`, prop, ty, action, path) +} + +func (rule *RuleAction) checkInvalidRunnerProps(pos *Pos, r *ActionMetadataRuns, ty, action, path string, props []string) { + for _, prop := range props { + invalid := prop == "main" && r.Main != "" || + prop == "pre" && r.Pre != "" || + prop == "pre-if" && r.PreIf != "" || + prop == "post" && r.Post != "" || + prop == "post-if" && r.PostIf != "" || + prop == "steps" && len(r.Steps) > 0 || + prop == "image" && r.Image != "" || + prop == "pre-entrypoint" && r.PreEntrypoint != "" || + prop == "entrypoint" && r.Entrypoint != "" || + prop == "post-entrypoint" && r.PostEntrypoint != "" || + prop == "args" && r.Args != nil || + prop == "env" && r.Env != nil + + if invalid { + rule.Errorf(pos, `%q is not allowed at "runs" section because %q is a %s action. it is defined at %q`, prop, action, ty, path) + } + } } // https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#runs-for-javascript-actions // https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#runs-for-docker-container-actions // https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#runs-for-composite-actions func (rule *RuleAction) checkLocalActionRunner(path string, meta *ActionMetadata, pos *Pos) { - u := meta.Runs.Using - if u == "docker" || u == "composite" { + r := &meta.Runs + u := r.Using + if u == "" { + rule.Errorf(pos, `"runs.using" is missing in local action %q defined at %q`, meta.Name, path) return } - if u == "" { - rule.Errorf(pos, `"runs.using" is missing in the local action %q defined at %q`, meta.Name, path) + + if u == "docker" { + if r.Image == "" { + rule.missingRunnerProp(pos, "image", "Docker", meta.Name, path) + } + rule.checkInvalidRunnerProps(pos, r, "Docker", meta.Name, path, []string{"main", "pre", "pre-if", "post", "post-if", "steps"}) + return + } + + if u == "composite" { + if len(r.Steps) == 0 { + rule.missingRunnerProp(pos, "steps", "Composite", meta.Name, path) + } + rule.checkInvalidRunnerProps(pos, r, "Composite", meta.Name, path, []string{"main", "pre", "pre-if", "post", "post-if", "image", "pre-entrypoint", "entrypoint", "post-entrypoint", "args", "env"}) return } + if !strings.HasPrefix(u, "node") { rule.invalidRunnerName(pos, u, meta.Name, path) return } + v, err := strconv.ParseUint(u[len("node"):], 10, 0) if err != nil { rule.invalidRunnerName(pos, u, meta.Name, path) @@ -137,7 +182,7 @@ func (rule *RuleAction) checkLocalActionRunner(path string, meta *ActionMetadata if v < MinimumNodeRunnerVersion { rule.Errorf( pos, - `%q runner at "runs.using" is unavailable since the Node.js version is too old (%d < %d) in the local action %q defined at %q. see https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#runs-for-javascript-actions`, + `%q runner at "runs.using" is unavailable since the Node.js version is too old (%d < %d) in local action %q defined at %q. see https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#runs-for-javascript-actions`, u, v, MinimumNodeRunnerVersion, @@ -145,6 +190,10 @@ func (rule *RuleAction) checkLocalActionRunner(path string, meta *ActionMetadata path, ) } + if r.Main == "" { + rule.missingRunnerProp(pos, "main", "JavaScript", meta.Name, path) + } + rule.checkInvalidRunnerProps(pos, r, "JavaScript", meta.Name, path, []string{"steps", "image", "pre-entrypoint", "entrypoint", "post-entrypoint", "args", "env"}) } // https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions#example-using-the-github-packages-container-registry diff --git a/testdata/projects/local_action_invalid_runners.out b/testdata/projects/local_action_invalid_runners.out index 54f0b88c0..017021cb2 100644 --- a/testdata/projects/local_action_invalid_runners.out +++ b/testdata/projects/local_action_invalid_runners.out @@ -1,4 +1,4 @@ -/workflows/test\.yaml:7:15: "node14" runner at "runs.using" is unavailable since the Node\.js version is too old \(14 < 16\) in the local action "Old Node\.js" defined at "\./old_node"\. see https://.+ \[action\]/ -/workflows/test\.yaml:8:15: "runs\.using" is missing in the local action "Missing runner name" defined at "\./missing_runner" \[action\]/ -/workflows/test\.yaml:9:15: invalid runner name "what-is-this-runner" at runs\.using in the local action "Unknown runner name" defined at "\./unknown_runner"\. see https://.+ to know valid runner names \[action\]/ -/workflows/test\.yaml:10:15: invalid runner name "nodenext" at runs\.using in the local action "Invalid node version" defined at "\./invalid_node_version"\. see https://.+ to know valid runner names \[action\]/ +/workflows/test\.yaml:7:15: "node14" runner at "runs.using" is unavailable since the Node\.js version is too old \(14 < 16\) in local action "Old Node\.js" defined at "\./old_node"\. see https://.+ \[action\]/ +/workflows/test\.yaml:8:15: "runs\.using" is missing in local action "Missing runner name" defined at "\./missing_runner" \[action\]/ +/workflows/test\.yaml:9:15: invalid runner name "what-is-this-runner" at runs\.using in local action "Unknown runner name" defined at "\./unknown_runner"\. see https://.+ to know valid runner names \[action\]/ +/workflows/test\.yaml:10:15: invalid runner name "nodenext" at runs\.using in local action "Invalid node version" defined at "\./invalid_node_version"\. see https://.+ to know valid runner names \[action\]/