mpalmer / action-validator

Tool to validate GitHub Action and Workflow YAML files
GNU General Public License v3.0
271 stars 25 forks source link

failure to specify shell in action.yml doesn't get cause an error #68

Closed till closed 4 months ago

till commented 6 months ago

Heya! 👋🏼

First off, thanks for creating this tool. :-)


I think I found a bug, the following action.yml should trigger an error from your tool — but it does not (yet):

name: 'foo/bar'
description: ''
branding:
  icon: 'zap'
  color: 'yellow'
runs:
  using: "composite"
  steps:
  - run: echo "Hello world"

Error on Github Actions is: Required property is missing: shell

The fix for it is:

# ...
runs:
  using: "composite"
  steps:
  - run: echo "Hello world"
    shell: bash

Could be a problem with the json schema files? I see "shell" there, but not sure how that gets enforced.

So maybe the fix is there and they need to be updated. I am also not sure if you do anything "custom" on top of it, I have to admit I haven't checked.

Cheers!

mpalmer commented 6 months ago

Thanks for the failing example. I'll add it as a new test and see what's going on.

mpalmer commented 5 months ago

After going off on a massive wild-goose-chase, I'm back on track, but now I'm having trouble reproducing the error you reported. The example you gave fails validation for me with the code on main, and adding a shell: line in the right place makes validation pass.

Thinking that the problem might have been in a previous release, and been subsequently fixed, I've checked out older versions of the codebase, at points when the JSON schema we use was changed, and each of those also fail to validate the example you provided.

Would you be able to provide more details of how you were able to make the (non-)failing example you provided pass action-validator? The output of action-validator --version would also be helpful, to ensure we're testing the same thing.

To make sure we don't backslide in the future, I've added a new test case.

till commented 5 months ago

I think I am using 0.5.4, it's still hardcoded in my workflow, haven't found a way yet to install this better so I do a curl against that release?

mpalmer commented 5 months ago

Yeah, downloading the release binary is typically the best way to install quickly. On the off-chance that the release was b0rked, I downloaded the linux_amd64 binary for the 0.5.4 release and tried it... and it fails on the example, as I expected. Is your workflow run public? If you can point me at it, I'll take a look and see if anything seems amiss.

till commented 5 months ago

Apologies for the slow replies. The action run is not public, but I am trying to build an example. Give me a few days, planning to get it done by Friday.

till commented 4 months ago

@mpalmer sorry for the delay.

I just downloaded the latest version (v0.5.4) from Github and tested it on a composite action. It now fails to validate it completely:

# action-test.yml
name: 'test'
description: 'description'
runs:
  using: "composite"
  steps:
  - run: mkdir -p .kube

Output:

❯ ./action-validator -v action-test.yml
Treating action-test.yml as a Workflow definition
Fatal error validating action-test.yml
Validation failed: ValidationState {
    action_type: Some(
        Workflow,
    ),
    file_path: Some(
        "action-test.yml",
    ),
    errors: [
        Properties {
            code: "properties",
            detail: Some(
                "Additional property 'description' is not allowed",
            ),
            path: "",
            title: "Property conditions are not met",
        },
        Properties {
            code: "properties",
            detail: Some(
                "Additional property 'runs' is not allowed",
            ),
            path: "",
            title: "Property conditions are not met",
        },
        Required {
            code: "required",
            detail: None,
            path: "/on",
            title: "This property is required",
        },
        Required {
            code: "required",
            detail: None,
            path: "/jobs",
            title: "This property is required",
        },
    ],
}
till commented 4 months ago

OK, I think I understand why it doesn't work — I have to call the file action.yml for your tool to determine that it's a Github Action and not an arbitrary workflow.

Now using the latest version (v0.5.4), it errors when shell is missing. :) It's a bit verbose, but that works for me!

mpalmer commented 4 months ago

Glad it's all sorted out.

I have to call the file action.yml for your tool to determine that it's a Github Action

Yeah, that's a bit of a trap for the unwary. I did it like that because the way I thought people would use it would be to validate the files they'd already put in .github/{workflows,actions}, and GitHub requires action definitions to be named action.yml. Is there some way you can think of I can improve the documentation / discoverability of this behaviour?

till commented 4 months ago

I think you're reasoning makes a lot of sense.