Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hopefully reusable workflow agrees to support ${{inputs. XXX}} expression parameter passing #1976

Open
qjkcienet opened this issue Jun 28, 2022 · 12 comments
Labels
enhancement New feature or request

Comments

@qjkcienet
Copy link

qjkcienet commented Jun 28, 2022

Describe the enhancement
I wanted to pass the parameters of the workflow_dispatch input in a reusable workflow,
and with ${{inputs.username}} expression, the value could not be read and without an error message.

But if using ${{github.event.inputs.username}} it works fine, I don't know why.
The official documentation does not detail the difference between the two and which to use in reusable workflows

Code Snippet

name: Caller

on:
  workflow_dispatch:
    inputs:
      username:
        type: string
        description: 'username'
        required: false
        default: 'john-doe'
        
jobs:
  Test-1:
      uses: ./.github/workflows/test_1.yml
      with:
        username: ${{ inputs.username }}       #${{ github.event.inputs.username }}
      secrets: inherit
name: Executor

on:
  push:
  workflow_call:
    inputs:
      username:
        type: string
        description: 'A username passed from the caller workflow'
        default: 'john-doe'
        required: false
        
      
jobs:
  Test-1:
    runs-on: ubuntu-latest
    steps:
      - name: Print the input name to STDOUT
        run: echo The username is ${{ inputs.username }}

Additional information
Add any other context about the feature here.

NOTE: if the feature request has been agreed upon then the assignee will create an ADR. See docs/adrs/README.md

@qjkcienet qjkcienet added the enhancement New feature or request label Jun 28, 2022
@nathanchapman
Copy link

nathanchapman commented Jun 28, 2022

I'm noticing the same and this feels like a bug. Before the inputs unification announcement, the Executor in your example would correctly use inputs.username. I could have sworn this was working even after the announcement and I think something may have changed even more recently with this behavior. Now, it looks like the inputs context contains all of the fields for both the Caller and the Executor, but only the fields from the event that triggered the Caller are being populated in the inputs context. This becomes more evident when you change the name of the inputs in the Executor to something like providedUsername and echo the inputs with toJSON in one of the Executors steps, you'll see something like:

{
  username: user1,
  providedUsername: 
}

@mukerjee
Copy link

seeing similar issues here to what @nathanchapman pointed out.

Last week this worked:

   Deploy:
     uses: ./.github/workflows/_deploy.yml
     with:
      environment: ${{ inputs.environment }}
    secrets: inherit

(in my called workflow i set the environment using ${{ inputs.environment }})

Now this week it doesn't work (no environment is getting set on the summary page and all the secrets in the called workflow are now empty).

Changing it to this suddenly works again:

   Deploy:
     uses: ./.github/workflows/_deploy.yml
     with:
      environment: ${{ github.event.inputs.environment }}
    secrets: inherit

this is almost definitely a regression...

@nathanchapman
Copy link

nathanchapman commented Jun 28, 2022

Same here ☝️

I'm filing a bug report with our GitHub Enterprise Cloud support. Here are some more details and examples for anyone interested.

Now that inputs have been unified across manual and reusable workflows, we've moved to using the inputs context across all workflow files. However, as of the last week or so, we've noticed a transparent change where the inputs context is broken within reusable workflows when those values are computed from inputs to the dispatched workflow. In other words, the reusable workflow's inputs context is trampled and/or muddied by the dispatched workflow's inputs context, depending on how inputs are passed.

Example 1

In this example, we're just setting up the workflow files.

name: Dispatchable Workflow

on:
  workflow_dispatch:
    inputs:
      gitRef:
        type: string
        description: The branch, tag, or SHA to checkout
        required: false

jobs:
  tests:
    name: Tests
    uses: ./.github/workflows/tests.yml
    with:
      gitRef: hardcoded
      otherInput: hardcoded
    secrets: inherit
name: Tests

on:
  workflow_call:
    inputs:
      gitRef:
        type: string
        description: The branch, tag, or SHA to test
        required: true
      otherInput:
        type: string
        description: Input only for example
        required: true

jobs:
  unitTests:
    name: Unit Tests
    runs-on: ubuntu-latest

    steps:
      - name: Debug Inputs
        run: echo "${{ toJSON(inputs) }}"

Running this workflow, whether a value is set for gitRef or not, results in the following expected output:

{
  gitRef: hardcoded,
  otherInput: hardcoded
}

Example 2

The reusable workflow remains the same, but we've modified how it's being called (with the inputs context):

name: Dispatchable Workflow

on:
  workflow_dispatch:
    inputs:
      gitRef:
        type: string
        description: The branch, tag, or SHA to checkout
        required: false

jobs:
  tests:
    name: Tests
    uses: ./.github/workflows/tests.yml
    with:
      gitRef: ${{ inputs.gitRef }}
      otherInput: hardcoded
    secrets: inherit

results in the following when example is passed as the gitRef to the dispatchable workflow:

{
  gitRef: ,
  otherInput: hardcoded
}

Example 3

The previous example looks like a possible name clash, so let's modify our dispatchable workflow even further to avoid a possible clash:

name: Dispatchable Workflow

on:
  workflow_dispatch:
    inputs:
      checkoutRef:
        type: string
        description: The branch, tag, or SHA to checkout
        required: false

jobs:
  tests:
    name: Tests
    uses: ./.github/workflows/tests.yml
    with:
      gitRef: ${{ inputs.checkoutRef || 'master' }}
      otherInput: hardcoded
    secrets: inherit

results in the following when example is passed as the checkoutRef to the dispatchable workflow:

{
  checkoutRef: example,
  gitRef: master,
  otherInput: hardcoded
}

Still not what we're expecting as output within the reusable Tests workflow. I'd expect:

{
  gitRef: example,
  otherInput: hardcoded
}

It appears that inputs.checkoutRef was not properly evaluated when setting the value for gitRef and somehow both fields ended up getting passed to the reusable workflow.

Example 4

Now, let's switch from using the inputs context to using github.event.inputs within the dispatchable workflow. Again, nothing has changed in the reusable Tests workflow since the beginning.

name: Dispatchable Workflow

on:
  workflow_dispatch:
    inputs:
      checkoutRef:
        type: string
        description: The branch, tag, or SHA to checkout
        required: false

jobs:
  tests:
    name: Tests
    uses: ./.github/workflows/tests.yml
    with:
      gitRef: ${{ github.event.inputs.checkoutRef || 'master' }}
      otherInput: hardcoded
    secrets: inherit

results in the following when example is passed as the checkoutRef to the dispatchable workflow:

{
  checkoutRef: example,
  gitRef: example,
  otherInput: hardcoded
}

this finally gives us the correct behavior in gitRef, but again, still doesn't quite match the inputs context I'd expect within the reusable Tests workflow, given that we're providing inputs explicitly with the with keyword. Expected output:

{
  gitRef: example,
  otherInput: hardcoded
}

Example 5

In this example, we're unifying the names of the inputs once again.

name: Dispatchable Workflow

on:
  workflow_dispatch:
    inputs:
      gitRef:
        type: string
        description: The branch, tag, or SHA to checkout
        required: false

jobs:
  tests:
    name: Tests
    uses: ./.github/workflows/tests.yml
    with:
      gitRef: ${{ github.event.inputs.gitRef || 'master' }}
      otherInput: hardcoded
    secrets: inherit

results in the following when example is passed as the gitRef to the dispatchable workflow:

{
  gitRef: example,
  otherInput: hardcoded
}

and the following when nothing is passed as the gitRef to the dispatchable workflow:

{
  gitRef: master,
  otherInput: hardcoded
}

This is close to the correct behavior. It allows computation (namely, default / fallback values) and the inputs context is correct within the reusable Tests workflow.

However, it's only not "muddied" because the input names match. Any additional inputs added to the dispatchable workflow end up in the inputs context for the reusable workflow even though they're not explicitly passed (muddying). If the reusable workflow has an input name that matches an input name from the dispatchable workflow, we end up with an empty / nullish value in the inputs context within the reusable workflow (trampling).

Requested Change

The inputs context is scoped to the appropriate workflow and inputs into reusable workflows can be computed from inputs into dispatched workflows.

@ajaykn
Copy link
Contributor

ajaykn commented Jun 29, 2022

We have noticed this regression and waiting for the fix to be rolled out.
Apologies for the inconvenience :(

cc @genieCS

@JavaScriptBach
Copy link

We have noticed this regression and waiting for the fix to be rolled out. Apologies for the inconvenience :(

cc @genieCS

Hi @ajaykn, is there an ETA for the fix to be rolled out? Trying to decide whether I should patch our workflows or wait for the fix. Thanks!

@ajaykn
Copy link
Contributor

ajaykn commented Jul 1, 2022

We have noticed this regression and waiting for the fix to be rolled out. Apologies for the inconvenience :(
cc @genieCS

Hi @ajaykn, is there an ETA for the fix to be rolled out? Trying to decide whether I should patch our workflows or wait for the fix. Thanks!

Hi @JavaScriptBach, this will mostly go in a day. If it's getting delayed, i will update.

@ChristopherHX
Copy link
Contributor

This issue seems to be resolved for me, the inputs context now has valid workflow_dispatch input values to pass to the reusable workflow and no longer a null inputs context. I guess this is deployed in waves and some people still have to wait.

Requested Change

The inputs context is scoped to the appropriate workflow and inputs into reusable workflows can be computed from inputs into dispatched workflows.

They are still merged, workflow_call inputs overrides workflow_dispatch inputs if they have different names both are in the inputs context.

I think merged inputs are intensional, but unexpected

@nathanchapman
Copy link

Also, @ChristopherHX made a great point here that this context availability isn't yet documented on https://docs.github.com/en/enterprise-cloud@latest/actions/learn-github-actions/contexts#context-availability

@ajaykn
Copy link
Contributor

ajaykn commented Jul 2, 2022

@nathanchapman @ChristopherHX @JavaScriptBach and others:

The changes have been rolled out for every one and hope there are no issues.

They are still merged, workflow_call inputs overrides workflow_dispatch inputs if they have different names both are in the inputs context.

I think merged inputs are intensional, but unexpected

Yes, we are passing dispatch inputs into reusable wfs too.
So inputs context will have both dispatch and workflow_call inputs. But if they have same name, then worklow_call inputs takes precedence.

Also, @ChristopherHX made a great point #1483 (comment) that this context availability isn't yet documented on https://docs.github.com/en/enterprise-cloud@latest/actions/learn-github-actions/contexts#context-availability

Regarding the docs update, good point. we will be updating soon.

Thanks everyone for sharing details and improving our product.

@Ingvord
Copy link

Ingvord commented Apr 7, 2023

Would be great if inputs from dispatched workflow will be implicitly passed to a callable one, or at least have some sort of short syntax for that, e.g.

jobs:
  release:
    uses: ../.github/workflows/release.yaml@master
    with: 
      inputs: inherited  # forwarded OR ${{ github.inputs }}  etc

Thanks!

@allwalte
Copy link

Would also be great to know what version of Actions this is in, and how that corresponds to GHE version, for those of us on GHE. I find it extremely difficult to track what I should be expecting to see/work in the version we have or not, or if I want to request an upgrade from our admins for a certain feature what version I should be asking for, etc...

@johannesnormannjensen
Copy link

johannesnormannjensen commented Nov 28, 2023

Seems that inputs are inherited now even though only secrets are specified to be inherited.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

9 participants