runner icon indicating copy to clipboard operation
runner copied to clipboard

Validate required inputs are set before invoking an action

Open NorseGaud opened this issue 4 years ago • 30 comments

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 avatar Jun 01 '20 19:06 NorseGaud

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?

NorseGaud avatar Jun 01 '20 19:06 NorseGaud

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.

thboop avatar Jun 25 '20 22:06 thboop

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 :-)

aggenebbisj avatar Jan 14 '21 19:01 aggenebbisj

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.

thboop avatar Apr 30 '21 19:04 thboop

bump

NorseGaud avatar Jun 09 '21 15:06 NorseGaud

So what's the point of required? 🤔

anukul avatar Aug 12 '21 10:08 anukul

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

gjadi-mej avatar Nov 03 '21 07:11 gjadi-mej

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

gajus avatar Dec 08 '21 20:12 gajus

@gajus This might help:

: "${INPUT_APP:?input \"app\" not set or empty}"

anukul avatar Dec 08 '21 20:12 anukul

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 avatar Dec 08 '21 20:12 gajus

@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 avatar Dec 08 '21 21:12 gajus

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; }

gajus avatar Dec 08 '21 21:12 gajus

Any updates with validating required inputs?

ViacheslavKudinov avatar Jun 07 '22 15:06 ViacheslavKudinov

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

balaji-nordic avatar Jun 17 '22 08:06 balaji-nordic

Would help as Im facing the same issue

Rarisma avatar Aug 16 '22 19:08 Rarisma

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

Eshnek avatar Oct 06 '22 21:10 Eshnek

Hi, any update about it?

alexquitiaquez avatar Feb 17 '23 16:02 alexquitiaquez

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!

kpet avatar Jun 03 '23 11:06 kpet

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 😬

hytromo avatar Jun 21 '23 11:06 hytromo

Run into this as well

christianbuerk0 avatar Jun 21 '23 13:06 christianbuerk0

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 ?

jonelleamio avatar Aug 25 '23 13:08 jonelleamio

Still not fixed.

mwestphal avatar Sep 08 '23 05:09 mwestphal

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 avatar Nov 30 '23 09:11 GoodMirek

So what's the point of required? 🤔

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

GoodMirek avatar Nov 30 '23 09:11 GoodMirek

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.

erikahansen avatar Jan 08 '24 10:01 erikahansen

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.

ChristopherHX avatar Jan 08 '24 22:01 ChristopherHX

🤔 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)

alexkuc avatar Jan 23 '24 12:01 alexkuc

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.

Bloodsucker avatar Jan 23 '24 12:01 Bloodsucker

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.

kkroening avatar Feb 25 '24 03:02 kkroening

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.

ianlewis avatar Feb 26 '24 00:02 ianlewis