actions / runner

The Runner for GitHub Actions :rocket:
https://github.com/features/actions
MIT License
4.79k stars 936 forks source link

Validate required inputs are set before invoking an action #1070

Open NorseGaud opened 4 years ago

NorseGaud commented 4 years ago

Describe the bug

I have the following action.yml:

name: 'Anka Prepare VM and execute inside'
description: 'Prepare an Anka VM and execute commands inside'
author: Veertu
branding:
  icon: 'anchor'
  color: 'blue'
inputs:
  anka-template:
    description: 'name or UUID of Anka Template'
    required: true

My index.js looks like:

const core = require('@actions/core');
const io = require('@actions/io');
const prepare = require('./prepare');
const execute = require('./execute');

// most @actions toolkit packages have async methods
async function run() {
  try { 
    const ankaVirtCLIPath = await io.which('anka', true)
    console.log(`Anka Virtualization CLI found at: ${ankaVirtCLIPath}`)
    const ankaTemplate = core.getInput('anka-template');
    const ankaTag = core.getInput('anka-tag');
    const ankaCommands = core.getInput('commands');
    const hostPreCommands = core.getInput('host-pre-commands');
    const hostPostCommands = core.getInput('host-post-commands');
    console.log("=========================" + "\n" + `${hostPreCommands}` + "\n" + "=================================")
    console.log("=========================" + "\n" + `${hostPostCommands}` + "\n" + "=================================")
    console.log("=========================" + "\n" + `${ankaCommands}` + "\n" + "=================================")
    if (hostPreCommands) {
      await execute.nodeCommands(hostPreCommands)
    }
    // Prepare VM
    await prepare.ankaRegistryPull(ankaTemplate,ankaTag)
    // Run commands inside VM
    // Cleanup
    if (hostPostCommands) {
      await execute.nodeCommands(hostPostCommands)
    }
  } catch (error) {
    core.setFailed(error);
  }
}

run()

and finally, the important part of my .github/workflows/test.yml:

    - name: basic commands
      id: basic
      uses: ./
      with:
        commands: |
          env
          ls -laht ./
          ls -laht ../
          pwd
          echo "HERE" && \
          echo "THERE HERE WHERE"

Yet, there is no failure for missing anka-template... It fails for a reason inside of the ankaRegistryPull function which is well after it tries to load the input.

https://help.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#inputs seems to indicate that it shouldn't allow the action to run if it's missing that input.

What am I missing?

NorseGaud commented 4 years ago

More info from my testing:

    const ankaTemplate = core.getInput('anka-template');
    console.log(`${typeof(ankaTemplate)}`)
    console.log(`TEMPLATE: ${ankaTemplate}`)
    if (typeof(ankaTemplate) === 'undefined') { 
      throw new Error('anka-template is not set!'); 
    }
Screen Shot 2020-06-01 at 3 46 47 PM

if I use .length ===0 I can get it to fail... But again, shouldn't this be handled by required: true?

thboop commented 4 years ago

We do provide the ability to set an input as required in core.getInput(), which will fail if an input is not set.

The action.yaml is mainly for the benefit of the the runner/action author, we don't really use it in the toolkit. We do some validation on inputs in the runner already for unexpected inputs, we could also fail the step if the required inputs are not set.

cc @TingluoHuang , @ericsciple , I'd love to hear your thoughts as well.

aggenebbisj commented 3 years ago

I just ran into the same issue. Have to say it's quite unexpected to mark something as required and then finding out it blows up silently. Especially since toolkit is the official GitHub toolkit for actions.

we could also fail the step if the required inputs are not set.

I think this would make a lot of sense :-)

thboop commented 3 years ago

This would improve the experience of developing actions, but would also break users that have erroneously set required in their action.yaml.

That being said, this isn't really a toolkit issue, the runner controls this. I'm going to transfer this issue to that repository.

NorseGaud commented 3 years ago

bump

anukul commented 3 years ago

So what's the point of required? 🤔

gjadi-mej commented 2 years ago

Bump, I was also surprised by this behavior (https://github.community/t/inputs-required-not-enforced-with-no-default/206745/3).

gajus commented 2 years ago

Does anyone have a one-liner workaround to enforce required parameters?

anukul commented 2 years ago

@gajus This might help:

: "${INPUT_APP:?input \"app\" not set or empty}"
gajus commented 2 years ago

Thanks @anukul , I was more wondering if there a way to write generic check, e.g. using JavaScript actions. However, it seems that it is impossible to get parent inputs.

gajus commented 2 years ago

@anukul Looks like your suggestion doesn't work either. At least in composite actions, env does not include INPUT_.

https://github.com/actions/runner/issues/665

gajus commented 2 years ago

Something like this works:

- run: |
    [[ "${{ inputs.docker_image_name }}" ]] || { echo "docker_image_name input is empty" ; exit 1; }
    [[ "${{ inputs.doppler_token }}" ]] || { echo "doppler_token input is empty" ; exit 1; }
ViacheslavKudinov commented 2 years ago

Any updates with validating required inputs?

balaji-nordic commented 2 years ago

Also having the same issue. Would be nice to get this fixed.

Rarisma commented 2 years ago

Would help as Im facing the same issue

Eshnek commented 1 year ago

+1, had to turn debug logging on to realize this was my issue

alexquitiaquez commented 1 year ago

Hi, any update about it?

kpet commented 1 year ago

Just ran into this as well. It'd be nice to get a clean failure of the using step when one or more required inputs are not provided. An error message should be printed and state which required input(s) were not provided. It'd be nice to have all the required but missing inputs printed in the message to avoid back and forth (hit the error, add one, hit the error again, ...). Thanks!

hytromo commented 1 year ago

Run into this as well, spent quite some time debugging an issue that would have been solved in 1' if github had let me know that a required input was missing 😬

christianbuerk0 commented 1 year ago

Run into this as well

jonelleamio commented 1 year ago

Something like this works:

- run: |
    [[ "${{ inputs.docker_image_name }}" ]] || { echo "docker_image_name input is empty" ; exit 1; }
    [[ "${{ inputs.doppler_token }}" ]] || { echo "doppler_token input is empty" ; exit 1; }

It's been a couple of years, is it still the best good answer ? or did they fixed it ?

mwestphal commented 1 year ago

Still not fixed.

GoodMirek commented 9 months ago

I ran into this issue too. It looks as such a basic thing which competing CI systems have for many years. Here, it is open for three years and still not solved.

I can imagine it cannot be just changed, as it would break too many things. But it could be an opt-in feature.

GoodMirek commented 9 months ago

So what's the point of required? 🤔

I have the same question. required currently seems completely useless in Github Actions.

erikahansen commented 8 months ago

Is this resolved now? I just got this:

Invalid workflow file: .github/workflows/development.yml#L10
The workflow is not valid. .github/workflows/development.yml (Line: 10, Col: 11): Input x is required, but not provided while calling.
ChristopherHX commented 8 months ago

Not fixed for actions.

Is this resolved now? I just got this:

This was never a problem for reusable workflows (your example), these are a newer concept.

I agree GitHub Actions (CI/CD Product name) vs. actions (those are the steps of a job < this issue is about that part) vs. (reusable) Workflows (those are files having an on key and one or more jobs) might be an easy to mix buzzwords in GitHub.

alexkuc commented 8 months ago

🤔 Maybe something got changed but when I changed input to required: true, the workflow_dispatch form made the field mandatory making it impossible to submit the form w/o supplying a value (HTML5 validation)

Bloodsucker commented 8 months ago

the workflow_dispatch form

The issue people are describing doesn't affect the dispatch form that you're referring to in your comment. The people are complaining about the input required setting being ignored when calling a job from another job. The expectation was for the job to fail if not all required parameters are set, however it just continues like nothing.

kkroening commented 6 months ago

Does required: true for composite action inputs do anything? It doesn't seem like it - the action happily chugs along without a peep when the input is unspecified, and then something somewhere down the line ends up receiving a blank string, wreaking havoc without an obvious cause. I guess required: true is just a no-op, unless I'm missing something or not understanding the purpose.

Maybe it's mentioned somewhere in the fine print above that this is indeed a no-op. Might be worth calling it out in the inputs.<input_id>.required API reference or something though.

ianlewis commented 6 months ago

Does required: true for composite action inputs do anything?

It doesn't do anything except to provide an intention to users. It's basically just documentation. Actions authors have to do all the input validation themselves currently.

ramonpetgrave64 commented 4 months ago

It seems like input types are also not enforced. I had assumed types number and boolean would keep my workflows safe from script injection. But the only way is to use them as environment variables.

ianlewis commented 4 months ago

It seems like input types are also not enforced. I had assumed types number and boolean would keep my workflows safe from script injection. But the only way is to use them as environment variables.

Inputs are provided to actions as environment variables so they are only strings. The actions.yml metadata format for inputs does not even had a type field. Types for inputs are only valid on reusable workflows.